Adding filtering and pagination to collectionList APIs#12436
Adding filtering and pagination to collectionList APIs#12436stevenwinship wants to merge 2 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
5a0bf16 to
c73b373
Compare
This comment has been minimized.
This comment has been minimized.
c73b373 to
eca44a4
Compare
This comment has been minimized.
This comment has been minimized.
eca44a4 to
6c33308
Compare
This comment has been minimized.
This comment has been minimized.
6c33308 to
98baff1
Compare
This comment has been minimized.
This comment has been minimized.
98baff1 to
e166e23
Compare
This comment has been minimized.
This comment has been minimized.
e166e23 to
e1391a1
Compare
This comment has been minimized.
This comment has been minimized.
qqmyers
left a comment
There was a problem hiding this comment.
I've looked through the code and it looks like it addresses the issue.
However, it seems to be a one off approach - no other paginated apis use 'nextOffset' and 'previousOffset', this api asks for offset (in # of items, not pages) and pageSize, while /api/datasets/{id}/versions/{versionId}/files uses offset and limit, this api returns a 'count' while the /files api and three others return 'totalCount', written by an ok() method while this one sends a 'count' (in all cases, but I think this is the number returned when paging, not all that exist) along with a duplicate 'pageSize' when paging is used and these are all done in the jsonArray(LIst, Pager) call (rather than somewhere reusable).
I don't know how much to push into any give PR, but, since pagination is a common theme in the API changes for the modern interface, can we standardize an approach (has this been discussed already?)? Are offsets in terms of pages or items? Do API calls return prev and next? Are the items or page numbers? Etc. Can the apis use the same keys? In the same json structure?
I've marked this as requesting changes, but I think we made need some discussion to decide what (if anything) should actually be changed before this goes forward.
e1391a1 to
08ef2bb
Compare
|
📦 Pushed preview images as 🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name. |
What this PR does / why we need it: Result lists for collections can be quite large so a way to filter and use pagination is needed.
Which issue(s) this PR closes:#12423
Special notes for your reviewer:
Suggestions on how to test this:
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Is there a release notes update needed for this change?: included
Additional documentation: