Bug 29223 - Auto-renewals can fail when not digested per branch and patron requests digest
Summary: Auto-renewals can fail when not digested per branch and patron requests digest
Status: Signed Off
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: Testopia
URL:
Keywords:
Depends on: 28581
Blocks:
  Show dependency treegraph
 
Reported: 2021-10-13 15:18 UTC by Nick Clemens
Modified: 2021-10-14 10:44 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 29223: Use patron's branchcode for digest if not sending digest per branch (2.66 KB, patch)
2021-10-13 15:48 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29223: Use patron's branchcode for digest if not sending digest per branch (2.72 KB, patch)
2021-10-13 17:22 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2021-10-13 15:18:38 UTC
Bug 28581 updated send_digest in automatic_renewals.pl to use $library->from_email_address, however, there is no branchcode parameter passed when not doing 'digest_as_branch' and so the call will fail
Comment 1 Nick Clemens 2021-10-13 15:48:59 UTC
Created attachment 126222 [details] [review]
Bug 29223: Use patron's branchcode for digest if not sending digest per branch

To test:
 1 - Set system preference  AutoRenewalNotices to 'according to patron messaging preferences'
 2 - Set circ rule for auto renewal - 100 renewals allowed - no renewal before 100
    This shoud make a checkout eligible for many auto renewals for testing
 3 - Checkout an item to a patron, ensure item is from a different branch than patron
 4 - Ensure the patron has an email, and set an email for patron's branch and items branch
 5 - Set the patron messaging preferences for auto-renewal to 'email' and 'wants digest'
 6 - perl misc/cronjobs/automatic_renewals.pl --send-notices --confirm
 7 - Job dies:
    Can't call method "from_email_address" on an undefined value at /usr/share/koha/bin/cronjobs/automatic_renewals.pl line 286.
 8 - Apply patch
 9 - Repeat
10 - NO errors
11 - Check patron notices tab, should be a notice enqueued, check DB to see from address is patron's branch
12 - perl misc/cronjobs/automatic_renewals.pl --send-notices --confirm --digest-per-branch
13 - No errors
14 - Check patron notices tab, should be a notice enqueued, check DB to see from address is item's branch
Comment 2 Andrew Fuerste-Henry 2021-10-13 17:22:47 UTC
Created attachment 126228 [details] [review]
Bug 29223: Use patron's branchcode for digest if not sending digest per branch

To test:
 1 - Set system preference  AutoRenewalNotices to 'according to patron messaging preferences'
 2 - Set circ rule for auto renewal - 100 renewals allowed - no renewal before 100
    This shoud make a checkout eligible for many auto renewals for testing
 3 - Checkout an item to a patron, ensure item is from a different branch than patron
 4 - Ensure the patron has an email, and set an email for patron's branch and items branch
 5 - Set the patron messaging preferences for auto-renewal to 'email' and 'wants digest'
 6 - perl misc/cronjobs/automatic_renewals.pl --send-notices --confirm
 7 - Job dies:
    Can't call method "from_email_address" on an undefined value at /usr/share/koha/bin/cronjobs/automatic_renewals.pl line 286.
 8 - Apply patch
 9 - Repeat
10 - NO errors
11 - Check patron notices tab, should be a notice enqueued, check DB to see from address is patron's branch
12 - perl misc/cronjobs/automatic_renewals.pl --send-notices --confirm --digest-per-branch
13 - No errors
14 - Check patron notices tab, should be a notice enqueued, check DB to see from address is item's branch

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 3 Andrew Fuerste-Henry 2021-10-13 17:29:32 UTC
This worked as described. 
In step 14, the from_address matches the item's *holding branch.* I tried it with both possible values for HomeOrHolding, in case it was following that, but it didn't make a difference. I wasn't clear if that was the intended behavior, from the test plan.