Bug 32503 - Holds awaiting pickup doesn't sort dates correctly
Summary: Holds awaiting pickup doesn't sort dates correctly
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-12-20 10:28 UTC by Martin Renvoize
Modified: 2023-12-28 20:42 UTC (History)
6 users (show)

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


Attachments
Bug 32503: Fix date sorting for holds awaiting pickup tables (1.90 KB, patch)
2022-12-28 22:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32503: Fix date sorting for holds awaiting pickup tables (1.97 KB, patch)
2022-12-29 11:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32503: (follow-up) Move data-order attributes (3.13 KB, patch)
2022-12-29 11:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32503: Fix date sorting for holds awaiting pickup tables (1.97 KB, patch)
2023-01-04 11:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32503: (follow-up) Move data-order attributes (3.13 KB, patch)
2023-01-04 11:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32503: Use first cancellation for date ordering (2.39 KB, patch)
2023-01-04 11:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32503: Fix date sorting for holds awaiting pickup tables (2.02 KB, patch)
2023-01-04 23:22 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 32503: (follow-up) Move data-order attributes (3.19 KB, patch)
2023-01-04 23:22 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 32503: Use first cancellation for date ordering (2.44 KB, patch)
2023-01-04 23:22 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 32503: Fix date sorting for holds awaiting pickup tables (2.08 KB, patch)
2023-02-22 13:27 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 32503: (follow-up) Move data-order attributes (3.24 KB, patch)
2023-02-22 13:27 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 32503: Use first cancellation for date ordering (2.50 KB, patch)
2023-02-22 13:27 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2022-12-20 10:28:30 UTC
Dates in the waiting reserves page are not sorting correctly when UK dates are used.

When you view the waiting reserves page with a large number of resereves, and you sort them by 'Waiting since' or 'Date hold placed', if the system uses UK date format (DD/MM/YYYY) the sorting doesn't working correctly - for example, you might see:

02/02/2017
03/03/2017
08/02/2017
10/01/2017
16/12/2016

instead of the correct order, which would be:

16/12/2016
10/01/2017
02/02/2017
08/02/2017
03/03/2017
Comment 1 Katrin Fischer 2022-12-28 22:43:55 UTC
Created attachment 144884 [details] [review]
Bug 32503: Fix date sorting for holds awaiting pickup tables

This patch is a little experimental, but seems to do the trick.
Comment 2 Katrin Fischer 2022-12-28 22:49:06 UTC
Usually this works:
https://wiki.koha-community.org/wiki/DataTables_HowTo#Sorting_dates_regardless_of_date_format_preference

But this was already implemented here:
<td><span data-order="[% reserveloo.waitingdate | html %]">[% reserveloo.waitingdate | $KohaDates %]</span></td>
<td><span data-order="[% reserveloo.reservedate | html %]">[% reserveloo.reservedate | $KohaDates %]</span></td>
<td><span data-order="[% reserveloo.expirationdate | html %]">[% reserveloo.expirationdate | $KohaDates %]</span></td>

And the source shows, it looks like we want it to:
<td><span data-order="2022-12-28">28/12/2022</span></td>
<td><span data-order="2022-12-28">28/12/2022</span></td>
<td><span data-order="2023-03-03">03/03/2023</span></td>

But... the sorting doesn't work.

So I googled.... and came up with this: <th data-type="@data-sort"></th> which seems to do the trick.

But as we use data-order a lot and this might affect other pages where this worked before, it definitely needs more investigation/someone more knowledgeable about datatables to have a look here.

Tested 3 holds with the following dates:
02/02/2017
03/03/2017
08/02/2017
Comment 3 Martin Renvoize 2022-12-29 11:37:59 UTC
Created attachment 144885 [details] [review]
Bug 32503: Fix date sorting for holds awaiting pickup tables

This patch is a little experimental, but seems to do the trick.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 4 Martin Renvoize 2022-12-29 11:38:04 UTC
Created attachment 144886 [details] [review]
Bug 32503: (follow-up) Move data-order attributes

It looks like the data-order attributes were at the wrong level to me,
i.e. not part of the 'td' but instead part of the cell content.

This patch moves those data-order attributes where possible and drops
the data-type definitions on the headers.

However, for the cancellation case, this isn't as clear cut as we may
well display more than one cancellation request date in the same cell.
For this case I've left Katrins data-type="@data-order" trick.
Comment 5 Martin Renvoize 2022-12-29 11:40:14 UTC
My Google foo is weak today.. where did you find the suggestion to use `data-type="@data-sort"` Katrin?

I think I spotted the issue with most of the sorting and have removed the data-type definition stuff.. but the cancellation date is more challenging.
Comment 6 Martin Renvoize 2023-01-04 11:00:35 UTC
Created attachment 144980 [details] [review]
Bug 32503: Fix date sorting for holds awaiting pickup tables

This patch is a little experimental, but seems to do the trick.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2023-01-04 11:00:39 UTC
Created attachment 144981 [details] [review]
Bug 32503: (follow-up) Move data-order attributes

It looks like the data-order attributes were at the wrong level to me,
i.e. not part of the 'td' but instead part of the cell content.

This patch moves those data-order attributes where possible and drops
the data-type definitions on the headers.

However, for the cancellation case, this isn't as clear cut as we may
well display more than one cancellation request date in the same cell.
For this case I've left Katrins data-type="@data-order" trick.
Comment 8 Martin Renvoize 2023-01-04 11:00:44 UTC
Created attachment 144982 [details] [review]
Bug 32503: Use first cancellation for date ordering
Comment 9 Lucas Gass 2023-01-04 23:22:35 UTC
Created attachment 145051 [details] [review]
Bug 32503: Fix date sorting for holds awaiting pickup tables

This patch is a little experimental, but seems to do the trick.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 10 Lucas Gass 2023-01-04 23:22:40 UTC
Created attachment 145052 [details] [review]
Bug 32503: (follow-up) Move data-order attributes

It looks like the data-order attributes were at the wrong level to me,
i.e. not part of the 'td' but instead part of the cell content.

This patch moves those data-order attributes where possible and drops
the data-type definitions on the headers.

However, for the cancellation case, this isn't as clear cut as we may
well display more than one cancellation request date in the same cell.
For this case I've left Katrins data-type="@data-order" trick.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 11 Lucas Gass 2023-01-04 23:22:44 UTC
Created attachment 145053 [details] [review]
Bug 32503: Use first cancellation for date ordering

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 12 Tomás Cohen Arazi 2023-02-22 13:27:31 UTC
Created attachment 147142 [details] [review]
Bug 32503: Fix date sorting for holds awaiting pickup tables

This patch is a little experimental, but seems to do the trick.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Tomás Cohen Arazi 2023-02-22 13:27:34 UTC
Created attachment 147143 [details] [review]
Bug 32503: (follow-up) Move data-order attributes

It looks like the data-order attributes were at the wrong level to me,
i.e. not part of the 'td' but instead part of the cell content.

This patch moves those data-order attributes where possible and drops
the data-type definitions on the headers.

However, for the cancellation case, this isn't as clear cut as we may
well display more than one cancellation request date in the same cell.
For this case I've left Katrins data-type="@data-order" trick.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 14 Tomás Cohen Arazi 2023-02-22 13:27:38 UTC
Created attachment 147144 [details] [review]
Bug 32503: Use first cancellation for date ordering

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 15 Tomás Cohen Arazi 2023-02-27 14:42:21 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 16 Matt Blenkinsop 2023-03-01 09:35:09 UTC
Nice work everyone!

Pushed to stable for 22.11.x
Comment 17 Lucas Gass 2023-03-10 14:12:35 UTC
Merge conflicts with 22.05.x, no backport.