To recreate: 1. Have the default notices set for AUTO_RENEWALS and AUTO_RENEWALS_DGST. 2. Set the ItemsDeniedRenewal system preference. ( itemlost: [1,2,3,4,5] ) 3. Set the AutoRenewalNotices system preference to 'According to messaging preferences'. 4. Don't mark lost items as returned ( MarkLostItemsAsReturned ). 5. Set up some auto renewals and then mark the items as lost. 6. Run automatic_renewals.pl 7. You get a notice telling you the item cannot be renewed but no reason is specified.
This is causing our patrons to receive an auto renewal digest notice the day after their items are marked as long overdue, stating that the item can't be renewed. We don't want to send this notice, as it is could be confusing to patrons who just received a bill for those lost items.
Created attachment 179464 [details] [review] Bug 34498: Make autorenewal notices consider ItemsDeniedRenewal This fix ensures that autorenewals (and corresponding notices) are not generated for items that are configured to be denied renewal in the ItemsDeniedRenewal system preference. To test: 1. Have the default notices set for AUTO_RENEWALS and AUTO_RENEWALS_DGST. 2. Set the ItemsDeniedRenewal system preference. ( itemlost: [1,2,3,4,5] ) 3. Set the AutoRenewalNotices system preference to 'According to messaging preferences'. 4. Don't mark lost items as returned ( MarkLostItemsAsReturned ). 5. Set up some auto renewals and then mark the items as lost. 6. Run automatic_renewals.pl 7. You get a notice telling you the item cannot be renewed but no reason is specified. 8. Apply the patch and restart services 9. Run through the test plan again and confirm no notices are generated. Sponsored-by: New Zealand Parliamentary Library
Created attachment 180304 [details] [review] Bug 34498: Make autorenewal notices consider ItemsDeniedRenewal This fix ensures that autorenewals (and corresponding notices) are not generated for items that are configured to be denied renewal in the ItemsDeniedRenewal system preference. To test: 1. Have the default notices set for AUTO_RENEWALS and AUTO_RENEWALS_DGST. 2. Set the ItemsDeniedRenewal system preference. ( itemlost: [1,2,3,4,5] ) 3. Set the AutoRenewalNotices system preference to 'According to messaging preferences'. 4. Don't mark lost items as returned ( MarkLostItemsAsReturned ). 5. Set up some auto renewals and then mark the items as lost. 6. Run automatic_renewals.pl 7. You get a notice telling you the item cannot be renewed but no reason is specified. 8. Apply the patch and restart services 9. Run through the test plan again and confirm no notices are generated. Sponsored-by: New Zealand Parliamentary Library Signed-off-by: Magnus Enger <magnus@libriotech.no>
I don't think skipping the item completely in the auto-renewal cron job is the way to go here. If the item is denied renewal based on a permanent attribute, like item type, then it's either a bug or a configuration issue if that item is getting flagged for auto-renewal in the first place. On the other hand, if it's denied renewal based on a temporary status such as Lost status, that means it was eligible for auto-renewal at some point, and libraries might want to notify patrons rather than have it silently fail to renew. We should adjust the default notice template as the bug title suggests to better handle items_denied_renewal, and leave libraries the choice to make their own revisions based on whether they want to notify of that failure reason or not.
(In reply to Emily Lamancusa (emlam) from comment #4) > I don't think skipping the item completely in the auto-renewal cron job is > the way to go here. If the item is denied renewal based on a permanent > attribute, like item type, then it's either a bug or a configuration issue > if that item is getting flagged for auto-renewal in the first place. On the > other hand, if it's denied renewal based on a temporary status such as Lost > status, that means it was eligible for auto-renewal at some point, and > libraries might want to notify patrons rather than have it silently fail to > renew. We should adjust the default notice template as the bug title > suggests to better handle items_denied_renewal, and leave libraries the > choice to make their own revisions based on whether they want to notify of > that failure reason or not. Hey Emily, thanks for your thoughts here. The particular use case for our patch was that the library is using the ItemsDeniedRenewal system preference to exclude items from renewal based on collectioncode. So it's more permanent than a lost status, and I'm sure they had a reason for not using an item type which would've allowed them to use the circulation rules. It means that patrons are getting empty autorenewal notices if they have items from this collection checked out, which aren't eligible for auto-renew. Does that explain the purpose of this better?
Thanks for the explanation, Aleisha! I think there are two pieces here: One is that the checkout code should be checking ItemsDeniedRenewal, and not setting issues.auto-renew in the first place if its renewal is denied by the syspref. Then the cron wouldn't pick up those items, and we wouldn't need a separate loop to exclude them. For the notices issue, we have something similar - it's not specific to ItemsDeniedRenewal, but in our system we generally don't want an auto-renewal notice going out at all if nothing actually renewed. To prevent that, we wrapped our notice template in: [%- UNLESS success == 0 %] <notice template contents> [%- END %] Would something like that work for this library's use case?