Fix pagination bug #251
Fix pagination bug #251
Conversation
alexdunnjpl
requested review from
jimmie,
jordanpadams,
nutjob4life and
tloubrieu-jpl
|
|
||
| @Test | ||
| public void testComplexAddTrimHeadAndTailSpanning() { | ||
| PaginationLidvidBuilder pageBuilder = new PaginationLidvidBuilder(new LidvidsContextStub(2, 5)); |
There was a problem hiding this comment.
Try any negatives like LidvidsContextStub(2, -5) or LidvidsContextStub(-2, -5)?
There was a problem hiding this comment.
Seemed defensible (albeit naive) to assume that a passed LidvidsContext would be valid. If that validation isn't already baked into the existing LidvidsContext implementations I'll see what I can do there - might be trivial but I need to verify that start/limit aren't used in any context where negatives are (say) expected, but handled after instantiation.
Plus I'll need to see how to throw in a way that will return a 422, which may be more trouble than is worth doing right this second (but I'll add it to the backlog).
There was a problem hiding this comment.
Ok to test raise an exception when limit or start is negative. Ok also to keep 422 error for later. 400 with a dedicated message sounds more than enough to me for now. Thanks
|
Merging, and will open a new issue for lower bounds (as it's out of scope for 240) and fix asap. |
🗒️ Summary
Fixes a bug in
PaginationLidvidBuilderwhich resulted in incorrect slicing, as well as crashes when providedstartparameter was greater than providedlimit⚙️ Test Data and/or Report
Added comprehensive unit tests for
PaginationLidvidBuilder.addAll()♻️ Related Issues
Fixes #240
#250 outstanding