Add ability to manage adjustments #1344
Conversation
csutter
force-pushed
the
adj2
branch
3 times, most recently
from
f1ca61c to
01235be
Compare
There was a problem hiding this comment.
Wow! That was a mind bender to review. 😱 Well done for figuring this all out ⭐
This is looking really good. There are some inline comments, most are about making things easier for future developers to understand. So a couple of requests to expand commit messages and about naming. I think for changes relating to Discovery Engine we need to assume that anyone looking at this code have no prior knowledge about it.
| # Saves the record and creates or updates the corresponding control in Discovery Engine depending | ||
| # on whether the record is new. Rolls back in case of error, and adds details of any remote API | ||
| # errors to the record so they can be displayed to the user. | ||
| def save_with_control |
There was a problem hiding this comment.
Should this methods be wrapped in an included block? as in: https://api.rubyonrails.org/classes/ActiveSupport/Concern.html
There was a problem hiding this comment.
No, because we want these methods will be added as instance methods on the including class, as with a vanilla (non-concern) Ruby module 💡
included is used for macros and other things that should be evaluated on a class level when the concern is included.
| it "does not persist the changes to the record" do | ||
| record.save_with_control | ||
|
|
||
| expect(record).to be_changed |
There was a problem hiding this comment.
Should this be not_to rather than to?
There was a problem hiding this comment.
Agree this doesn't read amazingly, but changed? refers to unsaved changes, so we're expecting it to be true because the record has failed to save (and thus is still "changed").
csutter
force-pushed
the
adj2
branch
5 times, most recently
from
6c393a0 to
8027b22
Compare
An adjustment is our local representation of a Discovery Engine control of kind `boost` or `filter`. These have very similar semantics (they both "adjust" the results a user gets for a given query) so it makes sense to consolidate them into a single model on the Search Admin side. `kind` is set up as an ActiveRecord enum, with the `suffix` option to give us more meaningful predicate methods (`#boost_kind?` and `#filter_kind?`, instead of `#boost?` and `#filter?`). Both kinds of adjustment have a `filter_expression` (in a Discovery Engine-specific query language) that defines what documents they apply to. The difference between them is that `boost` adjustments will apply a positive or negative rank weighting to all documents matched by their `filter_expression`, whereas `filter` adjustments will just exclude them from results outright. For the sake of simplicity, this uses an ActiveRecord enum to represent a control's kind (action/type), rather than Single Table Inheritance or Delegated Types.
- Add `AdjustmentsController` with `#index` and `#show` actions, plus view templates and routes - Add link to adjustments list page to application menu - Add `ModelTranslationHelper#t_model_enum_value` for less convoluted enum values translation
- Add `#new` and `#create` to `AdjustmentsController`, plus view templates - Add buttons to create filters and boosts to index page - Ensure new action can only be shown for valid kinds of adjustments
- Add `#edit` and `#update` to `AdjustmentsController`, plus view templates - Add "Edit" link to show page
- Add `#destroy` to `AdjustmentsController` - Add delete button to show template
- Add Discovery Engine gem and set up configuration from environment - Add `grpc_mock` gem to disable network connections over gRPC (in case one of our tests wants to run away) - Ensure appropriate Discovery Engine versioned libraries are loaded and available in tests
This adds a `DiscoveryEngine::Control` model to represent the remote control resources on Discovery Engine, and provide a simple create/update/delete API. It also adds a `#control_action` method to `Adjustment` to return the action specification for a given adjustment.
This adds a `HasDiscoveryEngineControl` model concern used by `Adjustment` (and eventually other control types) to persist themselves to Discovery Engine via the `DiscoveryEngine::Control` model. This happens in a transaction to make sure we don't persist the local record if remote modification fails.
This changes the `AdjustmentsController` to now call the new `_and_sync` methods provided by the `DiscoveryEngineSyncable` concern on `Adjustment`.
This adds the ability to manage adjustments to Search Admin.
An adjustment is a specific modification of the search results returned on GOV.UK. It consists of a descriptive name, and a filter expression that describes what content items (documents) it applies to based on their metadata.
An adjustment can either change the ranking of matching content in results by a specific factor ("boost") or remove matching documents from results entirely ("filter"). Both of these kinds of adjustment are represented as serving controls ("controls") on the Discovery Engine API for Vertex AI Search. There are also additional types of serving controls available that we will use in upcoming features, so the ability to manage the remote control resources is kept generic from the start.
This adds a basic CRUD UI for adjustments to the application, as well as transactional synchronisation of adjustments to Discovery Engine.
This is the first iteration of this feature, and the following are not part of this PR to avoid it getting too big:
Screenshots