Define pagination order for Paginator

[andrewjstone]

Pagination was previously hardcoded to be Ascending. I ran into an issue when paginating across multiple pages when descending order was required.

I fixed this by adding a parameter for the desired order into Paginator::new rather than hardcoding it. The rest is fixes to existing callsites.

[davepacheco]

Thanks for doing this.

[davepacheco]

I would have expected that you'd need to change L512 (new code) to use first() instead of last() if the pagination order is descending. If you don't, then I'd expect instead of:

• page 1: items 20-29
• page 2: items 10-19
• page 3: items 0-9

You'd get:

• page 1: items 20-30
• page 2: items 19-29
• page 3: items 18-28

I might be wrong? It seems worth adding a test for this.

[andrewjstone]

Thanks for the quick review @davepacheco. it's definitely worth adding a test.

FWIW, on another branch I had done a quick test by adding a current_pagparams_desc that sorted in descending order and ran through inserted chicken list history that was more than a page long and it all seemed to work. However, this may be the case because of the order_by that I added on the datastore list side.

[andrewjstone]

I added a couple of tests in 0151f56 and everything seems to work.

[jgallagher]

FWIW, .last() working makes sense to me. Because we're ordering descending, I think this example:

• page 1: items 20-29
• page 2: items 10-19
• page 3: items 0-9

would have the batches ordered in reverse. Page 1 would give you [29, 28, ..., 20], so .last() would give you 20, leading the next page to be [19, 18, ..., 10] and so on.

[davepacheco]

Got it -- that makes sense!

[jgallagher]

FWIW, .last() working makes sense to me. Because we're ordering descending, I think this example:

• page 1: items 20-29
• page 2: items 10-19
• page 3: items 0-9

would have the batches ordered in reverse. Page 1 would give you [29, 28, ..., 20], so .last() would give you 20, leading the next page to be [19, 18, ..., 10] and so on.