Added ADR around nested facet logic implementation #3020
Conversation
|
|
||
| Pros: | ||
|
|
||
| - Consistency of 2 separate facets being displayed to users upon tagging and viewing metadata. |
There was a problem hiding this comment.
I'm not sure this consistency is really a pro, is it? These are two separate user groups so there isn't really any added value in having a similar UX
There was a problem hiding this comment.
yea agreed, i'm also not keen on having validation to cross check across the two fields - but we'd note it here in any case as part of the alternatives we'd considered.
docs/adr/0001-implement-nested-facets-tagging-for-specialist-documents-as-a-single-dropdown.md
Outdated
Show resolved
Hide resolved
minhngocd
force-pushed
the
adr-nested-facet-merged-list
branch
from
3facb55 to
59e3f15
Compare
|
@ryanb-gds updated |
|
|
||
| ### Have tagging done as 2 separate facets with validation | ||
|
|
||
| Even on publisher side, surface all of a facet's parents facets as one dropdown, and all of the sub-facets as a separate dropdown. As outlined on [this branch](https://github.com/alphagov/specialist-publisher/compare/intellectual-property-office-finder_2?expand=1). |
There was a problem hiding this comment.
Suggest linking to the commit instead, as the branch can easily be deleted in future, but the commit should live forever (citation needed!)
| Even on publisher side, surface all of a facet's parents facets as one dropdown, and all of the sub-facets as a separate dropdown. As outlined on [this branch](https://github.com/alphagov/specialist-publisher/compare/intellectual-property-office-finder_2?expand=1). | |
| Even on publisher side, surface all of a facet's parents facets as one dropdown, and all of the sub-facets as a separate dropdown. As outlined in [this commit](https://github.com/alphagov/specialist-publisher/commit/c7346db771df5ccafd5fff8f7f687a8211436f6a). |
| Cons: | ||
|
|
||
| - Repeat of information. The sub-facets are also prefixed with parent facet in any case (e.g. to differentiate between multiple "Not Applicable" sub-facet entries). Users will have to tag both parent facet and the sub-facet with the prefix. | ||
| - There is no structural tie between the parent facet and the sub-facet when tagging. Nothing stopping the user from tagging mismatching pairs or not tagging a sub-facet. |
There was a problem hiding this comment.
"Not tagging a sub-facet" is allowed though, isn't it? As per our assumptions.
There was a problem hiding this comment.
hmm I'm not sure we clarified between
- Having a mixture of leaf nodes - hence tagging a main-facet because it doesn't have sub-facets
- Tagging a main-facet only despite it also having sub-facets
I think in the future we do want to support tagging main-facet only despite having sub-facets, but we felt like we could clarify that later down the line, as the current MVP doesn't require it.
| What we have implemented: | ||
|
|
||
| - On the publisher side for tagging, a single dropdown that merges the facets' values (main facet and sub facets, essentially leaf nodes) | ||
| - 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 in `specialist-publisher/app/models/document.rb` |
There was a problem hiding this comment.
Just checking whether what's written here is what's been implemented. The ADR implies (to me) that publishers can choose both "sub facets" and parent "facets" from the dropdown - but I'm only seeing subfacets in the dropdown on https://specialist-publisher.integration.publishing.service.gov.uk/trademark-decisions/new.
There was a problem hiding this comment.
@ChrisBAshton similar to point above - in our implementation:
- we allow main facets to be defined without subfacets. It would show up in the dropdown if that is the case. Unfortunately for this finder, it doesn't have any main facets without sub facets.
- what we don't allow, is if the main facet has sub-facets, then you can only choose the sub-facets (leaf nodes). Reason being we still have things to clarify about that behaviour (is it equivalent to tagging all subs, or no subs). Since the current finder doesn't require it, i thought de-scoping would be fine.
happy to discuss further
minhngocd
force-pushed
the
adr-nested-facet-merged-list
branch
from
59e3f15 to
1d8af10
Compare
.adr-dir file checked in for using the tool `adr-tools` for ADR template generation and management
minhngocd
force-pushed
the
adr-nested-facet-merged-list
branch
from
1d8af10 to
fb44823
Compare
https://trello.com/c/OA4zE8m0/3430-ipo-finder-request
https://trello.com/c/4RJusHld/3374-nested-facets-add-new-document
PR where implementation took place: #2923
Follow these steps if you are doing a Rails upgrade.