Bug 36585

Summary: Report 'Patrons with the most checkouts' doesn't have the total when outputting to CSV
Product: Koha Reporter: Michael Hafen <michael.hafen>
Component: ReportsAssignee: Michael Hafen <michael.hafen>
Status: Pushed to main --- QA Contact: Lisette Scheer <lisette>
Severity: enhancement    
Priority: P5 - low CC: david, lisette, lucas
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38915
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
This improves the 'Patrons with the most checkouts' report to: 1. Add the total to the CSV output. 2. Change the screen output to only show the patrons name once, instead of for each group by column. 3. Add total check out when output to screen. 4. Change item type group by from biblioitems.itemtype to items.itype. Note: This does not fix the SQL query so that it can run if the database is in strict mode.
Version(s) released in:
25.05.00
Circulation function:
Attachments: Improve bor_issues_top report
improve bor_issues_top report
Bug 36585: Improve 'Patrons with the most checkouts' report
Bug 36585: (QA follow-up) Fix tab spaces
Bug 36585: Improve 'Patrons with the most checkouts' report
Bug 36585: Improve 'Patrons with the most checkouts' report
Bug 36585: (QA follow-up) Fix tab spaces

Description Michael Hafen 2024-04-11 22:33:12 UTC
I will submit a patch which:
Adds the total to csv output.
Changes screen output to only show the patrons name once, instead of with each group by column.
Also show total checkout when output to screen.
Changes item type group by from biblioitems.itemtype to items.itype.
Comment 1 Michael Hafen 2024-04-11 22:41:50 UTC
Created attachment 164782 [details] [review]
Improve bor_issues_top report
Comment 2 Michael Hafen 2024-04-11 22:43:07 UTC
[ The test plan is in the patch, but I forgot to copy it here. ]

Try the following with and without a group by, specifically item type.

1. Generate the report with output to screen with a group by.
2. Observe that the patron name is repeated for each column group.
3. Observe that there is no total column.
4. Generate the report with output to csv.
5. Observe that the total check outs is not in the csv.
6. Apply patch. restart Koha.
7. Generate the report to csv again.
8. Observe that the total checkouts is in the csv.
9. Generate the report to screen again.
10. Observe that there is a total column and the patron name is only in one
column.
Comment 3 David Nind 2024-05-25 19:04:34 UTC
Hi Michael.

There doesn't seem to be a patch attached, as far as I can see.

I've changed the assignee to yourself.

David
Comment 4 Michael Hafen 2024-05-29 19:52:34 UTC
Created attachment 167248 [details] [review]
improve bor_issues_top report

Try the following with and without a group by, specifically item type.

1. Generate the report with output to screen with a group by.
2. Observe that the patron name is repeated for each column group.
3. Observe that there is no total column.
4. Generate the report with output to csv.
5. Observe that the total check outs is not in the csv.
6. Apply patch. restart Koha.
7. Generate the report to csv again.
8. Observe that the total checkouts is in the csv.
9. Generate the report to screen again.
10. Observe that there is a total column and the patron name is only in one
column.
Comment 5 David Nind 2024-05-30 01:17:53 UTC
Changing status back to needs sign off.

The commit message needs formatting according to the https://wiki.koha-community.org/wiki/Commit_messages

I will update the commit message when I sign it off.
Comment 6 David Nind 2024-05-30 01:21:46 UTC
Created attachment 167251 [details] [review]
Bug 36585: Improve 'Patrons with the most checkouts' report

Update the 'Patrons with the most checkouts' report (bor_issues_top.pl)
to:
1. Add the total to the CSV output.
2. Change the screen output to only show the patrons name once, instead
   of for each group by column.
3. Add total check out when output to screen.
4. Change item type group by from biblioitems.itemtype to items.itype.

Note: This does not fix the SQL query so that it can run if the
database is in strict mode.

Test plan:
1. If using koha-testing-docker (KTD), set strict SQL mode for your
   database to 0 (that is, turn it off).
   (Edit <strict_sql_modes>0</strict_sql_modes> in
   /etc/koha/sites/kohadev/koha-conf.xml
   (and then flush_memcached and restart_all))
2. Add some data so that a useful report can be generated:
   . Check out and return some items to different patrons in different
     libraries:
     ==> Minimum end result:
         . two items issued to a patron for one library, 1 item issued
           and returned for the same patron at another library
         . one item issued to another patron from another patron
           category for one of the libraries used previously
3. Generate the report with output to screen with a group by, for
   example: 'By' option set to Category code
4. Observe that the patron name is repeated for each column group.
5. Observe that there is no total column.
6. Generate the report with output to CSV.
7. Observe that the total checkouts is not in the CSV.
8. Apply the patch and restart Koha (restart_all).
9. Generate the CSV report again.
10. Observe that the total checkouts is in the CSV.
11. Generate the report to screen again.
12. Observe that there is a total column and the patron name is only
    shown in one column.

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 Lucas Gass (lukeg) 2024-08-27 19:05:39 UTC
Why the following line?

[% IF ( loopro.hilighted ) %]<td>[% ELSE %]<td>[% END %]
Comment 8 Michael Hafen 2024-08-27 19:37:31 UTC
(In reply to Lucas Gass from comment #7)
> Why the following line?
> 
> [% IF ( loopro.hilighted ) %]<td>[% ELSE %]<td>[% END %]

I don't know either, that line was there when I started.  I'm guessing it's for table row zebra stripping.

Yeah, digging through the history, that line used to toggle a 'hilighted' class, until 17 years ago in commit 1a90d0db7d.
Comment 9 Lisette Scheer 2025-01-02 18:17:07 UTC
The page to run the report won't load and throws an error:

CGI::Compile::ROOT::kohadevbox_koha_reports_bor_issues_top_2epl::calculate(): DBI Exception: DBD::mysql::st execute failed: 'koha_kohadev.borrowers.surname' isn't in GROUP BY at /kohadevbox/koha/reports/bor_issues_top.pl line 58
 at /usr/share/perl5/DBIx/Class/Exception.pm line 77

Additionally it fails the qa tool for using the tab character instead of 4 spaces

 FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/reports/bor_issues_top.tt
   FAIL   forbidden patterns
                forbidden pattern: tab char (line 90)
                forbidden pattern: tab char (line 91)
                forbidden pattern: tab char (line 93)
                forbidden pattern: tab char (line 105)
                forbidden pattern: tab char (line 106)
                forbidden pattern: tab char (line 107)
                forbidden pattern: tab char (line 108)
                forbidden pattern: tab char (line 109)

 FAIL   reports/bor_issues_top.pl
   FAIL   forbidden patterns
                forbidden pattern: tab char (line 83)
                forbidden pattern: tab char (line 89)
                forbidden pattern: tab char (line 250)
Comment 10 Lucas Gass (lukeg) 2025-01-02 18:46:53 UTC
(In reply to Lisette Scheer from comment #9)
> The page to run the report won't load and throws an error:
> 
 CGI::Compile::ROOT::kohadevbox_koha_reports_bor_issues_top_2epl::calculate():
> DBI Exception: DBD::mysql::st execute failed:
> 'koha_kohadev.borrowers.surname' isn't in GROUP BY at
> /kohadevbox/koha/reports/bor_issues_top.pl line 58
>  at /usr/share/perl5/DBIx/Class/Exception.pm line 77

Likely due to running in SQL strict mode
Comment 11 Lucas Gass (lukeg) 2025-01-02 18:48:24 UTC
Created attachment 176100 [details] [review]
Bug 36585: (QA follow-up) Fix tab spaces

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 12 Lisette Scheer 2025-01-02 19:05:18 UTC
Created attachment 176102 [details] [review]
Bug 36585: Improve 'Patrons with the most checkouts' report

Update the 'Patrons with the most checkouts' report (bor_issues_top.pl)
to:
1. Add the total to the CSV output.
2. Change the screen output to only show the patrons name once, instead
   of for each group by column.
3. Add total check out when output to screen.
4. Change item type group by from biblioitems.itemtype to items.itype.

Note: This does not fix the SQL query so that it can run if the
database is in strict mode.

Test plan:
1. If using koha-testing-docker (KTD), set strict SQL mode for your
   database to 0 (that is, turn it off).
   (Edit <strict_sql_modes>0</strict_sql_modes> in
   /etc/koha/sites/kohadev/koha-conf.xml
   (and then flush_memcached and restart_all))
2. Add some data so that a useful report can be generated:
   . Check out and return some items to different patrons in different
     libraries:
     ==> Minimum end result:
         . two items issued to a patron for one library, 1 item issued
           and returned for the same patron at another library
         . one item issued to another patron from another patron
           category for one of the libraries used previously
3. Generate the report with output to screen with a group by, for
   example: 'By' option set to Category code
4. Observe that the patron name is repeated for each column group.
5. Observe that there is no total column.
6. Generate the report with output to CSV.
7. Observe that the total checkouts is not in the CSV.
8. Apply the patch and restart Koha (restart_all).
9. Generate the CSV report again.
10. Observe that the total checkouts is in the CSV.
11. Generate the report to screen again.
12. Observe that there is a total column and the patron name is only
    shown in one column.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 13 Lisette Scheer 2025-01-02 19:05:57 UTC
Created attachment 176103 [details] [review]
Bug 36585: Improve 'Patrons with the most checkouts' report

Update the 'Patrons with the most checkouts' report (bor_issues_top.pl)
to:
1. Add the total to the CSV output.
2. Change the screen output to only show the patrons name once, instead
   of for each group by column.
3. Add total check out when output to screen.
4. Change item type group by from biblioitems.itemtype to items.itype.

Note: This does not fix the SQL query so that it can run if the
database is in strict mode.

Test plan:
1. If using koha-testing-docker (KTD), set strict SQL mode for your
   database to 0 (that is, turn it off).
   (Edit <strict_sql_modes>0</strict_sql_modes> in
   /etc/koha/sites/kohadev/koha-conf.xml
   (and then flush_memcached and restart_all))
2. Add some data so that a useful report can be generated:
   . Check out and return some items to different patrons in different
     libraries:
     ==> Minimum end result:
         . two items issued to a patron for one library, 1 item issued
           and returned for the same patron at another library
         . one item issued to another patron from another patron
           category for one of the libraries used previously
3. Generate the report with output to screen with a group by, for
   example: 'By' option set to Category code
4. Observe that the patron name is repeated for each column group.
5. Observe that there is no total column.
6. Generate the report with output to CSV.
7. Observe that the total checkouts is not in the CSV.
8. Apply the patch and restart Koha (restart_all).
9. Generate the CSV report again.
10. Observe that the total checkouts is in the CSV.
11. Generate the report to screen again.
12. Observe that there is a total column and the patron name is only
    shown in one column.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 14 Lisette Scheer 2025-01-02 19:06:28 UTC
Created attachment 176104 [details] [review]
Bug 36585: (QA follow-up) Fix tab spaces

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 15 Katrin Fischer 2025-01-16 16:03:18 UTC
Note:
These strings are not translatable:
+        print "Patron".$sep;
+        print "Total".$sep;

But neither was the already existing:
-        print $sep . "Total\n";

Can you please file a bug for this issue? Thank you.
Comment 16 Katrin Fischer 2025-01-16 16:41:54 UTC
Pushed for 25.05!

Well done everyone, thank you!