diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 759af8678..34358ea9a 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -18,6 +18,7 @@ $govuk-use-legacy-palette: false; @import 'helpers/add-title-margin'; @import "helpers/content-bottom-margin"; @import "helpers/publisher-metadata-with-logo"; +@import "helpers/attachments"; // Components from this application @import 'components/*'; diff --git a/app/assets/stylesheets/helpers/_attachments.scss b/app/assets/stylesheets/helpers/_attachments.scss new file mode 100644 index 000000000..7aed5f99c --- /dev/null +++ b/app/assets/stylesheets/helpers/_attachments.scss @@ -0,0 +1,3 @@ +.attachment { + margin-bottom: govuk-spacing(6); +} diff --git a/app/presenters/consultation_presenter.rb b/app/presenters/consultation_presenter.rb index 26393817a..55e3a49c4 100644 --- a/app/presenters/consultation_presenter.rb +++ b/app/presenters/consultation_presenter.rb @@ -5,6 +5,7 @@ class ConsultationPresenter < ContentItemPresenter include ContentItem::Political include ContentItem::Shareable include ContentItem::TitleAndContext + include ContentItem::Attachments def opening_date_time content_item["details"]["opening_date"] @@ -50,20 +51,20 @@ def final_outcome_detail content_item["details"]["final_outcome_detail"] end - def final_outcome_documents? - final_outcome_documents_list.any? - end - def final_outcome_documents - final_outcome_documents_list.join("") + content_item["details"]["final_outcome_documents"].to_a.join("") end - def public_feedback_documents? - public_feedback_documents_list.any? + def final_outcome_attachments + content_item["details"]["final_outcome_attachments"].to_a end def public_feedback_documents - public_feedback_documents_list.join("") + content_item["details"]["public_feedback_documents"].to_a.join("") + end + + def public_feedback_attachments + content_item["details"]["public_feedback_attachments"].to_a end def public_feedback_detail @@ -78,12 +79,12 @@ def held_on_another_website_url content_item["details"]["held_on_another_website_url"] end - def documents? - documents_list.any? + def documents + content_item["details"]["documents"].to_a.join("") end - def documents - documents_list.join("") + def featured_attachments + content_item["details"]["featured_attachments"].to_a end def ways_to_respond? @@ -111,7 +112,8 @@ def attachment_url end def add_margin? - final_outcome? || public_feedback_detail || public_feedback_documents? + final_outcome? || public_feedback_detail || + public_feedback_documents.present? || public_feedback_attachments.any? end private @@ -133,16 +135,4 @@ def display_date_and_time(date, rollback_midnight = false) def ways_to_respond content_item["details"]["ways_to_respond"] end - - def final_outcome_documents_list - content_item["details"]["final_outcome_documents"] || [] - end - - def public_feedback_documents_list - content_item["details"]["public_feedback_documents"] || [] - end - - def documents_list - content_item["details"]["documents"] || [] - end end diff --git a/app/presenters/content_item/attachments.rb b/app/presenters/content_item/attachments.rb new file mode 100644 index 000000000..628e8d625 --- /dev/null +++ b/app/presenters/content_item/attachments.rb @@ -0,0 +1,9 @@ +module ContentItem + module Attachments + def attachment_details(attachment_id) + content_item["details"]["attachments"].find do |attachment| + attachment["id"] == attachment_id + end + end + end +end diff --git a/app/presenters/publication_presenter.rb b/app/presenters/publication_presenter.rb index 97032933c..c2553fb29 100644 --- a/app/presenters/publication_presenter.rb +++ b/app/presenters/publication_presenter.rb @@ -3,17 +3,18 @@ class PublicationPresenter < ContentItemPresenter include ContentItem::NationalApplicability include ContentItem::NationalStatisticsLogo include ContentItem::Political + include ContentItem::Attachments def details content_item["details"]["body"] end def documents - documents_list.join("") + content_item["details"]["documents"].to_a.join("") end - def documents_count - documents_list.size + def featured_attachments + content_item["details"]["featured_attachments"].to_a end def national_statistics? @@ -23,10 +24,4 @@ def national_statistics? def dataset? %(national_statistics official_statistics transparency).include? document_type end - -private - - def documents_list - content_item["details"]["documents"] - end end diff --git a/app/views/content_items/_attachments.html.erb b/app/views/content_items/_attachments.html.erb new file mode 100644 index 000000000..81c191f83 --- /dev/null +++ b/app/views/content_items/_attachments.html.erb @@ -0,0 +1,20 @@ +<% if legacy_pre_rendered_documents.present? || attachments.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', + content: legacy_pre_rendered_documents.html_safe, + direction: page_text_direction %> + <% else %> + <% attachments.each do |attachment_id| %> +
+ <%= render 'govuk_publishing_components/components/attachment', + attachment: @content_item.attachment_details(attachment_id) %> +
+ <% end %> + <% end %> +
+<% end %> diff --git a/app/views/content_items/_publication_inline_body.html.erb b/app/views/content_items/_publication_inline_body.html.erb deleted file mode 100644 index 864ae4b7a..000000000 --- a/app/views/content_items/_publication_inline_body.html.erb +++ /dev/null @@ -1,21 +0,0 @@ -<%= render 'govuk_publishing_components/components/heading', - text: t("publication.documents", count: 5), # This should always be pluralised. - id: "documents-title", - mobile_top_margin: true %> - -
- <%= render 'govuk_publishing_components/components/govspeak', - content: @content_item.documents.html_safe, - direction: page_text_direction %> -
- -<%= render 'govuk_publishing_components/components/heading', - text: t("publication.details"), - id: "details-title", - mobile_top_margin: true %> - -
- <%= render 'govuk_publishing_components/components/govspeak', - content: @content_item.details.html_safe, - direction: page_text_direction %> -
diff --git a/app/views/content_items/consultation.html.erb b/app/views/content_items/consultation.html.erb index 4db3e95f3..803f7cf69 100644 --- a/app/views/content_items/consultation.html.erb +++ b/app/views/content_items/consultation.html.erb @@ -40,14 +40,10 @@ <% elsif @content_item.final_outcome? %> <%= render 'govuk_publishing_components/components/notice', title: 'This consultation has concluded' %> - <% if @content_item.final_outcome_documents? %> - <%= render 'govuk_publishing_components/components/heading', text: "Download the full outcome", mobile_top_margin: true %> -
- <%= render 'govuk_publishing_components/components/govspeak', - content: @content_item.final_outcome_documents.html_safe, - direction: page_text_direction %> -
- <% end %> + <%= render "attachments", + title: "Download the full outcome", + legacy_pre_rendered_documents: @content_item.final_outcome_documents, + attachments: @content_item.final_outcome_attachments %> <%= render 'govuk_publishing_components/components/heading', text: "Detail of outcome", mobile_top_margin: true %>
@@ -57,14 +53,10 @@
<% end %> - <% if @content_item.public_feedback_documents? %> - <%= render 'govuk_publishing_components/components/heading', text: "Feedback received", mobile_top_margin: true %> -
- <%= render 'govuk_publishing_components/components/govspeak', - content: @content_item.public_feedback_documents.html_safe, - direction: page_text_direction %> -
- <% end %> + <%= render "attachments", + title: "Feedback received", + legacy_pre_rendered_documents: @content_item.public_feedback_documents, + attachments: @content_item.public_feedback_attachments %> <% if @content_item.public_feedback_detail %> <%= render 'govuk_publishing_components/components/heading', text: "Detail of feedback received", mobile_top_margin: true %> @@ -115,14 +107,10 @@ <%= render 'govuk_publishing_components/components/heading', text: "Consultation description", mobile_top_margin: true %> <%= render 'govuk_publishing_components/components/govspeak', @content_item.govspeak_body %> - <% if @content_item.documents? %> - <%= render 'govuk_publishing_components/components/heading', text: "Documents", mobile_top_margin: true %> -
- <%= render 'govuk_publishing_components/components/govspeak', - content: @content_item.documents.html_safe, - direction: page_text_direction %> -
- <% end %> + <%= render "attachments", + title: "Documents", + legacy_pre_rendered_documents: @content_item.documents, + attachments: @content_item.featured_attachments %> <% if @content_item.ways_to_respond? %> diff --git a/app/views/content_items/publication.html.erb b/app/views/content_items/publication.html.erb index 2b7d4638a..100bed212 100644 --- a/app/views/content_items/publication.html.erb +++ b/app/views/content_items/publication.html.erb @@ -29,7 +29,21 @@
<%= render 'components/important-metadata', items: @content_item.important_metadata %>
- <%= render 'publication_inline_body' %> + <%= render "attachments", + title: t("publication.documents", count: 5), # This should always be pluralised. + legacy_pre_rendered_documents: @content_item.documents, + attachments: @content_item.featured_attachments %> + + +
+ <%= render 'govuk_publishing_components/components/heading', + text: t("publication.details"), + mobile_top_margin: true %> + + <%= render 'govuk_publishing_components/components/govspeak', + content: @content_item.details.html_safe, + direction: page_text_direction %> +
<%= render 'components/published-dates', { diff --git a/test/integration/consultation_test.rb b/test/integration/consultation_test.rb index 9e9f8b95b..5821ee55b 100644 --- a/test/integration/consultation_test.rb +++ b/test/integration/consultation_test.rb @@ -22,12 +22,20 @@ class ConsultationTest < ActionDispatch::IntegrationTest end end - test "consultation documents render" do + test "renders document attachments (as-is and directly)" do setup_and_visit_content_item("closed_consultation") - within ".consultation-documents" do + assert page.has_text?("Documents") + within "#documents" do assert page.has_text?("Museums Review Terms of Reference") end + + setup_and_visit_content_item("consultation_outcome_with_featured_attachments") + + assert page.has_text?("Documents") + within "#documents" do + assert page.has_text?("Setting the grade standards of new GCSEs in England – part 2") + end end test "link to external consultations" do @@ -87,19 +95,34 @@ class ConsultationTest < ActionDispatch::IntegrationTest end end - test "consultation outcome documents render" do + test "renders consultation outcome attachments (as-is and directly)" do setup_and_visit_content_item("consultation_outcome") - within ".consultation-outcome" do + assert page.has_text?("Download the full outcome") + within "#download-the-full-outcome" do assert page.has_text?("Employee Share Schemes: NIC elections - consulation response") end + + setup_and_visit_content_item("consultation_outcome_with_featured_attachments") + + assert page.has_text?("Download the full outcome") + within "#download-the-full-outcome" do + assert page.has_text?("Equalities impact assessment: setting the grade standards of new GCSEs in England – part 2") + end end - test "public feedback documents render" do + test "renders public feedback attachments (as-is and directly)" do setup_and_visit_content_item("consultation_outcome_with_feedback") assert page.has_text?("Feedback received") - within ".consultation-feedback-documents" do + within "#feedback-received" do + assert page.has_text?("Analysis of responses to our consultation on setting the grade standards of new GCSEs in England – part 2") + end + + setup_and_visit_content_item("consultation_outcome_with_featured_attachments") + + assert page.has_text?("Feedback received") + within "#feedback-received" do assert page.has_text?("Analysis of responses to our consultation on setting the grade standards of new GCSEs in England – part 2") end end diff --git a/test/integration/publication_test.rb b/test/integration/publication_test.rb index 53c8060f8..5a17dd8c3 100644 --- a/test/integration/publication_test.rb +++ b/test/integration/publication_test.rb @@ -11,7 +11,7 @@ class PublicationTest < ActionDispatch::IntegrationTest assert_has_component_title(@content_item["title"]) assert page.has_text?(@content_item["description"]) - within '[aria-labelledby="details-title"]' do + within "#details" do assert page.has_text?("Installation name: Leeming Biogas Facility") end end @@ -34,11 +34,17 @@ class PublicationTest < ActionDispatch::IntegrationTest assert_footer_has_published_dates("Published 3 May 2016") end - test "renders a govspeak block for attachments" do + test "renders document attachments (as-is and directly)" do setup_and_visit_content_item("publication") - within '[aria-labelledby="documents-title"]' do + within "#documents" do assert page.has_text?("Permit: Veolia ES (UK) Limited") end + + 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") + assert page.has_css?(".gem-c-attachment") + end end test "withdrawn publication" do diff --git a/test/presenters/consultation_presenter_test.rb b/test/presenters/consultation_presenter_test.rb index 22d8d8926..995241b7d 100644 --- a/test/presenters/consultation_presenter_test.rb +++ b/test/presenters/consultation_presenter_test.rb @@ -66,7 +66,6 @@ def schema_name presented = presented_item("closed_consultation") schema = schema_item("closed_consultation") - assert presented.documents? assert_equal schema["details"]["documents"].join(""), presented.documents end @@ -74,7 +73,6 @@ def schema_name presented = presented_item("consultation_outcome") schema = schema_item("consultation_outcome") - assert presented.final_outcome_documents? assert_equal schema["details"]["final_outcome_documents"].join(""), presented.final_outcome_documents end @@ -82,7 +80,6 @@ def schema_name presented = presented_item("consultation_outcome_with_feedback") schema = schema_item("consultation_outcome_with_feedback") - assert presented.public_feedback_documents? assert_equal schema["details"]["public_feedback_documents"].join(""), presented.public_feedback_documents end diff --git a/test/presenters/publication_presenter_test.rb b/test/presenters/publication_presenter_test.rb index f9b076df8..12c9446fa 100644 --- a/test/presenters/publication_presenter_test.rb +++ b/test/presenters/publication_presenter_test.rb @@ -17,10 +17,6 @@ def schema_name assert_equal "3 May 2016", presented_item.published end - test "counts documents attached to publication" do - assert_equal 2, presented_item.documents_count - end - test "presents the title of the publishing government" do assert_equal schema_item["links"]["government"][0]["title"], presented_item.publishing_government end diff --git a/test/views/content_items/attachments.html.erb_test.rb b/test/views/content_items/attachments.html.erb_test.rb new file mode 100644 index 000000000..6df55e025 --- /dev/null +++ b/test/views/content_items/attachments.html.erb_test.rb @@ -0,0 +1,40 @@ +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" }], + }, + ) + + 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