-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Aggregation updates #832
base: main
Are you sure you want to change the base?
Aggregation updates #832
Conversation
d78a698
to
ad48316
Compare
b4c189d
to
4cd967e
Compare
…ection/catalogue-api into Aggregation-updates
…ection/catalogue-api into Aggregation-updates
…ection/catalogue-api into Aggregation-updates
…ection/catalogue-api into Aggregation-updates
…ection/catalogue-api into Aggregation-updates
ab96b63
to
63957de
Compare
b0a6815
to
55ed86f
Compare
55ed86f
to
eab08b3
Compare
747b1de
to
5cd04a4
Compare
5cd04a4
to
0b00044
Compare
`source.genres.label`: Option[GenreFilter], | ||
`source.genres.concepts`: Option[GenreConceptFilter], | ||
`source.contributors.agent.label`: Option[ContributorsLabelFilter], | ||
`source.contributors.agent.id`: Option[ContributorsIdFilter], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the use of .id
in keeping with principle 1 in the API faceting RFC?
Filters are named by the JSON paths of the identified object that they filter or, if applied to an attribute other than the identifier, the path of that attribute
I would have expected the if filter to just be for example source.contributors.agent
, omitting the ID?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this should now be fixed
9cbb89f
to
153c8a6
Compare
What does this change?
wellcomecollection/platform#5825
Changes how we handle aggregations in Elasticsearch queries, following on changes to the catalogue pipeline.
The API now supports two aggregation types:
All existing production functionality should remain the same. From the frontend's perspective, the only change to the API involves renaming ID-based filters (
genres.concepts
→genres
), but the affected filters are only used behind a feature flag.Note
This branch has been rebased to #829.
Checklist
How to test
How can we measure success?
Added support for ID-based aggregations without negative side effects and without losing support for label-based concept aggregations.
Have we considered potential risks?