From 5c6e15ccc90a4cf85818aed5965bd7962da496d8 Mon Sep 17 00:00:00 2001 From: Andy Sellick Date: Tue, 23 Jan 2024 09:10:03 +0000 Subject: [PATCH 1/4] Make a new partial for publications - create a new partial for use and modification by publications - currently just a copy of the existing _attachments.html.erb --- .../content_items/_attachments_list.html.erb | 27 +++++++++++++++++++ app/views/content_items/publication.html.erb | 2 +- 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 app/views/content_items/_attachments_list.html.erb diff --git a/app/views/content_items/_attachments_list.html.erb b/app/views/content_items/_attachments_list.html.erb new file mode 100644 index 000000000..ee8f901a6 --- /dev/null +++ b/app/views/content_items/_attachments_list.html.erb @@ -0,0 +1,27 @@ +<% attachments ||= [] %> +<% attachment_details = attachments.filter_map { |id| @content_item&.attachment_details(id) } %> +<% if legacy_pre_rendered_documents.present? || attachment_details.any? %> +
+ <%= render 'govuk_publishing_components/components/heading', + text: title, + mobile_top_margin: true %> + + <% if legacy_pre_rendered_documents.present? %> + <%= render 'govuk_publishing_components/components/govspeak', { + direction: page_text_direction, + } do %> + <% add_gem_component_stylesheet("details") if legacy_pre_rendered_documents.include? "govuk\-details" %> + <%= raw(legacy_pre_rendered_documents) %> + <% end %> + <% else %> + <% attachment_details.each do |details| %> +
+ <%= render 'govuk_publishing_components/components/attachment', { + heading_level: 3, + attachment: details + } %> +
+ <% end %> + <% end %> +
+<% end %> diff --git a/app/views/content_items/publication.html.erb b/app/views/content_items/publication.html.erb index 66b40dd80..31bc45bef 100644 --- a/app/views/content_items/publication.html.erb +++ b/app/views/content_items/publication.html.erb @@ -49,7 +49,7 @@ <% end %>
- <%= render "attachments", + <%= render "attachments_list", title: t("publication.documents", count: 5), # This should always be pluralised. legacy_pre_rendered_documents: @content_item.documents, attachments: @content_item.featured_attachments From ba2ddc5eb0db4a4901caf5f33f020f37b243e885 Mon Sep 17 00:00:00 2001 From: Andy Sellick Date: Tue, 23 Jan 2024 09:18:43 +0000 Subject: [PATCH 2/4] Change data for publication attachments - modify the new partial and the publication presenter to use a different bit of the content item for generating the attachments list - specifically, use `details->attachments` instead of `details->documents`, which is pre-rendered HTML - `details->attachments` contains only the data for the attachments, allowing us to render them individually in the new partial using frontend components, which will allow us to add tracking later --- app/presenters/publication_presenter.rb | 3 +- .../content_items/_attachments_list.html.erb | 46 ++++++++----------- app/views/content_items/publication.html.erb | 4 +- test/presenters/publication_presenter_test.rb | 1 - 4 files changed, 24 insertions(+), 30 deletions(-) diff --git a/app/presenters/publication_presenter.rb b/app/presenters/publication_presenter.rb index 7514029e9..7039d0a97 100644 --- a/app/presenters/publication_presenter.rb +++ b/app/presenters/publication_presenter.rb @@ -11,7 +11,8 @@ def details end def documents - content_item["details"]["documents"].to_a.join("") + docs = content_item["details"]["attachments"].select { |a| a["locale"] == locale } if content_item["details"]["attachments"] + docs.each { |t| t["type"] = "html" unless t["content_type"] } end def featured_attachments diff --git a/app/views/content_items/_attachments_list.html.erb b/app/views/content_items/_attachments_list.html.erb index ee8f901a6..327bacba2 100644 --- a/app/views/content_items/_attachments_list.html.erb +++ b/app/views/content_items/_attachments_list.html.erb @@ -1,27 +1,21 @@ -<% attachments ||= [] %> -<% attachment_details = attachments.filter_map { |id| @content_item&.attachment_details(id) } %> -<% if legacy_pre_rendered_documents.present? || attachment_details.any? %> -
- <%= render 'govuk_publishing_components/components/heading', - text: title, - mobile_top_margin: true %> - - <% if legacy_pre_rendered_documents.present? %> - <%= render 'govuk_publishing_components/components/govspeak', { - direction: page_text_direction, - } do %> - <% add_gem_component_stylesheet("details") if legacy_pre_rendered_documents.include? "govuk\-details" %> - <%= raw(legacy_pre_rendered_documents) %> - <% end %> - <% else %> - <% attachment_details.each do |details| %> -
- <%= render 'govuk_publishing_components/components/attachment', { - heading_level: 3, - attachment: details - } %> -
- <% end %> +<% + attachments ||= nil + featured_attachments ||= [] + attachment_details = featured_attachments.filter_map { |id| @content_item&.attachment_details(id) } +%> +
+ <%= render 'govuk_publishing_components/components/heading', + text: title, + mobile_top_margin: true + %> + <% if attachment_details.length > 0 %> + <% add_gem_component_stylesheet("details") %> + <% attachment_details.each do |details| %> + <%= render 'govuk_publishing_components/components/attachment', { + heading_level: 3, + attachment: details, + margin_bottom: 6 + } %> <% end %> -
-<% end %> + <% end %> +
diff --git a/app/views/content_items/publication.html.erb b/app/views/content_items/publication.html.erb index 31bc45bef..322d5ea65 100644 --- a/app/views/content_items/publication.html.erb +++ b/app/views/content_items/publication.html.erb @@ -51,8 +51,8 @@
<%= render "attachments_list", title: t("publication.documents", count: 5), # This should always be pluralised. - legacy_pre_rendered_documents: @content_item.documents, - attachments: @content_item.featured_attachments + attachments: @content_item.documents, + featured_attachments: @content_item.featured_attachments %>
diff --git a/test/presenters/publication_presenter_test.rb b/test/presenters/publication_presenter_test.rb index ad11cba5d..94907fdde 100644 --- a/test/presenters/publication_presenter_test.rb +++ b/test/presenters/publication_presenter_test.rb @@ -10,7 +10,6 @@ def schema_name assert_equal schema_item["schema_name"], presented_item.schema_name assert_equal schema_item["title"], presented_item.title assert_equal schema_item["details"]["body"], presented_item.details - assert_equal schema_item["details"]["documents"].join(""), presented_item.documents end test "#published returns a formatted date of the day the content item became public" do From fbada616aecd66597e50f9295141b634df2f940e Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Thu, 11 Apr 2024 16:38:15 +0100 Subject: [PATCH 3/4] fix when no attachments --- app/presenters/publication_presenter.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/presenters/publication_presenter.rb b/app/presenters/publication_presenter.rb index 7039d0a97..f042be7e1 100644 --- a/app/presenters/publication_presenter.rb +++ b/app/presenters/publication_presenter.rb @@ -11,7 +11,9 @@ def details end def documents - docs = content_item["details"]["attachments"].select { |a| a["locale"] == locale } if content_item["details"]["attachments"] + return [] unless content_item["details"]["attachments"] + + docs = content_item["details"]["attachments"].select { |a| a["locale"] == locale } docs.each { |t| t["type"] = "html" unless t["content_type"] } end From 2f5cdec2faae1366d8e8b9a6294cc9e1a6746ff9 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Tue, 16 Apr 2024 14:14:38 +0100 Subject: [PATCH 4/4] Update tests - Add override data to support publication test - Update tests to make more relevant --- test/integration/publication_test.rb | 32 +++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/test/integration/publication_test.rb b/test/integration/publication_test.rb index ed2c3ac22..c0406ed3f 100644 --- a/test/integration/publication_test.rb +++ b/test/integration/publication_test.rb @@ -32,12 +32,38 @@ class PublicationTest < ActionDispatch::IntegrationTest assert_footer_has_published_dates("Published 3 May 2016") end - test "renders document attachments (as-is and directly)" do - setup_and_visit_content_item("publication") + test "does not render non-featured attachments" do + overrides = { + "details" => { + "attachments" => [{ + "accessible" => false, + "alternative_format_contact_email" => "customerservices@publicguardian.gov.uk", + "attachment_type" => "file", + "command_paper_number" => "", + "content_type" => "application/pdf", + "file_size" => 123_456, + "filename" => "veolia-permit.pdf", + "hoc_paper_number" => "", + "id" => "violia-permit", + "isbn" => "", + "locale" => "en", + "title" => "Permit: Veolia ES (UK) Limited", + "unique_reference" => "", + "unnumbered_command_paper" => false, + "unnumbered_hoc_paper" => false, + "url" => "https://assets.publishing.service.gov.uk/media/123abc/veolia-permit.zip", + }], + "featured_attachments" => [], + }, + } + + setup_and_visit_content_item("publication", overrides) within "#documents" do - assert page.has_text?("Permit: Veolia ES (UK) Limited") + assert page.has_no_text?("Permit: Veolia ES (UK) Limited") end + end + test "renders featured document attachments using components" do setup_and_visit_content_item("publication-with-featured-attachments") within "#documents" do assert page.has_text?("Number of ex-regular service personnel now part of FR20")