Bug 27820

Summary: plugins_nightly.pl script missing use
Product: Koha Reporter: Samir Shah <solaris.smoke>
Component: Plugin architectureAssignee: Fridolin Somers <fridolin.somers>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: andrewfh, fridolin.somers, jonathan.druart, kyle, martin.renvoize, solaris.smoke, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.04
Bug Depends on: 25245    
Bug Blocks:    
Attachments: Fix missing import in plugins_nightly.pl
Bug 27820: fix missing import in plugins_nightly.pl
Bug 27820: add all missing use in misc/cronjobs/plugins_nightly.pl
Bug 27820: fix missing import in plugins_nightly.pl
Bug 27820: add all missing use in misc/cronjobs/plugins_nightly.pl
Bug 27820: fix missing import in plugins_nightly.pl
Bug 27820: add all missing use in misc/cronjobs/plugins_nightly.pl

Description Samir Shah 2021-03-01 06:47:55 UTC
The new plugins_nightly.pl script that was introduced in Koha 20.11.03 doesn't work - it fails with the following error:

/etc/cron.daily/koha-common:
Undefined subroutine &main::cronlogaction called at /usr/share/koha/bin/cronjobs/plugins_nightly.pl line 8.

The issue appears to be a missing import - from looking at other files that use cronlogaction, the following is missing from this file:

use C4::Log;
Comment 1 Samir Shah 2021-03-01 07:51:11 UTC
Created attachment 117427 [details] [review]
Fix missing import in plugins_nightly.pl
Comment 2 Samir Shah 2021-03-01 07:52:17 UTC
I've added a patch that I think resolves the issue. I'm not familiar enough with Perl to be able to contribute more (e.g., tests, if required).
Comment 3 Fridolin Somers 2021-03-01 09:49:39 UTC
Good catch, I set status to need signoff
Comment 4 Fridolin Somers 2021-03-01 10:45:18 UTC
Created attachment 117435 [details] [review]
Bug 27820: fix missing import in plugins_nightly.pl

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 5 Fridolin Somers 2021-03-01 10:45:31 UTC
Created attachment 117436 [details] [review]
Bug 27820: add all missing use in misc/cronjobs/plugins_nightly.pl

Koha::Plugins and C4::Context where missing.
Try::Tiny was missing for try/catch.

Test plan :
1) Use a plugin using hook 'cronjob_nightly'
2) Run script : misc/cronjobs/plugins_nightly.pl
Comment 6 Fridolin Somers 2021-03-01 10:55:06 UTC
Piouf nearly all use where missing :D

In order to test, I've added a pull request on KitchenSink Koha plugin :
https://github.com/bywatersolutions/dev-koha-plugin-kitchen-sink/pull/15
Comment 7 Fridolin Somers 2021-03-01 10:55:39 UTC
Comment on attachment 117427 [details] [review]
Fix missing import in plugins_nightly.pl

I've signed first patch and created a second one that needs signoff
Comment 8 Jonathan Druart 2021-03-02 08:43:07 UTC
Please test this carefully.
Comment 9 Nick Clemens (kidclamp) 2021-03-11 11:16:08 UTC
Created attachment 118111 [details] [review]
Bug 27820: fix missing import in plugins_nightly.pl

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 10 Nick Clemens (kidclamp) 2021-03-11 11:16:10 UTC
Created attachment 118112 [details] [review]
Bug 27820: add all missing use in misc/cronjobs/plugins_nightly.pl

Koha::Plugins and C4::Context where missing.
Try::Tiny was missing for try/catch.

Test plan :
1) Use a plugin using hook 'cronjob_nightly'
2) Run script : misc/cronjobs/plugins_nightly.pl

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Martin Renvoize 2021-03-11 11:20:12 UTC
Created attachment 118113 [details] [review]
Bug 27820: fix missing import in plugins_nightly.pl

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2021-03-11 11:20:15 UTC
Created attachment 118114 [details] [review]
Bug 27820: add all missing use in misc/cronjobs/plugins_nightly.pl

Koha::Plugins and C4::Context where missing.
Try::Tiny was missing for try/catch.

Test plan :
1) Use a plugin using hook 'cronjob_nightly'
2) Run script : misc/cronjobs/plugins_nightly.pl

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2021-03-11 11:21:35 UTC
I had been avoiding QAing this one as I clearly made the mistakes in the first place.. but as no-one else is taking it on I thought I'd give it a spin.

Tested using the Kitchensink merge request as added by Fridolin.

All works as expected.  Passing QA
Comment 14 Jonathan Druart 2021-03-11 15:38:40 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 15 Jonathan Druart 2021-03-11 15:39:25 UTC
Congratulations Samir for your first patch pushed!
Comment 16 Fridolin Somers 2021-03-12 13:07:04 UTC
Pushed to 20.11.x for 20.11.04
Comment 17 Andrew Fuerste-Henry 2021-03-23 12:17:49 UTC
Missing dependency, not backported to 20.05