When the advance_notices.pl cronjob is run and the EnhancedMessagingPreferences syspref is set to don't allow, we get the following warning: The "EnhancedMessagingPreferences" syspref is off. Therefore, it is unlikely that this script will actually produce any messages to be sent. To change this, edit the "EnhancedMessagingPreferences" syspref. The advance_notices.pl script help says: -v Verbose. Without this flag set, only fatal errors are reported. However, the above warning is shown regardless of whether the script is run with the verbose flag. We need to add a verbosity check to this warn
Created attachment 125086 [details] [review] Bug 29054: Add verbose check for warn in advance_notices.pl To test: 1) Set EnhancedMessagingPreferences to Don't allow 2) In the koha-shell, run misc/cronjobs/advance_notices.pl -c 3) Note that you see the warning "The "EnhancedMessagingPreferences" syspref is off... etc." 4) Apply the patch and restart services 5) In the koha-shell, run misc/cronjobs/advance_notices.pl -c and note the warning no longer shows 6) Still in the shell, run misc/cronjobs/advance_notices.pl -c -v and note the warning does show Sponsored-by: Catalyst IT
I agree with that
Created attachment 127680 [details] [review] Bug 29054: Add verbose check for warn in advance_notices.pl To test: 1) Set EnhancedMessagingPreferences to Don't allow 2) In the koha-shell, run misc/cronjobs/advance_notices.pl -c 3) Note that you see the warning "The "EnhancedMessagingPreferences" syspref is off... etc." 4) Apply the patch and restart services 5) In the koha-shell, run misc/cronjobs/advance_notices.pl -c and note the warning no longer shows 6) Still in the shell, run misc/cronjobs/advance_notices.pl -c -v and note the warning does show Sponsored-by: Catalyst IT Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(In reply to Hayley Pelham from comment #0) > When the advance_notices.pl cronjob is run and the > EnhancedMessagingPreferences syspref is set to don't allow, we get the > following warning: > > The "EnhancedMessagingPreferences" syspref is off. > Therefore, it is unlikely that this script will actually produce any > messages to be sent. > To change this, edit the "EnhancedMessagingPreferences" syspref. > > The advance_notices.pl script help says: > > -v Verbose. Without this flag set, only fatal errors are reported. Not sure if we should remove a warn here or edit the above description? Removing fatal or replacing it by a less severe term? Needs feedback.
I agree with only giving this note with -v as at the moment if you have scheduled this on a server with multiple instances where the pref is turned off intentionally, it creates quite a bit of noise. It's not a fatal one I think, so would fit the description of -v, just referring to a configuration issue.
Setting back to Signed off after feedback from Katrin - thanks Katrin!
(In reply to Katrin Fischer from comment #5) > I agree with only giving this note with -v as at the moment if you have > scheduled this on a server with multiple instances where the pref is turned > off intentionally, it creates quite a bit of noise. > > It's not a fatal one I think, so would fit the description of -v, just > referring to a configuration issue. I dont oppose pushing it for the sake of multiple instances (where some instances would have enhanced messaging and others might not). But suppressing a warn that tells you that you are running a script that wouldnt do anything, is imo generally not what we should do.
Created attachment 129242 [details] [review] Bug 29054: Add verbose check for warn in advance_notices.pl To test: 1) Set EnhancedMessagingPreferences to Don't allow 2) In the koha-shell, run misc/cronjobs/advance_notices.pl -c 3) Note that you see the warning "The "EnhancedMessagingPreferences" syspref is off... etc." 4) Apply the patch and restart services 5) In the koha-shell, run misc/cronjobs/advance_notices.pl -c and note the warning no longer shows 6) Still in the shell, run misc/cronjobs/advance_notices.pl -c -v and note the warning does show Sponsored-by: Catalyst IT Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Marcel de Rooy from comment #7) > (In reply to Katrin Fischer from comment #5) > > I agree with only giving this note with -v as at the moment if you have > > scheduled this on a server with multiple instances where the pref is turned > > off intentionally, it creates quite a bit of noise. > > > > It's not a fatal one I think, so would fit the description of -v, just > > referring to a configuration issue. > > I dont oppose pushing it for the sake of multiple instances (where some > instances would have enhanced messaging and others might not). But > suppressing a warn that tells you that you are running a script that wouldnt > do anything, is imo generally not what we should do. We have other examples of script. membership_expiry.pl and preference MembershipExpiryDaysNotice: https://git.koha-community.org/Koha-community/Koha/src/commit/e653effe5783e149f5ab69f388aa5509db454ef4/misc/cronjobs/membership_expiry.pl#L182 Using -v is a very common way to know what a script does ;)
Created Bug 29928
This isnt enhancement, I change to minor to encourage backport
Pushed to master for 22.05, thanks to everybody involved [U+1F984]
Pushed to 21.11.x for 21.11.03
Pushed to 21.05.x for 21.05.11
Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed.