Bug 39015

Summary: Date sorting not working in cash register statistics wizard
Product: Koha Reporter: David Roberts <david.roberts>
Component: ReportsAssignee: Owen Leonard <oleonard>
Status: Pushed to main --- QA Contact: Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski>
Severity: normal    
Priority: P5 - low CC: baptiste.wojtkowski, david, jonathan.druart, martin.renvoize
Version: Main   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: Trivial patch Documentation contact:
Documentation submission: Text to go in the release notes:
This fixes date sorting for the cash register statistics wizard so that columns with dates sort correctly (it adds a "data-sort" attribute to the columns in the results table which contain dates - this allows DataTables to sort using the unformatted date). It also adds the "anti-the" class to the title column, so that it sorts excluding articles such as a, the, and an.
Version(s) released in:
25.05.00
Circulation function:
Bug Depends on: 38984    
Bug Blocks:    
Attachments: Bug 39015: Fix sorting by date on cash register statistics page
Bug 39015: Fix sorting by date on cash register statistics page
Bug 39015: Fix sorting by date on cash register statistics page
Bug 39015: Fix sorting by date on cash register statistics page
Bug 39015: Fix sorting by date on cash register statistics page

Description David Roberts 2025-02-03 11:28:16 UTC
If a site has the dateformat system preference set to dd/mm/yyyy, date sorting doesn't work in the statistics wizards - e.g. /cgi-bin/koha/reports/cash_register_stats.pl. It does seem to sort correctly if the dateformat is set to mm/dd/yyyy.

This might be connected to https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37875
[+] Comment 1 Owen Leonard 2025-02-03 14:52:55 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2025-02-03 15:23:51 UTC
Created attachment 177470 [details] [review]
Bug 39015: Fix sorting by date on cash register statistics page

This patch adds a "data-sort" attribute to the columns in the results
table which contain dates. This allows DataTables to sort using the
unformatted date.

The patch also adds the "anti-the" class to the column containing
titles, ensuring that the sort excludes articles.

To test, apply the patch and go to Reports -> Statistics wizards -> Cash
register. You must have multiple entries in the results, which could be
generated by using the "Create manual invoice" form under a patron's
account tab. Unfortunately that won't create entries with multiple dates
to sort on. I assume a proper test will require some manual SQL updates.

To test title sorting you should have some transactions which are tied
to an item. To generate this data:

- Set the 'WhenLostChargeReplacementFee' system preference to "Charge"
- Add a replacement cost to some items in the catalog.
- Check those items out ot a patron.
- From the "Item details" tab for those records, mark each item lost.
  - The charges should now be on the patron's account.

- On the Cash register statistics page, submit the form using parameters
  which will return multiple results.
- Test that the "Transaction date" and "Updated" columns work correctly
  with various settings of the "dateformat" system preference.
- Test that the "Bibliographic record title" column sorts correctly,
  excluding initial articles (a, an, the) from the sort.

Sponsored-by: Athens County Public Libraries
Comment 3 David Nind 2025-02-08 21:32:20 UTC
Created attachment 177662 [details] [review]
Bug 39015: Fix sorting by date on cash register statistics page

This patch adds a "data-sort" attribute to the columns in the results
table which contain dates. This allows DataTables to sort using the
unformatted date.

The patch also adds the "anti-the" class to the column containing
titles, ensuring that the sort excludes articles.

To test, apply the patch and go to Reports -> Statistics wizards -> Cash
register. You must have multiple entries in the results, which could be
generated by using the "Create manual invoice" form under a patron's
account tab. Unfortunately that won't create entries with multiple dates
to sort on. I assume a proper test will require some manual SQL updates.

To test title sorting you should have some transactions which are tied
to an item. To generate this data:

- Set the 'WhenLostChargeReplacementFee' system preference to "Charge"
- Add a replacement cost to some items in the catalog.
- Check those items out ot a patron.
- From the "Item details" tab for those records, mark each item lost.
  - The charges should now be on the patron's account.

- On the Cash register statistics page, submit the form using parameters
  which will return multiple results.
- Test that the "Transaction date" and "Updated" columns work correctly
  with various settings of the "dateformat" system preference.
- Test that the "Bibliographic record title" column sorts correctly,
  excluding initial articles (a, an, the) from the sort.

Sponsored-by: Athens County Public Libraries
Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2025-02-08 21:41:42 UTC
Testing notes using KTD:

1. I could replicate the date sorting issue with dates in the dd/mm/yyyy format, other date formats worked as expected.

2. Example query to update the date field:
   - koha-mysql kohadev
   - update accountlines set date="2025-01-15 12:00:00" where accountlines_id=3;
   - amend to reflect the data in your databases
   - list all transactions: select * from accountlines;
   - I amended the dates so that they were in different months and years

3. I amended the record titles to include the, a, and an at the start of the title - they now correctly sort by ignoring the article.

4. For the "Transaction type" field selector, I changed to "All transactions".
Comment 5 Owen Leonard 2025-02-26 18:26:35 UTC
Created attachment 178770 [details] [review]
Bug 39015: Fix sorting by date on cash register statistics page

This patch adds a "data-sort" attribute to the columns in the results
table which contain dates. This allows DataTables to sort using the
unformatted date.

The patch also adds the "anti-the" class to the column containing
titles, ensuring that the sort excludes articles.

To test, apply the patch and go to Reports -> Statistics wizards -> Cash
register. You must have multiple entries in the results, which could be
generated by using the "Create manual invoice" form under a patron's
account tab. Unfortunately that won't create entries with multiple dates
to sort on. I assume a proper test will require some manual SQL updates.

To test title sorting you should have some transactions which are tied
to an item. To generate this data:

- Set the 'WhenLostChargeReplacementFee' system preference to "Charge"
- Add a replacement cost to some items in the catalog.
- Check those items out ot a patron.
- From the "Item details" tab for those records, mark each item lost.
  - The charges should now be on the patron's account.

- On the Cash register statistics page, submit the form using parameters
  which will return multiple results.
- Test that the "Transaction date" and "Updated" columns work correctly
  with various settings of the "dateformat" system preference.
- Test that the "Bibliographic record title" column sorts correctly,
  excluding initial articles (a, an, the) from the sort.

Sponsored-by: Athens County Public Libraries
Signed-off-by: David Nind <david@davidnind.com>
Comment 6 Baptiste Wojtkowski (bwoj) 2025-02-27 10:49:12 UTC
Works perfectly even not on top of 38984, may I remove the dependency for QA ?
Comment 7 Katrin Fischer 2025-02-27 10:51:20 UTC
(In reply to Baptiste Wojtkowski (bwoj) from comment #6)
> Works perfectly even not on top of 38984, may I remove the dependency for QA
> ?

Please don't remove the dependency in Bugzilla. You can QA, but we'll need to set the status to BLOCKED if it passes, until 38984 has been processed.
Comment 8 Baptiste Wojtkowski (bwoj) 2025-02-27 10:54:21 UTC
Ok, then I'm fine with this one + QA tool passes
Comment 9 Katrin Fischer 2025-02-27 10:58:30 UTC
Please still add your sign-off lines :)
Comment 10 Baptiste Wojtkowski (bwoj) 2025-02-27 11:02:50 UTC
Created attachment 178780 [details] [review]
Bug 39015: Fix sorting by date on cash register statistics page

This patch adds a "data-sort" attribute to the columns in the results
table which contain dates. This allows DataTables to sort using the
unformatted date.

The patch also adds the "anti-the" class to the column containing
titles, ensuring that the sort excludes articles.

To test, apply the patch and go to Reports -> Statistics wizards -> Cash
register. You must have multiple entries in the results, which could be
generated by using the "Create manual invoice" form under a patron's
account tab. Unfortunately that won't create entries with multiple dates
to sort on. I assume a proper test will require some manual SQL updates.

To test title sorting you should have some transactions which are tied
to an item. To generate this data:

- Set the 'WhenLostChargeReplacementFee' system preference to "Charge"
- Add a replacement cost to some items in the catalog.
- Check those items out ot a patron.
- From the "Item details" tab for those records, mark each item lost.
  - The charges should now be on the patron's account.

- On the Cash register statistics page, submit the form using parameters
  which will return multiple results.
- Test that the "Transaction date" and "Updated" columns work correctly
  with various settings of the "dateformat" system preference.
- Test that the "Bibliographic record title" column sorts correctly,
  excluding initial articles (a, an, the) from the sort.

Sponsored-by: Athens County Public Libraries
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Comment 11 Martin Renvoize (ashimema) 2025-03-07 10:30:15 UTC
Why block a bug when it doesn't need to be blocked.. especially when what you're blocking it on is an enhancement which won't be backported.

I don't understand the logic here.
Comment 12 Katrin Fischer 2025-03-07 13:26:45 UTC
I would have set it to BLOCKED too - this is actually PQA, but the dependency on bug 38984 needs to move to PQA before we can push it.
Comment 13 Katrin Fischer 2025-03-07 13:27:31 UTC
(In reply to Katrin Fischer from comment #12)
> I would have set it to BLOCKED too - this is actually PQA, but the
> dependency on bug 38984 needs to move to PQA before we can push it.

I just checked - Owen is patch author here and Owen set the dependency, so I believe it to be correct.
Comment 14 Katrin Fischer 2025-03-07 13:31:56 UTC
Sorry, if I am misunderstanding the comment - might be missing something here.
Comment 15 Owen Leonard 2025-03-08 16:10:34 UTC
(In reply to Katrin Fischer from comment #13)
> > I would have set it to BLOCKED too - this is actually PQA, but the
> > dependency on bug 38984 needs to move to PQA before we can push it.

I go by what the RM says!
Comment 16 Katrin Fischer 2025-04-14 06:44:37 UTC
Oddly enough this applies without the depedency... ?
Comment 17 Katrin Fischer 2025-04-22 06:49:54 UTC
Pushed for 25.05!

Well done everyone, thank you!