From cbc845a7e526b8149adc051cebc5467e6d742edb Mon Sep 17 00:00:00 2001 From: Chris Yoong Date: Wed, 1 Dec 2021 15:37:47 +0000 Subject: [PATCH 1/2] Remove overview text The context_label is being defined by "schema_name: "publication" - should this be present this determines whether we class this as a "publication or consultation" Ensure title of page has "context_inside" Content prefixed to a "publication or consultation" Should content be [withdrawn] this also adds the "context_label" (or grey text) to the title scoped to items that have been identified as a "publication or consultation" Remove the loops around the attachments and just apply the context_label in all cases. Remove word "overview" from "context_inside" --- app/presenters/content_item/withdrawable.rb | 29 ++++++++++++++++++- .../content_items/_context_and_title.html.erb | 25 ++-------------- test/integration/fatality_notice_test.rb | 2 +- .../content_item/withdrawable_test.rb | 11 ++++++- 4 files changed, 42 insertions(+), 25 deletions(-) diff --git a/app/presenters/content_item/withdrawable.rb b/app/presenters/content_item/withdrawable.rb index c0733e09d..67b528dd2 100644 --- a/app/presenters/content_item/withdrawable.rb +++ b/app/presenters/content_item/withdrawable.rb @@ -5,7 +5,15 @@ def withdrawn? end def page_title - withdrawn? ? "[Withdrawn] #{title}" : title + if withdrawn? && context_title? && publication_overview? + "[Withdrawn] #{context_title}: #{title}" + elsif withdrawn? + "[Withdrawn] #{title}" + elsif context_title? && publication_overview? + "#{context_title}: #{title}" + else + title + end end def withdrawal_notice_component @@ -21,6 +29,19 @@ def withdrawal_notice_component private + def context_title? + context_title.present? + end + + def publication_overview? + publication_overview.present? + end + + def publication_overview + overview = (I18n.exists?("content_item.#{schema_name}") == "publication" || "statistical_data_set") + overview.presence + end + def withdrawal_notice content_item["withdrawn_notice"] end @@ -29,6 +50,12 @@ def withdrawal_notice_title "This #{withdrawal_notice_context.downcase} was withdrawn on #{withdrawal_notice_time}".html_safe end + def context_title + if I18n.exists?("content_item.schema_name.#{document_type}", count: 1, locale: :en) + I18n.t("content_item.schema_name.#{document_type}", count: 1, locale: :en) + end + end + def withdrawal_notice_context I18n.t("content_item.schema_name.#{schema_name}", count: 1, locale: :en) end diff --git a/app/views/content_items/_context_and_title.html.erb b/app/views/content_items/_context_and_title.html.erb index c68512a07..6eb2b7b7b 100644 --- a/app/views/content_items/_context_and_title.html.erb +++ b/app/views/content_items/_context_and_title.html.erb @@ -1,26 +1,7 @@ -<% - context_string = t("content_item.schema_name.#{@content_item.document_type}", count: 1); - context_inside = false; - %> - -<% @content_item&.featured_attachments.each do |fa| %> - <% return if !@content_item.attachment_details(fa).present? %> - <% if @content_item.attachment_details(fa)['title'] == @content_item.title %> - <% content_for :title do %> - <%= t("content_item.schema_name.#{@content_item.document_type}.overview", count: 1) %>: <%= @content_item.title %> - <% end %> - <% - context_string = t("content_item.schema_name.#{@content_item.document_type}.overview", count: 1) << ":" - context_inside = true - %> - <% break %> - <% end %> -<% end %> - <%= render 'govuk_publishing_components/components/title', - context: context_string, - context_locale: t_locale_fallback("content_item.schema_name.#{@content_item.document_type}", count: 1), - context_inside: context_inside, + context: t("content_item.schema_name.#{@content_item.document_type}", count: 1) << ":", + context_locale: t_locale_fallback("content_item.schema_name.#{@content_item.document_type}", count: 1) , + context_inside: true, title: @content_item.title, average_title_length: "long" %> diff --git a/test/integration/fatality_notice_test.rb b/test/integration/fatality_notice_test.rb index 7fbd034f7..5b5b3be4b 100644 --- a/test/integration/fatality_notice_test.rb +++ b/test/integration/fatality_notice_test.rb @@ -69,7 +69,7 @@ class FatalityNoticeTest < ActionDispatch::IntegrationTest setup_and_visit_content_item("withdrawn_fatality_notice") assert page.has_title?( - "[Withdrawn] Sir George Pomeroy Colley killed in Boer War - Fatality notice - GOV.UK", + "[Withdrawn] Fatality notice: Sir George Pomeroy Colley killed in Boer War - Fatality notice - GOV.UK", ) within ".gem-c-notice" do diff --git a/test/presenters/content_item/withdrawable_test.rb b/test/presenters/content_item/withdrawable_test.rb index 8935465c9..4d19b9be6 100644 --- a/test/presenters/content_item/withdrawable_test.rb +++ b/test/presenters/content_item/withdrawable_test.rb @@ -33,17 +33,26 @@ def title content_item["title"] end + def schema_name + content_item["schema_name"] + end + + def document_type + content_item["document_type"] + end + def content_item { "title" => "Proportion of residents who do any walking or cycling (at local authority level) (CW010)", "withdrawn_notice" => { "withdrawn_at" => "2016-07-12T09:47:15Z", }, + "document_type" => "statistical_data_set", } end end - assert_equal @withdrawable.page_title, "[Withdrawn] Proportion of residents who do any walking or cycling (at local authority level) (CW010)" + assert_equal @withdrawable.page_title, "[Withdrawn] Statistical data set: Proportion of residents who do any walking or cycling (at local authority level) (CW010)" end test "notice title and description are generated correctly" do From 0a10ea684a363b3d7f6827f6f3e4ea182c3bf016 Mon Sep 17 00:00:00 2001 From: Chris Yoong Date: Thu, 25 Nov 2021 14:22:05 +0000 Subject: [PATCH 2/2] Remove test No longer required --- .../attachments.html.erb_test.rb | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/test/views/content_items/attachments.html.erb_test.rb b/test/views/content_items/attachments.html.erb_test.rb index 64aea883c..913d549ba 100644 --- a/test/views/content_items/attachments.html.erb_test.rb +++ b/test/views/content_items/attachments.html.erb_test.rb @@ -43,23 +43,4 @@ class ContentItemsAttachmentsTest < ActionView::TestCase assert_includes rendered, "gem-c-govspeak" assert_includes rendered, "some html" end - - test "renders overview title when attachment title matches parent" do - @content_item = PublicationPresenter.new( - { "document_type" => "correspondence", - "title" => "Matching", - "details" => { "attachments" => [{ "id" => "attachment_id", - "title" => "Matching", - "url" => "some/url" }], - "featured_attachments" => %w[attachment_id] } }, - "/publication", - ApplicationController.new.view_context, - ) - render( - partial: "content_items/context_and_title", - ) - - assert_includes rendered, "Correspondence overview:" - assert_select "h1 span" - end end