From bb4233329c27acb204b6be14ab10a8b459f20f0d Mon Sep 17 00:00:00 2001 From: Christian Sutter Date: Thu, 28 Nov 2024 16:47:27 +0000 Subject: [PATCH 1/2] Homepage: Finish search autocomplete AB test This removes the AB testing code and makes the new search autocomplete component permanent. --- .../search_autocomplete_ab_testable.rb | 28 ----------------- app/controllers/homepage_controller.rb | 10 ------- app/views/homepage/_homepage_header.html.erb | 2 +- app/views/homepage/index.html.erb | 1 - spec/system/homepage_spec.rb | 30 +++---------------- 5 files changed, 5 insertions(+), 66 deletions(-) delete mode 100644 app/controllers/concerns/search_autocomplete_ab_testable.rb diff --git a/app/controllers/concerns/search_autocomplete_ab_testable.rb b/app/controllers/concerns/search_autocomplete_ab_testable.rb deleted file mode 100644 index 529b1b35c3..0000000000 --- a/app/controllers/concerns/search_autocomplete_ab_testable.rb +++ /dev/null @@ -1,28 +0,0 @@ -module SearchAutocompleteAbTestable - ALLOWED_VARIANTS = %w[A B Z].freeze - - def self.included(base) - base.helper_method :search_autocomplete_ab_test_variant - base.after_action :set_search_autocomplete_ab_test_response_header - end - - def search_autocomplete_ab_test - @search_autocomplete_ab_test ||= GovukAbTesting::AbTest.new( - "SearchAutocomplete", - allowed_variants: ALLOWED_VARIANTS, - control_variant: "Z", - ) - end - - def search_autocomplete_ab_test_variant - @search_autocomplete_ab_test_variant ||= search_autocomplete_ab_test.requested_variant(request.headers) - end - - def set_search_autocomplete_ab_test_response_header - search_autocomplete_ab_test_variant.configure_response(response) - end - - def show_search_autocomplete_test? - search_autocomplete_ab_test_variant.variant?("B") - end -end diff --git a/app/controllers/homepage_controller.rb b/app/controllers/homepage_controller.rb index 8a1c50ce55..244e1091ac 100644 --- a/app/controllers/homepage_controller.rb +++ b/app/controllers/homepage_controller.rb @@ -1,6 +1,5 @@ class HomepageController < ContentItemsController include Cacheable - include SearchAutocompleteAbTestable slimmer_template "gem_layout_homepage" @@ -10,15 +9,6 @@ def index private - def search_component - if show_search_autocomplete_test? - "search_with_autocomplete" - else - "search" - end - end - helper_method :search_component - def publication_class HomepagePresenter end diff --git a/app/views/homepage/_homepage_header.html.erb b/app/views/homepage/_homepage_header.html.erb index 48046745cb..ec0ef2e037 100644 --- a/app/views/homepage/_homepage_header.html.erb +++ b/app/views/homepage/_homepage_header.html.erb @@ -26,7 +26,7 @@ ga4_search_index_section_count: 6, } ) do %> - <%= render "govuk_publishing_components/components/#{search_component}", { + <%= render "govuk_publishing_components/components/search_with_autocomplete", { name: "keywords", button_text: t("homepage.index.search_button"), margin_bottom: 0, diff --git a/app/views/homepage/index.html.erb b/app/views/homepage/index.html.erb index f097f72704..6a7990bae7 100644 --- a/app/views/homepage/index.html.erb +++ b/app/views/homepage/index.html.erb @@ -2,7 +2,6 @@ <% content_for :extra_headers do %> "> - <%= search_autocomplete_ab_test_variant.analytics_meta_tag.html_safe %> <% end %> <% content_for :title, t("homepage.index.intro_title.text") %> <% content_for :body_classes, "homepage" %><%# The `homepage` body class is required for emergency banner. %> diff --git a/spec/system/homepage_spec.rb b/spec/system/homepage_spec.rb index 1b96d0e40b..9757a49c18 100644 --- a/spec/system/homepage_spec.rb +++ b/spec/system/homepage_spec.rb @@ -1,6 +1,4 @@ RSpec.describe "Homepage" do - include GovukAbTesting::RspecHelpers - before { stub_content_store_has_item("/", schema: "special_route", links: {}) } it "renders the homepage" do @@ -12,31 +10,11 @@ expect(page).not_to have_css(".homepage-inverse-header__title") end - context "search autocomplete AB test" do - it "does not render the search autocomplete on the A variant" do - with_variant(SearchAutocomplete: "A") do - visit "/" - - expect(page).not_to have_css(".gem-c-search-with-autocomplete") - end - end - - it "renders the search autocomplete on the B variant with the correct source URL" do - with_variant(SearchAutocomplete: "B") do - visit "/" - - expect(page).to have_css(".gem-c-search-with-autocomplete") - expect(page).to have_css("[data-source-url='http://www.dev.gov.uk/api/search/autocomplete.json']") - end - end - - it "does not render the search autocomplete on the Z variant" do - with_variant(SearchAutocomplete: "Z") do - visit "/" + it "renders the search with autocomplete component with the correct source URL" do + visit "/" - expect(page).not_to have_css(".gem-c-search-with-autocomplete") - end - end + expect(page).to have_css(".gem-c-search-with-autocomplete") + expect(page).to have_css("[data-source-url='http://www.dev.gov.uk/api/search/autocomplete.json']") end context "when visiting a Welsh content item first" do From 999e2fbe28224657ff5866e34340b9e67b04656c Mon Sep 17 00:00:00 2001 From: Leena Gupte Date: Fri, 6 Dec 2024 17:21:18 +0000 Subject: [PATCH 2/2] Allow search autocomplete to be easily disabled Autocomplete uses a denylist to stop certain terms from serving suggestions. The process to update the denylist is very manual and developer heavy. It might be difficult to update the denylist over the holiday period. In that situation the decision might be taken to temporarily turn off autocomplete entirely. An environment variable is used to determine whether or not to render search with autocomplete. The value of this variable will be applied globally across all applications that use site search. This environment variable will either exist, or not. The value of it doesn't really matter. If it exists, then search with autocomplete should be disabled. A value of "1" is used in the tests, as that is the default value being set in the helm charts. See https://github.com/alphagov/govuk-helm-charts/pull/2832 --- app/controllers/homepage_controller.rb | 13 ++++++++++ app/views/homepage/_homepage_header.html.erb | 2 +- spec/system/homepage_spec.rb | 25 ++++++++++++++++---- 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/app/controllers/homepage_controller.rb b/app/controllers/homepage_controller.rb index 244e1091ac..009228e145 100644 --- a/app/controllers/homepage_controller.rb +++ b/app/controllers/homepage_controller.rb @@ -9,7 +9,20 @@ def index private + def search_component + if show_search_autocomplete? + "search_with_autocomplete" + else + "search" + end + end + helper_method :search_component + def publication_class HomepagePresenter end + + def show_search_autocomplete? + true unless ENV["GOVUK_DISABLE_SEARCH_AUTOCOMPLETE"] + end end diff --git a/app/views/homepage/_homepage_header.html.erb b/app/views/homepage/_homepage_header.html.erb index ec0ef2e037..48046745cb 100644 --- a/app/views/homepage/_homepage_header.html.erb +++ b/app/views/homepage/_homepage_header.html.erb @@ -26,7 +26,7 @@ ga4_search_index_section_count: 6, } ) do %> - <%= render "govuk_publishing_components/components/search_with_autocomplete", { + <%= render "govuk_publishing_components/components/#{search_component}", { name: "keywords", button_text: t("homepage.index.search_button"), margin_bottom: 0, diff --git a/spec/system/homepage_spec.rb b/spec/system/homepage_spec.rb index 9757a49c18..e7b52b2b91 100644 --- a/spec/system/homepage_spec.rb +++ b/spec/system/homepage_spec.rb @@ -10,11 +10,28 @@ expect(page).not_to have_css(".homepage-inverse-header__title") end - it "renders the search with autocomplete component with the correct source URL" do - visit "/" + describe "search autocomplete" do + context "when autocomplete is enabled" do + it "renders the search with autocomplete component with the correct source URL" do + ClimateControl.modify GOVUK_DISABLE_SEARCH_AUTOCOMPLETE: nil do + visit "/" + + expect(page).to have_css(".gem-c-search-with-autocomplete") + expect(page).to have_css("[data-source-url='http://www.dev.gov.uk/api/search/autocomplete.json']") + end + end + end - expect(page).to have_css(".gem-c-search-with-autocomplete") - expect(page).to have_css("[data-source-url='http://www.dev.gov.uk/api/search/autocomplete.json']") + context "when autocomplete is disabled" do + it "does not render the search with autocomplete component" do + ClimateControl.modify GOVUK_DISABLE_SEARCH_AUTOCOMPLETE: "1" do + visit "/" + + expect(page).to_not have_css(".gem-c-search-with-autocomplete") + expect(page).to have_css(".gem-c-search") + end + end + end end context "when visiting a Welsh content item first" do