Skip to content

Commit

Permalink
Fix publication attachments
Browse files Browse the repository at this point in the history
- simplify and fix the code to display attachments on publication/form pages
- move most of the code into the presenter, to reduce ambiguity
- pass only one thing to the attachments_list component
- update tests
  • Loading branch information
andysellick committed Apr 18, 2024
1 parent ca194ef commit 1af04c1
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 20 deletions.
6 changes: 5 additions & 1 deletion app/presenters/publication_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,14 @@ def details
content_item["details"]["body"]
end

def attachments_for_components
documents.select { |doc| featured_attachments.include? doc["id"] }
end

def documents
return [] unless content_item["details"]["attachments"]

docs = content_item["details"]["attachments"].select { |a| a["locale"] == locale }
docs = content_item["details"]["attachments"].select { |a| !a.key?("locale") || a["locale"] == locale }
docs.each do |doc|
doc["type"] = "html" unless doc["content_type"]
doc["alternative_format_contact_email"] = nil if doc["accessible"] == true
Expand Down
23 changes: 9 additions & 14 deletions app/views/content_items/_attachments_list.html.erb
Original file line number Diff line number Diff line change
@@ -1,21 +1,16 @@
<%
attachments ||= nil
featured_attachments ||= []
attachment_details = featured_attachments.filter_map { |id| @content_item&.attachment_details(id) }
%>
<section id="<%= title.parameterize %>">
<%= render 'govuk_publishing_components/components/heading',
text: title,
mobile_top_margin: true
%>
<% if attachment_details.length > 0 %>
<% if attachments_for_components.any? %>
<section id="<%= title.parameterize %>">
<%= render 'govuk_publishing_components/components/heading',
text: title,
mobile_top_margin: true
%>
<% add_gem_component_stylesheet("details") %>
<% attachment_details.each do |details| %>
<% attachments_for_components.each do |details| %>
<%= render 'govuk_publishing_components/components/attachment', {
heading_level: 3,
attachment: details,
margin_bottom: 6
} %>
<% end %>
<% end %>
</section>
</section>
<% end %>
3 changes: 1 addition & 2 deletions app/views/content_items/publication.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@
<div class="responsive-bottom-margin">
<%= render "attachments_list",
title: t("publication.documents", count: 5), # This should always be pluralised.
attachments: @content_item.documents,
featured_attachments: @content_item.featured_attachments
attachments_for_components: @content_item.attachments_for_components
%>

<section id="details">
Expand Down
14 changes: 11 additions & 3 deletions test/integration/publication_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,7 @@ class PublicationTest < ActionDispatch::IntegrationTest
}

setup_and_visit_content_item("publication", overrides)
within "#documents" do
assert page.has_no_text?("Permit: Veolia ES (UK) Limited")
end
assert page.has_no_text?("Permit: Veolia ES (UK) Limited")
end

test "renders featured document attachments using components" do
Expand All @@ -71,6 +69,16 @@ class PublicationTest < ActionDispatch::IntegrationTest
end
end

test "doesn't render the documents section if no documents" do
overrides = {
"details" => {
"attachments" => [{}],
},
}
setup_and_visit_content_item("publication-with-featured-attachments", overrides)
assert page.has_no_text?("Documents")
end

test "renders accessible format option when accessible is false and email is supplied" do
overrides = {
"details" => {
Expand Down

0 comments on commit 1af04c1

Please sign in to comment.