From 1784ba36614fba3f65ceae8e15009df444677427 Mon Sep 17 00:00:00 2001 From: Jessica Jones Date: Fri, 14 Jun 2024 16:16:16 +0100 Subject: [PATCH 1/3] Create SearchDocuments parent class --- app/models/topical_event.rb | 16 +++++++++------ app/models/world_location_news.rb | 14 ++++++++----- app/services/search_documents.rb | 20 +++++++++++++------ .../topical_event_search_documents.rb | 9 +++++++++ ...=> topical_event_search_documents_spec.rb} | 6 +++--- 5 files changed, 45 insertions(+), 20 deletions(-) create mode 100644 app/services/topical_event_search_documents.rb rename spec/services/{search_documents_spec.rb => topical_event_search_documents_spec.rb} (94%) diff --git a/app/models/topical_event.rb b/app/models/topical_event.rb index 7b735567c..df6359eb7 100644 --- a/app/models/topical_event.rb +++ b/app/models/topical_event.rb @@ -3,7 +3,7 @@ class TopicalEvent def initialize(content_item) @content_item = content_item - @documents_service = SearchDocuments.new(slug, "filter_topical_events") + @documents_service = TopicalEventSearchDocuments end def self.find!(base_path) @@ -92,24 +92,28 @@ def ordered_featured_documents end end + def documents_service + @documents_service.new([slug], "filter_topical_events") + end + def latest - @latest ||= @documents_service.fetch_related_documents_with_format + @latest ||= documents_service.fetch_related_documents_with_format end def publications - @publications ||= @documents_service.fetch_related_documents_with_format({ filter_format: "publication" }) + @publications ||= documents_service.fetch_related_documents_with_format({ filter_format: "publication" }) end def consultations - @consultations ||= @documents_service.fetch_related_documents_with_format({ filter_format: "consultation" }) + @consultations ||= documents_service.fetch_related_documents_with_format({ filter_format: "consultation" }) end def announcements - @announcements ||= @documents_service.fetch_related_documents_with_format({ filter_content_purpose_supergroup: "news_and_communications", reject_content_purpose_subgroup: %w[decisions updates_and_alerts] }) + @announcements ||= documents_service.fetch_related_documents_with_format({ filter_content_purpose_supergroup: "news_and_communications", reject_content_purpose_subgroup: %w[decisions updates_and_alerts] }) end def guidance_and_regulation - @guidance_and_regulation ||= @documents_service.fetch_related_documents_with_format({ filter_content_purpose_supergroup: "guidance_and_regulation" }) + @guidance_and_regulation ||= documents_service.fetch_related_documents_with_format({ filter_content_purpose_supergroup: "guidance_and_regulation" }) end def organisations diff --git a/app/models/world_location_news.rb b/app/models/world_location_news.rb index d9bd5de21..ed25fbc9c 100644 --- a/app/models/world_location_news.rb +++ b/app/models/world_location_news.rb @@ -3,7 +3,7 @@ class WorldLocationNews def initialize(content_item) @content_item = content_item - @documents_service = SearchDocuments.new(slug, "filter_world_locations") + @documents_service = TopicalEventSearchDocuments end def self.find!(base_path) @@ -70,19 +70,23 @@ def latest @latest ||= [*announcements, *publications, *statistics] .select { |entry| entry.dig(:metadata, :public_updated_at).present? } .sort { |entry1, entry2| - (entry1.dig(:metadata, :public_updated_at) <=> entry2.dig(:metadata, :public_updated_at)) } - .first(SearchDocuments::DEFAULT_COUNT) + .first(@documents_service::DEFAULT_COUNT) + end + + def documents_service + @documents_service.new([slug], "filter_world_locations") end def announcements - @announcements ||= @documents_service.fetch_related_documents_with_format({ filter_content_purpose_supergroup: "news_and_communications" }) + @announcements ||= documents_service.fetch_related_documents_with_format({ filter_content_purpose_supergroup: "news_and_communications" }) end def publications - @publications ||= @documents_service.fetch_related_documents_with_format({ filter_content_purpose_supergroup: %w[guidance_and_regulation policy_and_engagement transparency] }) + @publications ||= documents_service.fetch_related_documents_with_format({ filter_content_purpose_supergroup: %w[guidance_and_regulation policy_and_engagement transparency] }) end def statistics - @statistics ||= @documents_service.fetch_related_documents_with_format({ filter_content_purpose_subgroup: "statistics" }) + @statistics ||= documents_service.fetch_related_documents_with_format({ filter_content_purpose_subgroup: "statistics" }) end def type diff --git a/app/services/search_documents.rb b/app/services/search_documents.rb index 64515ee92..72d3d08c6 100644 --- a/app/services/search_documents.rb +++ b/app/services/search_documents.rb @@ -2,23 +2,31 @@ class SearchDocuments include SearchApiFields DEFAULT_COUNT = 3 - def initialize(slug, filter_field) - @slug = slug + def initialize(slug_array, filter_field) + @slug_array = slug_array @filter_field = filter_field end def fetch_related_documents_with_format(filter_format = {}) - search_response = Services.search_api.search(default_search_options.merge(filter_format)) + search_response = Services.cached_search(default_search_options.merge(filter_format)) format_results(search_response) end + def fields + Raise NotImplementedError + end + + def order + Raise NotImplementedError + end + private def default_search_options - { @filter_field.to_sym => [@slug], + { @filter_field.to_sym => @slug_array, count: DEFAULT_COUNT, - order: "-public_timestamp", - fields: TOPICAL_EVENTS_SEARCH_FIELDS } + order:, + fields: } end def display_type(document) diff --git a/app/services/topical_event_search_documents.rb b/app/services/topical_event_search_documents.rb new file mode 100644 index 000000000..0d92c5f89 --- /dev/null +++ b/app/services/topical_event_search_documents.rb @@ -0,0 +1,9 @@ +class TopicalEventSearchDocuments < SearchDocuments + def fields + TOPICAL_EVENTS_SEARCH_FIELDS + end + + def order + "-public_timestamp" + end +end diff --git a/spec/services/search_documents_spec.rb b/spec/services/topical_event_search_documents_spec.rb similarity index 94% rename from spec/services/search_documents_spec.rb rename to spec/services/topical_event_search_documents_spec.rb index 65f053777..4a026dc80 100644 --- a/spec/services/search_documents_spec.rb +++ b/spec/services/topical_event_search_documents_spec.rb @@ -1,11 +1,11 @@ -RSpec.describe SearchDocuments do +RSpec.describe TopicalEventSearchDocuments do include SearchApiHelpers context "for topical events" do describe "#fetch_related_documents_with_format" do before do @slug = "/some/slug" - @documents_service = SearchDocuments.new(@slug, "filter_topical_events") + @documents_service = TopicalEventSearchDocuments.new([@slug], "filter_topical_events") end it "makes the correct call to search api" do @@ -70,7 +70,7 @@ describe "#fetch_related_documents_with_format" do before do @slug = "mock-country" - @documents_service = SearchDocuments.new(@slug, "filter_world_locations") + @documents_service = TopicalEventSearchDocuments.new([@slug], "filter_world_locations") end it "makes the correct call to search api" do From cd77564a22e1b59cd6206a08296ab3bc43d7e2d2 Mon Sep 17 00:00:00 2001 From: Jessica Jones Date: Fri, 14 Jun 2024 16:21:47 +0100 Subject: [PATCH 2/3] Add popular tasks from search to browse pages --- app/models/mainstream_browse_page.rb | 16 ++++++++++-- .../popular_browse_search_documents.rb | 9 +++++++ app/views/browse/show.html.erb | 22 +++++++++++++--- spec/controllers/browse_controller_spec.rb | 6 +++++ spec/features/mainstream_browsing_spec.rb | 2 +- spec/support/search_api_helpers.rb | 25 +++++++++++++++++++ 6 files changed, 74 insertions(+), 6 deletions(-) create mode 100644 app/services/popular_browse_search_documents.rb diff --git a/app/models/mainstream_browse_page.rb b/app/models/mainstream_browse_page.rb index cb4ebb510..73096198b 100644 --- a/app/models/mainstream_browse_page.rb +++ b/app/models/mainstream_browse_page.rb @@ -19,6 +19,7 @@ def self.find(base_path) def initialize(content_item) @content_item = content_item + @documents_service = PopularBrowseSearchDocuments end def top_level_browse_pages @@ -49,7 +50,18 @@ def lists @lists ||= ListSet.new("section", @content_item.content_id, details["groups"]) end - def slug - base_path.sub(%r{\A/browse/}, "") + def slug(path = base_path) + path.sub(%r{\A/browse/}, "") + end + + def display_popular_tasks? + %w[benefits business].include?(slug) + end + + def popular_tasks + return if second_level_browse_pages.blank? + + slugs = second_level_browse_pages.map { |page| slug(page.base_path) } + @documents_service.new(slugs, "filter_any_mainstream_browse_pages").fetch_related_documents_with_format end end diff --git a/app/services/popular_browse_search_documents.rb b/app/services/popular_browse_search_documents.rb new file mode 100644 index 000000000..0230edb89 --- /dev/null +++ b/app/services/popular_browse_search_documents.rb @@ -0,0 +1,9 @@ +class PopularBrowseSearchDocuments < SearchDocuments + def fields + TOPICAL_EVENTS_SEARCH_FIELDS + end + + def order + "-popularity" + end +end diff --git a/app/views/browse/show.html.erb b/app/views/browse/show.html.erb index 4031ffb37..d647312ca 100644 --- a/app/views/browse/show.html.erb +++ b/app/views/browse/show.html.erb @@ -31,7 +31,7 @@ } %> <% end %> -<% if display_action_links_for_slug?(page.slug) %> +<% if page.display_popular_tasks? %>
@@ -42,9 +42,25 @@ font_size: "m" } %>
    - <% action_link_data(page.slug).each do |link| %> + <% page.popular_tasks.each_with_index do |task, index| %>
  • - <%= render "shared/browse_action_link", locals: link %> + <%= render "govuk_publishing_components/components/action_link", { + text: task[:link][:text], + href: task[:link][:path], + dark_large_icon: true, + margin_bottom: 3, + data_attributes: { + module: "ga4-link-tracker", + ga4_track_links_only: "", + ga4_link: { + event_name: "navigation", + type: "action", + index_link: index + 1 , + index_total: page.popular_tasks.length, + text: "something in here" + } + } + } %>
  • <% end %>
diff --git a/spec/controllers/browse_controller_spec.rb b/spec/controllers/browse_controller_spec.rb index fce86187c..2c78cfb74 100644 --- a/spec/controllers/browse_controller_spec.rb +++ b/spec/controllers/browse_controller_spec.rb @@ -1,5 +1,6 @@ RSpec.describe BrowseController do include GovukAbTesting::RspecHelpers + include SearchApiHelpers render_views describe "GET index" do before do @@ -30,6 +31,11 @@ second_level_browse_pages:, }, ) + stub_search(body: { + "results" => [], + "start" => 0, + "total" => 0, + }) end it "sets correct expiry headers" do diff --git a/spec/features/mainstream_browsing_spec.rb b/spec/features/mainstream_browsing_spec.rb index cacb0551c..0b372ce19 100644 --- a/spec/features/mainstream_browsing_spec.rb +++ b/spec/features/mainstream_browsing_spec.rb @@ -9,7 +9,6 @@ schemas.each do |content_item| stub_content_store_has_item(content_item["base_path"], content_item) - search_api_has_documents_for_browse_page( content_item["content_id"], %w[ @@ -22,6 +21,7 @@ ], page_size: SearchApiSearch::PAGE_SIZE_TO_GET_EVERYTHING, ) + search_api_has_popular_documents_for_browse(content_item) visit content_item["base_path"] diff --git a/spec/support/search_api_helpers.rb b/spec/support/search_api_helpers.rb index c84d98da8..407a1a234 100644 --- a/spec/support/search_api_helpers.rb +++ b/spec/support/search_api_helpers.rb @@ -241,6 +241,31 @@ def search_api_has_documents_for_browse_page(browse_page_content_id, document_sl end end + def search_api_has_popular_documents_for_browse(browse_content_item) + return if browse_content_item["links"]["second_level_browse_pages"].blank? + + level_two_browse_slugs = + browse_content_item["links"]["second_level_browse_pages"].map do |link| + link["base_path"].sub(%r{\A/browse/}, "") + end + + fields = SearchApiFields::TOPICAL_EVENTS_SEARCH_FIELDS + + params = { + count: "3", + filter_any_mainstream_browse_pages: webmock_match_array(level_two_browse_slugs), + order: "-popularity", + fields: webmock_match_array(fields), + } + + results = %w[slug-a slug-2].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 = [] From fb9f8de4f99bc7b6cba0d598c50bb66f4d22c496 Mon Sep 17 00:00:00 2001 From: Jessica Jones Date: Mon, 29 Jul 2024 14:04:02 +0100 Subject: [PATCH 3/3] Only return popular service pages --- app/models/mainstream_browse_page.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/models/mainstream_browse_page.rb b/app/models/mainstream_browse_page.rb index 73096198b..c98d2cef7 100644 --- a/app/models/mainstream_browse_page.rb +++ b/app/models/mainstream_browse_page.rb @@ -62,6 +62,12 @@ def popular_tasks return if second_level_browse_pages.blank? slugs = second_level_browse_pages.map { |page| slug(page.base_path) } - @documents_service.new(slugs, "filter_any_mainstream_browse_pages").fetch_related_documents_with_format + service_pages_tagged_to_browse_subtopics(slugs) + end + + def service_pages_tagged_to_browse_subtopics(subtopic_slugs) + @documents_service.new( + subtopic_slugs, "filter_any_mainstream_browse_pages" + ).fetch_related_documents_with_format({ filter_format: "transaction" }) end end