When something is changed by a cronjob, and that entity is logged via action logs, we can know what changed, and that it was via a cronjob, but we cannot necessarily know which cronjob made that change. The closest we can come is to find the action logs for the cronjob module which ran before the change which is by no means reliable assuming the CronLog is even enabled. We should add a new column to action logs to store the name of the script ran for any action logs where the interface is "cron".
Created attachment 136252 [details] [review] Bug 30984 - Action logs should log the cronjob script name that generated the given log When something is changed by a cronjob, and that entity is logged via action logs, we can know what changed, and that it was via a cronjob, but we cannot necessarily know which cronjob made that change. The closest we can come is to find the action logs for the cronjob module which ran before the change which is by no means reliable assuming the CronLog is even enabled. We should add a new column to action logs to store the name of the script ran for any action logs where the interface is "cron". Test plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Enable all the Log/Logging sysprefs 4) Run some cronjobs that will generate action logs 5) Note the new action_logs column "script" contains the filename of the cronjob that caused the change.
Created attachment 136288 [details] [review] Bug 30984 - Action logs should log the cronjob script name that generated the given log When something is changed by a cronjob, and that entity is logged via action logs, we can know what changed, and that it was via a cronjob, but we cannot necessarily know which cronjob made that change. The closest we can come is to find the action logs for the cronjob module which ran before the change which is by no means reliable assuming the CronLog is even enabled. We should add a new column to action logs to store the name of the script ran for any action logs where the interface is "cron". Test plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Enable all the Log/Logging sysprefs 4) Run some cronjobs that will generate action logs 5) Note the new action_logs column "script" contains the filename of the cronjob that caused the change. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Nice improvement in logging.. I initially thought we'd see it passed in and part of Koha::Script rather than being populated from within C4::Log.. but I'm ambivalent as to where it should live. I wonder about background tasks, whether we'd want to record those as 'scripts' too somewhere along the line.. Signing off anyway as it works as described.
Processing additional checks * Commit title does not start with 'Bug XXXXX: ' - 5dd5449223
Created attachment 138243 [details] [review] Bug 30984: Action logs should log the cronjob script name that generated the given log When something is changed by a cronjob, and that entity is logged via action logs, we can know what changed, and that it was via a cronjob, but we cannot necessarily know which cronjob made that change. The closest we can come is to find the action logs for the cronjob module which ran before the change which is by no means reliable assuming the CronLog is even enabled. We should add a new column to action logs to store the name of the script ran for any action logs where the interface is "cron". Test plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Enable all the Log/Logging sysprefs 4) Run some cronjobs that will generate action logs 5) Note the new action_logs column "script" contains the filename of the cronjob that caused the change. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 138244 [details] [review] Bug 30984: (QA follow-up) Ternary formatting style Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Just mentioning here that mediumtext is not really needed for the script name; tinytext would suffice (256).
Created attachment 138294 [details] [review] Bug 30984: (QA follow-up) Switch from mediumtext to tinytext
Created attachment 138295 [details] [review] Bug 30984: (QA follow-up) Switch from mediumtext to varchar(255) My research indeicates that tinytext of less than 40 bytes uses about the same storage as a varchar(255). Also, using a TEXT field in a complex query may create a temp table. varchar fields are also more efficient for sorting on.
Pushed to master for 22.11. Nice work everyone, thanks!
Backported to 22.05.x for 22.05.06
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed. Nothing to document it seems, marking resolved.
Thanks! Pushed to 21.11 for 21.11.12