[Bug 34924] New: Add ability to send 'final auto renewal notice'
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Bug ID: 34924 Summary: Add ability to send 'final auto renewal notice' Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com See bugs 19014 and 31427 - users can end up receiving an auto renewal failure immediately after a success. 31427 updates to send this notice at the end of a circ period, but then the user doesn't know the book won't renew until it is nearly due We should have a way to mark that an issue has reached the end of the allowed renewals and warn the user that the item won't be renewed when it becomes due. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=19014, | |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=31427 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 156239 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=156239&action=edit Bug 34924: Add Koha::Checkout->attempt_auto_renew This patch moves the actual renewal out of the auto_renewals cronjob script and into the object and adds tests. The logic for notices is still handled in the script. To test: 1 - prove -v t/db_dependent/Koha/Checkouts.t 2 - Add a circ rule with auto_renew checked 3 - Checkout an item to a patron and set due date in the past 4 - Checkout an item to a patron and set due date in the future 5 - perl misc/cronjobs/automatic_renewals.pl -v 6 - Confirm one would be renewed and the other is too_soon 7 - perl misc/cronjobs/automatic_renewals.pl -v --confirm 8 - Confirm the expected issue is successfully renewed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 --- Comment #2 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 156240 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=156240&action=edit Bug 34924: Add 'auto_renew_final' and 'auto_unseen_final' return to CanBookBeRenewed There is a desire for auto_renewals to treat the final renewal differently. We would like to notify the patron of the final renewal - but not again when the next renewal fails. This patch adds the new return value and tests. To test: prove -v t/db_dependent/Circulation.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 --- Comment #3 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 156241 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=156241&action=edit Bug 34924: Handle final renewal errors This patch updates Koha::Checkout->attempt_auto_renew to renew when the 'final' errors are passed, and to pass the error value back to the cronjob for processing The sample notice for AUTO_RENEW and AUTO_RENEW_DGST are both updated to handle the new error On the next cron the error will be updated to too_many or too_unseen, but a notice will not be sent. To test: 0 - Run reset_all to install the updated sample notices or copy the text 1 - Set system preference UnseenRenewals to 'Allow' 2 - Setup a circ rule to allow 2 renewals, 2 unseen renewals 3 - Checkout an item to a patron who has an email defined and auto renewals selected in messaging preferences 4 - Update the issue to be due/overdue: UPDATE issues SET date_due=DATE_SUB(NOW(), INTERVAL 1 DAYS); 5 - perl misc/cronjobs/automatic_renewals.pl -v -c 6 - Confirm patron notified and issue renewed 7 - Set issue due/overdue again 8 - perl misc/cronjobs/automatic_renewals.pl -v -c 9 - Confirm patron notified of final unseen renewal 10 - perl misc/cronjobs/automatic_renewals.pl -v -c 11 - Confirm issue not renewed, patron not notified 12 - Update circ rules t all 4 renewals total 13 - Force renewal on staff side to clear unseen 14 - perl misc/cronjobs/automatic_renewals.pl -v -c 15 - Confirm patron notified of final renewal (allowed 4, 2 unseen from cron, 1 manual, this unseen from cron) 16 - perl misc/cronjobs/automatic_renewals.pl -v -c 17 - Confirm patron not notified, issue not renewed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |emily.lamancusa@montgomeryc | |ountymd.gov, | |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Katie Bliss <kebliss@dmpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kebliss@dmpl.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #4 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- The script output looks right in terms of which items should renew, but the renewal itself doesn't seem to be happening. After running the cron with the --confirm switch, the item's due date doesn't change, and the patron's checkouts table line for that item still says "Scheduled for automatic renewal (2 of 2 renewals remaining and 2 of 2 unseen renewals remaining)". The patron does have a queued automatic renewal notice saying that the item renewed and is due on the original due date. I tried again with only the first patch applied and got the same result, so I'm guessing the problem is there somewhere. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #156239|0 |1 is obsolete| | Attachment #156240|0 |1 is obsolete| | Attachment #156241|0 |1 is obsolete| | --- Comment #5 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 157290 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157290&action=edit Bug 34924: Add Koha::Checkout->attempt_auto_renew This patch moves the actual renewal out of the auto_renewals cronjob script and into the object and adds tests. The logic for notices is still handled in the script. To test: 1 - prove -v t/db_dependent/Koha/Checkouts.t 2 - Add a circ rule with auto_renew checked 3 - Checkout an item to a patron and set due date in the past 4 - Checkout an item to a patron and set due date in the future 5 - perl misc/cronjobs/automatic_renewals.pl -v 6 - Confirm one would be renewed and the other is too_soon 7 - perl misc/cronjobs/automatic_renewals.pl -v --confirm 8 - Confirm the expected issue is successfully renewed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 --- Comment #6 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 157291 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157291&action=edit Bug 34924: Add 'auto_renew_final' and 'auto_unseen_final' return to CanBookBeRenewed There is a desire for auto_renewals to treat the final renewal differently. We would like to notify the patron of the final renewal - but not again when the next renewal fails. This patch adds the new return value and tests. To test: prove -v t/db_dependent/Circulation.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 --- Comment #7 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 157292 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157292&action=edit Bug 34924: Handle final renewal errors This patch updates Koha::Checkout->attempt_auto_renew to renew when the 'final' errors are passed, and to pass the error value back to the cronjob for processing The sample notice for AUTO_RENEW and AUTO_RENEW_DGST are both updated to handle the new error On the next cron the error will be updated to too_many or too_unseen, but a notice will not be sent. To test: 0 - Run reset_all to install the updated sample notices or copy the text 1 - Set system preference UnseenRenewals to 'Allow' 2 - Setup a circ rule to allow 2 renewals, 2 unseen renewals 3 - Checkout an item to a patron who has an email defined and auto renewals selected in messaging preferences 4 - Update the issue to be due/overdue: UPDATE issues SET date_due=DATE_SUB(NOW(), INTERVAL 1 DAYS); 5 - perl misc/cronjobs/automatic_renewals.pl -v -c 6 - Confirm patron notified and issue renewed 7 - Set issue due/overdue again 8 - perl misc/cronjobs/automatic_renewals.pl -v -c 9 - Confirm patron notified of final unseen renewal 10 - perl misc/cronjobs/automatic_renewals.pl -v -c 11 - Confirm issue not renewed, patron not notified 12 - Update circ rules t all 4 renewals total 13 - Force renewal on staff side to clear unseen 14 - perl misc/cronjobs/automatic_renewals.pl -v -c 15 - Confirm patron notified of final renewal (allowed 4, 2 unseen from cron, 1 manual, this unseen from cron) 16 - perl misc/cronjobs/automatic_renewals.pl -v -c 17 - Confirm patron not notified, issue not renewed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 --- Comment #8 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Emily Lamancusa from comment #4)
The script output looks right in terms of which items should renew, but the renewal itself doesn't seem to be happening. After running the cron with the --confirm switch, the item's due date doesn't change, and the patron's checkouts table line for that item still says "Scheduled for automatic renewal (2 of 2 renewals remaining and 2 of 2 unseen renewals remaining)". The patron does have a queued automatic renewal notice saying that the item renewed and is due on the original due date.
I tried again with only the first patch applied and got the same result, so I'm guessing the problem is there somewhere.
Ah, I neglected to pass through the confirm to attempt_auto_renewal - this should be working better now -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 --- Comment #9 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Thanks for the follow-up - that fixed it! It all works great now, except that the default text of the final renewal notice is contradictory: "The following item, Pro JavaScript techniques , has not been renewed because: The following item, Pro JavaScript techniques , has correctly been renewed and is now due on 10/23/2023 This item has reached the maximum number of unseen renewals and will need to be renewed at the library." It happens because the final renewal message is passed as an error even though it's not really an error...which isn't ideal. It's easy to fix in AUTO_RENEWALS, and a bit less so in AUTO_RENEWALS_DGST, but it does make me wonder if we'd be setting ourselves up for issues with maintainability down the road. Is there a better way to pass that information? Testing notes for 3rd patch (handle final renewal errors): Step 1 - Also need to set the system preference AutoRenwalNotices to "according to patron messaging preferences" Step 2 - The circ rule should set renewals to 4, unseen renewals to 2, and Automatic Renewal to "yes" (setting both to 2 won't generate the "final unseen" notice because the checkout will reach its final unseen renewal and its final overall auto-renewal at the same time, and the overall limit takes precedence in the notice) Step 12 - already done above Step 14 - need to update the issue to be due/overdue again before running the script -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- This looks exciting, but alas, I wasn't quick enough and it no longer applies. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #157290|0 |1 is obsolete| | Attachment #157291|0 |1 is obsolete| | Attachment #157292|0 |1 is obsolete| | --- Comment #11 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 157687 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157687&action=edit Bug 34924: Add Koha::Checkout->attempt_auto_renew This patch moves the actual renewal out of the auto_renewals cronjob script and into the object and adds tests. The logic for notices is still handled in the script. To test: 1 - prove -v t/db_dependent/Koha/Checkouts.t 2 - Add a circ rule with auto_renew checked 3 - Checkout an item to a patron and set due date in the past 4 - Checkout an item to a patron and set due date in the future 5 - perl misc/cronjobs/automatic_renewals.pl -v 6 - Confirm one would be renewed and the other is too_soon 7 - perl misc/cronjobs/automatic_renewals.pl -v --confirm 8 - Confirm the expected issue is successfully renewed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 --- Comment #12 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 157688 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157688&action=edit Bug 34924: Add 'auto_renew_final' and 'auto_unseen_final' return to CanBookBeRenewed There is a desire for auto_renewals to treat the final renewal differently. We would like to notify the patron of the final renewal - but not again when the next renewal fails. This patch adds the new return value and tests. To test: prove -v t/db_dependent/Circulation.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 --- Comment #13 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 157689 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157689&action=edit Bug 34924: Handle final renewal errors This patch updates Koha::Checkout->attempt_auto_renew to renew when the 'final' errors are passed, and to pass the error value back to the cronjob for processing The sample notice for AUTO_RENEW and AUTO_RENEW_DGST are both updated to handle the new error On the next cron the error will be updated to too_many or too_unseen, but a notice will not be sent. To test: 0 - Run reset_all to install the updated sample notices or copy the text 1 - Set system preference UnseenRenewals to 'Allow' 2 - Setup a circ rule to allow 2 renewals, 2 unseen renewals 3 - Checkout an item to a patron who has an email defined and auto renewals selected in messaging preferences 4 - Update the issue to be due/overdue: UPDATE issues SET date_due=DATE_SUB(NOW(), INTERVAL 1 DAYS); 5 - perl misc/cronjobs/automatic_renewals.pl -v -c 6 - Confirm patron notified and issue renewed 7 - Set issue due/overdue again 8 - perl misc/cronjobs/automatic_renewals.pl -v -c 9 - Confirm patron notified of final unseen renewal 10 - perl misc/cronjobs/automatic_renewals.pl -v -c 11 - Confirm issue not renewed, patron not notified 12 - Update circ rules t all 4 renewals total 13 - Force renewal on staff side to clear unseen 14 - perl misc/cronjobs/automatic_renewals.pl -v -c 15 - Confirm patron notified of final renewal (allowed 4, 2 unseen from cron, 1 manual, this unseen from cron) 16 - perl misc/cronjobs/automatic_renewals.pl -v -c 17 - Confirm patron not notified, issue not renewed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 --- Comment #14 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 157690 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157690&action=edit Bug 34924: Adjust auto renewal email to reflect success for final renewals -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 --- Comment #15 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Emily Lamancusa from comment #9)
It happens because the final renewal message is passed as an error even though it's not really an error...which isn't ideal. It's easy to fix in AUTO_RENEWALS, and a bit less so in AUTO_RENEWALS_DGST, but it does make me wonder if we'd be setting ourselves up for issues with maintainability down the road. Is there a better way to pass that information?
I adjusted the single renewal notice - the digest looks to me like it will come out correctly. I started trying to look into this, and it got much bigger. We could pass a 'success' flag for the regular notice, then check for the final in the auto_renew_error field. If we just consider it to be 'status' rather than 'error' it feels less bad, adding a new column for tracking final seems overkill to me. If acceptable, I'd like to see this work done on a new bug, rather than changing the structure and adding feature at the same time. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Marie <marie.hedbom@musikverket.se> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |marie.hedbom@musikverket.se -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 --- Comment #16 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- (In reply to Nick Clemens from comment #15)
I started trying to look into this, and it got much bigger. We could pass a 'success' flag for the regular notice, then check for the final in the auto_renew_error field. If we just consider it to be 'status' rather than 'error' it feels less bad, adding a new column for tracking final seems overkill to me. If acceptable, I'd like to see this work done on a new bug, rather than changing the structure and adding feature at the same time.
That sounds like a good solution - and I agree with making that change on a separate bug. Thanks for taking a look! Will retest (and hopefully sign off!) in the morning. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Also sounds good to me.. Looking through this and having run a few tests I'd be happy to QA this once the SO has come in ;P -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #157687|0 |1 is obsolete| | --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 157895 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157895&action=edit Bug 34924: Add Koha::Checkout->attempt_auto_renew This patch moves the actual renewal out of the auto_renewals cronjob script and into the object and adds tests. The logic for notices is still handled in the script. To test: 1 - prove -v t/db_dependent/Koha/Checkouts.t 2 - Add a circ rule with auto_renew checked 3 - Checkout an item to a patron and set due date in the past 4 - Checkout an item to a patron and set due date in the future 5 - perl misc/cronjobs/automatic_renewals.pl -v 6 - Confirm one would be renewed and the other is too_soon 7 - perl misc/cronjobs/automatic_renewals.pl -v --confirm 8 - Confirm the expected issue is successfully renewed Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #157688|0 |1 is obsolete| | --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 157896 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157896&action=edit Bug 34924: Add 'auto_renew_final' and 'auto_unseen_final' return to CanBookBeRenewed There is a desire for auto_renewals to treat the final renewal differently. We would like to notify the patron of the final renewal - but not again when the next renewal fails. This patch adds the new return value and tests. To test: prove -v t/db_dependent/Circulation.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #157689|0 |1 is obsolete| | --- Comment #20 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 157897 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157897&action=edit Bug 34924: Handle final renewal errors This patch updates Koha::Checkout->attempt_auto_renew to renew when the 'final' errors are passed, and to pass the error value back to the cronjob for processing The sample notice for AUTO_RENEW and AUTO_RENEW_DGST are both updated to handle the new error On the next cron the error will be updated to too_many or too_unseen, but a notice will not be sent. To test: 0 - Run reset_all to install the updated sample notices or copy the text 1 - Set system preference UnseenRenewals to 'Allow' 2 - Setup a circ rule to allow 2 renewals, 2 unseen renewals 3 - Checkout an item to a patron who has an email defined and auto renewals selected in messaging preferences 4 - Update the issue to be due/overdue: UPDATE issues SET date_due=DATE_SUB(NOW(), INTERVAL 1 DAYS); 5 - perl misc/cronjobs/automatic_renewals.pl -v -c 6 - Confirm patron notified and issue renewed 7 - Set issue due/overdue again 8 - perl misc/cronjobs/automatic_renewals.pl -v -c 9 - Confirm patron notified of final unseen renewal 10 - perl misc/cronjobs/automatic_renewals.pl -v -c 11 - Confirm issue not renewed, patron not notified 12 - Update circ rules t all 4 renewals total 13 - Force renewal on staff side to clear unseen 14 - perl misc/cronjobs/automatic_renewals.pl -v -c 15 - Confirm patron notified of final renewal (allowed 4, 2 unseen from cron, 1 manual, this unseen from cron) 16 - perl misc/cronjobs/automatic_renewals.pl -v -c 17 - Confirm patron not notified, issue not renewed Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #157690|0 |1 is obsolete| | --- Comment #21 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 157898 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157898&action=edit Bug 34924: Adjust auto renewal email to reflect success for final renewals Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #22 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Of course.. Emily is on the QA team.. so we can swap.. added my signoff and passing to Emily for the final QA :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #157895|0 |1 is obsolete| | --- Comment #23 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 157929 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157929&action=edit Bug 34924: Add Koha::Checkout->attempt_auto_renew This patch moves the actual renewal out of the auto_renewals cronjob script and into the object and adds tests. The logic for notices is still handled in the script. To test: 1 - prove -v t/db_dependent/Koha/Checkouts.t 2 - Add a circ rule with auto_renew checked 3 - Checkout an item to a patron and set due date in the past 4 - Checkout an item to a patron and set due date in the future 5 - perl misc/cronjobs/automatic_renewals.pl -v 6 - Confirm one would be renewed and the other is too_soon 7 - perl misc/cronjobs/automatic_renewals.pl -v --confirm 8 - Confirm the expected issue is successfully renewed Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #157896|0 |1 is obsolete| | --- Comment #24 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 157930 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157930&action=edit Bug 34924: Add 'auto_renew_final' and 'auto_unseen_final' return to CanBookBeRenewed There is a desire for auto_renewals to treat the final renewal differently. We would like to notify the patron of the final renewal - but not again when the next renewal fails. This patch adds the new return value and tests. To test: prove -v t/db_dependent/Circulation.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #157897|0 |1 is obsolete| | --- Comment #25 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 157931 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157931&action=edit Bug 34924: Handle final renewal errors This patch updates Koha::Checkout->attempt_auto_renew to renew when the 'final' errors are passed, and to pass the error value back to the cronjob for processing The sample notice for AUTO_RENEW and AUTO_RENEW_DGST are both updated to handle the new error On the next cron the error will be updated to too_many or too_unseen, but a notice will not be sent. To test: 0 - Run reset_all to install the updated sample notices or copy the text 1 - Set system preference UnseenRenewals to 'Allow' 2 - Setup a circ rule to allow 2 renewals, 2 unseen renewals 3 - Checkout an item to a patron who has an email defined and auto renewals selected in messaging preferences 4 - Update the issue to be due/overdue: UPDATE issues SET date_due=DATE_SUB(NOW(), INTERVAL 1 DAYS); 5 - perl misc/cronjobs/automatic_renewals.pl -v -c 6 - Confirm patron notified and issue renewed 7 - Set issue due/overdue again 8 - perl misc/cronjobs/automatic_renewals.pl -v -c 9 - Confirm patron notified of final unseen renewal 10 - perl misc/cronjobs/automatic_renewals.pl -v -c 11 - Confirm issue not renewed, patron not notified 12 - Update circ rules t all 4 renewals total 13 - Force renewal on staff side to clear unseen 14 - perl misc/cronjobs/automatic_renewals.pl -v -c 15 - Confirm patron notified of final renewal (allowed 4, 2 unseen from cron, 1 manual, this unseen from cron) 16 - perl misc/cronjobs/automatic_renewals.pl -v -c 17 - Confirm patron not notified, issue not renewed Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #157898|0 |1 is obsolete| | --- Comment #26 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 157932 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157932&action=edit Bug 34924: Adjust auto renewal email to reflect success for final renewals Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 --- Comment #27 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 157933 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157933&action=edit Bug 34924: (QA follow-up) correctly handle digest data compilation for successful renewal automatic_renewals.pl does some data compilation for digest notices that depends on the error value returned from attempt_auto_renew. However, on successful renewal $error is undefined. Add additional checks on value of $success and definedness of $error to compile data accurately and avoid warns for undefined variable Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #28 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Looks great! Added a follow-up to fix some checks when compiling result data for the digest notices - CanBookBeRenewed was returning an $error value of 'auto-renew' for successful autorenewals, while attempt_auto_renew returns an undefined $error value in that situation (which makes sense - the subroutine already assumes we're in an auto-renewal context after all). This was actually causing some incorrect data to be passed to the digest notice, as well as a bunch of warns for trying to access $error when it was undefined. Won't cause issues anywhere else, though, since attempt_auto_renew isn't used outside the cron. Other than that, QA tests pass, no regressions found, and everything looks good. Thanks for the nice refactor and excellent added clarity to auto-renewal notices! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|martin.renvoize@ptfs-europe |emily.lamancusa@montgomeryc |.com |ountymd.gov -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |23.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 --- Comment #29 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Needs documenting CC| |fridolin.somers@biblibre.co | |m --- Comment #30 from Fridolin Somers <fridolin.somers@biblibre.com> --- Enhancement not pushed to 23.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Georgia Newman <g.newman@arts.ac.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |g.newman@arts.ac.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 --- Comment #31 from Lucas Gass <lucas@bywatersolutions.com> ---
I started trying to look into this, and it got much bigger. We could pass a 'success' flag for the regular notice, then check for the final in the auto_renew_error field. If we just consider it to be 'status' rather than 'error' it feels less bad, adding a new column for tracking final seems overkill to me. If acceptable, I'd like to see this work done on a new bug, rather than changing the structure and adding feature at the same time.
Has this new bug been filed yet? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Rebecca Coert <rcoert@arlingtonva.us> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |rcoert@arlingtonva.us -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Ray Delahunty <r.delahunty@arts.ac.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |r.delahunty@arts.ac.uk --- Comment #32 from Ray Delahunty <r.delahunty@arts.ac.uk> --- We would be very disappointed if changes to the existing functionality added here (as suggested in comments 9 to 16) led to a reduction in functionality. We are using the AUTO_RENEWALS_DGST as in effect the first notification of an item being overdue. We have removed the ‘success’ TT code from the notice so no emails are sent if all items renew without an error state. No pointless emails are sent, which is proving very popular here. The ‘auto_renewal_final’ situation is something the reader needs to be told about (of course), so having the ‘auto_renewal_final’ as an error is key to our 23.11 auto-renewals functionality. In dozens of tests to configure auto-renewals we have never been able to replicate point 11 in the test plan for this bug. An email is prepared for the ‘auto_renewal_final’ and one is also sent for the ‘too_many’ error, which is good as it supports our four overdue notifications workflow. (Our first ‘official’ ODUE notice is sent 7 days after the item fails to auto-renew). If we wanted to achieve test point 11 and have no notifications sent for the ‘too_many’ error I think we could simply remove that error from the TT code in the notice. But why would we do that, given the delights of no ‘success’ renewal email nags and 4 overdue notification functionality? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 --- Comment #33 from Ray Delahunty <r.delahunty@arts.ac.uk> --- Created attachment 168642 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=168642&action=edit What we were trying to achieve -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 --- Comment #34 from Ray Delahunty <r.delahunty@arts.ac.uk> --- I need to update my comments. Extended testing has shown that my removing the ‘success content’ in the digest triggers a nasty side effect that we cannot tolerate. And further tests show that the renewal reminder being triggered by the too_many error is also caused by my removing the ‘success content’. Now that we have reverted to using the default digest, when we upgrade to 23.11 tomorrow, the digest will work in accordance with this bug. The attachment outlines what we were trying to achieve, and I plan to raise a separate bug to request the optional prevention of error-less renewal notifications, and further to optionally have a digest prepared as well for the too_many error. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|martin.renvoize@ptfs-europe | |.com | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |caroline.cyr-la-rose@inlibr | |o.com Resolution|--- |FIXED Status|Needs documenting |RESOLVED --- Comment #35 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- This doesn't seem to change anything on the staff side (or OPAC), so nothing to change in the manual. Please reopen with details if this is not the case. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34924 Michaela Sieber <michaela.sieber@kit.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |michaela.sieber@kit.edu -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org