Summary: | GET /acquisitions/orders doesn't honour sorting | ||
---|---|---|---|
Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> |
Component: | Acquisitions | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Status: | CLOSED FIXED | QA Contact: | Jonathan Druart <jonathan.druart> |
Severity: | major | ||
Priority: | P5 - low | CC: | aholt, andrew, barbara.johnson, catherine.ingram, jonathan.druart, kyle, lucas, nick, tomascohen, victor |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | Sponsored | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
22.05.00,21.05.09,21.11.03
|
|
Circulation function: | |||
Bug Depends on: | 20212 | ||
Bug Blocks: | 29570 | ||
Attachments: |
Bug 29464: Regression tests
Bug 29464: Make GET /acquisitions/orders honour sorting Bug 29464: Regression tests Bug 29464: Make GET /acquisitions/orders honour sorting Bug 29464: Regression tests Bug 29464: Make GET /acquisitions/orders honour sorting |
Description
Nick Clemens (kidclamp)
2021-11-12 11:00:27 UTC
Confirmed the issue :( *** Bug 29570 has been marked as a duplicate of this bug. *** This was certainly working on bug 20212. Will try to check what else changed after it was pushed. Created attachment 128395 [details] [review] Bug 29464: Regression tests Created attachment 128396 [details] [review] Bug 29464: Make GET /acquisitions/orders honour sorting When bug 20212 was pushed, we lost (didn't dig enough to find out how) the sorting feature for this route. This is mainly because biblio+biblioitems filtering prevents the route to use plain $c->objects->search, so it has baked a custom version of it. In this case, it missed the call to the dbic_merge_sorting helper, that takes care of reading the query parameters and inject the sorting attributes to the resultset. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/acquisitions_orders.t => FAIL: Tests fail, sorting doesn't actually work 3. Apply this patch 4. Repeat 2 => SUCCESS: Sorting works! 5. Test sorting the pending orders on parcel.tt 6. Sign off :-D Sponsored-by: ByWater Solutions (In reply to Nick Clemens from comment #0) > This seems to be from bug 20212 - the sort options in the headers of the > pending_orders table no longer function. Done. > Additionally, the 'Summary' column is now marked as 'orderable: false' - it > was possible to sort on this column before those patches Will handle this on bug 29570. Created attachment 128397 [details] [review] Bug 29464: Regression tests Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Created attachment 128398 [details] [review] Bug 29464: Make GET /acquisitions/orders honour sorting When bug 20212 was pushed, we lost (didn't dig enough to find out how) the sorting feature for this route. This is mainly because biblio+biblioitems filtering prevents the route to use plain $c->objects->search, so it has baked a custom version of it. In this case, it missed the call to the dbic_merge_sorting helper, that takes care of reading the query parameters and inject the sorting attributes to the resultset. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/acquisitions_orders.t => FAIL: Tests fail, sorting doesn't actually work 3. Apply this patch 4. Repeat 2 => SUCCESS: Sorting works! 5. Test sorting the pending orders on parcel.tt 6. Sign off :-D Sponsored-by: ByWater Solutions Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Tests pass and sorting works so I signed off. Making the Summary/title sortable again will happen in Bug 29570? (In reply to Lucas Gass from comment #9) > Tests pass and sorting works so I signed off. Making the Summary/title > sortable again will happen in Bug 29570? Yes, that's much more complex to solve. *** Bug 29570 has been marked as a duplicate of this bug. *** This is a bug that is costing my staff time every day. If we receive an order of 30 books and have to go into Acquisitions and click "received" on each title - it is much easy to find the titles when they are in alphabetical order. Otherwise it is a hunt and seek when the titles are listed in random order. So it might take anywhere from a few extra minutes for a small order to an extra 10 minutes for a larger order. Thank you, Catherine Ingram +1 It has negatively impacted our workflow as well. (In reply to Tomás Cohen Arazi from comment #10) > (In reply to Lucas Gass from comment #9) > > Tests pass and sorting works so I signed off. Making the Summary/title > > sortable again will happen in Bug 29570? > > Yes, that's much more complex to solve. (In reply to Tomás Cohen Arazi from comment #11) > *** Bug 29570 has been marked as a duplicate of this bug. *** Why is this marked as duplicate? We are not solving the problem for the summary column (and so the sort by title) (In reply to Jonathan Druart from comment #14) > (In reply to Tomás Cohen Arazi from comment #10) > > (In reply to Lucas Gass from comment #9) > > > Tests pass and sorting works so I signed off. Making the Summary/title > > > sortable again will happen in Bug 29570? > > > > Yes, that's much more complex to solve. > > (In reply to Tomás Cohen Arazi from comment #11) > > *** Bug 29570 has been marked as a duplicate of this bug. *** > > Why is this marked as duplicate? We are not solving the problem for the > summary column (and so the sort by title) duplicate status has been reverted, I missed that. Created attachment 129136 [details] [review] Bug 29464: Regression tests Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 129137 [details] [review] Bug 29464: Make GET /acquisitions/orders honour sorting When bug 20212 was pushed, we lost (didn't dig enough to find out how) the sorting feature for this route. This is mainly because biblio+biblioitems filtering prevents the route to use plain $c->objects->search, so it has baked a custom version of it. In this case, it missed the call to the dbic_merge_sorting helper, that takes care of reading the query parameters and inject the sorting attributes to the resultset. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/acquisitions_orders.t => FAIL: Tests fail, sorting doesn't actually work 3. Apply this patch 4. Repeat 2 => SUCCESS: Sorting works! 5. Test sorting the pending orders on parcel.tt 6. Sign off :-D Sponsored-by: ByWater Solutions Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to master for 22.05, thanks to everybody involved [U+1F984] This is quite a pain point for many libraries, is there a chance to get this one backported? Updating severity a little and also +1 for backporting :) Pushed to 21.11.x for 21.11.03 Pushed to 21.05.x for 21.05.09 Missing dependencies for 20.11.x, it shouldn't be affected, no backport. |