Bug 7019 - Circulation History Sort Order Wrong
Summary: Circulation History Sort Order Wrong
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin Somers
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 5766
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-12 20:47 UTC by Nicole C. Engard
Modified: 2014-05-26 21:04 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
screenshot (10.15 KB, image/png)
2011-10-13 13:09 UTC, Nicole C. Engard
Details
Proposed fix (1.67 KB, patch)
2011-10-13 16:09 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Fix for Bug 7019 - Circulation History Sort Order Wrong (1.71 KB, patch)
2011-10-14 13:40 UTC, Nicole C. Engard
Details | Diff | Splinter Review
Proposed patch (3.11) (1.71 KB, patch)
2013-02-21 15:37 UTC, Fridolin Somers
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7019: Circulation History Sort Order Wrong (in datatable) (1.80 KB, patch)
2013-02-26 17:44 UTC, Owen Leonard
Details | Diff | Splinter Review
Proposed patch (revisited) (2.18 KB, patch)
2013-07-11 08:31 UTC, Fridolin Somers
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7019: Circulation History Sort Order Wrong (in datatable) (2.22 KB, patch)
2013-07-17 03:04 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 7019: Circulation History Sort Order Wrong (in datatable) (2.29 KB, patch)
2013-07-17 14:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7019: [3.10.x] Circulation History Sort Order Wrong (in datatable) (3.69 KB, patch)
2013-10-23 09:35 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2011-10-12 20:47:10 UTC
If any items are checked out, the return date is treated as text instead of a date, so it's sorted month first, even across years.
Comment 1 Nicole C. Engard 2011-10-13 13:09:38 UTC
Created attachment 5874 [details]
screenshot
Comment 2 Owen Leonard 2011-10-13 16:09:16 UTC Comment hidden (obsolete)
Comment 3 Nicole C. Engard 2011-10-14 13:40:36 UTC Comment hidden (obsolete)
Comment 4 Ian Walls 2011-10-14 13:46:47 UTC
Template-only changes, string-free, improves sorting.  While not perfect, it's MUCH better than what we had, so marking as Passed QA.
Comment 5 Chris Cormack 2011-10-15 01:20:28 UTC
Pushed, please test
Comment 6 José Anjos 2012-11-12 12:19:01 UTC
Using Koha 3.08.03 with Systempref dateformat= dd/mm/yyyy
the date are not well sorted. If I change it to yyyy/mm/dd it's OK.
Comment 7 Fridolin Somers 2013-02-06 11:16:28 UTC
Very odd, datatables.js method "dt_add_type_uk_date" does not use UK format "MM/DD/YYYY" but metric format "DD/MM/YYYY".
Comment 8 Nicole C. Engard 2013-02-13 13:02:57 UTC
Still an issue in 3.8.8.

Nicole
Comment 9 Fridolin Somers 2013-02-13 13:31:17 UTC
(In reply to comment #7)
> Very odd, datatables.js method "dt_add_type_uk_date" does not use UK format
> "MM/DD/YYYY" but metric format "DD/MM/YYYY".

My coworkers explained me : UK = metric format (for dates only), it is US format who is "MM/DD/YYYY".
Comment 10 Fridolin Somers 2013-02-13 13:36:37 UTC
Datatables looks for datas to choose the default sort algorithm.
When looking at Checked Out and dates, it will use alphabetical sort algorithm.
Date format must be forced for metric ('uk_date') AND us format ('date').

Note that ISO format sorted by alphabetical order is correct.
Comment 11 Fridolin Somers 2013-02-21 15:37:45 UTC Comment hidden (obsolete)
Comment 12 Owen Leonard 2013-02-26 17:44:12 UTC Comment hidden (obsolete)
Comment 13 Marcel de Rooy 2013-03-29 12:25:33 UTC
QA Comment:
I have the strong impression that this is already fixed in master. Probably by the dt_add_type_uk_date stuff?
Just test in current master with dd/mm/yy. If it would sort alphabetically, it would not sort right but it does on date. Same for mm/dd/yy.
Please clarify.
Comment 14 Fridolin Somers 2013-06-24 16:02:41 UTC
> Comment 13
Had you a checked out row in circulation history ?
It is this text "Checked Out" which, even with dt_add_type_uk_date stuff, prevents the library from sorting by date because column does not contain only dates.
You should see that "Date due" column sorting is always fine whereas "Return date" not when having some "Checked Out".
Comment 15 Jonathan Druart 2013-06-27 15:20:49 UTC
Fridolyn,

I reproduce the issue and your patch fixes something, but it is not perfect. I got:
Checked Out
20/04/2012
13/04/2012
19/03/2013
Checked Out
12/09/2012
Checked Out
12/09/2012
12/09/2012

In fact when the sort plugin find something else than a date, it get all mixed up.


But try (members/readingrec.tt ~l.83):

-            [% issue.returndate | $KohaDates %]
+            <span title="[% issue.returndate %]">[% issue.returndate | $KohaDates %]</span>

It works for me.
Comment 16 Fridolin Somers 2013-07-11 08:31:13 UTC Comment hidden (obsolete)
Comment 17 Srdjan Jankovic 2013-07-17 03:04:51 UTC Comment hidden (obsolete)
Comment 18 Jonathan Druart 2013-07-17 14:09:37 UTC
QA comment:
The last patch works great.
Marked as Passed QA.
Comment 19 Jonathan Druart 2013-07-17 14:09:56 UTC
Created attachment 19723 [details] [review]
Bug 7019: Circulation History Sort Order Wrong (in datatable)

The "return date" column sort is not using the date sort algorithm because it is
confused by the combination of date data and text data ("Checked out").

This patch adds the use of datatable "title-string" sort type.
With that, the date in ISO format are used for sorting and because letters are before numbers, "Checked out" rows are more recent than any date.

Test plan :
- set dateformat syspref to "dd/mm/yyyy"
- Go to members/readingrec.pl for a borrower having a old issues and
checked out issues
=> Check that sort of "Return date" is correct.
For example :
With patch :
  20/01/2011
  06/03/2011
  20/03/2011
  Checked Out
Without patch :
  06/03/2011
  20/01/2011
  20/03/2011
  Checked Out
- Test also with dateformat syspref "mm/dd/yyyy"

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 20 Galen Charlton 2013-07-17 15:01:32 UTC
Pushed to master.  Thanks, Fridolyn!
Comment 21 Tomás Cohen Arazi 2013-08-12 17:08:18 UTC
This patch has been pushed to 3.12.x, will be in 3.12.4.

Thanks Fridolyn!
Comment 22 Sophie MEYNIEUX 2013-10-23 09:16:16 UTC
Would it be possible to have it in 3.10.x ?
Comment 23 Jonathan Druart 2013-10-23 09:35:27 UTC
Created attachment 22305 [details] [review]
Bug 7019: [3.10.x] Circulation History Sort Order Wrong (in datatable)

The "return date" column sort is not using the date sort algorithm because it is
confused by the combination of date data and text data ("Checked out").

This patch adds the use of datatable "title-string" sort type.
With that, the date in ISO format are used for sorting and because letters are before numbers, "Checked out" rows are more recent than any date.

Test plan :
- set dateformat syspref to "dd/mm/yyyy"
- Go to members/readingrec.pl for a borrower having a old issues and
checked out issues
=> Check that sort of "Return date" is correct.
For example :
With patch :
  20/01/2011
  06/03/2011
  20/03/2011
  Checked Out
Without patch :
  06/03/2011
  20/01/2011
  20/03/2011
  Checked Out
- Test also with dateformat syspref "mm/dd/yyyy"