From 24f1a0f3e074c3a23d8053ce6357172b4d4cdc65 Mon Sep 17 00:00:00 2001 From: Andy Sellick Date: Fri, 7 Jun 2024 16:28:28 +0100 Subject: [PATCH 1/2] Use components for calls to evidence attachments - output attachments in calls for evidence using the data from the content item, rather than the pre-rendered markup from the content item --- app/presenters/call_for_evidence_presenter.rb | 40 +++- .../content_items/call_for_evidence.html.erb | 16 +- test/integration/call_for_evidence_test.rb | 219 +++++++++++++++++- .../call_for_evidence_presenter_test.rb | 26 ++- 4 files changed, 276 insertions(+), 25 deletions(-) diff --git a/app/presenters/call_for_evidence_presenter.rb b/app/presenters/call_for_evidence_presenter.rb index 3ddbaae11..29799e6e6 100644 --- a/app/presenters/call_for_evidence_presenter.rb +++ b/app/presenters/call_for_evidence_presenter.rb @@ -48,28 +48,50 @@ def outcome_detail content_item["details"]["outcome_detail"] end + # Read the full outcome, top of page def outcome_documents - content_item["details"]["outcome_documents"]&.join("") + # content_item["details"]["outcome_documents"]&.join("") + documents.select { |doc| outcome_attachments.include? doc["id"] } + end + + # Documents, bottom of page + def general_documents + documents.select { |doc| featured_attachments.include? doc["id"] } + end + + def attachments_with_details + items = [].push(*outcome_documents) + items.push(*general_documents) + items.select { |doc| doc["accessible"] == false && doc["alternative_format_contact_email"] }.count end def outcome_attachments content_item["details"]["outcome_attachments"] end - def held_on_another_website? - held_on_another_website_url.present? + def featured_attachments + content_item["details"]["featured_attachments"] end - def held_on_another_website_url - content_item["details"]["held_on_another_website_url"] + def documents + # content_item["details"]["documents"]&.join("") + return [] unless content_item["details"]["attachments"] + + 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["type"] = "external" if doc["attachment_type"] == "external" + doc["preview_url"] = "#{doc['url']}/preview" if doc["preview_url"] + doc["alternative_format_contact_email"] = nil if doc["accessible"] == true + end end - def documents - content_item["details"]["documents"]&.join("") + def held_on_another_website? + held_on_another_website_url.present? end - def featured_attachments - content_item["details"]["featured_attachments"] + def held_on_another_website_url + content_item["details"]["held_on_another_website_url"] end def ways_to_respond? diff --git a/app/views/content_items/call_for_evidence.html.erb b/app/views/content_items/call_for_evidence.html.erb index 3ecef5dc9..2da7638fa 100644 --- a/app/views/content_items/call_for_evidence.html.erb +++ b/app/views/content_items/call_for_evidence.html.erb @@ -42,10 +42,10 @@ <% elsif @content_item.outcome? %> <%= render 'govuk_publishing_components/components/notice', title: t("call_for_evidence.closed") %> - <%= render "attachments", - title: t("call_for_evidence.download_outcome"), - legacy_pre_rendered_documents: @content_item.outcome_documents, - attachments: @content_item.outcome_attachments %> + <%= render "attachments_list", + title: t("call_for_evidence.download_outcome"), + attachments_for_components: @content_item.outcome_documents + %> <%= render 'govuk_publishing_components/components/heading', text: t("call_for_evidence.detail_of_outcome"), mobile_top_margin: true %>
@@ -116,10 +116,10 @@ <%= raw(@content_item.govspeak_body[:content]) %> <% end %> - <%= render "attachments", - title: t("call_for_evidence.documents"), - legacy_pre_rendered_documents: @content_item.documents, - attachments: @content_item.featured_attachments %> + <%= render "attachments_list", + title: t("call_for_evidence.documents"), + attachments_for_components: @content_item.general_documents + %>
<% if @content_item.ways_to_respond? %> diff --git a/test/integration/call_for_evidence_test.rb b/test/integration/call_for_evidence_test.rb index fc43d0afe..be90222ef 100644 --- a/test/integration/call_for_evidence_test.rb +++ b/test/integration/call_for_evidence_test.rb @@ -11,6 +11,81 @@ def teardown Timecop.return end + general_overrides = { + "details" => { + "attachments" => [ + { + "accessible" => false, + "alternative_format_contact_email" => "publications@ofqual.gov.uk", + "attachment_type" => "file", + "command_paper_number" => "", + "content_type" => "application/pdf", + "file_size" => 803, + "filename" => "Setting_grade_standards_part_2.pdf", + "hoc_paper_number" => "", + "id" => "01", + "isbn" => "", + "number_of_pages" => 33, + "title" => "Setting the grade standards of new GCSEs in England – part 2", + "unique_reference" => "Ofqual/16/5939", + "unnumbered_command_paper" => false, + "unnumbered_hoc_paper" => false, + "url" => "https://assets.publishing.service.gov.uk/media/5a7f7b63ed915d74e33f6b3d/Setting_grade_standards_part_2.pdf", + }, + { + "accessible" => false, + "alternative_format_contact_email" => "publications@ofqual.gov.uk", + "attachment_type" => "file", + "command_paper_number" => "", + "content_type" => "application/pdf", + "file_size" => 365, + "filename" => "Decisions_-_setting_GCSE_grade_standards_-_part_2.pdf", + "hoc_paper_number" => "", + "id" => "02", + "isbn" => "", + "number_of_pages" => 10, + "title" => "Decisions on setting the grade standards of new GCSEs in England - part 2", + "unique_reference" => "Ofqual/16/6102", + "unnumbered_command_paper" => false, + "unnumbered_hoc_paper" => false, + "url" => "https://assets.publishing.service.gov.uk/media/5a817d87ed915d74e62328cf/Decisions_-_setting_GCSE_grade_standards_-_part_2.pdf", + }, + { + "accessible" => false, + "alternative_format_contact_email" => "publications@ofqual.gov.uk", + "attachment_type" => "file", + "command_paper_number" => "", + "content_type" => "application/pdf", + "file_size" => 646, + "filename" => "Grading-consulation-Equalities-Impact-Assessment.pdf", + "hoc_paper_number" => "", + "id" => "03", + "isbn" => "", + "number_of_pages" => 5, + "title" => "Equalities impact assessment: setting the grade standards of new GCSEs in England – part 2", + "unique_reference" => "Ofqual/16/6104", + "unnumbered_command_paper" => false, + "unnumbered_hoc_paper" => false, + "url" => "https://assets.publishing.service.gov.uk/media/5a8014d6ed915d74e622c5af/Grading-consulation-Equalities-Impact-Assessment.pdf", + }, + { + "accessible" => false, + "alternative_format_contact_email" => "publications@ofqual.gov.uk", + "attachment_type" => "file", + "content_type" => "application/pdf", + "file_size" => 175, + "filename" => "Grading-consultation-analysis-of-responses.pdf", + "id" => "04", + "number_of_pages" => 24, + "title" => "Analysis of responses to our consultation on setting the grade standards of new GCSEs in England – part 2", + "url" => "https://assets.publishing.service.gov.uk/media/5a819d85ed915d74e6233377/Grading-consultation-analysis-of-responses.pdf", + }, + ], + "outcome_attachments" => %w[01], + "featured_attachments" => %w[02], + }, + } + test "call for evidence" do setup_and_visit_content_item("open_call_for_evidence", { "public_updated_at" => "2022-05-15T15:52:59.000+00:00", @@ -41,11 +116,11 @@ def teardown end test "renders document attachments" do - setup_and_visit_content_item("closed_call_for_evidence") + setup_and_visit_content_item("closed_call_for_evidence", general_overrides) assert page.has_text?("Documents") within "#documents" do - assert page.has_text?("Net Zero Review: Call for evidence") + assert page.has_text?("Decisions on setting the grade standards of new GCSEs in England - part 2") end end @@ -58,6 +133,142 @@ def teardown end end + test "renders accessible format option when accessible is false and email is supplied" do + overrides = { + "details" => { + "attachments" => [ + { + "accessible" => false, + "alternative_format_contact_email" => "ddc-modinternet@mod.gov.uk", + "attachment_type" => "file", + "id" => "01", + "title" => "Number of ex-regular service personnel now part of FR20", + "url" => "https://assets.publishing.service.gov.uk/government/uploads/system/uploads/attachment_data/file/315163/PUBLIC_1392629965.pdf", + "content_type" => "application/pdf", + "filename" => "PUBLIC_1392629965.pdf", + "locale" => "en", + }, + ], + "featured_attachments" => %w[01], + }, + } + setup_and_visit_content_item("call_for_evidence_outcome_with_featured_attachments", overrides) + within "#documents" do + assert page.has_text?("Request an accessible format") + end + end + + test "doesn't render accessible format option when accessible is true and email is supplied" do + overrides = { + "details" => { + "attachments" => [ + { + "accessible" => true, + "alternative_format_contact_email" => "ddc-modinternet@mod.gov.uk", + "attachment_type" => "file", + "id" => "01", + "title" => "Number of ex-regular service personnel now part of FR20", + "url" => "https://assets.publishing.service.gov.uk/government/uploads/system/uploads/attachment_data/file/315163/PUBLIC_1392629965.pdf", + "content_type" => "application/pdf", + "filename" => "PUBLIC_1392629965.pdf", + "locale" => "en", + }, + ], + "featured_attachments" => %w[01], + }, + } + setup_and_visit_content_item("call_for_evidence_outcome_with_featured_attachments", overrides) + within "#documents" do + assert page.has_no_text?("Request an accessible format") + end + end + + test "doesn't render accessible format option when accessible is false and email is not supplied" do + overrides = { + "details" => { + "attachments" => [ + { + "accessible" => false, + "attachment_type" => "file", + "id" => "01", + "title" => "Number of ex-regular service personnel now part of FR20", + "url" => "https://assets.publishing.service.gov.uk/government/uploads/system/uploads/attachment_data/file/315163/PUBLIC_1392629965.pdf", + "content_type" => "application/pdf", + "filename" => "PUBLIC_1392629965.pdf", + "locale" => "en", + }, + ], + "featured_attachments" => %w[01], + }, + } + setup_and_visit_content_item("call_for_evidence_outcome_with_featured_attachments", overrides) + within "#documents" do + assert page.has_no_text?("Request an accessible format") + end + end + + test "tracks details elements in attachments correctly" do + overrides = { + "details" => { + "attachments" => [ + { + "accessible" => false, + "alternative_format_contact_email" => "ddc-modinternet@mod.gov.uk", + "id" => "01", + "title" => "Attachment 1 - should have details element", + "url" => "https://assets.publishing.service.gov.uk/government/uploads/system/uploads/attachment_data/file/315163/PUBLIC_1392629965.pdf", + "content_type" => "application/pdf", + "filename" => "PUBLIC_1392629965.pdf", + "locale" => "en", + }, + { + "accessible" => true, + "alternative_format_contact_email" => "ddc-modinternet@mod.gov.uk", + "id" => "02", + "title" => "Attachment 2", + "url" => "https://assets.publishing.service.gov.uk/government/uploads/system/uploads/attachment_data/file/315163/PUBLIC_1392629965.pdf", + "content_type" => "application/pdf", + "filename" => "PUBLIC_1392629965.pdf", + "locale" => "en", + }, + { + "accessible" => true, + "alternative_format_contact_email" => nil, + "id" => "03", + "title" => "Attachment 3", + "url" => "https://assets.publishing.service.gov.uk/government/uploads/system/uploads/attachment_data/file/315163/PUBLIC_1392629965.pdf", + "content_type" => "application/pdf", + "filename" => "PUBLIC_1392629965.pdf", + "locale" => "en", + }, + { + "accessible" => false, + "alternative_format_contact_email" => "ddc-modinternet@mod.gov.uk", + "id" => "04", + "title" => "Attachment 4 - should have details element", + "url" => "https://assets.publishing.service.gov.uk/government/uploads/system/uploads/attachment_data/file/315163/PUBLIC_1392629965.pdf", + "content_type" => "application/pdf", + "filename" => "PUBLIC_1392629965.pdf", + "locale" => "en", + }, + ], + "outcome_attachments" => %w[01 02], + "featured_attachments" => %w[03 04], + }, + } + setup_and_visit_content_item("call_for_evidence_outcome_with_featured_attachments", overrides) + attachments = page.find_all(".gem-c-attachment") + assert_equal attachments.length, overrides["details"]["attachments"].length + + attachments.each do |attachment| + next unless attachment.has_css?(".govuk-details__summary") + + details = attachment.find(".govuk-details__summary")["data-ga4-event"] + actual_tracking = JSON.parse(details) + assert_equal actual_tracking["index_section_count"], 2 + end + end + test "link to external calls for evidence" do setup_and_visit_content_item("open_call_for_evidence") @@ -117,12 +328,12 @@ def teardown end test "renders call for evidence outcome attachments" do - setup_and_visit_content_item("call_for_evidence_outcome") + setup_and_visit_content_item("call_for_evidence_outcome", general_overrides) assert page.has_text?("This call for evidence has closed") assert page.has_text?("Read the full outcome") within "#read-the-full-outcome" do - assert page.has_text?("Employee Share Schemes: NIC elections - consulation response") + assert page.has_text?("Setting the grade standards of new GCSEs in England – part 2") end end diff --git a/test/presenters/call_for_evidence_presenter_test.rb b/test/presenters/call_for_evidence_presenter_test.rb index 6fa2cb38d..fa8eb6834 100644 --- a/test/presenters/call_for_evidence_presenter_test.rb +++ b/test/presenters/call_for_evidence_presenter_test.rb @@ -6,6 +6,18 @@ def schema_name "call_for_evidence" end + test_documents = [ + { + "id" => "01", + }, + { + "id" => "02", + }, + { + "id" => "03", + }, + ] + test "presents the schema name" do assert_equal schema_item("open_call_for_evidence")["document_type"], presented_item("open_call_for_evidence").document_type assert_equal schema_item("open_call_for_evidence")["details"]["body"], presented_item("open_call_for_evidence").body @@ -64,18 +76,24 @@ def schema_name test "presents call for evidence documents" do schema = schema_item("closed_call_for_evidence") - schema["details"]["documents"] = %W[
\n

a

\n
\n

b

\n
\n

c

\n
] + schema["details"]["attachments"] = test_documents + schema["details"]["featured_attachments"] = %w[01 02] presented = presented_item("closed_call_for_evidence", schema) - assert_equal "
\n

a

\n
\n

b

\n
\n

c

\n
", presented.documents + assert_equal presented.general_documents.length, 2 + assert_equal presented.general_documents[0]["id"], "01" + assert_equal presented.general_documents[1]["id"], "02" end test "presents outcome documents" do schema = schema_item("call_for_evidence_outcome") - schema["details"]["outcome_documents"] = %W[
\n

a

\n
\n

b

\n
\n

c

\n
] + schema["details"]["attachments"] = test_documents + schema["details"]["outcome_attachments"] = %w[02 03] presented = presented_item("call_for_evidence_outcome", schema) - assert_equal "
\n

a

\n
\n

b

\n
\n

c

\n
", presented.outcome_documents + assert_equal presented.outcome_documents.length, 2 + assert_equal presented.outcome_documents[0]["id"], "02" + assert_equal presented.outcome_documents[1]["id"], "03" end test "presents URL for calls for evidence held on another website" do From fdd3a8bc2e9d26e3f56667bd8fe5f1fa8314f1dc Mon Sep 17 00:00:00 2001 From: Andy Sellick Date: Fri, 7 Jun 2024 16:32:03 +0100 Subject: [PATCH 2/2] Remove unused partial --- app/views/content_items/_attachments.html.erb | 27 ----------- app/views/content_items/consultation.html.erb | 6 --- .../attachments.html.erb_test.rb | 46 ------------------- 3 files changed, 79 deletions(-) delete mode 100644 app/views/content_items/_attachments.html.erb delete mode 100644 test/views/content_items/attachments.html.erb_test.rb diff --git a/app/views/content_items/_attachments.html.erb b/app/views/content_items/_attachments.html.erb deleted file mode 100644 index ee8f901a6..000000000 --- a/app/views/content_items/_attachments.html.erb +++ /dev/null @@ -1,27 +0,0 @@ -<% 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/consultation.html.erb b/app/views/content_items/consultation.html.erb index 0b90882ac..f144e2570 100644 --- a/app/views/content_items/consultation.html.erb +++ b/app/views/content_items/consultation.html.erb @@ -148,12 +148,6 @@ title: t("consultation.documents"), attachments_for_components: @content_item.documents_attachments_for_components %> - <% if false %> - <%= render "attachments", - title: t("consultation.documents"), - legacy_pre_rendered_documents: @content_item.documents, - attachments: @content_item.featured_attachments %> - <% end %> <% if @content_item.ways_to_respond? %> diff --git a/test/views/content_items/attachments.html.erb_test.rb b/test/views/content_items/attachments.html.erb_test.rb deleted file mode 100644 index 913d549ba..000000000 --- a/test/views/content_items/attachments.html.erb_test.rb +++ /dev/null @@ -1,46 +0,0 @@ -require "test_helper" - -class ContentItemsAttachmentsTest < ActionView::TestCase - test "it shows pre-rendered attachments by default" do - render( - partial: "content_items/attachments", - locals: { title: "Documents", - legacy_pre_rendered_documents: "some html" }, - ) - - assert_includes rendered, "gem-c-govspeak" - assert_includes rendered, "some html" - end - - test "can render attachments using their metadata" do - @content_item = PublicationPresenter.new( - { "details" => { "attachments" => [{ "id" => "attachment_id", - "title" => "Some title", - "url" => "some/url" }] } }, - "/publication", - ApplicationController.new.view_context, - ) - - render( - partial: "content_items/attachments", - locals: { title: "Documents", - legacy_pre_rendered_documents: "", - attachments: %w[attachment_id] }, - ) - - assert_includes rendered, "gem-c-attachment" - assert_includes rendered, "Some title" - end - - test "it prioritises pre-rendered attachments" do - render( - partial: "content_items/attachments", - locals: { title: "Documents", - legacy_pre_rendered_documents: "some html", - attachments: %w[attachment_id] }, - ) - - assert_includes rendered, "gem-c-govspeak" - assert_includes rendered, "some html" - end -end