From b61e51986f145b3d5c40c6e8ba74f39ef67e9eeb Mon Sep 17 00:00:00 2001 From: hannako Date: Mon, 14 Jun 2021 11:51:26 +0100 Subject: [PATCH 1/6] Improve structure of the brexit link hash We are using the brexit_link hash to provide data to the frontend for different sections of the page. So have given the hash some nesting to reflect that. --- .../content_item/brexit_hub_page.rb | 19 +++++++++++++------ .../content_items/detailed_guide.html.erb | 8 ++++---- .../detailed_guide/_brexit_links.html.erb | 19 +++++++++---------- 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/app/presenters/content_item/brexit_hub_page.rb b/app/presenters/content_item/brexit_hub_page.rb index f2e3b39ab..843ffed9d 100644 --- a/app/presenters/content_item/brexit_hub_page.rb +++ b/app/presenters/content_item/brexit_hub_page.rb @@ -1,21 +1,28 @@ module ContentItem module BrexitHubPage BREXIT_BUSINESS_PAGE_CONTENT_ID = "91cd6143-69d5-4f27-99ff-a52fb0d51c78".freeze - BREXIT_CITIZEN_PAGE_CONTENT_ID = "6555e0bf-c270-4cf9-a0c5-d20b95fab7f1".freeze - BREXIT_HUB_PAGE_CONTENT_IDS = [BREXIT_BUSINESS_PAGE_CONTENT_ID, BREXIT_CITIZEN_PAGE_CONTENT_ID].freeze BREXIT_BUSINESS_PAGE_PATH = "/guidance/brexit-guidance-for-businesses".freeze + + BREXIT_CITIZEN_PAGE_CONTENT_ID = "6555e0bf-c270-4cf9-a0c5-d20b95fab7f1".freeze BREXIT_CITIZEN_PAGE_PATH = "/guidance/brexit-guidance-for-individuals-and-families".freeze def brexit_links { ContentItem::BrexitHubPage::BREXIT_BUSINESS_PAGE_CONTENT_ID => { - text: I18n.t("brexit.citizen_link"), - path: BREXIT_CITIZEN_PAGE_PATH, + nav_link: { + text: I18n.t("brexit.citizen_link"), + path: BREXIT_CITIZEN_PAGE_PATH, + track_label: "Guidance nav link", + }, track_category: "brexit-business-page", }, ContentItem::BrexitHubPage::BREXIT_CITIZEN_PAGE_CONTENT_ID => { - text: I18n.t("brexit.business_link"), - path: BREXIT_BUSINESS_PAGE_PATH, + nav_link: { + text: I18n.t("brexit.business_link"), + path: BREXIT_BUSINESS_PAGE_PATH, + track_label: "Guidance nav link", + + }, track_category: "brexit-citizen-page", }, } diff --git a/app/views/content_items/detailed_guide.html.erb b/app/views/content_items/detailed_guide.html.erb index 8b56be063..f3b78c638 100644 --- a/app/views/content_items/detailed_guide.html.erb +++ b/app/views/content_items/detailed_guide.html.erb @@ -17,13 +17,13 @@

<%= I18n.t("brexit.heading_prefix") %> <%= link_to( - brexit_link[:text], - brexit_link[:path], + brexit_link[:nav_link][:text], + brexit_link[:nav_link][:path], class: "govuk-link", data: { - track_action: brexit_link[:path], + track_action: brexit_link[:nav_link][:path], track_category: brexit_link[:track_category], - track_label: "Guidance nav link", + track_label: brexit_link[:nav_link][:track_label], module: 'gem-track-click', }) %>.

diff --git a/app/views/content_items/detailed_guide/_brexit_links.html.erb b/app/views/content_items/detailed_guide/_brexit_links.html.erb index 392789dac..2a275d482 100644 --- a/app/views/content_items/detailed_guide/_brexit_links.html.erb +++ b/app/views/content_items/detailed_guide/_brexit_links.html.erb @@ -9,16 +9,15 @@ } %> <% end %> <% if section[:links].present? %> - <% track_category = brexit_link[:track_category] %> - <% links = section[:links].map do |link| - link_to(link[:text], link[:path], class: "govuk-link", data: { - track_action: link[:path], - track_category: track_category, - track_label: section[:title][:text] || "", - module: 'gem-track-click', - }) - end - %> + <% links = section[:links].map do |link| + link_to(link[:text], link[:path], class: "govuk-link", data: { + track_action: link[:path], + track_category: brexit_link[:track_category], + track_label: section[:title][:text] || "", + module: 'gem-track-click', + }) + end + %> <%= render "govuk_publishing_components/components/list", { items: links, visible_counters: true, From 6389e8a2e5d53227cb53a9989f225dc728cc65fe Mon Sep 17 00:00:00 2001 From: hannako Date: Mon, 14 Jun 2021 12:26:41 +0100 Subject: [PATCH 2/6] Remove reference to hub pages UCD and analytics colleagues are refering to child taxon pages rather than hub pages. Better if we all use the same language, so amending now. --- .../content_item/{brexit_hub_page.rb => brexit_taxons.rb} | 6 +++--- app/presenters/content_item_presenter.rb | 2 +- test/integration/detailed_guide_test.rb | 6 +++--- test/test_helper.rb | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) rename app/presenters/content_item/{brexit_hub_page.rb => brexit_taxons.rb} (85%) diff --git a/app/presenters/content_item/brexit_hub_page.rb b/app/presenters/content_item/brexit_taxons.rb similarity index 85% rename from app/presenters/content_item/brexit_hub_page.rb rename to app/presenters/content_item/brexit_taxons.rb index 843ffed9d..b307f04a1 100644 --- a/app/presenters/content_item/brexit_hub_page.rb +++ b/app/presenters/content_item/brexit_taxons.rb @@ -1,5 +1,5 @@ module ContentItem - module BrexitHubPage + module BrexitTaxons BREXIT_BUSINESS_PAGE_CONTENT_ID = "91cd6143-69d5-4f27-99ff-a52fb0d51c78".freeze BREXIT_BUSINESS_PAGE_PATH = "/guidance/brexit-guidance-for-businesses".freeze @@ -8,7 +8,7 @@ module BrexitHubPage def brexit_links { - ContentItem::BrexitHubPage::BREXIT_BUSINESS_PAGE_CONTENT_ID => { + ContentItem::BrexitTaxons::BREXIT_BUSINESS_PAGE_CONTENT_ID => { nav_link: { text: I18n.t("brexit.citizen_link"), path: BREXIT_CITIZEN_PAGE_PATH, @@ -16,7 +16,7 @@ def brexit_links }, track_category: "brexit-business-page", }, - ContentItem::BrexitHubPage::BREXIT_CITIZEN_PAGE_CONTENT_ID => { + ContentItem::BrexitTaxons::BREXIT_CITIZEN_PAGE_CONTENT_ID => { nav_link: { text: I18n.t("brexit.business_link"), path: BREXIT_BUSINESS_PAGE_PATH, diff --git a/app/presenters/content_item_presenter.rb b/app/presenters/content_item_presenter.rb index 547500e6e..f8e828c39 100644 --- a/app/presenters/content_item_presenter.rb +++ b/app/presenters/content_item_presenter.rb @@ -1,6 +1,6 @@ class ContentItemPresenter include ContentItem::Withdrawable - include ContentItem::BrexitHubPage + include ContentItem::BrexitTaxons attr_reader :content_item, :requested_path, diff --git a/test/integration/detailed_guide_test.rb b/test/integration/detailed_guide_test.rb index 73c23f5c2..176c71626 100644 --- a/test/integration/detailed_guide_test.rb +++ b/test/integration/detailed_guide_test.rb @@ -104,13 +104,13 @@ class DetailedGuideTest < ActionDispatch::IntegrationTest # renders description field as a custom link assert_not page.has_text?(@content_item["description"]) link_text = "Brexit guidance for individuals and families" - assert page.has_link?(link_text, href: ContentItem::BrexitHubPage::BREXIT_CITIZEN_PAGE_PATH) + assert page.has_link?(link_text, href: ContentItem::BrexitTaxons::BREXIT_CITIZEN_PAGE_PATH) setup_and_visit_brexit_child_taxon("citizen") assert_not page.has_text?(@content_item["description"]) link_text = "Brexit guidance for businesses" - assert page.has_link?(link_text, href: ContentItem::BrexitHubPage::BREXIT_BUSINESS_PAGE_PATH) + assert page.has_link?(link_text, href: ContentItem::BrexitTaxons::BREXIT_BUSINESS_PAGE_PATH) # adds GA tracking to the li links track_action = find_link("Foreign travel advice")["data-track-action"] @@ -126,7 +126,7 @@ class DetailedGuideTest < ActionDispatch::IntegrationTest track_category = find_link("Brexit guidance for businesses")["data-track-category"] track_label = find_link("Brexit guidance for businesses")["data-track-label"] - assert_equal ContentItem::BrexitHubPage::BREXIT_BUSINESS_PAGE_PATH, track_action + assert_equal ContentItem::BrexitTaxons::BREXIT_BUSINESS_PAGE_PATH, track_action assert_equal "brexit-citizen-page", track_category assert_equal "Guidance nav link", track_label end diff --git a/test/test_helper.rb b/test/test_helper.rb index 50c127ee2..fcc887260 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -187,11 +187,11 @@ def setup_and_visit_brexit_child_taxon(type = nil) end def brexit_citizen_id - ContentItem::BrexitHubPage::BREXIT_CITIZEN_PAGE_CONTENT_ID + ContentItem::BrexitTaxons::BREXIT_CITIZEN_PAGE_CONTENT_ID end def brexit_business_id - ContentItem::BrexitHubPage::BREXIT_BUSINESS_PAGE_CONTENT_ID + ContentItem::BrexitTaxons::BREXIT_BUSINESS_PAGE_CONTENT_ID end def brexit_body_content From 17de2418ddc1679b71c8e02091fd65aa565c2464 Mon Sep 17 00:00:00 2001 From: hannako Date: Mon, 14 Jun 2021 12:04:19 +0100 Subject: [PATCH 3/6] Rename partial to reflect that it contains sections (heading plus links) --- app/views/content_items/detailed_guide.html.erb | 2 +- .../{_brexit_links.html.erb => _brexit_sections.html.erb} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename app/views/content_items/detailed_guide/{_brexit_links.html.erb => _brexit_sections.html.erb} (100%) diff --git a/app/views/content_items/detailed_guide.html.erb b/app/views/content_items/detailed_guide.html.erb index f3b78c638..8db1c7fcd 100644 --- a/app/views/content_items/detailed_guide.html.erb +++ b/app/views/content_items/detailed_guide.html.erb @@ -49,7 +49,7 @@ } unless brexit_link %> <% if brexit_link %> - <%= render partial: 'content_items/detailed_guide/brexit_links', locals: { brexit_link: brexit_link } %> + <%= render partial: 'content_items/detailed_guide/brexit_sections', locals: { brexit_link: brexit_link } %> <% else %> <%= render 'govuk_publishing_components/components/govspeak', {} do %> <%= raw(@content_item.govspeak_body[:content]) %> diff --git a/app/views/content_items/detailed_guide/_brexit_links.html.erb b/app/views/content_items/detailed_guide/_brexit_sections.html.erb similarity index 100% rename from app/views/content_items/detailed_guide/_brexit_links.html.erb rename to app/views/content_items/detailed_guide/_brexit_sections.html.erb From 4bd374fd82e76c2c2c96350053d696fa83f4e4b5 Mon Sep 17 00:00:00 2001 From: hannako Date: Mon, 14 Jun 2021 12:16:30 +0100 Subject: [PATCH 4/6] Rename brexit_link --> brexit_child_taxon Using the term brexit_link was a bit confusing now there is a partial and a parser to build links for these brexit pages. --- app/presenters/content_item/brexit_taxons.rb | 6 ++-- .../content_items/detailed_guide.html.erb | 28 +++++++++---------- .../detailed_guide/_brexit_sections.html.erb | 2 +- app/views/layouts/application.html.erb | 2 +- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/app/presenters/content_item/brexit_taxons.rb b/app/presenters/content_item/brexit_taxons.rb index b307f04a1..f04202f02 100644 --- a/app/presenters/content_item/brexit_taxons.rb +++ b/app/presenters/content_item/brexit_taxons.rb @@ -6,7 +6,7 @@ module BrexitTaxons BREXIT_CITIZEN_PAGE_CONTENT_ID = "6555e0bf-c270-4cf9-a0c5-d20b95fab7f1".freeze BREXIT_CITIZEN_PAGE_PATH = "/guidance/brexit-guidance-for-individuals-and-families".freeze - def brexit_links + def brexit_child_taxons { ContentItem::BrexitTaxons::BREXIT_BUSINESS_PAGE_CONTENT_ID => { nav_link: { @@ -28,8 +28,8 @@ def brexit_links } end - def brexit_link - brexit_links[content_item["content_id"]] + def brexit_child_taxon + brexit_child_taxons[content_item["content_id"]] end end end diff --git a/app/views/content_items/detailed_guide.html.erb b/app/views/content_items/detailed_guide.html.erb index 8db1c7fcd..b7d3bfaa1 100644 --- a/app/views/content_items/detailed_guide.html.erb +++ b/app/views/content_items/detailed_guide.html.erb @@ -6,24 +6,24 @@
- <% brexit_link = @content_item.brexit_link %> - <% title_and_context = brexit_link ? { title: @content_item.title } : @content_item.title_and_context %> + <% brexit_child_taxon = @content_item.brexit_child_taxon %> + <% title_and_context = brexit_child_taxon ? { title: @content_item.title } : @content_item.title_and_context %> <%= render 'govuk_publishing_components/components/title', title_and_context %>
<%= render 'shared/translations' %>
- <% if brexit_link %> + <% if brexit_child_taxon %>

<%= I18n.t("brexit.heading_prefix") %> <%= link_to( - brexit_link[:nav_link][:text], - brexit_link[:nav_link][:path], + brexit_child_taxon[:nav_link][:text], + brexit_child_taxon[:nav_link][:path], class: "govuk-link", data: { - track_action: brexit_link[:nav_link][:path], - track_category: brexit_link[:track_category], - track_label: brexit_link[:nav_link][:track_label], + track_action: brexit_child_taxon[:nav_link][:path], + track_category: brexit_child_taxon[:track_category], + track_label: brexit_child_taxon[:nav_link][:track_label], module: 'gem-track-click', }) %>.

@@ -34,7 +34,7 @@
-<%= render 'shared/publisher_metadata_with_logo' unless brexit_link %> +<%= render 'shared/publisher_metadata_with_logo' unless brexit_child_taxon %> <%= render 'shared/history_notice', content_item: @content_item %> <%= render 'govuk_publishing_components/components/notice', @content_item.withdrawal_notice_component %> @@ -46,10 +46,10 @@ <%= render "govuk_publishing_components/components/print_link", { margin_top: 0, margin_bottom: 6, - } unless brexit_link %> + } unless brexit_child_taxon %> - <% if brexit_link %> - <%= render partial: 'content_items/detailed_guide/brexit_sections', locals: { brexit_link: brexit_link } %> + <% if brexit_child_taxon %> + <%= render partial: 'content_items/detailed_guide/brexit_sections', locals: { brexit_child_taxon: brexit_child_taxon } %> <% else %> <%= render 'govuk_publishing_components/components/govspeak', {} do %> <%= raw(@content_item.govspeak_body[:content]) %> @@ -61,13 +61,13 @@ published: @content_item.published, last_updated: @content_item.updated, history: @content_item.history - } unless brexit_link %> + } unless brexit_child_taxon %>
<% end %> <%= render "govuk_publishing_components/components/print_link", { margin_top: 0, margin_bottom: 6, - } unless brexit_link %> + } unless brexit_child_taxon %> <%= render 'shared/sidebar_navigation' %> diff --git a/app/views/content_items/detailed_guide/_brexit_sections.html.erb b/app/views/content_items/detailed_guide/_brexit_sections.html.erb index 2a275d482..278f9db97 100644 --- a/app/views/content_items/detailed_guide/_brexit_sections.html.erb +++ b/app/views/content_items/detailed_guide/_brexit_sections.html.erb @@ -12,7 +12,7 @@ <% links = section[:links].map do |link| link_to(link[:text], link[:path], class: "govuk-link", data: { track_action: link[:path], - track_category: brexit_link[:track_category], + track_category: brexit_child_taxon[:track_category], track_label: section[:title][:text] || "", module: 'gem-track-click', }) diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 661ad7551..01996d918 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -42,7 +42,7 @@ <% unless @do_not_show_breadcrumbs %> <% if @content_item.try(:back_link) %> <%= render 'govuk_publishing_components/components/back_link', href: @content_item.back_link %> - <% elsif @content_item.brexit_link %> + <% elsif @content_item.brexit_child_taxon %> <%= render 'govuk_publishing_components/components/breadcrumbs', breadcrumbs: [ { url: "/", title: "Home" } , { url: "/transition", title: "Brexit" } ] %> <% else %> <%= render 'govuk_publishing_components/components/contextual_breadcrumbs', content_item: @content_item.content_item.parsed_content %> From 3ae3a1f9a95d0dd092d390db52865422cba1de0e Mon Sep 17 00:00:00 2001 From: hannako Date: Mon, 14 Jun 2021 15:36:31 +0100 Subject: [PATCH 5/6] Update url in breadcrumbs --- app/views/layouts/application.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 01996d918..335711159 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -43,7 +43,7 @@ <% if @content_item.try(:back_link) %> <%= render 'govuk_publishing_components/components/back_link', href: @content_item.back_link %> <% elsif @content_item.brexit_child_taxon %> - <%= render 'govuk_publishing_components/components/breadcrumbs', breadcrumbs: [ { url: "/", title: "Home" } , { url: "/transition", title: "Brexit" } ] %> + <%= render 'govuk_publishing_components/components/breadcrumbs', breadcrumbs: [ { url: "/", title: "Home" } , { url: "/brexit", title: "Brexit" } ] %> <% else %> <%= render 'govuk_publishing_components/components/contextual_breadcrumbs', content_item: @content_item.content_item.parsed_content %> <% end %> From 32d1e9a0553a3eb9e6ce64d88def600090fe60fc Mon Sep 17 00:00:00 2001 From: hannako Date: Tue, 15 Jun 2021 10:37:47 +0100 Subject: [PATCH 6/6] remove namespacing --- app/presenters/content_item/brexit_taxons.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/presenters/content_item/brexit_taxons.rb b/app/presenters/content_item/brexit_taxons.rb index f04202f02..593dd4902 100644 --- a/app/presenters/content_item/brexit_taxons.rb +++ b/app/presenters/content_item/brexit_taxons.rb @@ -8,7 +8,7 @@ module BrexitTaxons def brexit_child_taxons { - ContentItem::BrexitTaxons::BREXIT_BUSINESS_PAGE_CONTENT_ID => { + BREXIT_BUSINESS_PAGE_CONTENT_ID => { nav_link: { text: I18n.t("brexit.citizen_link"), path: BREXIT_CITIZEN_PAGE_PATH, @@ -16,7 +16,7 @@ def brexit_child_taxons }, track_category: "brexit-business-page", }, - ContentItem::BrexitTaxons::BREXIT_CITIZEN_PAGE_CONTENT_ID => { + BREXIT_CITIZEN_PAGE_CONTENT_ID => { nav_link: { text: I18n.t("brexit.business_link"), path: BREXIT_BUSINESS_PAGE_PATH,