[Bug 27680] New: API DataTables Wrapper fails for ordering on multi-data-field columns
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Bug ID: 27680 Summary: API DataTables Wrapper fails for ordering on multi-data-field columns Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org We can make a single column contain data from multiple 'data' points in the ajax response using a mixture of a render function and concatenating the data points with ':' in the data field. This allows for a nice display and searching to work.. but the ordering call fails :( The final patch on bug 26273 highlights this: { "data": "manager.firstname:manager.surname", "searchable": true, "orderable": true, "render": function(data, type, row, meta) { var fullname = row.manager.firstname + " " + row.manager.surname; return fullname; } }, Assuming manager.firstname = 'Bob' and manager.surname = 'Geldoff'.. we will display "Bob Geldoff". Searching for 'Bob' will add a suitable search header searching for 'Bob' in both 'manager.firstname' and 'manager.surname' fields.. but hitting order by on this column will result in a 500 error as we add '_order_by: +manager.firstname:manager.surname' to the query parameters. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org, | |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |26273 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26273 [Bug 26273] Expose cashup summary history for a cash register -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 116794 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116794&action=edit :x Bug 27680: Take first order column for ordering -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 116795 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116795&action=edit Bug 27680: [DO NOT PUSH] Example patch on cities -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116794|0 |1 is obsolete| | --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 116796 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116796&action=edit Bug 27680: Take first order column for ordering -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Jonathan Druart from comment #3)
Created attachment 116796 [details] [review] Bug 27680: Take first order column for ordering
We can go with this patch for now. Are the REST API routes support ordering on several attributes? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Jonathan Druart from comment #4)
(In reply to Jonathan Druart from comment #3)
Created attachment 116796 [details] [review] [review] Bug 27680: Take first order column for ordering
We can go with this patch for now.
Are the REST API routes support ordering on several attributes?
It appears to: https://git.koha-community.org/Koha-community/Koha/src/branch/master/Koha/RE... :) -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Jonathan Druart from comment #4)
(In reply to Jonathan Druart from comment #3)
Created attachment 116796 [details] [review] [review] Bug 27680: Take first order column for ordering
We can go with this patch for now.
Are the REST API routes support ordering on several attributes?
Looks like we do 95 if ( reftype($order_by) and reftype($order_by) eq 'ARRAY' ) { So either we go first with this quick fix, or we need to implement it for the DT API wrapper. Currently it does not work if you select more than 1 column. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 116829 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116829&action=edit Bug 27680: Add multi-field support -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Still working here.. I found a series of little bugs and am writing some extra unit tests -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116796|0 |1 is obsolete| | --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 116831 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116831&action=edit Bug 27680: Take first order column for ordering -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116829|0 |1 is obsolete| | --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 116832 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116832&action=edit Bug 27680: Add multi-field support -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 116833 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116833&action=edit Bug 27680: Make collectionFormat open for order_by -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 116834 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116834&action=edit Bug 27680: Add tests for various ways of passing multi-params -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |In Discussion -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- As the final tests expose.. this raises a question. What form of 'multi-param' sending do we actually want to support? Prior to my patches we were "in theory" limiting to pipe delimited strings.. but the tests weren't passing any pipe delimited parameters and were passing for 'traditional multiple params' (i.e ?param1=+thingy¶m2=+thangy). I've added tests to cover the following.. but clearly many of them currently fail. 1. Multi param traditional `?param=+this¶m=+that` - Passing 2. Multi param PHP `?param[]=+this¶m[]=+that` - Failing 3. CSV param `?param=+this,+that` - Failing I like option 1, but feel we should fail in a more formal way for any/all other options.. right now we just explode horribly. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |agustinmoyano@theke.io, | |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- It does not work for me. https://snipboard.io/tzlfnE.jpg generates: /api/v1/cities?&_page=1&_per_page=20&q=%5B%5D&_match=contains&_order_by=-me.city_id -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Actually I forgot to restart_all. Now the table does not display at all OpenAPI <<< GET api/v1/cities [{"message":"Expected array - got string.","path":"\/_order_by"}] -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116831|0 |1 is obsolete| | --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 116843 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116843&action=edit Bug 27680: Take first order column for ordering -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 116844 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116844&action=edit Bug 27680: Add support for sorting fields with multiple data points This patch adds proper handling for sorting a single column that is constructed of multiple data entities.. i.e `"data": "string1:string2"` It does NOT add support for filtering on multiple columns yet. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116834|0 |1 is obsolete| | --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 116845 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116845&action=edit Bug 27680: Add tests for various ways of passing multi-params This patch adds unit test to increase the coverage of parameter that accept multiple values. There are a number of different ways end users can send such parameters and we should test to ensure we are recieving the correct option. Options `?param1=this¶m1=that` - traditional multiple pass params `?param1[]=this¶m1[]=that` - php multiple pass params `?param1=this,that` - comma delimited list param `?param1=this|that` - pipe delimited list param -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 116846 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116846&action=edit Bug 27680: Allow traditional multi-params to work This patch allows the preceeding test to pass. Strictly, we want a comma delimited string for our _order_by parameter, but we cannot easily block a traditional multi-passed parameter. As such the 'nice' thing to do is handle it when such a thing is passed as it will pass through validation regardless. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116832|0 |1 is obsolete| | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116833|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #20 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- This is the most pragmatic solution I could come up with. * We set the datatable wrapper to send _order_by as a comma delimited string of fields. * We update the collectionFormat spec to 'csv' from 'pipes'.. pipes was a bad value to start with as it's 'pipe' and 'csv' is more common. * We add handling to allow for traditional multiple passed params by name (as we can't rule them out via validation for some reason so it's nicer to handle them than it is to throw a nasty serverside error. * We add notes to some tests that are expected to result in a nasty 500 error. We cannot prevent arbitrary parameters from being sent to the API (which includes the _order_by[] PHP style multiple pass option, and non-csv delimited string options). Looking for feedback as to whether this is enough.. I'm thinking perhaps we need to throw a better Koha level exception when catching bad column names in our query string perhaps? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.com -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116795|0 |1 is obsolete| | --- Comment #21 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117335 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117335&action=edit Bug 27680: [DO NOT PUSH] Example patch on cities -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116843|0 |1 is obsolete| | --- Comment #22 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117336 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117336&action=edit Bug 27680: Take first order column for ordering -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116844|0 |1 is obsolete| | --- Comment #23 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117337 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117337&action=edit Bug 27680: Add support for sorting fields with multiple data points This patch adds proper handling for sorting a single column that is constructed of multiple data entities.. i.e `"data": "string1:string2"` It does NOT add support for filtering on multiple columns yet. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116845|0 |1 is obsolete| | --- Comment #24 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117338 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117338&action=edit Bug 27680: Add tests for various ways of passing multi-params This patch adds unit test to increase the coverage of parameter that accept multiple values. There are a number of different ways end users can send such parameters and we should test to ensure we are recieving the correct option. Options `?param1=this¶m1=that` - traditional multiple pass params `?param1[]=this¶m1[]=that` - php multiple pass params `?param1=this,that` - comma delimited list param `?param1=this|that` - pipe delimited list param -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116846|0 |1 is obsolete| | --- Comment #25 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117339 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117339&action=edit Bug 27680: Allow traditional multi-params to work This patch allows the preceeding test to pass. Strictly, we want a comma delimited string for our _order_by parameter, but we cannot easily block a traditional multi-passed parameter. As such the 'nice' thing to do is handle it when such a thing is passed as it will pass through validation regardless. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #26 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117340 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117340&action=edit Bug 27680: (follow-up) Add Koha::Exception for bad queries This patch adds a PropertyNotFound exception tothe 'as_list' method to catch DBIx::Class excpetions caused by passing bad column names into a query. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #27 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117341 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117341&action=edit Bug 27680: (follow-up) Attempt to catch the exception Help: I'm struggling to catch the point at which the exception should be thrown to convert it to a 400 error response in the API. '->search' does not actually trigger the DB hit until we reference some data from it.. so we can't use that.. so I tried '->count', but of course.. that drops 'order_by' which is the failure in this case and so we pass through. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117341|0 |1 is obsolete| | --- Comment #28 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117375 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117375&action=edit Bug 27680: (follow-up) Attempt to catch the exception Help: I'm struggling to catch the point at which the exception should be thrown to convert it to a 400 error response in the API. '->search' does not actually trigger the DB hit until we reference some data from it.. so we can't use that.. so I tried '->count', but of course.. that drops 'order_by' which is the failure in this case and so we pass through. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |27806 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27806 [Bug 27806] Exceptions thrown during a call to ->render are not caught and passed to ->unhandled_exception -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117340|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117375|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #29 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I've moved the followup bugs to try and render exceptions more cleanly to another bug now.. I'd really like to get this bug unblocked so the dependant cash management bug can start moving again. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |27807 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27807 [Bug 27807] API DataTables Wrapper fails for ordered on multiple columns -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117336|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117337|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117338|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117339|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117335|0 |1 is obsolete| | --- Comment #30 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 117388 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117388&action=edit Bug 27680: [DO NOT PUSH] Example patch on cities Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #31 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 117389 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117389&action=edit Bug 27680: Take first order column for ordering Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #32 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 117390 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117390&action=edit Bug 27680: Add support for sorting fields with multiple data points This patch adds proper handling for sorting a single column that is constructed of multiple data entities.. i.e `"data": "string1:string2"` It does NOT add support for filtering on multiple columns yet. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #33 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 117391 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117391&action=edit Bug 27680: Add tests for various ways of passing multi-params This patch adds unit test to increase the coverage of parameter that accept multiple values. There are a number of different ways end users can send such parameters and we should test to ensure we are recieving the correct option. Options `?param1=this¶m1=that` - traditional multiple pass params `?param1[]=this¶m1[]=that` - php multiple pass params `?param1=this,that` - comma delimited list param `?param1=this|that` - pipe delimited list param Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #34 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 117392 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117392&action=edit Bug 27680: Allow traditional multi-params to work This patch allows the preceeding test to pass. Strictly, we want a comma delimited string for our _order_by parameter, but we cannot easily block a traditional multi-passed parameter. As such the 'nice' thing to do is handle it when such a thing is passed as it will pass through validation regardless. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #35 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 117393 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117393&action=edit Bug 27680: Add support for param[] syntax While not that common nowadays, it is the syntax PHP uses, and DataTables also generates such thing in 'traditional' mode. We should support it as well. This patch adds support for that. It does so by adding _order_by[] to the reserved param names, and proper handling on the dbic_merge_sorting helper. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3- Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #36 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 117394 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117394&action=edit Bug 27680: (QA follow-up) Minor perlcritic issue Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117389|0 |1 is obsolete| | Attachment #117390|0 |1 is obsolete| | Attachment #117391|0 |1 is obsolete| | Attachment #117392|0 |1 is obsolete| | Attachment #117393|0 |1 is obsolete| | Attachment #117394|0 |1 is obsolete| | --- Comment #37 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 117438 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117438&action=edit Bug 27680: Take first order column for ordering Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #38 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 117439 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117439&action=edit Bug 27680: Add support for sorting fields with multiple data points This patch adds proper handling for sorting a single column that is constructed of multiple data entities.. i.e `"data": "string1:string2"` It does NOT add support for filtering on multiple columns yet. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #39 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 117440 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117440&action=edit Bug 27680: Add tests for various ways of passing multi-params This patch adds unit test to increase the coverage of parameter that accept multiple values. There are a number of different ways end users can send such parameters and we should test to ensure we are recieving the correct option. Options `?param1=this¶m1=that` - traditional multiple pass params `?param1[]=this¶m1[]=that` - php multiple pass params `?param1=this,that` - comma delimited list param `?param1=this|that` - pipe delimited list param Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #40 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 117441 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117441&action=edit Bug 27680: Allow traditional multi-params to work This patch allows the preceeding test to pass. Strictly, we want a comma delimited string for our _order_by parameter, but we cannot easily block a traditional multi-passed parameter. As such the 'nice' thing to do is handle it when such a thing is passed as it will pass through validation regardless. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #41 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 117442 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117442&action=edit Bug 27680: Add support for param[] syntax While not that common nowadays, it is the syntax PHP uses, and DataTables also generates such thing in 'traditional' mode. We should support it as well. This patch adds support for that. It does so by adding _order_by[] to the reserved param names, and proper handling on the dbic_merge_sorting helper. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3- Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Bug 27680: (QA follow-up) Minor perlcritic issue Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |21.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #42 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 21.05, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #43 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- We have a test that is failing: # Failed test 'exact match for JSON Pointer "/order_by"' # at t/Koha/REST/Plugin/Query.t line 338. # Structures begin differing at: # $got = ARRAY(0x55702f09b360) # $expected = HASH(0x55702f094400) # Looks like you failed 1 test of 20. # Failed test 'dbic_merge_sorting() tests' # at t/Koha/REST/Plugin/Query.t line 343. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #44 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117476 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117476&action=edit Bug 27680: (QA follow-up) Update test for dbic_merge_sorting The order_by handling will now always return an arrayref of ordering hashrefs even when only one ordering hash is present. This is interpreted by dbic as equivilent so I just update the test to pass here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 --- Comment #45 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- follow-up pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|21.05.00 |21.05.00,20.11.04 released in| | Status|Pushed to master |Pushed to stable CC| |fridolin.somers@biblibre.co | |m --- Comment #46 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 20.11.x for 20.11.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable Version(s)|21.05.00,20.11.04 |21.05.00,20.11.04,20.05.10 released in| | CC| |andrew@bywatersolutions.com --- Comment #47 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Pushed to 20.05.x for 20.05.10 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27680 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to oldstable |RESOLVED CC| |victor@tuxayo.net --- Comment #48 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org