diff --git a/app/helpers/browse_helper.rb b/app/helpers/browse_helper.rb index a577ec9f7..1d3fa2ec2 100644 --- a/app/helpers/browse_helper.rb +++ b/app/helpers/browse_helper.rb @@ -1,21 +1,10 @@ module BrowseHelper - def display_popular_links_for_slug?(slug) - popular_links_data(slug).present? - end - - def popular_links_data(slug) - if I18n.exists?("browse.popular_links.#{slug}", :en) - I18n.t("browse.popular_links.#{slug}") - end - end - - def popular_links_for_slug(slug) - links = popular_links_data(slug) - count = links.length - links.map.with_index(1) do |link, index| + def with_tracking_data(popular_content) + count = popular_content.length + popular_content.map.with_index(1) do |link, index| { text: link[:title], - href: link[:url], + href: link[:link], data_attributes: { module: "ga4-link-tracker", ga4_track_links_only: "", diff --git a/app/models/mainstream_browse_page.rb b/app/models/mainstream_browse_page.rb index cb4ebb510..72c76a50d 100644 --- a/app/models/mainstream_browse_page.rb +++ b/app/models/mainstream_browse_page.rb @@ -45,10 +45,20 @@ def second_level_pages_curated? details["second_level_ordering"] == "curated" end + def top_level_browse_page? + top_level_browse_pages.any? { |page| page.base_path == base_path } + end + def lists @lists ||= ListSet.new("section", @content_item.content_id, details["groups"]) end + def popular_list + return unless top_level_browse_page? + + @popular_list ||= PopularListSet.fetch(@content_item) + end + def slug base_path.sub(%r{\A/browse/}, "") end diff --git a/app/models/popular_list_set.rb b/app/models/popular_list_set.rb new file mode 100644 index 000000000..66cd5feb1 --- /dev/null +++ b/app/models/popular_list_set.rb @@ -0,0 +1,40 @@ +class PopularListSet + def initialize(content_item) + @content_item = content_item + end + + def self.fetch(content_item) + new(content_item).formatted_results + end + + def formatted_results + if search_response["results"].present? + search_response["results"].map do |result| + { + title: result["title"], + link: result["link"], + } + end + end + end + +private + + attr_reader :content_item + + def search_response + params = { + filter_any_mainstream_browse_page_content_ids: second_level_browse_content_ids, + count: 3, + order: "-popularity", + fields: SearchApiFields::POPULAR_BROWSE_SEARCH_FIELDS, + } + Services.cached_search(params) + end + + def second_level_browse_content_ids + content_item + .linked_items("second_level_browse_pages") + .map { |content_item| content_item.content_item_data["content_id"] } + end +end diff --git a/app/services/search_api_fields.rb b/app/services/search_api_fields.rb index 4da5d9ab3..2e0f5e02c 100644 --- a/app/services/search_api_fields.rb +++ b/app/services/search_api_fields.rb @@ -35,4 +35,10 @@ module SearchApiFields content_id organisations document_collections].freeze + + POPULAR_BROWSE_SEARCH_FIELDS = %w[ title + link + public_timestamp + display_type + content_store_document_type].freeze end diff --git a/app/views/browse/show.html.erb b/app/views/browse/show.html.erb index 2d00fa7f4..ca1fa5b82 100644 --- a/app/views/browse/show.html.erb +++ b/app/views/browse/show.html.erb @@ -34,7 +34,7 @@ } %> <% end %> -<% if display_popular_links_for_slug?(page.slug) %> +<% if page.popular_list %>
@@ -45,7 +45,7 @@ font_size: "m" } %>
    - <% popular_links_for_slug(page.slug).each do |link| %> + <% with_tracking_data(page.popular_list).each do |link| %>
  • <%= render partial: "shared/browse_action_link", locals: {link:} %>
    • @@ -68,7 +68,7 @@ <% total_links = page.second_level_browse_pages.count.to_s %> <%= render "shared/browse_cards_container" do %> <%= render "govuk_publishing_components/components/cards", { - heading: display_popular_links_for_slug?(page.slug) ? t("browse.topics") : nil, + heading: page.popular_list ? t("browse.topics") : nil, items: page.second_level_browse_pages.map.with_index do |second_level_browse_page, index| { link: { @@ -86,6 +86,6 @@ description: second_level_browse_page.description, } end, - sub_heading_level: display_popular_links_for_slug?(page.slug) ? 3 : 2, + sub_heading_level: page.popular_list ? 3 : 2, } %> <% end %> diff --git a/features/step_definitions/viewing_browse_steps.rb b/features/step_definitions/viewing_browse_steps.rb index 13d0dd110..83d41cb4b 100644 --- a/features/step_definitions/viewing_browse_steps.rb +++ b/features/step_definitions/viewing_browse_steps.rb @@ -19,6 +19,9 @@ ], page_size: SearchApiSearch::PAGE_SIZE_TO_GET_EVERYTHING, ) + + content_ids = second_level_browse_pages.map { |link| link[:content_id] } + search_api_has_popular_documents_for_level_one_browse(content_ids) end Given(/^that there are curated second level browse pages$/) do @@ -41,6 +44,9 @@ order_type: "curated", ) add_second_level_browse_pages(second_level_browse_pages) + + content_ids = second_level_browse_pages.map { |link| link[:content_id] } + search_api_has_popular_documents_for_level_one_browse(content_ids) end Then(/^I see the links tagged to the browse page/) do diff --git a/spec/controllers/browse_controller_spec.rb b/spec/controllers/browse_controller_spec.rb index fce86187c..f76349476 100644 --- a/spec/controllers/browse_controller_spec.rb +++ b/spec/controllers/browse_controller_spec.rb @@ -1,5 +1,7 @@ RSpec.describe BrowseController do include GovukAbTesting::RspecHelpers + include SearchApiHelpers + render_views describe "GET index" do before do @@ -21,15 +23,20 @@ describe "GET top_level_browse_page" do describe "for a valid browse page" do before do - stub_content_store_has_item( - "/browse/benefits", + content_item = { base_path: "/browse/benefits", title: "foo", links: { top_level_browse_pages:, second_level_browse_pages:, }, + } + stub_content_store_has_item( + "/browse/benefits", + content_item, ) + + search_api_has_popular_documents_for_level_one_browse(level_two_browse_content_ids) end it "sets correct expiry headers" do @@ -75,4 +82,8 @@ def second_level_browse_pages base_path: "/browse/benefits/entitlement", }] end + + def level_two_browse_content_ids + second_level_browse_pages.map { |link| link[:content_id] } + end end diff --git a/spec/features/mainstream_browsing_spec.rb b/spec/features/mainstream_browsing_spec.rb index 3cd71acc1..1e598ede3 100644 --- a/spec/features/mainstream_browsing_spec.rb +++ b/spec/features/mainstream_browsing_spec.rb @@ -23,6 +23,11 @@ page_size: SearchApiSearch::PAGE_SIZE_TO_GET_EVERYTHING, ) + if content_item["links"]["second_level_browse_pages"].present? + content_ids = content_item["links"]["second_level_browse_pages"].map { |link| link["content_id"] } + search_api_has_popular_documents_for_level_one_browse(content_ids) + end + visit content_item["base_path"] expect(page.status_code).to eq(200) @@ -30,41 +35,46 @@ end end - it "renders popular tasks on level 1 browse pages" do - level_one_browse_slugs = %w[ abroad - benefits - births-deaths-marriages - business - childcare-parenting - citizenship - disabilities - driving - education - employing-people - environment-countryside - housing-local-services - justice - tax - visas-immigration - working] - - level_one_browse_slugs.each do |slug| - content_item = GovukSchemas::Example.find("mainstream_browse_page", example_name: "top_level_page").tap do |item| - item["base_path"] = "/browse/#{slug}" - end + describe "popular tasks" do + it "renders on level one browse pages" do + content_item = GovukSchemas::Example.find("mainstream_browse_page", example_name: "top_level_page") stub_content_store_has_item(content_item["base_path"], content_item) + content_ids = content_item["links"]["second_level_browse_pages"].map { |link| link["content_id"] } + search_api_has_popular_documents_for_level_one_browse(content_ids) + visit content_item["base_path"] expect(page).to have_css(".browse__action-links") expect(page).to have_content("Popular tasks") end end + it "does not render on the root browse page" do + content_item = GovukSchemas::Example.find("mainstream_browse_page", example_name: "root_page") + stub_content_store_has_item(content_item["base_path"], content_item) + + visit content_item["base_path"] + expect(page).not_to have_css(".browse__action-links") + expect(page).not_to have_content("Popular tasks") + end + + it "does not render on level two browse pages" do + content_item = GovukSchemas::Example.find("mainstream_browse_page", example_name: "level_2_page") + stub_content_store_has_item(content_item["base_path"], content_item) + search_api_has_documents_for_browse_page(content_item["content_id"], %w[a-slug], page_size: 1000) + + visit content_item["base_path"] + expect(page).not_to have_css(".browse__action-links") + expect(page).not_to have_content("Popular tasks") + end + it "renders the GOV.UK Chat promo" do content_item = GovukSchemas::Example.find("mainstream_browse_page", example_name: "top_level_page").tap do |item| item["base_path"] = GovukChatPromoHelper::GOVUK_CHAT_PROMO_BASE_PATHS.first end stub_content_store_has_item(content_item["base_path"], content_item) + content_ids = content_item["links"]["second_level_browse_pages"].map { |link| link["content_id"] } + search_api_has_popular_documents_for_level_one_browse(content_ids) ClimateControl.modify GOVUK_CHAT_PROMO_ENABLED: "true" do visit content_item["base_path"] diff --git a/spec/helpers/browse_helper_spec.rb b/spec/helpers/browse_helper_spec.rb index 06b301ca9..08b1f39fd 100644 --- a/spec/helpers/browse_helper_spec.rb +++ b/spec/helpers/browse_helper_spec.rb @@ -1,20 +1,25 @@ RSpec.describe BrowseHelper do - describe "#display_popular_links_for_slug?" do - it "returns true if data exists" do - expect(helper.display_popular_links_for_slug?("business")).to be(true) - end - - it "returns false if no data exists" do - expect(helper.display_popular_links_for_slug?("random12345")).to be(false) - end - end + describe "#with_tracking_data" do + it "returns formatted popular links" do + popular_content_from_search = [ + { + 'link': "/foo/policy_paper", + 'title': "Policy on World Locations", + }, + { + 'link': "/foo/news_story", + 'title': "PM attends summit on world location news pages", + }, + { + 'link': "/foo/anything", + 'title': "Anything", + }, + ] - describe "#popular_links_for_slug" do - it "adds GA tracking data to links" do expected_links = [ { - text: "HMRC online services: sign in or set up an account", - href: "/log-in-register-hmrc-online-services", + text: "Policy on World Locations", + href: "/foo/policy_paper", data_attributes: { module: "ga4-link-tracker", ga4_track_links_only: "", @@ -23,14 +28,14 @@ type: "action", index_link: 1, index_total: 3, - text: "HMRC online services: sign in or set up an account", + text: "Policy on World Locations", section: "Popular tasks", }, }, }, { - text: "Get information about a company", - href: "/get-information-about-a-company", + text: "PM attends summit on world location news pages", + href: "/foo/news_story", data_attributes: { module: "ga4-link-tracker", ga4_track_links_only: "", @@ -39,14 +44,14 @@ type: "action", index_link: 2, index_total: 3, - text: "Get information about a company", + text: "PM attends summit on world location news pages", section: "Popular tasks", }, }, }, { - text: "Find an energy certificate", - href: "/find-energy-certificate", + text: "Anything", + href: "/foo/anything", data_attributes: { module: "ga4-link-tracker", ga4_track_links_only: "", @@ -55,13 +60,13 @@ type: "action", index_link: 3, index_total: 3, - text: "Find an energy certificate", + text: "Anything", section: "Popular tasks", }, }, }, ] - expect(helper.popular_links_for_slug("business")).to eq(expected_links) + expect(helper.with_tracking_data(popular_content_from_search)).to eq(expected_links) end end end diff --git a/spec/models/mainstream_browse_page_spec.rb b/spec/models/mainstream_browse_page_spec.rb index 2a312d708..953135e53 100644 --- a/spec/models/mainstream_browse_page_spec.rb +++ b/spec/models/mainstream_browse_page_spec.rb @@ -209,4 +209,37 @@ expect(page.lists).to be_an_instance_of(ListSet) end end + + describe "popular_lists" do + let(:top_level_browse_page) { GovukSchemas::Example.find("mainstream_browse_page", example_name: "top_level_page") } + let(:second_level_browse_page) { GovukSchemas::Example.find("mainstream_browse_page", example_name: "level_2_page") } + let(:root_browse_page) { GovukSchemas::Example.find("mainstream_browse_page", example_name: "root_page") } + + it "passes the content item to PopularListSet on top level browse pages" do + api_data["base_path"] = top_level_browse_page["base_path"] + api_data["links"] = top_level_browse_page["links"] + list = [{ title: "foo", link: "/foo" }] + + allow(PopularListSet) + .to receive(:fetch) + .with(content_item) + .and_return(list) + + expect(page.popular_list).to eq list + end + + it "returns nil on the root browse page" do + api_data["base_path"] = root_browse_page["base_path"] + api_data["links"] = root_browse_page["links"] + + expect(page.popular_list).to be_nil + end + + it "returns nil on second level browse pages" do + api_data["base_path"] = second_level_browse_page["base_path"] + api_data["links"] = second_level_browse_page["links"] + + expect(page.popular_list).to be_nil + end + end end diff --git a/spec/models/popular_list_spec.rb b/spec/models/popular_list_spec.rb new file mode 100644 index 000000000..61010cf3b --- /dev/null +++ b/spec/models/popular_list_spec.rb @@ -0,0 +1,43 @@ +RSpec.describe PopularListSet do + include SearchApiHelpers + + describe "#formatted_results" do + let(:top_level_browse_page) { GovukSchemas::Example.find("mainstream_browse_page", example_name: "top_level_page") } + let(:content_item) { ContentItem.new(top_level_browse_page) } + let(:popular_list) { described_class.new(content_item) } + + it "formats the response from SearchAPI" do + content_ids = content_item + .linked_items("second_level_browse_pages") + .map { |content_item| content_item.content_item_data["content_id"] } + + params = { + filter_any_mainstream_browse_page_content_ids: webmock_match_array(content_ids), + count: "3", + order: "-popularity", + fields: webmock_match_array(SearchApiFields::POPULAR_BROWSE_SEARCH_FIELDS), + } + + search_response = { + "results" => [ + search_api_document_for_browse("tagged-to-a-benefits-topic"), + search_api_document_for_browse("tagged-to-another-benefits-topic"), + ], + } + + expected_results = [ + { + title: "Tagged to a benefits topic", + link: "/tagged-to-a-benefits-topic", + }, + { + title: "Tagged to another benefits topic", + link: "/tagged-to-another-benefits-topic", + }, + ] + + stub_search(params:, body: search_response) + expect(popular_list.formatted_results).to eq expected_results + end + end +end diff --git a/spec/support/search_api_helpers.rb b/spec/support/search_api_helpers.rb index c84d98da8..7ee07717a 100644 --- a/spec/support/search_api_helpers.rb +++ b/spec/support/search_api_helpers.rb @@ -178,6 +178,18 @@ def search_api_document_for_supergroup_section(slug, content_store_document_type } end + def search_api_document_for_browse(slug) + { "content_store_document_type" => "transaction", + "link" => "/#{slug}", + "public_timestamp" => "2019-11-21T14:44:36Z", + "title" => slug.titleize.humanize.to_s, + "index" => "govuk", + "es_score" => nil, + "_id" => "/#{slug}", + "elasticsearch_type" => "edition", + "document_type" => "edition" } + end + def search_api_has_latest_documents_for_subtopic(subtopic_content_id, document_slugs, page_size: 50) results = document_slugs.map.with_index do |slug, i| search_api_document_for_slug(slug, (i + 1).hours.ago) @@ -241,6 +253,24 @@ def search_api_has_documents_for_browse_page(browse_page_content_id, document_sl end end + def search_api_has_popular_documents_for_level_one_browse(browse_content_ids) + fields = SearchApiFields::POPULAR_BROWSE_SEARCH_FIELDS + + params = { + count: "3", + filter_any_mainstream_browse_page_content_ids: webmock_match_array(browse_content_ids), + order: "-popularity", + fields: webmock_match_array(fields), + } + + results = %w[anything anything_else].map do |slug| + search_api_document_for_slug(slug, 1.hour.ago, "guide") + end + + body = { results: } + stub_search(params:, body:) + end + def section_tagged_content_list(doc_type, count = 1) content_list = []