From dd193ef1ab9eb4808ba17fce9c526cbc6fde4e22 Mon Sep 17 00:00:00 2001 From: Jessica Jones Date: Fri, 6 Dec 2024 15:47:53 +0000 Subject: [PATCH 1/6] Rename method --- .../content_item/single_page_notification_button.rb | 8 ++++++-- app/presenters/content_item_presenter.rb | 2 +- .../_published_dates_with_notification_button.html.erb | 4 ++-- .../shared/_single_page_notification_button.html.erb | 2 +- test/presenters/call_for_evidence_presenter_test.rb | 2 +- test/presenters/consultation_presenter_test.rb | 2 +- test/presenters/detailed_guide_presenter_test.rb | 2 +- test/presenters/publication_presenter_test.rb | 2 +- 8 files changed, 14 insertions(+), 10 deletions(-) diff --git a/app/presenters/content_item/single_page_notification_button.rb b/app/presenters/content_item/single_page_notification_button.rb index a048e1f8c..45690af32 100644 --- a/app/presenters/content_item/single_page_notification_button.rb +++ b/app/presenters/content_item/single_page_notification_button.rb @@ -8,8 +8,12 @@ module SinglePageNotificationButton 5f9c6c15-7631-11e4-a3cb-005056011aef ].freeze - def has_single_page_notifications? - !EXEMPTION_LIST.include? content_item["content_id"] + def page_is_on_exemption_list? + EXEMPTION_LIST.include? content_item["content_id"] + end + + def display_single_page_notification_button? + !page_is_on_exemption_list? end end end diff --git a/app/presenters/content_item_presenter.rb b/app/presenters/content_item_presenter.rb index 1d1a7187c..848b58773 100644 --- a/app/presenters/content_item_presenter.rb +++ b/app/presenters/content_item_presenter.rb @@ -47,7 +47,7 @@ def requesting_a_service_sign_in_page? false end - def has_single_page_notifications? + def display_single_page_notification_button? false end diff --git a/app/views/shared/_published_dates_with_notification_button.html.erb b/app/views/shared/_published_dates_with_notification_button.html.erb index 96937c742..1d6f4a336 100644 --- a/app/views/shared/_published_dates_with_notification_button.html.erb +++ b/app/views/shared/_published_dates_with_notification_button.html.erb @@ -10,7 +10,7 @@

- <% if @content_item.has_single_page_notifications? %> + <% if @content_item.display_single_page_notification_button? %> <%= I18n.t("common.email_and_print_link") %> <% else %> <%= I18n.t("common.print_link") %> @@ -33,7 +33,7 @@ section: "Footer" } } - } if @content_item.has_single_page_notifications? %> + } if @content_item.display_single_page_notification_button? %> <%= render "govuk_publishing_components/components/print_link", { margin_top: 0, margin_bottom: 8, diff --git a/app/views/shared/_single_page_notification_button.html.erb b/app/views/shared/_single_page_notification_button.html.erb index 64be0a5a1..64b435139 100644 --- a/app/views/shared/_single_page_notification_button.html.erb +++ b/app/views/shared/_single_page_notification_button.html.erb @@ -21,4 +21,4 @@ margin_bottom: 6, button_location: "top", skip_account: skip_account, -} if @content_item.has_single_page_notifications? %> +} if @content_item.display_single_page_notification_button? %> diff --git a/test/presenters/call_for_evidence_presenter_test.rb b/test/presenters/call_for_evidence_presenter_test.rb index fa8eb6834..fdc98c56c 100644 --- a/test/presenters/call_for_evidence_presenter_test.rb +++ b/test/presenters/call_for_evidence_presenter_test.rb @@ -156,7 +156,7 @@ def schema_name schema = schema_item("open_call_for_evidence") presented = presented_item("open_call_for_evidence", schema) - assert presented.has_single_page_notifications? + assert presented.display_single_page_notification_button? end end end diff --git a/test/presenters/consultation_presenter_test.rb b/test/presenters/consultation_presenter_test.rb index 2de445a3c..4fbe64ccc 100644 --- a/test/presenters/consultation_presenter_test.rb +++ b/test/presenters/consultation_presenter_test.rb @@ -175,7 +175,7 @@ def schema_name schema = schema_item("open_consultation") presented = presented_item("open_consultation", schema) - assert presented.has_single_page_notifications? + assert presented.display_single_page_notification_button? end end end diff --git a/test/presenters/detailed_guide_presenter_test.rb b/test/presenters/detailed_guide_presenter_test.rb index 439e6dd66..bb1f08037 100644 --- a/test/presenters/detailed_guide_presenter_test.rb +++ b/test/presenters/detailed_guide_presenter_test.rb @@ -86,6 +86,6 @@ def schema_name test "presents the single page notification button" do presented = presented_item("national_applicability_detailed_guide") - assert presented.has_single_page_notifications? + assert presented.display_single_page_notification_button? end end diff --git a/test/presenters/publication_presenter_test.rb b/test/presenters/publication_presenter_test.rb index 7f0f99f34..3ed4c1edb 100644 --- a/test/presenters/publication_presenter_test.rb +++ b/test/presenters/publication_presenter_test.rb @@ -76,7 +76,7 @@ def schema_name test "presents the single page notification button" do presented = presented_item("statistics_publication") - assert presented.has_single_page_notifications? + assert presented.display_single_page_notification_button? end test "hide_from_search_engines? returns false" do From 767f72d19bf62b89d369e6bfe6ae18faf0dc21ca Mon Sep 17 00:00:00 2001 From: Jessica Jones Date: Fri, 6 Dec 2024 15:52:49 +0000 Subject: [PATCH 2/6] Add check to document collections pages This was missed from https://github.com/alphagov/government-frontend/pull/2943/commits/4d2c8a49244f9b1a2e00095974d23525b94d978d No behavioural changes here, just making sure that we follow the same pattern for all document types --- app/views/shared/_document_collections_email_signup.html.erb | 2 +- test/presenters/document_collection_presenter_test.rb | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/views/shared/_document_collections_email_signup.html.erb b/app/views/shared/_document_collections_email_signup.html.erb index df50c701c..c934329bd 100644 --- a/app/views/shared/_document_collections_email_signup.html.erb +++ b/app/views/shared/_document_collections_email_signup.html.erb @@ -10,7 +10,7 @@ margin_bottom: 6 } %>

-<% else %> +<% elsif @content_item.display_single_page_notification_button? %> <%= render 'shared/single_page_notification_button', { content_item: @content_item, skip_account: @has_govuk_account ? "false" : "true" diff --git a/test/presenters/document_collection_presenter_test.rb b/test/presenters/document_collection_presenter_test.rb index 35d5c13f2..324c45009 100644 --- a/test/presenters/document_collection_presenter_test.rb +++ b/test/presenters/document_collection_presenter_test.rb @@ -119,6 +119,10 @@ class PresentedDocumentCollection < TestCase assert_nil nil, public_updated_at end end + + test "renders with the single page notification button" do + assert presented_item.display_single_page_notification_button? + end end class GroupWithMissingDocument < TestCase From 93531b0bb2652759242f39b7791eb952ea90d5e4 Mon Sep 17 00:00:00 2001 From: Jessica Jones Date: Fri, 6 Dec 2024 14:04:16 +0000 Subject: [PATCH 3/6] Hide email button if content not in English GOV.UK does not support email subscriptions to foreign language content. The single page notification button is present on the following document types, regardless of their locale: document collections, detailed guides, publications, call for evidence and consultations (except those pages that have a hardcoded exception [1]). The consequence has been users signing up to an email subscription that will never match a content change [2] We could make the button work so that clicking the button on eg a welsh page results in the user signing up to the equivalent content in English. But we decided against this: 1. There is no indication that there is a user need for that behaviour 2. The code to fetch the english base-path would be added to the button component in the gem. The logic for this button is already very complicated, and adding further complexity feels like a bad idea. 3. Not all our publishing applications create foreign content with a link back to the english version. So if the button was added to other content types in the future it wouldn't work as expected eg https://www.gov.uk/api/content/budd-dal-plant-16-19 4. Hiding the signup button is the same approach used elsewhere [3] [1] https://github.com/alphagov/government-frontend/blob/e7b9db33536f487991ba907502080e0625a066cc/app/presenters/content_item/single_page_notification_button.rb#L4 [2] https://github.com/alphagov/email-alert-service/blob/main/email_alert_service/models/major_change_message_processor.rb#L23-L26 [3] https://github.com/alphagov/collections/blob/ea307473313aa5138e6db6dd16eb26f1bf453bfd/app/views/world_location_news/show.html.erb#L56-L86 --- .../single_page_notification_button.rb | 2 +- test/integration/call_for_evidence_test.rb | 7 ++++++- test/integration/consultation_test.rb | 7 ++++++- test/integration/detailed_guide_test.rb | 7 ++++++- .../email_notifications_test.rb | 12 +++++++++--- test/integration/publication_test.rb | 7 ++++++- .../call_for_evidence_presenter_test.rb | 19 +++++++++++++++---- .../presenters/consultation_presenter_test.rb | 19 ++++++++++++++----- .../detailed_guide_presenter_test.rb | 15 ++++++++++++--- .../document_collection_presenter_test.rb | 12 ++++++++++-- test/presenters/publication_presenter_test.rb | 15 ++++++++++++--- test/test_helper.rb | 3 ++- 12 files changed, 99 insertions(+), 26 deletions(-) diff --git a/app/presenters/content_item/single_page_notification_button.rb b/app/presenters/content_item/single_page_notification_button.rb index 45690af32..d7d10e6b1 100644 --- a/app/presenters/content_item/single_page_notification_button.rb +++ b/app/presenters/content_item/single_page_notification_button.rb @@ -13,7 +13,7 @@ def page_is_on_exemption_list? end def display_single_page_notification_button? - !page_is_on_exemption_list? + !page_is_on_exemption_list? && I18n.locale == :en end end end diff --git a/test/integration/call_for_evidence_test.rb b/test/integration/call_for_evidence_test.rb index da0bb628b..8ba4e56b1 100644 --- a/test/integration/call_for_evidence_test.rb +++ b/test/integration/call_for_evidence_test.rb @@ -371,11 +371,16 @@ def teardown assert page.has_css?("a", text: "Twitter") end - test "renders with the single page notification button" do + test "renders with the single page notification button on English language pages" do setup_and_visit_content_item("open_call_for_evidence") assert page.has_css?(".gem-c-single-page-notification-button") end + test "does not render the single page notification button on foreign language pages" do + setup_and_visit_content_item("open_call_for_evidence", "locale" => "cy") + assert_not page.has_css?(".gem-c-single-page-notification-button") + end + test "does not render the single page notification button on exempt pages" do setup_and_visit_notification_exempt_page("open_call_for_evidence") assert_not page.has_css?(".gem-c-single-page-notification-button") diff --git a/test/integration/consultation_test.rb b/test/integration/consultation_test.rb index f54d2ee72..895a3d3d9 100644 --- a/test/integration/consultation_test.rb +++ b/test/integration/consultation_test.rb @@ -372,7 +372,7 @@ class ConsultationTest < ActionDispatch::IntegrationTest assert page.has_css?("a", text: "Twitter") end - test "renders with the single page notification button" do + test "renders with the single page notification button on English language pages" do setup_and_visit_content_item("open_consultation") assert page.has_css?(".gem-c-single-page-notification-button") @@ -391,4 +391,9 @@ class ConsultationTest < ActionDispatch::IntegrationTest setup_and_visit_notification_exempt_page("open_consultation") assert_not page.has_css?(".gem-c-single-page-notification-button") end + + test "does not render the single page notification button on foreign language pages" do + setup_and_visit_notification_exempt_page("open_consultation", "locale" => "cy") + assert_not page.has_css?(".gem-c-single-page-notification-button") + end end diff --git a/test/integration/detailed_guide_test.rb b/test/integration/detailed_guide_test.rb index e46d662bc..be34f53fd 100644 --- a/test/integration/detailed_guide_test.rb +++ b/test/integration/detailed_guide_test.rb @@ -82,7 +82,7 @@ class DetailedGuideTest < ActionDispatch::IntegrationTest assert_not_equal faq_schema["mainEntity"], [] end - test "renders with the single page notification button" do + test "renders with the single page notification button on English language pages" do setup_and_visit_content_item("detailed_guide") assert page.has_css?(".gem-c-single-page-notification-button") @@ -101,4 +101,9 @@ class DetailedGuideTest < ActionDispatch::IntegrationTest setup_and_visit_notification_exempt_page("detailed_guide") assert_not page.has_css?(".gem-c-single-page-notification-button") end + + test "does not render the single page notification button on foreign language pages" do + setup_and_visit_notification_exempt_page("detailed_guide", "locale" => "cy") + assert_not page.has_css?(".gem-c-single-page-notification-button") + end end diff --git a/test/integration/document_collection/email_notifications_test.rb b/test/integration/document_collection/email_notifications_test.rb index b2d272dbe..2673bfb60 100644 --- a/test/integration/document_collection/email_notifications_test.rb +++ b/test/integration/document_collection/email_notifications_test.rb @@ -20,8 +20,9 @@ def email_alert_frontend_signup_endpoint_enforce_account "/email/subscriptions/single-page/new" end - test "renders a signup link if the document collection has a taxonomy topic email override" do + test "renders a signup link if the document collection has a taxonomy topic email override and the page is in English" do content_item = get_content_example("document_collection") + content_item["locale"] = "en" content_item["links"]["taxonomy_topic_email_override"] = [{ "base_path" => taxonomy_topic_base_path.to_s }] stub_content_store_has_item(content_item["base_path"], content_item) visit_with_cachebust(content_item["base_path"]) @@ -31,7 +32,7 @@ def email_alert_frontend_signup_endpoint_enforce_account end test "renders the single page notification button with a form action of email-alert-frontend's non account signup endpoint" do - setup_and_visit_content_item("document_collection") + setup_and_visit_content_item("document_collection", "locale" => "en") form = page.find("form.gem-c-single-page-notification-button") assert_match(email_alert_frontend_signup_endpoint_no_account, form["action"]) @@ -55,7 +56,7 @@ def email_alert_frontend_signup_endpoint_enforce_account # Need to use Rack as Selenium, the default driver, doesn't provide header access, and we need to set a govuk_account_session header Capybara.current_driver = :rack_test mock_logged_in_session - setup_and_visit_content_item("document_collection") + setup_and_visit_content_item("document_collection", "locale" => "en") form = page.find("form.gem-c-single-page-notification-button") assert_match(email_alert_frontend_signup_endpoint_enforce_account, form["action"]) @@ -78,5 +79,10 @@ def email_alert_frontend_signup_endpoint_enforce_account # reset back to default driver Capybara.use_default_driver end + + test "does not render the single page notification button if the page is in a foreign language" do + setup_and_visit_content_item("document_collection", "locale" => "cy") + assert_not page.has_css?(".gem-c-single-page-notification-button") + end end end diff --git a/test/integration/publication_test.rb b/test/integration/publication_test.rb index f67875101..35a621d9e 100644 --- a/test/integration/publication_test.rb +++ b/test/integration/publication_test.rb @@ -269,7 +269,7 @@ class PublicationTest < ActionDispatch::IntegrationTest ]) end - test "renders with the single page notification button" do + test "renders with the single page notification button on English language pages" do setup_and_visit_content_item("publication") assert page.has_css?(".gem-c-single-page-notification-button") @@ -289,6 +289,11 @@ class PublicationTest < ActionDispatch::IntegrationTest assert_not page.has_css?(".gem-c-single-page-notification-button") end + test "does not render the single page notification button on foreign language pages" do + setup_and_visit_content_item("publication", "locale" => "cy") + assert_not page.has_css?(".gem-c-single-page-notification-button") + end + test "adds the noindex meta tag to '/government/publications/pension-credit-claim-form--2'" do overrides = { "base_path" => "/government/publications/pension-credit-claim-form--2" } setup_and_visit_content_item("publication-with-featured-attachments", overrides) diff --git a/test/presenters/call_for_evidence_presenter_test.rb b/test/presenters/call_for_evidence_presenter_test.rb index fdc98c56c..ca80f8919 100644 --- a/test/presenters/call_for_evidence_presenter_test.rb +++ b/test/presenters/call_for_evidence_presenter_test.rb @@ -152,11 +152,22 @@ def schema_name assert_equal "https://twitter.com/share?url=https%3A%2F%2Fwww.test.gov.uk%2Fgovernment%2Fcall_for_evidence%2Fyouth-vaping-call-for-evidence&text=Youth%20Vaping", presented_item("open_call_for_evidence").share_links[1][:href] end - test "presents the single page notification button" do - schema = schema_item("open_call_for_evidence") - presented = presented_item("open_call_for_evidence", schema) + test "displays the single page notification button on English pages" do + I18n.with_locale("en") do + schema = schema_item("open_call_for_evidence") + presented = presented_item("open_call_for_evidence", schema) + + assert presented.display_single_page_notification_button? + end + end + + test "does not display the single page notification button on foreign language pages" do + I18n.with_locale("fr") do + schema = schema_item("open_call_for_evidence") + presented = presented_item("open_call_for_evidence", schema) - assert presented.display_single_page_notification_button? + assert_not presented.display_single_page_notification_button? + end end end end diff --git a/test/presenters/consultation_presenter_test.rb b/test/presenters/consultation_presenter_test.rb index 4fbe64ccc..630b620e5 100644 --- a/test/presenters/consultation_presenter_test.rb +++ b/test/presenters/consultation_presenter_test.rb @@ -171,11 +171,20 @@ def schema_name assert_equal "https://twitter.com/share?url=https%3A%2F%2Fwww.test.gov.uk%2Fgovernment%2Fconsultations%2Fpostgraduate-doctoral-loans&text=Postgraduate%20doctoral%20loans", presented_item("open_consultation").share_links[1][:href] end - test "presents the single page notification button" do - schema = schema_item("open_consultation") - presented = presented_item("open_consultation", schema) - - assert presented.display_single_page_notification_button? + test "displays the single page notification button on English pages" do + I18n.with_locale("en") do + schema = schema_item("open_consultation") + presented = presented_item("open_consultation", schema) + assert presented.display_single_page_notification_button? + end + end + + test "does not display the single page notification button on foreign language pages" do + I18n.with_locale("fr") do + schema = schema_item("open_consultation") + presented = presented_item("open_consultation", schema) + assert_not presented.display_single_page_notification_button? + end end end end diff --git a/test/presenters/detailed_guide_presenter_test.rb b/test/presenters/detailed_guide_presenter_test.rb index bb1f08037..26d384318 100644 --- a/test/presenters/detailed_guide_presenter_test.rb +++ b/test/presenters/detailed_guide_presenter_test.rb @@ -84,8 +84,17 @@ def schema_name assert_equal presented.logo, expected end - test "presents the single page notification button" do - presented = presented_item("national_applicability_detailed_guide") - assert presented.display_single_page_notification_button? + test "displays the single page notification button on English pages" do + I18n.with_locale("en") do + presented = presented_item("national_applicability_detailed_guide") + assert presented.display_single_page_notification_button? + end + end + + test "does not display the single page notification button on foreign language pages" do + I18n.with_locale("fr") do + presented = presented_item("national_applicability_detailed_guide") + assert_not presented.display_single_page_notification_button? + end end end diff --git a/test/presenters/document_collection_presenter_test.rb b/test/presenters/document_collection_presenter_test.rb index 324c45009..efe9d7cd2 100644 --- a/test/presenters/document_collection_presenter_test.rb +++ b/test/presenters/document_collection_presenter_test.rb @@ -120,8 +120,16 @@ class PresentedDocumentCollection < TestCase end end - test "renders with the single page notification button" do - assert presented_item.display_single_page_notification_button? + test "displays the single page notification button on English pages" do + I18n.with_locale("en") do + assert presented_item.display_single_page_notification_button? + end + end + + test "does not display the single page notification button on foreign language pages" do + I18n.with_locale("fr") do + assert_not presented_item.display_single_page_notification_button? + end end end diff --git a/test/presenters/publication_presenter_test.rb b/test/presenters/publication_presenter_test.rb index 3ed4c1edb..2a85d1414 100644 --- a/test/presenters/publication_presenter_test.rb +++ b/test/presenters/publication_presenter_test.rb @@ -74,9 +74,18 @@ def schema_name assert_equal(presented.national_applicability[:wales][:alternative_url], "http://wales.gov.uk/topics/statistics/headlines/housing2012/121025/?lang=en") end - test "presents the single page notification button" do - presented = presented_item("statistics_publication") - assert presented.display_single_page_notification_button? + test "displays the single page notification button on English pages" do + I18n.with_locale("en") do + presented = presented_item("statistics_publication") + assert presented.display_single_page_notification_button? + end + end + + test "does not display the single page notification button on foreign language pages" do + I18n.with_locale("fr") do + presented = presented_item("statistics_publication") + assert_not presented.display_single_page_notification_button? + end end test "hide_from_search_engines? returns false" do diff --git a/test/test_helper.rb b/test/test_helper.rb index f036dc4d0..4d24c0ee4 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -194,8 +194,9 @@ def setup_and_visit_content_item_with_taxonomy_topic_email_override(name) end end - def setup_and_visit_notification_exempt_page(name) + def setup_and_visit_notification_exempt_page(name, overrides = {}) @content_item = get_content_example(name).tap do |item| + item.deep_merge(overrides) item["content_id"] = ContentItem::SinglePageNotificationButton::EXEMPTION_LIST[0] stub_content_store_has_item(item["base_path"], item.to_json) visit_with_cachebust((item["base_path"]).to_s) From f1944ac3b689e8d129bf7ca37bec14c015fad6fb Mon Sep 17 00:00:00 2001 From: Jessica Jones Date: Fri, 6 Dec 2024 16:06:42 +0000 Subject: [PATCH 4/6] Create DocumentCollection::SignupLink module Following the pattern in this repo of having small module that are inherited by the document type presenter. --- .../document_collection/signup_link.rb | 11 +++++++ .../document_collection_presenter.rb | 5 +-- ...document_collections_email_signup.html.erb | 2 +- .../document_collection/signup_link_test.rb | 33 +++++++++++++++++++ 4 files changed, 46 insertions(+), 5 deletions(-) create mode 100644 app/presenters/document_collection/signup_link.rb create mode 100644 test/presenters/document_collection/signup_link_test.rb diff --git a/app/presenters/document_collection/signup_link.rb b/app/presenters/document_collection/signup_link.rb new file mode 100644 index 000000000..e19112614 --- /dev/null +++ b/app/presenters/document_collection/signup_link.rb @@ -0,0 +1,11 @@ +module DocumentCollection + module SignupLink + def show_email_signup_link? + taxonomy_topic_email_override_base_path.present? + end + + def taxonomy_topic_email_override_base_path + content_item.dig("links", "taxonomy_topic_email_override", 0, "base_path") + end + end +end diff --git a/app/presenters/document_collection_presenter.rb b/app/presenters/document_collection_presenter.rb index d94fb03b1..67440920c 100644 --- a/app/presenters/document_collection_presenter.rb +++ b/app/presenters/document_collection_presenter.rb @@ -5,6 +5,7 @@ class DocumentCollectionPresenter < ContentItemPresenter include ContentItem::TitleAndContext include ContentItem::ContentsList include ContentItem::SinglePageNotificationButton + include DocumentCollection::SignupLink def contents_items groups.map do |group| @@ -54,10 +55,6 @@ def group_heading(group) ) end - def taxonomy_topic_email_override_base_path - content_item.dig("links", "taxonomy_topic_email_override", 0, "base_path") - end - private def group_document_link_public_updated_at(link) diff --git a/app/views/shared/_document_collections_email_signup.html.erb b/app/views/shared/_document_collections_email_signup.html.erb index c934329bd..136912b68 100644 --- a/app/views/shared/_document_collections_email_signup.html.erb +++ b/app/views/shared/_document_collections_email_signup.html.erb @@ -1,4 +1,4 @@ -<% if @content_item.taxonomy_topic_email_override_base_path.present? %> +<% if @content_item.show_email_signup_link? %>
"/a-taxonomy-topic", + }, + ] + assert item.show_email_signup_link? + end +end From bf2821d3c8f96ee9e3a9aadbf2cea12bd25aa4dc Mon Sep 17 00:00:00 2001 From: Jessica Jones Date: Fri, 6 Dec 2024 14:41:22 +0000 Subject: [PATCH 5/6] Only allow email sign up link on english document collections --- .../document_collection/signup_link.rb | 2 +- .../email_notifications_test.rb | 9 ++++++ .../document_collection/signup_link_test.rb | 30 ++++++++++++++----- 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/app/presenters/document_collection/signup_link.rb b/app/presenters/document_collection/signup_link.rb index e19112614..e6b509551 100644 --- a/app/presenters/document_collection/signup_link.rb +++ b/app/presenters/document_collection/signup_link.rb @@ -1,7 +1,7 @@ module DocumentCollection module SignupLink def show_email_signup_link? - taxonomy_topic_email_override_base_path.present? + taxonomy_topic_email_override_base_path.present? && I18n.locale == :en end def taxonomy_topic_email_override_base_path diff --git a/test/integration/document_collection/email_notifications_test.rb b/test/integration/document_collection/email_notifications_test.rb index 2673bfb60..b716c8fd1 100644 --- a/test/integration/document_collection/email_notifications_test.rb +++ b/test/integration/document_collection/email_notifications_test.rb @@ -84,5 +84,14 @@ def email_alert_frontend_signup_endpoint_enforce_account setup_and_visit_content_item("document_collection", "locale" => "cy") assert_not page.has_css?(".gem-c-single-page-notification-button") end + + test "does not render the email signup link if the page is in a foreign language" do + content_item = get_content_example("document_collection") + content_item["links"]["taxonomy_topic_email_override"] = [{ "base_path" => taxonomy_topic_base_path.to_s }] + content_item["locale"] = "cy" + stub_content_store_has_item(content_item["base_path"], content_item) + visit_with_cachebust(content_item["base_path"]) + assert_not page.has_css?(".gem-c-signup-link") + end end end diff --git a/test/presenters/document_collection/signup_link_test.rb b/test/presenters/document_collection/signup_link_test.rb index 10c58e004..30f8acdb2 100644 --- a/test/presenters/document_collection/signup_link_test.rb +++ b/test/presenters/document_collection/signup_link_test.rb @@ -21,13 +21,27 @@ def initialize assert_equal false, item.show_email_signup_link? end - test "show_email_signup_link? returns true if there is a linked taxonomy_topic_email_override" do - item = DummyContentItem.new - item.content_item["links"]["taxonomy_topic_email_override"] = [ - { - "base_path" => "/a-taxonomy-topic", - }, - ] - assert item.show_email_signup_link? + test "show_email_signup_link? returns false if the locale is not en" do + I18n.with_locale("fr") do + item = DummyContentItem.new + item.content_item["links"]["taxonomy_topic_email_override"] = [ + { + "base_path" => "/a-taxonomy-topic", + }, + ] + assert_equal false, item.show_email_signup_link? + end + end + + test "show_email_signup_link? returns true if there is a linked taxonomy_topic_email_override and the locale is en" do + I18n.with_locale("en") do + item = DummyContentItem.new + item.content_item["links"]["taxonomy_topic_email_override"] = [ + { + "base_path" => "/a-taxonomy-topic", + }, + ] + assert item.show_email_signup_link? + end end end From 61e907461479c9d8be6cc1ff3df1bdaa81497da1 Mon Sep 17 00:00:00 2001 From: Jessica Jones Date: Mon, 9 Dec 2024 14:37:35 +0000 Subject: [PATCH 6/6] Small refactor --- .../_single_page_notification_button.html.erb | 44 ++++++++++--------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/app/views/shared/_single_page_notification_button.html.erb b/app/views/shared/_single_page_notification_button.html.erb index 64b435139..9ec9afdba 100644 --- a/app/views/shared/_single_page_notification_button.html.erb +++ b/app/views/shared/_single_page_notification_button.html.erb @@ -1,24 +1,26 @@ -<% - default_ga4_data_attributes = { - module: "ga4-link-tracker", - ga4_link: { - event_name: "navigation", - type: "subscribe", - index_link: 1, - index_total: 2, - section: "Top" +<% if @content_item.display_single_page_notification_button? %> + <% + default_ga4_data_attributes = { + module: "ga4-link-tracker", + ga4_link: { + event_name: "navigation", + type: "subscribe", + index_link: 1, + index_total: 2, + section: "Top" + } } - } -%> + %> -<% ga4_data_attributes = ga4_data_attributes || default_ga4_data_attributes %> -<% skip_account = skip_account || "false" %> + <% ga4_data_attributes = ga4_data_attributes || default_ga4_data_attributes %> + <% skip_account = skip_account || "false" %> -<%= render 'govuk_publishing_components/components/single_page_notification_button', { - base_path: @content_item.base_path, - js_enhancement: @has_govuk_account, - ga4_data_attributes: ga4_data_attributes, - margin_bottom: 6, - button_location: "top", - skip_account: skip_account, -} if @content_item.display_single_page_notification_button? %> + <%= render 'govuk_publishing_components/components/single_page_notification_button', { + base_path: @content_item.base_path, + js_enhancement: @has_govuk_account, + ga4_data_attributes: ga4_data_attributes, + margin_bottom: 6, + button_location: "top", + skip_account: skip_account, + } %> +<% end %>