Bug 29381 - Auto-renewal digest messages are sent on every cron run
Summary: Auto-renewal digest messages are sent on every cron run
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Joonas Kylmälä
QA Contact: Nick Clemens
URL:
Keywords:
: 29555 (view as bug list)
Depends on: 18532
Blocks: 30222
  Show dependency treegraph
 
Reported: 2021-10-31 13:59 UTC by Joonas Kylmälä
Modified: 2022-12-12 21:24 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes an issue with automatic renewal digest messages - these were being sent on every cron run, even if there was nothing to renew or no renewal errors. (This error was caused by a regression in 21.05 from Bug 18532: Add individual issues to digest notice and hide auto_renewals messaging preference when not needed.)
Version(s) released in:
21.11.00,21.05.09


Attachments
Bug 29381: Prevent sending of automatic renewal messages on every cron run (3.04 KB, patch)
2021-10-31 15:08 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 29381: Prevent sending of automatic renewal messages on every cron run (3.08 KB, patch)
2021-11-07 20:18 UTC, David Nind
Details | Diff | Splinter Review
Bug 29381: Prevent sending of automatic renewal messages on every cron run (3.14 KB, patch)
2021-11-23 11:42 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Joonas Kylmälä 2021-10-31 13:59:53 UTC
If you have patrons with checkouts that have auto-renewal enabled and the patron has checked in their messaging preferences that they want auto-renewal messages as digests then they get every day / cron run a message with al list of their checkouts that has auto-renewal enabled.
Comment 1 Joonas Kylmälä 2021-10-31 15:08:17 UTC
Created attachment 127141 [details] [review]
Bug 29381: Prevent sending of automatic renewal messages on every cron run

Currently the auto-renewal digest messages are sent on every cron run
even if there was nothing to renew or no renewal errors.

This regression was introduced in the commit "Bug 18532: Add
individual issues to digest notice and hide auto_renewals messaging
preference when not needed".

To test:
 1) set syspref AutoRenewalNotices to be according to patron
    preferences
 2) Enable renewal digest messages on a patron's messaging preferences
 3) Checkout a book for patron, during the checkout use the Checkout
 settings menu to check the box "Automatic renewal"
 4) Run
    $ perl misc/cronjobs/automatic_renewals.pl --send-notices --confirm --digest-per-branch
    $ perl misc/cronjobs/automatic_renewals.pl --send-notices --confirm --digest-per-branch
 5) Notice you have now two renewal messages for the patron
 6) Apply patch
 7) repeat step 4) and notice you don't get anymore these unnecessary
    renewal messages
Comment 2 Joonas Kylmälä 2021-10-31 15:09:01 UTC
To complete the test plan you need to apply also fix from bug 29380, though it is unrelated to this bug.
Comment 3 David Cook 2021-11-01 22:23:43 UTC
I can't even get auto-renewal digest messages to send...

Isn't "--send-notices" a deprecated option?
Comment 4 Joonas Kylmälä 2021-11-02 20:35:19 UTC
(In reply to David Cook from comment #3)
> I can't even get auto-renewal digest messages to send...

Not sure where you got stuck. To test you can check the Notices tab of the patron via staff interface as well, no need to have have a working email setup.
Comment 5 David Cook 2021-11-03 22:27:35 UTC
(In reply to Joonas Kylmälä from comment #4)
> (In reply to David Cook from comment #3)
> > I can't even get auto-renewal digest messages to send...
> 
> Not sure where you got stuck. To test you can check the Notices tab of the
> patron via staff interface as well, no need to have have a working email
> setup.

Ah sorry. I wasn't referring to your patches. I meant that I can't get auto-renewal digest messages to send in production in Koha haha. I can't tell if Koha is broken or if I'm doing something wrong, but it sounds like you're able to send auto-renewal digest messages...
Comment 6 David Nind 2021-11-07 20:18:50 UTC
Created attachment 127416 [details] [review]
Bug 29381: Prevent sending of automatic renewal messages on every cron run

Currently the auto-renewal digest messages are sent on every cron run
even if there was nothing to renew or no renewal errors.

This regression was introduced in the commit "Bug 18532: Add
individual issues to digest notice and hide auto_renewals messaging
preference when not needed".

To test:
 1) set syspref AutoRenewalNotices to be according to patron
    preferences
 2) Enable renewal digest messages on a patron's messaging preferences
 3) Checkout a book for patron, during the checkout use the Checkout
 settings menu to check the box "Automatic renewal"
 4) Run
    $ perl misc/cronjobs/automatic_renewals.pl --send-notices --confirm --digest-per-branch
    $ perl misc/cronjobs/automatic_renewals.pl --send-notices --confirm --digest-per-branch
 5) Notice you have now two renewal messages for the patron
 6) Apply patch
 7) repeat step 4) and notice you don't get anymore these unnecessary
    renewal messages

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Cook 2021-11-16 00:03:52 UTC
I don't think that I have any QA powers any more but this sounds very important.

(Bug 29223 was the issue I was having with automatic renewal digests.)
Comment 8 David Cook 2021-11-22 22:16:03 UTC
*** Bug 29555 has been marked as a duplicate of this bug. ***
Comment 9 David Cook 2021-11-22 22:16:42 UTC
(In reply to David Cook from comment #8)
> *** Bug 29555 has been marked as a duplicate of this bug. ***

Nick made different changes on bug 29555 and after comparing both patches I'm thinking that maybe Nick and Joonas should talk and combine the patches?
Comment 10 Nick Clemens 2021-11-23 11:42:58 UTC
Created attachment 127947 [details] [review]
Bug 29381: Prevent sending of automatic renewal messages on every cron run

Currently the auto-renewal digest messages are sent on every cron run
even if there was nothing to renew or no renewal errors.

This regression was introduced in the commit "Bug 18532: Add
individual issues to digest notice and hide auto_renewals messaging
preference when not needed".

To test:
 1) set syspref AutoRenewalNotices to be according to patron
    preferences
 2) Enable renewal digest messages on a patron's messaging preferences
 3) Checkout a book for patron, during the checkout use the Checkout
 settings menu to check the box "Automatic renewal"
 4) Run
    $ perl misc/cronjobs/automatic_renewals.pl --send-notices --confirm --digest-per-branch
    $ perl misc/cronjobs/automatic_renewals.pl --send-notices --confirm --digest-per-branch
 5) Notice you have now two renewal messages for the patron
 6) Apply patch
 7) repeat step 4) and notice you don't get anymore these unnecessary
    renewal messages

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Jonathan Druart 2021-11-24 08:43:06 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2021-11-24 08:43:21 UTC
There is a terrible lack of tests in this area :-/
Comment 13 Jonathan Druart 2021-11-24 08:51:37 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 14 Andrew Fuerste-Henry 2022-01-03 16:11:11 UTC
Pushed to 21.05.x for 21.05.09
Comment 15 Victor Grousset/tuxayo 2022-01-12 00:59:15 UTC
Missing dependencies for 20.11.x, it shouldn't be affected, no backport.