Bug 36013

Summary: update_totalissues.pl should not log
Product: Koha Reporter: Fridolin Somers <fridolin.somers>
Component: Command-line UtilitiesAssignee: Fridolin Somers <fridolin.somers>
Status: In Discussion --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: david, magnus, nick, robin
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21487
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10326
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36474
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This updates logging when the misc/cronjobs/update_totalissues.pl script is run and CataloguingLog is enabled). This previously added one entry to the log for each record updated - this information is not useful and can take up a lot of database space.
Version(s) released in:
Attachments: Bug 36013: update_totalissues.pl should not log
Bug 36013: update_totalissues.pl should not log
Bug 36013: update_totalissues.pl should not log

Description Fridolin Somers 2024-02-07 10:08:52 UTC
Even when CataloguingLog is on, misc/cronjobs/update_totalissues.pl should not log the changes.
It is useless and takes database space.

Do like bulkmarcimport.pl :
https://git.koha-community.org/Koha-community/Koha/src/branch/23.11.x/misc/migration_tools/bulkmarcimport.pl#L165
Comment 1 Fridolin Somers 2024-02-07 10:15:20 UTC Comment hidden (obsolete)
Comment 2 David Nind 2024-02-10 21:46:34 UTC
I had a go at testing.

However, I'm not sure what I should see in the log viewer before the patch is applied.

With CataloguingLog enabled, I don't see any entries in the log viewer after running the cron job.

If I enable the CronjobLog log system preference, I see a single entry for the cron job running (Info column: misc/cronjobs/update_totalissues.pl --use-stats --commit=1000 -v).
Comment 3 Magnus Enger 2024-03-08 12:03:18 UTC
I had a problem where a bad record would stop the script from completing: 

$ misc/cronjobs/update_totalissues.pl --use-stats --commit=1000 -v
Processing bib 1 (0 issues)
Processing bib 2 (0 issues)
Processing bib 3 (0 issues)
...
Processing bib 368 (0 issues)
Processing bib 369 (0 issues)
:8: parser error : PCDATA invalid Char value 31
  <controlfield tag="001">00aD000015937</controlfield>
                            ^
:9: parser error : PCDATA invalid Char value 31
  <controlfield tag="004">00satmrnu0</controlfield>
                            ^
:9: parser error : PCDATA invalid Char value 31
  <controlfield tag="004">00satmrnu0</controlfield>
                               ^
:9: parser error : PCDATA invalid Char value 31
  <controlfield tag="004">00satmrnu0</controlfield>
                                  ^
:9: parser error : PCDATA invalid Char value 31
  <controlfield tag="004">00satmrnu0</controlfield>
                                     ^
:10: parser error : PCDATA invalid Char value 31
  <controlfield tag="008">00ar19881981bdkldan</controlfield>

After deleting that record the script runs as it should:

...
Processing bib 437 (0 issues)
Processing bib 438 (0 issues)
Update total issues count script report
=======================================================
Run started at:                         03/08/24 11:48:53
Run ended at:                           03/08/24 11:48:57
Total run time:                         4751 ms
Number of bibs modified:                435
Number of bibs with error:              0

After this the cataloguing log shows one entry per record, so that will quickley amount to a lot of unnecessary data!
Comment 4 Magnus Enger 2024-03-08 12:04:55 UTC
Created attachment 162957 [details] [review]
Bug 36013: update_totalissues.pl should not log

Even when CataloguingLog is on, misc/cronjobs/update_totalissues.pl should not log the changes.
It is useless and takes database space.

Do like bulkmarcimport.pl :
https://git.koha-community.org/Koha-community/Koha/src/branch/23.11.x/misc/migration_tools/bulkmarcimport.pl#L165

Test plan :
1) Enable system preference CataloguingLog
2) Perform some issues and returns
3) Run : misc/cronjobs/update_totalissues.pl --use-stats --commit=1000 -v
4) Go to Tools > Log viewer
5) Check there are no new cataloguing logs

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Works as advertised. Before the patch, running the script adds one
entry to action_logs per record, with the patch nothing is logged.
Comment 5 David Nind 2024-03-09 20:52:44 UTC
Created attachment 163012 [details] [review]
Bug 36013: update_totalissues.pl should not log

Even when CataloguingLog is on, misc/cronjobs/update_totalissues.pl should not log the changes.
It is useless and takes database space.

Do like bulkmarcimport.pl :
https://git.koha-community.org/Koha-community/Koha/src/branch/23.11.x/misc/migration_tools/bulkmarcimport.pl#L165

Test plan :
1) Enable system preference CataloguingLog
2) Perform some issues and returns
3) Run : misc/cronjobs/update_totalissues.pl --use-stats --commit=1000 -v
4) Go to Tools > Log viewer
5) Check there are no new cataloguing logs

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Works as advertised. Before the patch, running the script adds one
entry to action_logs per record, with the patch nothing is logged.
Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2024-03-09 21:02:55 UTC
THanks Magnus!

Have added my sign off in case that helps with getting through QA.
Comment 7 Nick Clemens (kidclamp) 2024-04-01 15:13:05 UTC
I don't love just overriding the syspref here - or at least not without making it an option.

Please see bug 36474 - I reduce the logs by not updating a biblio when there are no changes. Beyond that I think it should be a switch in the command --nolog or something.

I am willing to be convinced otherwise, but I think 36474 should be addressed first