Spike: Create API Pagination Pattern
Description
Attachments
- 10 Jan 2017, 07:47 AM
- 09 Jan 2017, 11:40 PM
- 06 Jan 2017, 09:16 PM
blocks
causes
Confluence content
QAlity Plus - Test Management
Checklists
Activity
Looks good to me. Moving to done.
Hi @Josh Zamor,
Thank you very much – both for the code reviews and addition to the style guide. I made an extremely minor update to the later, but think it looks great. I also…
Removed most of the pagination-related branches from the repository. (The “feature/OLMIS-1512_experimental” one would be nice to keep for now.)
Added a pagination-related post to the dev forum.
Hope we’re good to go with the UI team, given the addition of tickets OLMIS-1709, OLMIS-1721, and OLMIS-1728.
Thanks again!
~ Ben
Review's look good and have been completed. I realized that we should put in a blurb in the styleguide for some of the decisions we made in this ticket and so I added that here: https://github.com/OpenLMIS/openlmis-template-service/commit/3486cdc744407b3685a4e6cc6c5ae4fa93c3fd00
@Ben Leibert: a couple questions:
do you agree with what I just added to the style guide? Would you add anything?
should we delete the https://openlmis.atlassian.net/browse/OLMIS-1512#icft=OLMIS-1512 branches or do they hold something we might want to revisit later?
I'm wondering if we need to do some socializing of this new pattern. A call would be ideal, though then we might need it with SD and TW. Could you instead post to the dev forum, describe the new pattern briefly, link to the style guide as well as perhaps a code example of what they should follow (could just be the /api/requisitions/search endpoint)?
I'm encountering permissions problems while trying to create another review in Fisheye. For now, @Josh Zamor, I hope it's thus okay to mention here that commits 4fbdd92 and 0b012e6 in the reference-data service are ready for review.
I'll also mention that I spent a lot of time trying to come up with a pattern which doesn't duplicate pagination-related response properties like "totalElements," "numberOfElements," and "totalPages." I bailed after getting to a point where:
A) The checkApiIsRaml task confirmed the RAML was valid, but
B) The parser used during integration tests would complain about schema which very clearly looked correct. I think this may involve a bug in our tooling, and would be glad to elaborate/demonstrate.
I thus stuck with a pattern similar to the one we currently use. It, along with our RAML in general, can hopefully be improved in subsequent releases. (On a related note, one of the tools we use released beta support for RAML 1.0 just this week.)
Hi @Josh Zamor. I think you had mentioned that I should assign this ticket to you when it's ready to be reviewed. It's thus yours now. : ) I created the review in Fisheye and hope you'll let me know what you think. Because there are aspects of the ticket which aren't covered by the review, I hope you'll reassign it to me afterward.
Details
Details
Assignee
Ben LeibertReporter
Mary Jo KochendorferStory Points
Components
Sprint
Fix versions
Priority
BlockerTime Assistant
Open Time Assistant
Time Assistant
Pick a pattern—don't reinvent the wheel.
Acceptance Criteria
Document of a proposed pattern for pagination, including how the API returns it
Sample RAML for pagination
Check into how our automated RAML checker tools will test this (this could be applied to one example endpoint and back-end code to illustrate the changes; or explained in the document)
Talk with Nick/UI component lead about how pagination may or may not impact the UI offline mode.
UPDATE:
Update requisitions/search endpoint such that it includes pagination. (This will require changes to the custom repository's implementation.)
Tell the UI team that the requisition search endpoint has been updated, and that they need to update the UI accordingly.
Update the above endpoint's RAML and make sure tests pass with it.
Choose an obscure, unused, endpoint that uses a Spring repository, and add pagination support to it. (Don't choose an endpoint used by the UI.)