Bug 27926 - Date of birth sorting with British English format is broken
Summary: Date of birth sorting with British English format is broken
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff Client (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on: 15400
Blocks:
  Show dependency treegraph
 
Reported: 2021-03-11 11:47 UTC by Martin Renvoize
Modified: 2021-04-24 17:02 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.05


Attachments
Bug 27926: Add 'title-string' for dates on circ-patron-results (2.18 KB, patch)
2021-03-11 12:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27926: Add 'title-string' for dates on circ-patron-results (2.26 KB, patch)
2021-03-11 13:59 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 27926: (QA follow-up) Switch to using data-order (2.07 KB, patch)
2021-03-12 08:51 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27926: Add 'title-string' for dates on circ-patron-results (2.31 KB, patch)
2021-04-03 14:57 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 27926: (QA follow-up) Switch to using data-order (2.13 KB, patch)
2021-04-03 14:57 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2021-03-11 11:47:37 UTC
Bug 15400 added an include to always output date of birth properly formatted via $KohaDates and with `(age XX)` appended.

This has broken the sorting in some tables; for example the patron selection table when searching for a patron for circulation.
Comment 1 Martin Renvoize 2021-03-11 12:19:35 UTC
Created attachment 118118 [details] [review]
Bug 27926: Add 'title-string' for dates on circ-patron-results

This patch adds the title string sorting for the data of birth field in
the circulation patron results table.  This has the side effect of
adding a tooltip that displays the date of birth in ISO format, so I'm
not entirely sure it's the right approach.

Test plan
1/ Set your date format preferences to British English (dd-mm-yyyy)
2/ Do a search that will yield multiple patrons via "Check out"
3/ Try sorting on date of birth and not the order is incorrect.
4/ Apply the patch
5/ Refresh the page and try sorting on date again
6/ The order should now be correct
7/ Signoff
Comment 2 Martin Renvoize 2021-03-11 12:22:04 UTC
I'm not sure about the solution chosen here.. see the caveat in the commit message.

We will also need to deal with the same issue on guarantor_search and members_results at least.. maybe more.
Comment 3 ByWater Sandboxes 2021-03-11 13:59:21 UTC
Created attachment 118124 [details] [review]
Bug 27926: Add 'title-string' for dates on circ-patron-results

This patch adds the title string sorting for the data of birth field in
the circulation patron results table.  This has the side effect of
adding a tooltip that displays the date of birth in ISO format, so I'm
not entirely sure it's the right approach.

Test plan
1/ Set your date format preferences to British English (dd-mm-yyyy)
2/ Do a search that will yield multiple patrons via "Check out"
3/ Try sorting on date of birth and not the order is incorrect.
4/ Apply the patch
5/ Refresh the page and try sorting on date again
6/ The order should now be correct
7/ Signoff

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 4 Sally 2021-03-11 14:00:56 UTC
Works for me with both DD/MM/YYYY and MM/DD/YYYY formats.  

Happy to test again once it's ready for sign off.
Comment 5 Martin Renvoize 2021-03-11 15:32:37 UTC
OK, Going for pragmatic here.. I'm going to set this one to NSO and leave it as only fixing the circ search case.

I'll open another bug for the other patron search results tables.. they're somewhat more complicated being old style ajax tables.
Comment 6 Martin Renvoize 2021-03-11 15:36:33 UTC
Thanks Sally :)
Comment 7 Martin Renvoize 2021-03-12 08:51:07 UTC
Created attachment 118181 [details] [review]
Bug 27926: (QA follow-up) Switch to using data-order

This patch switches away from using the title attribute for ordering and
instead uses data-order which is supported in modern DataTables.  This
has the effect of allowing our ordering to continue to work but without
adding the unwanted tooltip on hover.

Test plan
1/ Ensure sorting is still working as expected
2/ Hover over any of the date of birth fields and note that you are not
longer faced with a tooltip.
3/ Signoff
Comment 8 Katrin Fischer 2021-04-03 14:57:27 UTC
Created attachment 119159 [details] [review]
Bug 27926: Add 'title-string' for dates on circ-patron-results

This patch adds the title string sorting for the data of birth field in
the circulation patron results table.  This has the side effect of
adding a tooltip that displays the date of birth in ISO format, so I'm
not entirely sure it's the right approach.

Test plan
1/ Set your date format preferences to British English (dd-mm-yyyy)
2/ Do a search that will yield multiple patrons via "Check out"
3/ Try sorting on date of birth and not the order is incorrect.
4/ Apply the patch
5/ Refresh the page and try sorting on date again
6/ The order should now be correct
7/ Signoff

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Katrin Fischer 2021-04-03 14:57:31 UTC
Created attachment 119160 [details] [review]
Bug 27926: (QA follow-up) Switch to using data-order

This patch switches away from using the title attribute for ordering and
instead uses data-order which is supported in modern DataTables.  This
has the effect of allowing our ordering to continue to work but without
adding the unwanted tooltip on hover.

Test plan
1/ Ensure sorting is still working as expected
2/ Hover over any of the date of birth fields and note that you are not
longer faced with a tooltip.
3/ Signoff

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Jonathan Druart 2021-04-06 13:58:13 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 11 Fridolin Somers 2021-04-15 10:44:54 UTC
Pushed to 20.11.x for 20.11.05
Comment 12 Andrew Fuerste-Henry 2021-04-24 17:02:39 UTC
Missing dependencies, not backported to 20.05