Some finessing from QA - item filters and undefined sorting #406
Conversation
charmingduchess
commented
Sep 13, 2024
charmingduchess
commented
- refactored sort method to actually work
- added check make sure inner_hits query still happen when item filters are applied
| done() | ||
| }) | ||
| }) | ||
| it('reverts to filtering on inner_hits when item filters are included in the query and returns default item size', (done) => { |
There was a problem hiding this comment.
this is not actually a documented requirement, but I think item filters should override all_items until we have the work done in discovery api to filter and return more than items_size number of items
| } | ||
| return a[property]?.[0] > b[property]?.[0] ? -1 : 1 | ||
| } | ||
| } |
| const itemFilterQuery = queryKeys | ||
| .filter((param) => param.startsWith('item_')).length > 0 | ||
| const returnAllItems = queryKeys?.includes('all_items') && | ||
| !itemFilterQuery |
There was a problem hiding this comment.
This is fine, but I kind of think use of ?all_items with any item filter should return a 4xx earlier, since it's nonsensical. That would mean we don't have to handle that possibility here. As written, it looks like we'll just ignore all_items if any item filter is applied. But above is also fine.
There was a problem hiding this comment.
Ah yes there was a decision made around this that I forgot to explain. The front end spec is that a user can filter items, and then click view all to de-paginate the results. We decided to expedite the release of this endpoint by letting the front end decide whether to use all_items or to revert to the already implemented batched filtering solution. Eventually, we actually do want to have some combination of params that will return all of a bib's items for a given filter.
