From 092b64e86ffb028d9e5ff9ecd048cecf002ed723 Mon Sep 17 00:00:00 2001 From: hannako Date: Fri, 28 May 2021 18:13:56 +0100 Subject: [PATCH 1/5] Create parser to process html body content - Whitehall sends the body content of detailed_guides as a string of html in the details attribute of the content item. The brexit child taxon pages are detailed_guides but we need to add some tracking to the links contained in this blob. So we need to parse it first. --- app/lib/body_parser.rb | 67 ++++++++++++++++++++++ test/unit/body_parser_test.rb | 104 ++++++++++++++++++++++++++++++++++ 2 files changed, 171 insertions(+) create mode 100644 app/lib/body_parser.rb create mode 100644 test/unit/body_parser_test.rb diff --git a/app/lib/body_parser.rb b/app/lib/body_parser.rb new file mode 100644 index 000000000..b2ec8d0f8 --- /dev/null +++ b/app/lib/body_parser.rb @@ -0,0 +1,67 @@ +class BodyParser + def initialize(body) + @body = body + end + + def title_and_link_sections + if raw_title_and_link_sections.any? + raw_title_and_link_sections.map do |section| + s = parse(section) + { + title: parsed_title(s), + links: parsed_links(s), + } + end + end + end + +private + + attr_reader :body + + def parse(html) + Nokogiri::HTML.parse(html) + rescue Nokogiri::XML::SyntaxError + "" + end + + def parsed_body + @parsed_body ||= parse(body) + end + + def raw_body_content + if parsed_body.present? + parsed_body.search("div.govspeak").children + end + end + + def raw_title_and_link_sections + if raw_body_content.any? + raw_body_content.to_s.split(/(?=

'\ + '

Travel to the EU

\n \n'\ + '\n'\ + '

Travel to the UK

\n \n'\ + '\n'\ + "" + end + + def html_missing_section_headings + '
'\ + '\n'\ + '\n'\ + "
" + end + + def subject + @subject ||= BodyParser.new(html) + end + + test "#title_and_link_sections" do + expected = [ + { + title: "Travel to the EU", + links: [ + { + path: "/foreign-travel-advice", + text: "Foreign travel advice", + }, + { + path: "/visit-eu", + text: "Travelling to the EU", + }, + ], + }, + { + title: "Travel to the UK", + links: [ + { + path: "/local-travel-advice", + text: "Local travel advice", + }, + { + path: "/visit-uk", + text: "Travelling to the UK", + }, + ], + }, + ] + + assert_equal expected, subject.title_and_link_sections + end + + test "when parsing html missing the section headings" do + subject = BodyParser.new(html_missing_section_headings) + expected = [ + { + title: "", + links: [ + { + path: "/foreign-travel-advice", + text: "Foreign travel advice", + }, + { + path: "/visit-eu", + text: "Travelling to the EU", + }, + { + path: "/local-travel-advice", + text: "Local travel advice", + }, + { + path: "/visit-uk", + text: "Travelling to the UK", + }, + ], + }, + ] + assert_equal expected, subject.title_and_link_sections + end +end From a12c5b09a8a9f37b05687dbad621459f05a0831e Mon Sep 17 00:00:00 2001 From: hannako Date: Wed, 9 Jun 2021 15:22:39 +0100 Subject: [PATCH 2/5] Small refactor of detailed_guide view --- app/views/content_items/detailed_guide.html.erb | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/app/views/content_items/detailed_guide.html.erb b/app/views/content_items/detailed_guide.html.erb index 25484c57d..b98f7619c 100644 --- a/app/views/content_items/detailed_guide.html.erb +++ b/app/views/content_items/detailed_guide.html.erb @@ -6,15 +6,16 @@
- <% title_and_context = @content_item.brexit_link ? { title: @content_item.title } : @content_item.title_and_context %> + <% brexit_link = @content_item.brexit_link %> + <% title_and_context = brexit_link ? { title: @content_item.title } : @content_item.title_and_context %> <%= render 'govuk_publishing_components/components/title', title_and_context %>
<%= render 'shared/translations' %>
- <% if @content_item.brexit_link %> + <% if brexit_link %>

- <%= I18n.t("brexit.heading_prefix") %> <%= link_to @content_item.brexit_link[:text], @content_item.brexit_link[:path] %>. + <%= I18n.t("brexit.heading_prefix") %> <%= link_to brexit_link[:text], brexit_link[:path] %>.

<% else %> @@ -23,7 +24,7 @@
-<%= render 'shared/publisher_metadata_with_logo' unless @content_item.brexit_link %> +<%= render 'shared/publisher_metadata_with_logo' unless brexit_link %> <%= render 'shared/history_notice', content_item: @content_item %> <%= render 'govuk_publishing_components/components/notice', @content_item.withdrawal_notice_component %> @@ -35,7 +36,7 @@ <%= render "govuk_publishing_components/components/print_link", { margin_top: 0, margin_bottom: 6, - } unless @content_item.brexit_link %> + } unless brexit_link %> <%= render 'govuk_publishing_components/components/govspeak', {} do %> <%= raw(@content_item.govspeak_body[:content]) %> @@ -46,13 +47,13 @@ published: @content_item.published, last_updated: @content_item.updated, history: @content_item.history - } unless @content_item.brexit_link %> + } unless brexit_link %> <% end %> <%= render "govuk_publishing_components/components/print_link", { margin_top: 0, margin_bottom: 6, - } unless @content_item.brexit_link %> + } unless brexit_link %> <%= render 'shared/sidebar_navigation' %> From 1a1c8e9d935d40ddbc02b66f04c0367d67298e2c Mon Sep 17 00:00:00 2001 From: hannako Date: Wed, 9 Jun 2021 16:05:25 +0100 Subject: [PATCH 3/5] Create a partial to display the parsed body content --- app/presenters/content_item/body.rb | 4 ++++ .../content_items/detailed_guide.html.erb | 8 +++++-- .../detailed_guide/_brexit_links.html.erb | 23 +++++++++++++++++++ 3 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 app/views/content_items/detailed_guide/_brexit_links.html.erb diff --git a/app/presenters/content_item/body.rb b/app/presenters/content_item/body.rb index b910b6158..4b2c4c56c 100644 --- a/app/presenters/content_item/body.rb +++ b/app/presenters/content_item/body.rb @@ -10,5 +10,9 @@ def govspeak_body direction: text_direction, } end + + def title_and_link_sections + BodyParser.new(body.html_safe).title_and_link_sections + end end end diff --git a/app/views/content_items/detailed_guide.html.erb b/app/views/content_items/detailed_guide.html.erb index b98f7619c..86ea00244 100644 --- a/app/views/content_items/detailed_guide.html.erb +++ b/app/views/content_items/detailed_guide.html.erb @@ -38,8 +38,12 @@ margin_bottom: 6, } unless brexit_link %> - <%= render 'govuk_publishing_components/components/govspeak', {} do %> - <%= raw(@content_item.govspeak_body[:content]) %> + <% if brexit_link %> + <%= render partial: 'content_items/detailed_guide/brexit_links', locals: { brexit_link: brexit_link } %> + <% else %> + <%= render 'govuk_publishing_components/components/govspeak', {} do %> + <%= raw(@content_item.govspeak_body[:content]) %> + <% end %> <% end %>
diff --git a/app/views/content_items/detailed_guide/_brexit_links.html.erb b/app/views/content_items/detailed_guide/_brexit_links.html.erb new file mode 100644 index 000000000..9e6bcbb3d --- /dev/null +++ b/app/views/content_items/detailed_guide/_brexit_links.html.erb @@ -0,0 +1,23 @@ +
+ <% @content_item.title_and_link_sections.each do |section| %> + <% if section[:title].present? %> + <%= render "govuk_publishing_components/components/heading", { + text: section[:title], + heading_level: 2, + font_size: "m", + margin_bottom: 6, + } %> + <% 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") + end + %> + <%= render "govuk_publishing_components/components/list", { + items: links, + visible_counters: true, + } %> + <% end %> + <% end %> +
From 76228b365a18c6c98e0cd6021660647072fc0004 Mon Sep 17 00:00:00 2001 From: hannako Date: Wed, 9 Jun 2021 16:08:54 +0100 Subject: [PATCH 4/5] Add tracking to the bulleted list of links --- .../content_item/brexit_hub_page.rb | 2 ++ .../detailed_guide/_brexit_links.html.erb | 7 ++++++- test/integration/detailed_guide_test.rb | 9 ++++++++ test/test_helper.rb | 21 +++++++++++++++++++ 4 files changed, 38 insertions(+), 1 deletion(-) diff --git a/app/presenters/content_item/brexit_hub_page.rb b/app/presenters/content_item/brexit_hub_page.rb index e7592c02f..f2e3b39ab 100644 --- a/app/presenters/content_item/brexit_hub_page.rb +++ b/app/presenters/content_item/brexit_hub_page.rb @@ -11,10 +11,12 @@ def brexit_links ContentItem::BrexitHubPage::BREXIT_BUSINESS_PAGE_CONTENT_ID => { text: I18n.t("brexit.citizen_link"), path: BREXIT_CITIZEN_PAGE_PATH, + track_category: "brexit-business-page", }, ContentItem::BrexitHubPage::BREXIT_CITIZEN_PAGE_CONTENT_ID => { text: I18n.t("brexit.business_link"), path: BREXIT_BUSINESS_PAGE_PATH, + track_category: "brexit-citizen-page", }, } end 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 9e6bcbb3d..bf1082c61 100644 --- a/app/views/content_items/detailed_guide/_brexit_links.html.erb +++ b/app/views/content_items/detailed_guide/_brexit_links.html.erb @@ -11,7 +11,12 @@ <% 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") + link_to(link[:text], link[:path], class: "govuk-link", data: { + track_action: link[:path], + track_category: track_category, + track_label: section[:title] || "", + module: 'gem-track-click', + }) end %> <%= render "govuk_publishing_components/components/list", { diff --git a/test/integration/detailed_guide_test.rb b/test/integration/detailed_guide_test.rb index 166f1bef1..48652fd96 100644 --- a/test/integration/detailed_guide_test.rb +++ b/test/integration/detailed_guide_test.rb @@ -111,5 +111,14 @@ class DetailedGuideTest < ActionDispatch::IntegrationTest 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) + + # adds GA tracking to the links + track_action = find_link("Foreign travel advice")["data-track-action"] + track_category = find_link("Foreign travel advice")["data-track-category"] + track_label = find_link("Foreign travel advice")["data-track-label"] + + assert_equal "/foreign-travel-advice", track_action + assert_equal "brexit-citizen-page", track_category + assert_equal "Travel to the EU", track_label end end diff --git a/test/test_helper.rb b/test/test_helper.rb index c2175c742..50c127ee2 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -179,6 +179,8 @@ def setup_and_visit_content_item_with_taxons(name, taxons) def setup_and_visit_brexit_child_taxon(type = nil) @content_item = get_content_example("detailed_guide").tap do |item| item["content_id"] = type == "business" ? brexit_business_id : brexit_citizen_id + item["details"]["body"] = brexit_body_content + stub_content_store_has_item(item["base_path"], item.to_json) visit_with_cachebust((item["base_path"]).to_s) end @@ -192,6 +194,25 @@ def brexit_business_id ContentItem::BrexitHubPage::BREXIT_BUSINESS_PAGE_CONTENT_ID end + def brexit_body_content + '
'\ + '

Travel to the EU

\n \n'\ + '\n'\ + '

Travel to the UK

\n \n'\ + '\n'\ + "
" + end + def setup_and_visit_random_content_item(document_type: nil) content_item = GovukSchemas::RandomExample.for_schema(frontend_schema: schema_type) do |payload| payload.merge!("document_type" => document_type) unless document_type.nil? From a6e81c7d94a36b5632da6e60ef226fafd5fce112 Mon Sep 17 00:00:00 2001 From: hannako Date: Fri, 28 May 2021 21:44:04 +0100 Subject: [PATCH 5/5] Add some tracking to the description link --- app/views/content_items/detailed_guide.html.erb | 12 +++++++++++- test/integration/detailed_guide_test.rb | 11 ++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/app/views/content_items/detailed_guide.html.erb b/app/views/content_items/detailed_guide.html.erb index 86ea00244..6eface360 100644 --- a/app/views/content_items/detailed_guide.html.erb +++ b/app/views/content_items/detailed_guide.html.erb @@ -15,7 +15,17 @@ <% if brexit_link %>

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

<% else %> diff --git a/test/integration/detailed_guide_test.rb b/test/integration/detailed_guide_test.rb index 48652fd96..db23a9a18 100644 --- a/test/integration/detailed_guide_test.rb +++ b/test/integration/detailed_guide_test.rb @@ -112,7 +112,7 @@ class DetailedGuideTest < ActionDispatch::IntegrationTest link_text = "Brexit guidance for businesses" assert page.has_link?(link_text, href: ContentItem::BrexitHubPage::BREXIT_BUSINESS_PAGE_PATH) - # adds GA tracking to the links + # adds GA tracking to the li links track_action = find_link("Foreign travel advice")["data-track-action"] track_category = find_link("Foreign travel advice")["data-track-category"] track_label = find_link("Foreign travel advice")["data-track-label"] @@ -120,5 +120,14 @@ class DetailedGuideTest < ActionDispatch::IntegrationTest assert_equal "/foreign-travel-advice", track_action assert_equal "brexit-citizen-page", track_category assert_equal "Travel to the EU", track_label + + # adds GA tracking to the description field links + track_action = find_link("Brexit guidance for businesses")["data-track-action"] + 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 "brexit-citizen-page", track_category + assert_equal "Brexit guidance for businesses", track_label end end