Bug 30354 - AUTO_RENEWALS_DGST notices are not generated if patron set to receive notice via SMS and no SMS notice defined
Summary: AUTO_RENEWALS_DGST notices are not generated if patron set to receive notice ...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact: Joonas Kylmälä
URL:
Keywords:
: 29721 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-03-24 14:49 UTC by Andrew Fuerste-Henry
Modified: 2023-06-08 22:26 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
If an SMS notice is not defined for AUTO_RENEWALS_DGST and a patron has selected to receive a digest notification by SMS when items are automatically renewed, it doesn't generate a notice (even though the item(s) is renewed). This fixes the issue so that an email message is generated.
Version(s) released in:
22.05.00,21.11.06,21.05.16


Attachments
Bug 30354: Don't assign warn as letter (2.95 KB, patch)
2022-03-24 15:35 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 30354: Don't assign warn as letter (3.00 KB, patch)
2022-04-30 21:05 UTC, David Nind
Details | Diff | Splinter Review
Bug 30354: Don't assign warn as letter (3.14 KB, patch)
2022-05-04 16:49 UTC, Joonas Kylmälä
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Fuerste-Henry 2022-03-24 14:49:58 UTC
To recreate:
- set AutoRenewalNotices to follow messaging prefs
- have a borrower with a checkout ready for auto-renewal
- set that borrower to receive both email and SMS AUTO_RENEWALS_DGST
- confirm your AUTO_RENEWALS_DGST does not have SMS content but does have email
- run the auto_renew cron
- confirm your renewal happens but the patron gets no notices

The cron spits out these errors:
No circulation AUTO_RENEWALS_DGST letter transported by sms at /kohadevbox/koha/C4/Letters.pm line 583.
no letter of type 'AUTO_RENEWALS_DGST' found for borrowernumber 5. Please see sample_notices.sql at misc/cronjobs/automatic_renewals.pl line 305.
Can't use string ("1") as a HASH ref while "strict refs" in use at /kohadevbox/koha/C4/Letters.pm line 898.

When we check to see if an SMS notice exists, the warning gets passed forward as the $letter.
Comment 1 Lucas Gass 2022-03-24 14:58:11 UTC
*** Bug 29721 has been marked as a duplicate of this bug. ***
Comment 2 Nick Clemens 2022-03-24 15:35:07 UTC
Created attachment 132180 [details] [review]
Bug 30354: Don't assign warn as letter

TO test:
1 - Check out an item marked for autop renewal to a patron and make it overdue
2 - Set system preference AutoRenewalNotices to follow messaging prefs
3 - set that borrower to receive both email and SMS AUTO_RENEWALS_DGST
4 - confirm your AUTO_RENEWALS_DGST does not have SMS content but does have email
5 - run the auto_renew cron
6 - item is renewed, but error from cron, and cron dies:

No circulation AUTO_RENEWALS_DGST letter transported by sms at /kohadevbox/koha/C4/Letters.pm line 583.
no letter of type 'AUTO_RENEWALS_DGST' found for borrowernumber 5. Please see sample_notices.sql at misc/cronjobs/automatic_renewals.pl line 305.
Can't use string ("1") as a HASH ref while "strict refs" in use at /kohadevbox/koha/C4/Letters.pm line 898.

7 - Apply patch
8 - Make item eligible for auto renewal agian (or checkin/checkout)
9 - Run the cron
10 - There is still 2 warn, but cron does not die:

No circulation AUTO_RENEWALS_DGST letter transported by sms at /kohadevbox/koha/C4/Letters.pm line 583.
no letter of type 'AUTO_RENEWALS_DGST' found for borrowernumber 5. Please see sample_notices.sql at misc/cronjobs/    automatic_renewals.pl line 305.

11 - Patron receives email and item is renewed
Comment 3 David Nind 2022-04-30 21:05:51 UTC
Created attachment 134435 [details] [review]
Bug 30354: Don't assign warn as letter

TO test:
1 - Check out an item marked for autop renewal to a patron and make it overdue
2 - Set system preference AutoRenewalNotices to follow messaging prefs
3 - set that borrower to receive both email and SMS AUTO_RENEWALS_DGST
4 - confirm your AUTO_RENEWALS_DGST does not have SMS content but does have email
5 - run the auto_renew cron
6 - item is renewed, but error from cron, and cron dies:

No circulation AUTO_RENEWALS_DGST letter transported by sms at /kohadevbox/koha/C4/Letters.pm line 583.
no letter of type 'AUTO_RENEWALS_DGST' found for borrowernumber 5. Please see sample_notices.sql at misc/cronjobs/automatic_renewals.pl line 305.
Can't use string ("1") as a HASH ref while "strict refs" in use at /kohadevbox/koha/C4/Letters.pm line 898.

7 - Apply patch
8 - Make item eligible for auto renewal agian (or checkin/checkout)
9 - Run the cron
10 - There is still 2 warn, but cron does not die:

No circulation AUTO_RENEWALS_DGST letter transported by sms at /kohadevbox/koha/C4/Letters.pm line 583.
no letter of type 'AUTO_RENEWALS_DGST' found for borrowernumber 5. Please see sample_notices.sql at misc/cronjobs/    automatic_renewals.pl line 305.

11 - Patron receives email and item is renewed

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2022-04-30 21:25:56 UTC
Testing notes (using koha-testing-docker):

- To set things up:
  . Changed the circulation rule so that for All All, Automatic renewal is set to yes
  . Added an email address to the patron used to check items out to (someone@example.com) (not sure that this is required for this to work)
  . To get the SMS column to appear under patron messaging preferences, set the  SMSSendDriver system preference to Email (otherwise the error message isn't generated when the cron script is run)

- To run the cron job: cd misc/cronjobs, perl automatic_renewals.pl -c -d -v

- To check whether a message is "sent" or not, check under the notices section for the patron: before the patch is applied there should be no notice listed, even though the item is renewed; after the patch is applied the item is renewed and there is a pending notice
Comment 5 Joonas Kylmälä 2022-05-04 16:49:52 UTC
Created attachment 134616 [details] [review]
Bug 30354: Don't assign warn as letter

TO test:
1 - Check out an item marked for autop renewal to a patron and make it overdue
2 - Set system preference AutoRenewalNotices to follow messaging prefs
3 - set that borrower to receive both email and SMS AUTO_RENEWALS_DGST
4 - confirm your AUTO_RENEWALS_DGST does not have SMS content but does have email
5 - run the auto_renew cron
6 - item is renewed, but error from cron, and cron dies:

No circulation AUTO_RENEWALS_DGST letter transported by sms at /kohadevbox/koha/C4/Letters.pm line 583.
no letter of type 'AUTO_RENEWALS_DGST' found for borrowernumber 5. Please see sample_notices.sql at misc/cronjobs/automatic_renewals.pl line 305.
Can't use string ("1") as a HASH ref while "strict refs" in use at /kohadevbox/koha/C4/Letters.pm line 898.

7 - Apply patch
8 - Make item eligible for auto renewal agian (or checkin/checkout)
9 - Run the cron
10 - There is still 2 warn, but cron does not die:

No circulation AUTO_RENEWALS_DGST letter transported by sms at /kohadevbox/koha/C4/Letters.pm line 583.
no letter of type 'AUTO_RENEWALS_DGST' found for borrowernumber 5. Please see sample_notices.sql at misc/cronjobs/    automatic_renewals.pl line 305.

11 - Patron receives email and item is renewed

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

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 6 Fridolin Somers 2022-05-05 21:20:54 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 7 Kyle M Hall 2022-05-13 17:57:45 UTC
Pushed to 21.11.x for 21.11.06
Comment 8 Victor Grousset/tuxayo 2022-06-25 21:22:06 UTC
Backported: Pushed to 21.05.x branch for 21.05.16