Summary: | API DataTables Wrapper fails for ordered on multiple columns | ||
---|---|---|---|
Product: | Koha | Reporter: | Martin Renvoize (ashimema) <martin.renvoize> |
Component: | Architecture, internals, and plumbing | Assignee: | Martin Renvoize (ashimema) <martin.renvoize> |
Status: | CLOSED FIXED | QA Contact: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Severity: | normal | ||
Priority: | P5 - low | CC: | agustinmoyano, andrew, fridolin.somers, jonathan.druart, oleonard, sally.healey, tomascohen, victor |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
21.05.00,20.11.05,20.05.11
|
|
Circulation function: | |||
Bug Depends on: | 27680 | ||
Bug Blocks: | |||
Attachments: |
Bug 27807: Add multi-column ordering support to datatables wrapper
Bug 27807: (QA follow-up) Unique values only Bug 27807: (QA follow-up) Unique values only Bug 27807: Add multi-column ordering support to datatables wrapper Bug 27807: (QA follow-up) Unique values only Bug 27807: Add multi-column ordering support to datatables wrapper Bug 27807: (QA follow-up) Unique values only |
Description
Martin Renvoize (ashimema)
2021-02-26 13:48:15 UTC
Created attachment 117502 [details] [review] Bug 27807: Add multi-column ordering support to datatables wrapper This patch add mutli-column ordering support to the datatables api wrapper. Test plan 1/ Add a series of cities to the system with various combinations of city and state such that you can distinguish sorting on city + state in city ascending + state ascending, city ascending + state descending etc. 2/ Attempt to sort on the two fields in the table (Click the 'City' heading to sort on city name, then Shift Click on 'State' to add 'state' ordering on top.. Shift click again on state to reverse the 'state' ordering but maintain the 'City' ordering. 3/ Confirm the various ordering comes out correctly. 4/ Signoff I've applied "Bug 27680: [DO NOT PUSH] Example patch on cities" and I managed to generate queries like: _order_by=+me.name,+me.state,+me.name,+me.city_id or even: _order_by=+me.name,+me.state,+me.name,-me.city_id I know it is not an usual case, and certainly only hypothetical, but maybe we should unique orderArray? Good call, I didn't think about the uniqueness challenge Created attachment 117504 [details] [review] Bug 27807: (QA follow-up) Unique values only This patch filters the final _order_by clause such that we only have unique data fields present and we take the first occurence of a field. (In reply to Jonathan Druart from comment #2) > I've applied "Bug 27680: [DO NOT PUSH] Example patch on cities" and I > managed to generate queries like: > > _order_by=+me.name,+me.state,+me.name,+me.city_id > or even: > _order_by=+me.name,+me.state,+me.name,-me.city_id > > I know it is not an usual case, and certainly only hypothetical, but maybe > we should unique orderArray? Good catch. Created attachment 117506 [details] [review] Bug 27807: (QA follow-up) Unique values only This patch filters the final _order_by clause such that we only have unique data fields present and we take the first occurence of a field. Updated the patch.. I was pushing an array onto an array.. effectively `[ thing2, [ thing2, thing3] ]` I've updated the patch to perform an array merge to we get a single level array back and thus the unique filtering works as expected. Created attachment 117514 [details] [review] Bug 27807: Add multi-column ordering support to datatables wrapper This patch add mutli-column ordering support to the datatables api wrapper. Test plan 1/ Add a series of cities to the system with various combinations of city and state such that you can distinguish sorting on city + state in city ascending + state ascending, city ascending + state descending etc. 2/ Attempt to sort on the two fields in the table (Click the 'City' heading to sort on city name, then Shift Click on 'State' to add 'state' ordering on top.. Shift click again on state to reverse the 'state' ordering but maintain the 'City' ordering. 3/ Confirm the various ordering comes out correctly. 4/ Signoff Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 117515 [details] [review] Bug 27807: (QA follow-up) Unique values only This patch filters the final _order_by clause such that we only have unique data fields present and we take the first occurence of a field. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 118825 [details] [review] Bug 27807: Add multi-column ordering support to datatables wrapper This patch add mutli-column ordering support to the datatables api wrapper. Test plan 1/ Add a series of cities to the system with various combinations of city and state such that you can distinguish sorting on city + state in city ascending + state ascending, city ascending + state descending etc. 2/ Attempt to sort on the two fields in the table (Click the 'City' heading to sort on city name, then Shift Click on 'State' to add 'state' ordering on top.. Shift click again on state to reverse the 'state' ordering but maintain the 'City' ordering. 3/ Confirm the various ordering comes out correctly. 4/ Signoff Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 118826 [details] [review] Bug 27807: (QA follow-up) Unique values only This patch filters the final _order_by clause such that we only have unique data fields present and we take the first occurence of a field. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Pushed to master for 21.05, thanks to everybody involved! Pushed to 20.11.x for 20.11.05 Pushed to 20.05.x for 20.05.11 Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed. |