Bug 10184 - Circulation History reverses sort order
Summary: Circulation History reverses sort order
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low trivial (vote)
Assignee: Colin Campbell
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-03 14:32 UTC by Colin Campbell
Modified: 2014-12-07 20:02 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
proposed patch (1.17 KB, patch)
2013-05-03 14:56 UTC, Colin Campbell
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 10184 - Circulation History reverses sort order (1.30 KB, patch)
2013-05-24 12:31 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 10184 - Circulation History reverses sort order (1.26 KB, patch)
2013-05-27 08:20 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Colin Campbell 2013-05-03 14:32:27 UTC
The script members/readingrec.pl specifically requests the users issues ordered with the most recent (by due date) first. But they are displayed in approximately the reverse order to this. The cause is that unless explicitly disabled the datatables sort  their data and not being given a specification they do so in ascending order by the first column. The pre display sort of already sorted data needs disabling
Comment 1 Colin Campbell 2013-05-03 14:56:29 UTC Comment hidden (obsolete)
Comment 2 Colin Campbell 2013-05-03 14:57:31 UTC
oops forgot to change status
Comment 3 Owen Leonard 2013-05-09 15:16:39 UTC
I think the added line should be:

"aaSorting": [[8, "desc"]]

...so that the sorted column reflects the sort state in the column header.
Comment 4 Colin Campbell 2013-05-10 07:05:23 UTC
(In reply to comment #3)
> I think the added line should be:
> 
> "aaSorting": [[8, "desc"]]
> 
> ...so that the sorted column reflects the sort state in the column header.

Does this not add an unnecessary re-sort, the data is sorted unless the user selects to resort it no further processing is required
Comment 5 Frédéric Demians 2013-05-15 14:23:18 UTC
The sorting algorithm doesn't take into account the 'dateformat' system preference. We just have a string sort. It works when dateformat = yyyy/mm/dd. It doesn't for dd/mm/yyyy.
Comment 6 Colin Campbell 2013-05-16 11:05:18 UTC
(In reply to comment #5)
> The sorting algorithm doesn't take into account the 'dateformat' system
> preference. We just have a string sort. It works when dateformat =
> yyyy/mm/dd. It doesn't for dd/mm/yyyy.

The sort is done by mysql on a mysql date column the koha system preference is not relevant.
Comment 7 Colin Campbell 2013-05-23 14:52:32 UTC
Status has been marked as failed QA but I cant see under what basis? Patch is stalled as a result and bug is out in the wild can someone clarify
Comment 8 Owen Leonard 2013-05-24 12:29:44 UTC
I set to "Failed QA" because of the issue with the table header sort indicator. I see your point about the forced re-sort. In my tests there was a minor difference in processing time, but I didn't test with an account with a large history (only ~120). The indicator is a minor point, so I'll give the re-sort question the benefit of the doubt and say it's not worth the extra processing.
Comment 9 Owen Leonard 2013-05-24 12:31:08 UTC Comment hidden (obsolete)
Comment 10 Chris Cormack 2013-05-27 08:20:14 UTC
Created attachment 18410 [details] [review]
Bug 10184 - Circulation History reverses sort order

Issue data is passed to the readingrec template ordered
most recent due date first. Datatables unless specified
otherwise do a presort on the data they will display
the default results in an order the reverse of what was
intended. Disabling the presort preserves the initial sort
order until the user selects a different sort

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 11 Galen Charlton 2013-05-28 15:06:58 UTC
Pushed to master.  Thanks, Colin!
Comment 12 Tomás Cohen Arazi 2013-06-03 15:53:20 UTC
This patch has been pushed to 3.12.x, will be in 3.12.1.
Comment 13 Bernardo Gonzalez Kriegel 2013-06-10 20:26:50 UTC
Pushed to 3.10.x, will be in 3.10.7
Comment 14 Chris Hall 2013-06-17 07:42:45 UTC
Pushed to 3.8.x, will be in 3.8.14