From 3a50337145605189769a92a1cf70ef75f72f8087 Mon Sep 17 00:00:00 2001 From: Christian Sutter Date: Sun, 9 Feb 2025 16:14:12 +0000 Subject: [PATCH 1/3] Add initial `ServingConfig` model This represents a serving config(uration) on Discovery Engine, which is an endpoint on an engine that can be used for querying. The main purpose of serving configs is to have controls attached which modify their behaviour, and each of an engine's serving configs can have zero-to-many of that engine's controls attached. Having several serving configs means we can test out different sets of controls and how they affect search results. Serving configs have one of three use cases: - `live` serving configs are used somewhere in our search stack to provide results to the public - `preview` serving configs are for use by Search Admin users to test new configurations (for example, when adding new controls) - `system` serving configs are for internal programmatic use and cannot be modified through the Search Admin UI (for example, we have an "empty" serving config to test having no controls at all) Serving configs also have a display name and remote resource identifier (like other Discovery Engine resources). As we cannot currently create or delete serving configs through the Discovery Engine Ruby client, initial creation of three serving configs has been done in `govuk-infrastructure` [1] and we use a Rake task to create the model records on this end. Depending on whether these three are enough for the long term, we may or may not want to add the ability to create, edit, and delete serving configs to Search Admin in the future. [1]: https://github.com/alphagov/govuk-infrastructure/pull/1680 --- app/models/serving_config.rb | 29 +++++++++++ config/locales/en.yml | 13 +++++ .../20250212115000_create_serving_configs.rb | 22 ++++++++ db/schema.rb | 12 ++++- lib/tasks/bootstrap.rake | 30 +++++++++++ spec/factories.rb | 7 +++ spec/models/serving_config_spec.rb | 51 +++++++++++++++++++ 7 files changed, 163 insertions(+), 1 deletion(-) create mode 100644 app/models/serving_config.rb create mode 100644 db/migrate/20250212115000_create_serving_configs.rb create mode 100644 lib/tasks/bootstrap.rake create mode 100644 spec/models/serving_config_spec.rb diff --git a/app/models/serving_config.rb b/app/models/serving_config.rb new file mode 100644 index 00000000..c79b9bf5 --- /dev/null +++ b/app/models/serving_config.rb @@ -0,0 +1,29 @@ +# Represents a serving config(uration) on Discovery Engine. +# +# A serving config is a specific endpoint on an engine which is used for querying. +# +# Having more than one serving config on an engine is helpful because each one can have different +# sets of controls attached to it. +# +# see https://cloud.google.com/ruby/docs/reference/google-cloud-discovery_engine-v1beta/latest/Google-Cloud-DiscoveryEngine-V1beta-ServingConfig +class ServingConfig < ApplicationRecord + include DiscoveryEngineNameable + + # Tracks what this serving config is used for + enum :use_case, { + # Used for actual public search on GOV.UK, for example the default serving config or any + # additional serving configs used for AB tests + live: 0, + # Used by search administrators for testing purposes, for example to try out a new control + # before adding it to a live serving config + preview: 1, + # Used internally, can only be modified programatically (not through the Search Admin UI) + system: 2, + }, validate: true + + # Don't allow changing remote_resource_id after creation + attr_readonly :remote_resource_id + + validates :display_name, presence: true + validates :remote_resource_id, presence: true, uniqueness: true +end diff --git a/config/locales/en.yml b/config/locales/en.yml index dd90a122..9cc6c65e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -77,6 +77,9 @@ en: recommended_link: one: external link other: external links + serving_config: + one: serving configuration + other: serving configurations attributes: control: comment: Comment @@ -94,6 +97,16 @@ en: description: Description keywords: Keywords comment: Comments + serving_config: + display_name: Name + use_case: Kind + use_case_values: + live: Live + preview: Preview + system: System + name: Google Cloud resource name + remote_resource_id: Google Cloud ID + description: Description errors: models: diff --git a/db/migrate/20250212115000_create_serving_configs.rb b/db/migrate/20250212115000_create_serving_configs.rb new file mode 100644 index 00000000..0b5043e2 --- /dev/null +++ b/db/migrate/20250212115000_create_serving_configs.rb @@ -0,0 +1,22 @@ +class CreateServingConfigs < ActiveRecord::Migration[8.0] + def change + create_table :serving_configs do |t| + t.integer :use_case, + null: false, + comment: "An enum declaring what use case this serving config is for" + t.string :display_name, + null: false, + comment: "A human-readable name" + t.string :description, + null: false, + comment: "A description of this serving config's purpose" + t.string :remote_resource_id, + null: false, + comment: "The ID of this serving config on Discovery Engine" + + t.timestamps + + t.index :remote_resource_id, unique: true + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 5f41d01d..2db751aa 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2025_02_10_143408) do +ActiveRecord::Schema[8.0].define(version: 2025_02_12_115000) do create_table "control_boost_actions", charset: "utf8mb3", force: :cascade do |t| t.string "filter_expression", null: false t.float "boost_factor", null: false @@ -47,6 +47,16 @@ t.index ["link"], name: "index_recommended_links_on_link", unique: true end + create_table "serving_configs", charset: "utf8mb3", force: :cascade do |t| + t.integer "use_case", null: false, comment: "An enum declaring what use case this serving config is for" + t.string "display_name", null: false, comment: "A human-readable name" + t.string "description", null: false, comment: "A description of this serving config's purpose" + t.string "remote_resource_id", null: false, comment: "The ID of this serving config on Discovery Engine" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["remote_resource_id"], name: "index_serving_configs_on_remote_resource_id", unique: true + end + create_table "users", charset: "utf8mb3", force: :cascade do |t| t.string "name" t.string "email" diff --git a/lib/tasks/bootstrap.rake b/lib/tasks/bootstrap.rake new file mode 100644 index 00000000..d125a2c2 --- /dev/null +++ b/lib/tasks/bootstrap.rake @@ -0,0 +1,30 @@ +# Tasks to seed the app's production environments, for example creating records for pre-existing +# resources. +namespace :bootstrap do + # As of Feb 2025, the Ruby client for Discovery Engine does not allow full lifecycle management + # of serving config resources (only updating existing ones) - so we created the following serving + # configs in Terraform in `govuk-infrastructure`. + # + # see https://github.com/alphagov/govuk-infrastructure/pull/1680 + desc "Creates ServingConfig records for resources already created in Terraform" + task seed_serving_configs: :environment do + ServingConfig.create!( + use_case: :live, + display_name: "Default", + description: "Used by live GOV.UK site search", + remote_resource_id: "govuk_default", + ) + ServingConfig.create!( + use_case: :preview, + display_name: "Preview", + description: "A preview serving config to test out changes", + remote_resource_id: "govuk_preview", + ) + ServingConfig.create!( + use_case: :system, + display_name: "Raw", + description: "A serving config without any controls applied", + remote_resource_id: "govuk_raw", + ) + end +end diff --git a/spec/factories.rb b/spec/factories.rb index 82edd533..cc95cd70 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -47,6 +47,13 @@ keywords { "tax, self assessment, hmrc" } end + factory :serving_config do + use_case { :live } + display_name { "Serving config" } + description { "A serving configuration" } + remote_resource_id { "serving-config" } + end + factory :user do factory :admin_user do permissions { %w[admin] } diff --git a/spec/models/serving_config_spec.rb b/spec/models/serving_config_spec.rb new file mode 100644 index 00000000..b26865da --- /dev/null +++ b/spec/models/serving_config_spec.rb @@ -0,0 +1,51 @@ +RSpec.describe ServingConfig, type: :model do + describe "validations" do + subject(:serving_config) { build(:serving_config) } + + it { is_expected.to be_valid } + + context "without a display name" do + before do + serving_config.display_name = nil + end + + it "is invalid" do + expect(serving_config).to be_invalid + expect(serving_config.errors).to be_of_kind(:display_name, :blank) + end + end + + context "without a remote resource ID" do + before do + serving_config.remote_resource_id = nil + end + + it "is invalid" do + expect(serving_config).to be_invalid + expect(serving_config.errors).to be_of_kind(:remote_resource_id, :blank) + end + end + + context "with a duplicate remote resource ID" do + before do + create(:serving_config, remote_resource_id: "dupe") + serving_config.remote_resource_id = "dupe" + end + + it "is invalid" do + expect(serving_config).to be_invalid + expect(serving_config.errors).to be_of_kind(:remote_resource_id, :taken) + end + end + end + + describe "#remote_resource_id" do + subject(:serving_config) { create(:serving_config, remote_resource_id: "hello") } + + it "is immutable after initial creation" do + expect { serving_config.update!(remote_resource_id: "goodbye") }.to raise_error( + ActiveRecord::ReadonlyAttributeError, + ) + end + end +end From e57333e40e3a7405a20c9f281d8bd70a2296fdc8 Mon Sep 17 00:00:00 2001 From: Christian Sutter Date: Tue, 11 Feb 2025 12:25:25 +0000 Subject: [PATCH 2/3] Serving configs: Add basic index/show UI As these aren't currently user-manageable in Search Admin, we don't need any UI to create, edit, or delete them. --- app/controllers/serving_configs_controller.rb | 19 +++++++++ app/helpers/serving_configs_helper.rb | 17 ++++++++ app/views/layouts/search.html.erb | 5 +++ app/views/serving_configs/index.html.erb | 18 ++++++++ app/views/serving_configs/show.html.erb | 42 +++++++++++++++++++ config/locales/en.yml | 4 ++ config/routes.rb | 1 + spec/system/serving_configs_spec.rb | 38 +++++++++++++++++ 8 files changed, 144 insertions(+) create mode 100644 app/controllers/serving_configs_controller.rb create mode 100644 app/helpers/serving_configs_helper.rb create mode 100644 app/views/serving_configs/index.html.erb create mode 100644 app/views/serving_configs/show.html.erb create mode 100644 spec/system/serving_configs_spec.rb diff --git a/app/controllers/serving_configs_controller.rb b/app/controllers/serving_configs_controller.rb new file mode 100644 index 00000000..323bf724 --- /dev/null +++ b/app/controllers/serving_configs_controller.rb @@ -0,0 +1,19 @@ +class ServingConfigsController < ApplicationController + before_action :set_serving_config, only: %i[show] + + self.primary_navigation_area = :search + self.secondary_navigation_area = :serving_configs + layout "search" + + def index + @serving_configs = ServingConfig.order(:use_case, :display_name) + end + + def show; end + +private + + def set_serving_config + @serving_config = ServingConfig.find(params[:id]) + end +end diff --git a/app/helpers/serving_configs_helper.rb b/app/helpers/serving_configs_helper.rb new file mode 100644 index 00000000..1581b9d4 --- /dev/null +++ b/app/helpers/serving_configs_helper.rb @@ -0,0 +1,17 @@ +module ServingConfigsHelper + SERVING_CONFIG_USE_CASE_TAG_COLOURS = { + live: "green", + preview: "yellow", + system: "grey", + }.freeze + + def serving_config_use_case_tag(serving_config) + scope = "activerecord.attributes.serving_config.use_case_values" + colour = SERVING_CONFIG_USE_CASE_TAG_COLOURS[serving_config.use_case.to_sym] + + tag.span( + t(serving_config.use_case, scope:), + class: "govuk-tag govuk-tag--#{colour}", + ) + end +end diff --git a/app/views/layouts/search.html.erb b/app/views/layouts/search.html.erb index 47b70887..74771456 100644 --- a/app/views/layouts/search.html.erb +++ b/app/views/layouts/search.html.erb @@ -12,6 +12,11 @@ href: controls_path, current: secondary_navigation_area == :controls, }, + { + label: t("serving_configs.index.page_title"), + href: serving_configs_path, + current: secondary_navigation_area == :serving_configs, + }, ] } %> <% end %> diff --git a/app/views/serving_configs/index.html.erb b/app/views/serving_configs/index.html.erb new file mode 100644 index 00000000..7bd5a21a --- /dev/null +++ b/app/views/serving_configs/index.html.erb @@ -0,0 +1,18 @@ +<%= render "page_title", title: t(".page_title") %> + +
+ <%= render "govuk_publishing_components/components/table", { + head: [ + { text: t_model_attr(:display_name) }, + { text: t_model_attr(:use_case) }, + { text: t_model_attr(:description) }, + ], + rows: @serving_configs.map do |serving_config| + [ + { text: link_to(serving_config.display_name, serving_config, class: "govuk-link") }, + { text: serving_config_use_case_tag(serving_config) }, + { text: serving_config.description }, + ] + end + } %> +
diff --git a/app/views/serving_configs/show.html.erb b/app/views/serving_configs/show.html.erb new file mode 100644 index 00000000..1d951ce0 --- /dev/null +++ b/app/views/serving_configs/show.html.erb @@ -0,0 +1,42 @@ +<% content_for(:breadcrumbs) do %> + <%= render "govuk_publishing_components/components/breadcrumbs", { + breadcrumbs: [ + { + title: t("serving_configs.index.page_title"), + url: serving_configs_path + }, + { + title: @serving_config.display_name + } + ] + } %> +<% end %> + +<%= render "page_title", { + title: @serving_config.display_name, +} %> + +<%= render "govuk_publishing_components/components/summary_list", { + items: [ + { + field: t_model_attr(:display_name), + value: @serving_config.display_name, + }, + { + field: t_model_attr(:use_case), + value: serving_config_use_case_tag(@serving_config), + }, + { + field: t_model_attr(:description), + value: @serving_config.description, + }, + { + field: t_model_attr(:remote_resource_id), + value: tag.code(@serving_config.remote_resource_id), + }, + { + field: t_model_attr(:name), + value: tag.code(@serving_config.name), + } + ] +} %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 9cc6c65e..86de7734 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -166,3 +166,7 @@ en: destroy: success: The external link was successfully deleted. failure: The external link could not be deleted. + + serving_configs: + index: + page_title: Serving configs diff --git a/config/routes.rb b/config/routes.rb index d4ee3876..cbb8a423 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -13,6 +13,7 @@ resources :with_boost_actions, controller: :controls, action_type: Control::BoostAction resources :with_filter_actions, controller: :controls, action_type: Control::FilterAction end + resources :serving_configs resources :recommended_links, path: "/recommended-links" diff --git a/spec/system/serving_configs_spec.rb b/spec/system/serving_configs_spec.rb new file mode 100644 index 00000000..1b8d89b9 --- /dev/null +++ b/spec/system/serving_configs_spec.rb @@ -0,0 +1,38 @@ +RSpec.describe "Serving configs", type: :system do + scenario "Viewing serving configs" do + given_several_serving_configs_exist + + when_i_visit_the_serving_configs_page + then_i_should_see_all_serving_configs + and_i_can_click_through_to_a_serving_config + end + + def given_several_serving_configs_exist + @live = create( + :serving_config, + use_case: :live, + remote_resource_id: "live-serving-config", + display_name: "Live serving config", + ) + @preview = create( + :serving_config, + use_case: :preview, + remote_resource_id: "preview-serving-config", + display_name: "Preview serving config", + ) + end + + def when_i_visit_the_serving_configs_page + visit serving_configs_path + end + + def then_i_should_see_all_serving_configs + expect(page).to have_link("Live serving config") + expect(page).to have_link("Preview serving config") + end + + def and_i_can_click_through_to_a_serving_config + click_link "Live serving config" + expect(page).to have_selector("h1", text: "Live serving config") + end +end From 271720405485a43eb6864a113880b33d12854453 Mon Sep 17 00:00:00 2001 From: Christian Sutter Date: Tue, 11 Feb 2025 12:25:54 +0000 Subject: [PATCH 3/3] Serving configs: Add Finder Frontend preview This allows users to preview the results from a serving config on Finder Frontend using the `debug_serving_config` parameter. --- app/models/serving_config.rb | 8 ++++++++ app/views/serving_configs/show.html.erb | 10 ++++++++++ config/locales/en.yml | 3 +++ spec/models/serving_config_spec.rb | 18 ++++++++++++++++++ 4 files changed, 39 insertions(+) diff --git a/app/models/serving_config.rb b/app/models/serving_config.rb index c79b9bf5..499413c7 100644 --- a/app/models/serving_config.rb +++ b/app/models/serving_config.rb @@ -26,4 +26,12 @@ class ServingConfig < ApplicationRecord validates :display_name, presence: true validates :remote_resource_id, presence: true, uniqueness: true + + # A URL to preview this serving config on Finder Frontend + def preview_url + FinderFrontendSearch.new( + keywords: "example search", + debug_serving_config: remote_resource_id, + ).url + end end diff --git a/app/views/serving_configs/show.html.erb b/app/views/serving_configs/show.html.erb index 1d951ce0..79e8701f 100644 --- a/app/views/serving_configs/show.html.erb +++ b/app/views/serving_configs/show.html.erb @@ -16,6 +16,16 @@ title: @serving_config.display_name, } %> +
+ <%= render "govuk_publishing_components/components/button", { + text: t(".buttons.preview"), + href: @serving_config.preview_url, + target: "_blank", + secondary_quiet: true, + inline_layout: true + } %> +
+ <%= render "govuk_publishing_components/components/summary_list", { items: [ { diff --git a/config/locales/en.yml b/config/locales/en.yml index 86de7734..a14cc66b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -170,3 +170,6 @@ en: serving_configs: index: page_title: Serving configs + show: + buttons: + preview: Preview on GOV.UK diff --git a/spec/models/serving_config_spec.rb b/spec/models/serving_config_spec.rb index b26865da..ea3ab358 100644 --- a/spec/models/serving_config_spec.rb +++ b/spec/models/serving_config_spec.rb @@ -48,4 +48,22 @@ ) end end + + describe "#preview_url" do + subject(:serving_config) { create(:serving_config, remote_resource_id: "hello") } + + let(:finder_frontend_search) do + instance_double(FinderFrontendSearch, url: "https://example.org") + end + + before do + allow(FinderFrontendSearch).to receive(:new) + .with(keywords: "example search", debug_serving_config: "hello") + .and_return(finder_frontend_search) + end + + it "returns the preview URL" do + expect(serving_config.preview_url).to eq("https://example.org") + end + end end