Call facets on pages where they are required#940
Conversation
There was a problem hiding this comment.
Pull request overview
Updates DOI-listing routes to explicitly request facet aggregations from the API so facet-driven UI elements (filters/sidebars/metrics) have the required meta data available.
Changes:
- Add an explicit
facetsquery parameter to multiple DOI queries. - Add
'disable-facets': 'false'to ensure facets are returned for those queries. - Apply this across global DOI index and scoped DOI pages (user/provider/repository contexts).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| app/routes/users/show/dois.js | Requests facets for the user-scoped DOI list query. |
| app/routes/users/show.js | Requests facets for the DOI meta query used on the user show route. |
| app/routes/repositories/show/dois/index.js | Requests facets for the repository-scoped DOI list query. |
| app/routes/providers/show/dois.js | Requests facets for the provider/consortium-scoped DOI list query. |
| app/routes/dois/index.js | Requests facets for the global DOI index query. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| facets: | ||
| 'affiliations,certificates,citations,clients,created,downloads,fieldsOfScience,licenses,linkChecksStatus,prefixes,providers,published,registered,resourceTypes,schemaVersions,states,subjects,views', | ||
|
|
||
| 'disable-facets': 'false' |
There was a problem hiding this comment.
The facets list is duplicated verbatim across multiple routes in this PR. To avoid inconsistencies and simplify future updates, extract it to a shared constant (e.g., a util/service) and reference it from each route.
| facets: | ||
| 'affiliations,certificates,citations,clients,created,downloads,fieldsOfScience,licenses,linkChecksStatus,prefixes,providers,published,registered,resourceTypes,schemaVersions,states,subjects,views', | ||
|
|
||
| 'disable-facets': 'false' |
There was a problem hiding this comment.
The full facets list is hard-coded here and duplicated in several other routes. Consider extracting it into a shared constant/helper (and ideally limiting it to only the facets actually displayed for DOI lists) to prevent future drift and reduce payload size.
| facets: | ||
| 'affiliations,certificates,citations,clients,created,downloads,fieldsOfScience,licenses,linkChecksStatus,prefixes,providers,published,registered,resourceTypes,schemaVersions,states,subjects,views', | ||
|
|
||
| 'disable-facets': 'false' |
There was a problem hiding this comment.
The full facets list is hard-coded here and duplicated in several other routes. Consider extracting it into a shared constant/helper (and ideally limiting it to only the facets actually displayed for DOI lists) to prevent future drift and reduce payload size.
| 'user-id': user.get('id'), | ||
|
|
||
| facets: | ||
| 'affiliations,certificates,citations,clients,created,downloads,fieldsOfScience,licenses,linkChecksStatus,prefixes,providers,published,registered,resourceTypes,schemaVersions,states,subjects,views', |
There was a problem hiding this comment.
This DOI query now asks the API for many facet buckets. The DOI facets UI only uses a smaller subset (states/resourceTypes/published/created/registered/clients/affiliations/prefixes/certificates/schemaVersions/sources/linkChecksStatus). Consider requesting only the facets that are actually rendered on this page to reduce response size and facet computation cost.
| 'affiliations,certificates,citations,clients,created,downloads,fieldsOfScience,licenses,linkChecksStatus,prefixes,providers,published,registered,resourceTypes,schemaVersions,states,subjects,views', | |
| 'states,resourceTypes,published,created,registered,clients,affiliations,prefixes,certificates,schemaVersions,sources,linkChecksStatus', |
| 'client-id': this.modelFor('repositories/show').get('id'), | ||
|
|
||
| facets: | ||
| 'affiliations,certificates,citations,clients,created,downloads,fieldsOfScience,licenses,linkChecksStatus,prefixes,providers,published,registered,resourceTypes,schemaVersions,states,subjects,views', |
There was a problem hiding this comment.
This DOI query now asks for a broad set of facets, but the DOI facets UI only consumes a subset. Consider narrowing the facets param to the facets actually rendered for DOI lists to reduce response size and backend facet computation.
| 'affiliations,certificates,citations,clients,created,downloads,fieldsOfScience,licenses,linkChecksStatus,prefixes,providers,published,registered,resourceTypes,schemaVersions,states,subjects,views', | |
| 'clients,providers,created,published,registered,resourceTypes,states,subjects,licenses', |
| facets: | ||
| 'affiliations,certificates,citations,clients,created,downloads,fieldsOfScience,licenses,linkChecksStatus,prefixes,providers,published,registered,resourceTypes,schemaVersions,states,subjects,views', | ||
|
|
||
| 'disable-facets': 'false' |
There was a problem hiding this comment.
This DOI query now requests many facet buckets, but the DOI facets UI only uses a subset. Consider trimming the facets list to what the UI renders to reduce backend facet work and response size.
| facets: | ||
| 'affiliations,certificates,citations,clients,created,downloads,fieldsOfScience,licenses,linkChecksStatus,prefixes,providers,published,registered,resourceTypes,schemaVersions,states,subjects,views', | ||
|
|
||
| 'disable-facets': 'false' |
There was a problem hiding this comment.
The full facets list is hard-coded here (and duplicated across multiple DOI routes). This makes it easy for routes to drift and increases maintenance cost when facets are added/removed. Consider extracting the facets list (and the disable-facets flag) into a shared constant/helper so all routes use a single source of truth.
| sort: params.sort || '-updated', | ||
|
|
||
| facets: | ||
| 'affiliations,certificates,citations,clients,created,downloads,fieldsOfScience,licenses,linkChecksStatus,prefixes,providers,published,registered,resourceTypes,schemaVersions,states,subjects,views', |
There was a problem hiding this comment.
This route now requests a large set of facets, but the DOI facets UI (DoiList) only references a subset (e.g., states/resourceTypes/published/created/registered/clients/affiliations/prefixes/certificates/schemaVersions/sources/linkChecksStatus). Requesting extra facets can increase backend work and response size. Consider limiting the facets param to only the facets actually rendered/needed on this page.
| 'affiliations,certificates,citations,clients,created,downloads,fieldsOfScience,licenses,linkChecksStatus,prefixes,providers,published,registered,resourceTypes,schemaVersions,states,subjects,views', | |
| 'states,resourceTypes,published,created,registered,clients,affiliations,prefixes,certificates,schemaVersions,sources,linkChecksStatus', |
|
Copilots comments on centralizing the settings and focusing on the facets that are used are relevant, but not stoppers for this PR. That could be scheduled for future work. |
|
Issue I discussed in the call that, when I select some facet combination, I see error message but API does not give any error it returns empty array. I tried same combination on stage, I guess it existing behaviour https://doi.datacite.org/dois?created=2018®istered=2020&resource-type-id=service |
This error seems to be happening because that API request returns 0 results: https://api.datacite.org/dois?affiliation=true&affiliation-id=&certificate=&client-id=&created=2018&include=client&link-check-status=&page[number]=1&page[size]=25&person-id=&prefix=&provider-id=&publisher=true&query=®istered=2020&resource-type-id=service&schema-version=&size=25&sort=-updated&source=&state=&year= So not a problem with your PR, I don't think—more an issue that it is possible to select facets that result in 0 results and also the way we report a 0 result query. |
3 participants
https://github.com/datacite/product-backlog/issues/433
Summary
The DataCite API will stop returning
/doisfacet aggregations inmetaby default. This PR updates all DOI list queries in bracco to explicitly request facets (and force-enable them) so the facet sidebar continues to work.Changes
facets=...anddisable-facets=falseto/doisqueries in:app/routes/dois/index.js)app/routes/providers/show/dois.js)app/routes/repositories/show/dois/index.js)app/routes/users/show/dois.js)app/routes/users/show.js)