From e068b382d5ca4d64efa2beb209ec28d668921b65 Mon Sep 17 00:00:00 2001 From: Lucas Gass Date: Wed, 17 Dec 2025 13:25:09 +0000 Subject: [PATCH] Bug 41437: Prevent sending an auto renewal failure if one already exists To test: 0. Set RestrictionBlockRenewing to 'block' 1. Set up a patron for auto rewenal and then check something out to them. Make sure the patron has an email address and has messaging preferences set to receive auto renewals. 2. Now do something that would make an auto renewal fail. I placed the item from step 1 on hold for someone else. 2. Run the auto renewals cron ( perl /kohadevbox/koha/misc/cronjobs/automatic_renewals.pl -c -v ) 3. A notice should be generated telling the patron the thing cannot renew with the error code of `on_reserve`. ( good ) 4. Restrict the patron, doing this manually is fine. 5. Run the auto renewals cron again. 6. Another auto renewal is sent tell the patron the thing cannot renew with the error code `restriction`. ( bad ) 7. APPLY PATCH, restart_all 8. Reset the test plan and try all the steps again. Tthis time at step 8 you should NOT get a 2nd consecutive failure notice. 9. Lift the restriction and run the automatic_renewals.pl script once again, you shold get a notice that the renewal succeeds. 10. Repeat this process for digest notices. --- misc/cronjobs/automatic_renewals.pl | 61 ++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 18 deletions(-) diff --git a/misc/cronjobs/automatic_renewals.pl b/misc/cronjobs/automatic_renewals.pl index 53c906c5031..93339da85c0 100755 --- a/misc/cronjobs/automatic_renewals.pl +++ b/misc/cronjobs/automatic_renewals.pl @@ -215,6 +215,9 @@ sub _ProcessRenewals { $wants_digest = 0; } + # capture the last error before we start + my $previous_error = $auto_renew->auto_renew_error; + my ( $success, $error, $updated ); eval { ( $success, $error, $updated ) = $auto_renew->attempt_auto_renew( { confirm => $confirm } ); }; if ($@) { @@ -255,32 +258,54 @@ sub _ProcessRenewals { $confirm ? 'will' : 'would', $error; } if ($updated) { + my $skip_notice = + defined($previous_error) + && $previous_error ne '' + && $previous_error ne 'auto_too_soon'; + push @{ $report{ $auto_renew->borrowernumber } }, $auto_renew if $error ne 'auto_too_soon' + && !$skip_notice && ( $wants_messages && !$wants_digest ); # Do not notify if it's too soon } + } if ($wants_digest) { - # cache this one to process after we've run through all of the items. - if ($digest_per_branch) { - $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{success}++ if $success; - $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{error}++ - unless $success || $error eq 'auto_too_soon'; - $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{results} - ->{ defined $error ? $error : 'auto-renew' }++; - push @{ $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{issues} }, - $auto_renew->itemnumber; - $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{updated} = 1 - if $updated && ( !$error || $error ne 'auto_too_soon' ); - } else { - $renew_digest->{ $auto_renew->borrowernumber }->{success}++ if $success; - $renew_digest->{ $auto_renew->borrowernumber }->{error}++ unless $success || $error eq 'auto_too_soon'; - $renew_digest->{ $auto_renew->borrowernumber }->{results}->{ defined $error ? $error : 'auto-renew' }++; - $renew_digest->{ $auto_renew->borrowernumber }->{updated} = 1 - if $updated && ( !$error || $error ne 'auto_too_soon' ); - push @{ $renew_digest->{ $auto_renew->borrowernumber }->{issues} }, $auto_renew->itemnumber; + #if there is already an error that is not auto_too_soon dont tell the patron again + my $skip_notice = + defined($previous_error) + && $previous_error ne '' + && $previous_error ne 'auto_too_soon' + && !$success; + + unless ($skip_notice) { + + # cache this one to process after we've run through all of the items. + if ($digest_per_branch) { + $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{success}++ + if $success; + $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{error}++ + unless $success || $error eq 'auto_too_soon'; + $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{results} + ->{ defined $error ? $error : 'auto-renew' }++; + + push @{ $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{issues} }, + + $auto_renew->itemnumber; + $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{updated} = 1 + if $updated && ( !$error || $error ne 'auto_too_soon' ); + } else { + $renew_digest->{ $auto_renew->borrowernumber }->{success}++ if $success; + $renew_digest->{ $auto_renew->borrowernumber }->{error}++ + unless $success || $error eq 'auto_too_soon'; + $renew_digest->{ $auto_renew->borrowernumber }->{results} + ->{ defined $error ? $error : 'auto-renew' }++; + $renew_digest->{ $auto_renew->borrowernumber }->{updated} = 1 + if $updated && ( !$error || $error ne 'auto_too_soon' ); + push @{ $renew_digest->{ $auto_renew->borrowernumber }->{issues} }, $auto_renew->itemnumber; + } } } -- 2.39.5