Summary: | AUTO_RENEWALS_DGST should skip auto_too_soon | ||
---|---|---|---|
Product: | Koha | Reporter: | David Cook <dcook> |
Component: | Notices | Assignee: | David Cook <dcook> |
Status: | Pushed to oldstable --- | QA Contact: | Lucas Gass (lukeg) <lucas> |
Severity: | normal | ||
Priority: | P5 - low | CC: | david, fridolin.somers, lucas, marie.hedbom, wainuiwitikapark |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: |
This fixes the default AUTO_RENEWALS_DGST notice so that items where it is too soon to renew aren't included in the notice output to patrons when the automatic renewals cron job is run (based on the circulation rules settings). These items were previously included in the notice.
NOTE: This notice is only updated for new installations. Existing installations should update this notice if they only want to show the actual items automatically renewed.
|
Version(s) released in: |
24.11.00,24.05.02,23.11.07
|
Circulation function: | |||
Attachments: |
Bug 36741: Skip auto_too_soon issues in auto renew digest
Bug 36741: Skip auto_too_soon issues in auto renew digest Bug 36741: Skip auto_too_soon issues in auto renew digest |
Description
David Cook
2024-05-01 04:53:02 UTC
It seems that all we need is the following: [% FOREACH checkout IN checkouts %] [% NEXT IF (checkout.auto_renew_error == 'auto_too_soon') %] That's the default that makes sense to me. People could do something else if they wanted, but we *must* capture this case and do something with it. I'm getting (totally valid) complaints about this at the moment. I don't think there's an easy way to update existing AUTO_RENEWALS_DGST notices, but we should update the mandatory sample notices at least. Created attachment 167054 [details] [review] Bug 36741: Skip auto_too_soon issues in auto renew digest This change adds a line to skip auto_too_soon issues/checkouts in the auto renew digest template. Since auto_too_soon do not trigger notifications and don't require any special action, let's skip them in the breakdown of checkouts in the AUTO_RENEWALS_DGST email. Test plan: 0. Apply the patch 1. reset_all (in koha-testing-docker) 2. Note the following line in the AUTO_RENEWALS_DGST template: [% NEXT IF (checkout.auto_renew_error == 'auto_too_soon') %] Created attachment 167137 [details] [review] Bug 36741: Skip auto_too_soon issues in auto renew digest This change adds a line to skip auto_too_soon issues/checkouts in the auto renew digest template. Since auto_too_soon do not trigger notifications and don't require any special action, let's skip them in the breakdown of checkouts in the AUTO_RENEWALS_DGST email. Test plan: 0. Apply the patch 1. reset_all (in koha-testing-docker) 2. Note the following line in the AUTO_RENEWALS_DGST template: [% NEXT IF (checkout.auto_renew_error == 'auto_too_soon') %] Signed-off-by: David Nind <david@davidnind.com> If you use [% NEXT IF (checkout.auto_renew_error == 'auto_too_soon') %] will it not lead to an erroneous count here: "There were [% error %] items that were not renewed." ? (In reply to Lucas Gass from comment #5) > If you use [% NEXT IF (checkout.auto_renew_error == 'auto_too_soon') %] will > it not lead to an erroneous count here: > > "There were [% error %] items that were not renewed." > > ? Nope. auto_too_soon isn't added to the error count, so it would actually be more accurate. $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{error}++ unless $success || $error eq 'auto_too_soon'; $renew_digest->{ $auto_renew->borrowernumber }->{error}++ unless $success || $error eq 'auto_too_soon'; (In reply to David Cook from comment #6) > (In reply to Lucas Gass from comment #5) > > If you use [% NEXT IF (checkout.auto_renew_error == 'auto_too_soon') %] will > > it not lead to an erroneous count here: > > > > "There were [% error %] items that were not renewed." > > > > ? > > Nope. auto_too_soon isn't added to the error count, so it would actually be > more accurate. > > $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber > }->{error}++ unless $success || $error eq 'auto_too_soon'; > > $renew_digest->{ $auto_renew->borrowernumber }->{error}++ unless $success || > $error eq 'auto_too_soon'; Ah, great! Created attachment 168140 [details] [review] Bug 36741: Skip auto_too_soon issues in auto renew digest This change adds a line to skip auto_too_soon issues/checkouts in the auto renew digest template. Since auto_too_soon do not trigger notifications and don't require any special action, let's skip them in the breakdown of checkouts in the AUTO_RENEWALS_DGST email. Test plan: 0. Apply the patch 1. reset_all (in koha-testing-docker) 2. Note the following line in the AUTO_RENEWALS_DGST template: [% NEXT IF (checkout.auto_renew_error == 'auto_too_soon') %] Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> I am wondering about this one. Won't this create an empty notice if all items are "auto_too_soon"? (In reply to Katrin Fischer from comment #9) > I am wondering about this one. Won't this create an empty notice if all > items are "auto_too_soon"? No, because notices only go out if the "updated" flag in the $renew_digest hashref is set to 1, and if all items are "auto_too_soon", the "updated" flag won't be set to 1: $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{updated} = 1 if $updated && (!$error || $error ne 'auto_too_soon'); $renew_digest->{ $auto_renew->borrowernumber }->{updated} = 1 if $updated && (!$error || $error ne 'auto_too_soon'); Resetting to Passed QA. -- I've had this running in production for about 2 months now, and so far so good. Thx for the reply :) I was wondering if we should not "remove" those items from the loop so we can do without additional code in the sample notices. The notices get a bit harder to read/understand for librarians that way, but I think it gives flexibility for those who might want to include a note about these items. Pushed for 24.11! Well done everyone, thank you! Backported to 24.05.x for upcoming 24.05.02 I'm not sure if I described this correctly in the release notes - please update to correct my mistakes. I've never really got my head around these settings for the circulation rules! Pushed to 23.11.x for 23.11.07 Not backporting to 23.05.x unless requested |