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.