IPO trademark decisions finder #2923

kashifatcha · 2024-12-19T13:39:58Z

This PR implements a new feature - nested facets - and uses the IPO trademark decision finder's config to test the implementation. The current version includes all finder requirements, and is ready to be released as draft, alongside its other PRs: publishing-api and search-api.

At the core of the implementation is the decision to tag a document with both facet and subfacet, in order to minimise the changes required in publishing-api and search-api. Details can be found in the associated ADR. The tagging has been tested in integration, including that search-api returns the correct tags. This query returns the subfacet, as expected:

{
results: [
{
trademark_decision_grounds_section: [
"section-3-1-graphical-representation",
"section-3-4-and-section-3-5-other-issues"
],
trademark_decision_grounds_sub_section: [
"section-3-1-graphical-representation-is-it-graphically-represented",
"section-3-4-and-section-3-5-other-issues-protected-emblems-etc"
],
index: "govuk",
es_score: null,
_id: "/trademark-decisions/new-td1",
elasticsearch_type: "trademark_decision",
document_type: "trademark_decision"
}
]...}

Upcoming work will address Finder Frontend filtering.

We're also using a feature that is not very commonly used, found elsewhere on this finder - to help search long lists of facet options. This will be used in other upcoming finders, and we've proven that it works successfully here (required setting show_option_select_filter in specialist publisher, and the field type as searchable_multivalued_text in search api).

Specialist Publisher Trademark Decisions view:

Formed filled out:
!Note: we only allow selecting a child if the parent has any children. Whilst this is not the case for this finder config, if a nested facet value did not have any children, the parent value would appear as a select option.

Summary page:

Document preview
!Note: This is quite verbose on the nested values - we will discuss a different approach.

Finder summary

Finder

Finder Date filter

Searchable filter

Free text search

Mark

Company

Nested facets filter
!Note: nested facets are not currently rendering on the Front End, so we're only rendering parent options

Trello nested facets
Trello IPO finder

ryanb-gds

Good stuff, couple of minor suggestions but nothing that should stop merging if you're wanting to 🚢

Edit: well, except the failing example 😅

ryanb-gds · 2025-02-20T11:00:40Z

app/controllers/documents_controller.rb

+    return unless @document
+
+    @document.finder_schema.nested_facets.pluck("key", "sub_facet_key").each do |facet_key, sub_facet_key|
+      next unless @document.send(sub_facet_key)


Would respond_to? be more semantically appropriate here?

unfortunately no, because respond_to? would return true, but the actual value of it may be blank or nil

alternatively, i've been changing the public_sends to assign_attributes so i'm sure we can change it to get_attribute of some sort

app/models/document.rb

ryanb-gds · 2025-02-20T11:07:30Z

app/models/document.rb

+      assign_attributes(clean_key(nested_facet_from_schema["key"]) => parent_facet_value.uniq) if parent_facet_value.any?
+      assign_attributes(clean_key(nested_facet_from_schema["sub_facet_key"]) => nested_facet_value.uniq) if nested_facet_value.any?
+    end
+  end


I think at this point it's probably worth creating a facet model to abstract some of this away from the document like we did for email alerts, but we can maybe pair on that in a separate PR if anyone is interested? Might also save us from all the merging in the controller, not sure about that though

sounds good! yes, maybe we can do that as a separate thing

ryanb-gds · 2025-02-20T11:11:11Z

spec/controllers/documents_controller_spec.rb

+      expect(SecureRandom).to receive(:uuid).and_return(stub_content_id)
+    end
+
+    it "should create a document and send draft to Publishing API" do


Bit worrying that this test was missing!

We do cover this in feature tests too, but it's handy to have a controller test, closer to the code.

Maybe it makes sense for the feature tests to stay more UI-focused, and bring the publishing API expectations in controller tests.

ryanb-gds · 2025-02-20T11:12:38Z

spec/controllers/documents_controller_spec.rb

+          ],
+        },
+        "save" => "",
+        "document_type_slug" => "trademark-decisions",


I wonder if we should create ourselves a test specialist finder that never gets published. Would be nice not to have to use actual finders in our generic tests.

I like this, for the next person 😅

ryanb-gds · 2025-02-20T11:15:20Z

app/components/document_metadata_component.rb

@@ -0,0 +1,31 @@
+class DocumentMetadataComponent < ViewComponent::Base


I like that this is a view component now 🙌

Bumps [govuk_sidekiq](https://github.com/alphagov/govuk_sidekiq) from 9.0.5 to 9.0.6. - [Changelog](https://github.com/alphagov/govuk_sidekiq/blob/main/CHANGELOG.md) - [Commits](alphagov/govuk_sidekiq@v9.0.5...v9.0.6) --- updated-dependencies: - dependency-name: govuk_sidekiq dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>

Bumps [aws-sdk-s3](https://github.com/aws/aws-sdk-ruby) from 1.180.0 to 1.181.0. - [Release notes](https://github.com/aws/aws-sdk-ruby/releases) - [Changelog](https://github.com/aws/aws-sdk-ruby/blob/version-3/gems/aws-sdk-s3/CHANGELOG.md) - [Commits](https://github.com/aws/aws-sdk-ruby/commits) --- updated-dependencies: - dependency-name: aws-sdk-s3 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>

Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.18.2 to 1.18.3. - [Release notes](https://github.com/sparklemotion/nokogiri/releases) - [Changelog](https://github.com/sparklemotion/nokogiri/blob/v1.18.3/CHANGELOG.md) - [Commits](sparklemotion/nokogiri@v1.18.2...v1.18.3) --- updated-dependencies: - dependency-name: nokogiri dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>

Bumps [aws-sdk-s3](https://github.com/aws/aws-sdk-ruby) from 1.181.0 to 1.182.0. - [Release notes](https://github.com/aws/aws-sdk-ruby/releases) - [Changelog](https://github.com/aws/aws-sdk-ruby/blob/version-3/gems/aws-sdk-s3/CHANGELOG.md) - [Commits](https://github.com/aws/aws-sdk-ruby/commits) --- updated-dependencies: - dependency-name: aws-sdk-s3 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>

… finder schema Options_for returns a tuple of strings to populate select dropdowns which feels like view logic rather than model logic. Moving this out and exposing allowed values on the facet instead.

Add a helper method to render nested facets. In the schema, the parent facet now has a `sub_facets` field containing facet-like objects with `label` and `value` fields. We implemented a single dropdown that merges the facets' values (main facet and sub facets, essentially leaf nodes). The user sees dasherised labels in the format 'Parent label - child label'. Moved to using generic specialist form views for trademark decision view. The nested facet options method works for both cases whether or not the facet value has sub-facets or not, so it should be safe to use by default. Remove options_for and replace with select_options_for_facet

When we send the document to Publishing API, we write logic into the document model to figure out whether the leaf node is a parent facet or sub-facet value in the dropdown, and split the metadata we send to Publishing API into its own facet and a separate sub-facet. The reasoning behind this is that if we send them as separate facets, we can keep Publishing API and Search API functionality unchanged..

When we load the edit page or new page again, the document is loaded from Publishing API, with the facet and sub-facet being separate facets, but the dropdown element expects a combined leaf node dropdown, so we have to merge them back together again into the main facet field. We need to collate the values for the `ipo_grounds_section` and `ipo_grounds_sub_section` fields in the one field we do render (`ipo_grounds_section`).

…a summary Additional lookup is required to find the correct label for a given key, when the facet is nested.

Since this facet has over 150 options, we're configuring it to be searchable.

lauraghiorghisor-tw force-pushed the intellectual-property-office-finder branch 5 times, most recently from 53eb8b4 to 7fd2e6a Compare February 7, 2025 17:03

minhngocd force-pushed the intellectual-property-office-finder branch 3 times, most recently from 263dcb2 to e466a4c Compare February 11, 2025 16:38

lauraghiorghisor-tw force-pushed the intellectual-property-office-finder branch 7 times, most recently from 0a11ae4 to ec3b023 Compare February 13, 2025 09:03

minhngocd force-pushed the intellectual-property-office-finder branch from dcaaede to adfb209 Compare February 13, 2025 09:43

lauraghiorghisor-tw force-pushed the intellectual-property-office-finder branch 11 times, most recently from f0059cd to 34e5917 Compare February 14, 2025 23:38

lauraghiorghisor-tw changed the title ~~WIP blocked by nested facets work~~ IPO trademark decisions finder Feb 15, 2025

lauraghiorghisor-tw force-pushed the intellectual-property-office-finder branch 2 times, most recently from 5af63d3 to 35d0043 Compare February 17, 2025 09:41

lauraghiorghisor-tw force-pushed the intellectual-property-office-finder branch 4 times, most recently from d3df0a6 to e679a78 Compare February 17, 2025 13:03

minhngocd force-pushed the intellectual-property-office-finder branch from 833a217 to 8f60fd6 Compare February 17, 2025 19:18

lauraghiorghisor-tw force-pushed the intellectual-property-office-finder branch 2 times, most recently from 56791ed to ed1383c Compare February 18, 2025 10:56

minhngocd force-pushed the intellectual-property-office-finder branch 2 times, most recently from ddeb768 to ec77e96 Compare February 18, 2025 17:05

minhngocd mentioned this pull request Feb 19, 2025

Added ADR around nested facet logic implementation #3020

Merged

lauraghiorghisor-tw force-pushed the intellectual-property-office-finder branch from ec77e96 to 41925aa Compare February 20, 2025 09:28

ryanb-gds approved these changes Feb 20, 2025

View reviewed changes

lauraghiorghisor-tw force-pushed the intellectual-property-office-finder branch from 41925aa to 81ce561 Compare February 20, 2025 12:04

dependabot bot and others added 14 commits February 20, 2025 12:15

Add pry-byebug as a dependency

68ad316

Add IPO trademark decision finder

bd136c0

Refactoring: Move options_for into a view helper rather than being in…

a867881

… finder schema Options_for returns a tuple of strings to populate select dropdowns which feels like view logic rather than model logic. Moving this out and exposing allowed values on the facet instead.

Add feature specs

da90912

Extract metadata into a component

92d5a77

Render correct subfacet name and selected option label in the metadat…

45434a8

…a summary Additional lookup is required to find the correct label for a given key, when the facet is nested.

Show filter for hearing officer facet

ab6c332

Since this facet has over 150 options, we're configuring it to be searchable.

lauraghiorghisor-tw force-pushed the intellectual-property-office-finder branch from 81ce561 to ab6c332 Compare February 20, 2025 12:15

minhngocd merged commit d523555 into main Feb 20, 2025
11 checks passed

minhngocd deleted the intellectual-property-office-finder branch February 20, 2025 13:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IPO trademark decisions finder #2923

IPO trademark decisions finder #2923

kashifatcha commented Dec 19, 2024 •

edited by lauraghiorghisor-tw

Loading

ryanb-gds left a comment •

edited

Loading

ryanb-gds Feb 20, 2025

minhngocd Feb 20, 2025

minhngocd Feb 20, 2025

ryanb-gds Feb 20, 2025

minhngocd Feb 20, 2025

ryanb-gds Feb 20, 2025

lauraghiorghisor-tw Feb 20, 2025

lauraghiorghisor-tw Feb 20, 2025

ryanb-gds Feb 20, 2025

lauraghiorghisor-tw Feb 20, 2025

ryanb-gds Feb 20, 2025

		@@ -0,0 +1,31 @@
		class DocumentMetadataComponent < ViewComponent::Base

IPO trademark decisions finder #2923

IPO trademark decisions finder #2923

Conversation

kashifatcha commented Dec 19, 2024 • edited by lauraghiorghisor-tw Loading

ryanb-gds left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kashifatcha commented Dec 19, 2024 •

edited by lauraghiorghisor-tw

Loading

ryanb-gds left a comment •

edited

Loading