When using the en-GB translation, column sorting of saved reports (../cgi-bin/koha/reports/guided_reports.pl?phase=Use%20saved) doesn't work properly when sorting by date. For example, sorting by date created could result in the following order: 02/04/2019 02/04/2020 02/08/2018 03/04/2019 This is presumably because the UK date format is dd/mm/yyyy, not mm/dd/yyyy. It works correctly in the en translation. Sorting by non-date columns in en-GB also works correctly.
Looks like it is fixed by Bug 27994
I see it is Bug 26234 that removed : { "sType": "title-string", "aTargets" : [ "title-string" ] }
Hi David, that seems to work correctly for me on master, with en-GB and dateformat=dd/mm/yyyy On which version of Koha do you see that problem? Can you recreate on a sandbox?
(In reply to Fridolin Somers from comment #2) > I see it is Bug 26234 that removed : > { "sType": "title-string", "aTargets" : [ "title-string" ] } Not related and not a bug, title-string has been moved to columns_settings.inc (not removed).
I reproduce this bug on our demo actually 20.11.09 : https://intranet-demo.biblibre.com/
Created attachment 125870 [details] [review] Bug 28349: (bug 26234 follow-up) Correct wrong array merge Bug 26234 allowed to remove the different classes we use in aoColumnDefs for sorting columns of table using DT. However there is a terrible mistake, the merge of existing aoColumnDefs array with the default one is totally wrong: $.extend(true, new_parameters, default_column_defs); When we actually want $.extend(true, new_parameters["aoColumnDefs"], default_column_defs); We want to merge default_column_defs with the existing aocolumnDefs. This bug only exists when there is an existing aocolumnDefs. See commit d3f3a55e0b1544ebcd3166d141e126a4324baab1 It happens on: * cataloguing/z3950_search.tt * reports/guided_reports_start.tt * serials/subscription-detail.tt * opac-course-reserves.tt * opac-detail.tt However this bug has been hidden as we have in master related bug from bug 27945, which removes title-string.
Problem and fix can be easily tested on v20.11.09 (sql reports), but this patch is for master and 21.05 as well.
Created attachment 125875 [details] [review] Bug 28349: (bug 26234 follow-up) Correct wrong array merge Bug 26234 allowed to remove the different classes we use in aoColumnDefs for sorting columns of table using DT. However there is a terrible mistake, the merge of existing aoColumnDefs array with the default one is totally wrong: $.extend(true, new_parameters, default_column_defs); When we actually wanted to do: $.extend(true, new_parameters["aoColumnDefs"], default_column_defs); But it's still wrong, extend is doing a deep copy and the array will be replaced by the other one, whereas we want to append. We want to merge default_column_defs with the existing aocolumnDefs, this patch is doing it explicitely. This bug only exists when there is an existing aocolumnDefs. See commit d3f3a55e0b1544ebcd3166d141e126a4324baab1 It happens on: * cataloguing/z3950_search.tt * reports/guided_reports_start.tt * serials/subscription-detail.tt * opac-course-reserves.tt * opac-detail.tt However this bug has been hidden as we have in master related bug from bug 27945, which removes title-string.
Created attachment 126179 [details] [review] Bug 28349: [DO NOT PUSH] Recreate the problem You can apply this patch to recreate the problem. NoSort is removed from the default DT settings but we are passing it from the KohaTable constructor. We are then expecting the first column (the checkbox) to not be orderable. Without the fix, the parameter is not taken into account (ie. not correctly merged into the DT default parameters).
Created attachment 127028 [details] [review] Bug 28349: (bug 26234 follow-up) Correct wrong array merge Bug 26234 allowed to remove the different classes we use in aoColumnDefs for sorting columns of table using DT. However there is a terrible mistake, the merge of existing aoColumnDefs array with the default one is totally wrong: $.extend(true, new_parameters, default_column_defs); When we actually wanted to do: $.extend(true, new_parameters["aoColumnDefs"], default_column_defs); But it's still wrong, extend is doing a deep copy and the array will be replaced by the other one, whereas we want to append. We want to merge default_column_defs with the existing aocolumnDefs, this patch is doing it explicitely. This bug only exists when there is an existing aocolumnDefs. See commit d3f3a55e0b1544ebcd3166d141e126a4324baab1 It happens on: * cataloguing/z3950_search.tt * reports/guided_reports_start.tt * serials/subscription-detail.tt * opac-course-reserves.tt * opac-detail.tt However this bug has been hidden as we have in master related bug from bug 27945, which removes title-string. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
That was somewhat fun to reproduce and test.. I'm going to be bold and go for straight QA here.. it's a clear fix, with no regressions, once I wrapped my head around it. Passing QA
Pushed to master for 21.11, thanks to everybody involved!