From 08b7bf56333ff6c3e8c16a1cd24f0e595b6ed3c9 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Fri, 11 Mar 2022 12:00:26 +0000 Subject: [PATCH] Bug 30222: Only send auto renewal digest wehn there is an update Currently, the code sends an email to the patron when there is any error that is not 'too_soon' - we also don't include issues that are 'too_soon', so emials may be missing information This patch changes the code to: 1 - push all issues that were checked to the report when a borrower wants a digest 2 - adds a variable to track when any of the issues has been updated, and checkes this when sending digests 3 - if nothing has been updated, no report is sent to the patron To test: 1 - Checkout an item, marked for autorenewal, backdated to be overdue to a patron 2 - Set preferences AutoRenewalNotices = 'according to patron messaging preferences' EnhancedMessagingPreferences = 'Allow' 3 - Set the patrons preference for auto renewal to email+digest 4 - Ensure the patron has an email 5 - Place an item level hold for another patron on the issued item 6 - perl misc/cronjobs/automatic_renewals.pl -v --confirm 7 - Check patron record/notices tab 8 - Note item not renewed, patron notified 9 - perl misc/cronjobs/automatic_renewals.pl -v --confirm 10 - Note item not renewed, status not changed, patron notified again 11 - Apply patch 12 - perl misc/cronjobs/automatic_renewals.pl -v --confirm 13 - Item not renewed, patron no notified 14 - Checkout another item marked for auto renewal to the patron, but due in the future ensure it is outside of 'no renewal before' 15 - perl misc/cronjobs/automatic_renewals.pl -v --confirm 16 - Patron not notified, items not renewd Change to status of 'auto_too_soon' is not notified 17 - Checkout a third item, marked for auto renew, overdue 18 - place an item level hold for another patorn on third item 19 - perl misc/cronjobs/automatic_renewals.pl -v --confirm 20 - patron is notified, nothing renewed, all statuses reported 21 - cancel the item level hold on third item 22 - perl misc/cronjobs/automatic_renewals.pl -v --confirm 23 - item renewed, patron notified, all statuses reported 24 - perl misc/cronjobs/automatic_renewals.pl -v --confirm 25 - nothing renewed, patron not notified, only change is one issue 'auto_too_soon' Signed-off-by: Andrew Fuerste-Henry Signed-off-by: Jonathan Druart Signed-off-by: Fridolin Somers Signed-off-by: Kyle M Hall (cherry picked from commit 3fac0c7aee782f96867d2384422962b038c5f7aa) Signed-off-by: Andrew Fuerste-Henry --- misc/cronjobs/automatic_renewals.pl | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/misc/cronjobs/automatic_renewals.pl b/misc/cronjobs/automatic_renewals.pl index 8927b958c8..43e41ce012 100755 --- a/misc/cronjobs/automatic_renewals.pl +++ b/misc/cronjobs/automatic_renewals.pl @@ -148,7 +148,9 @@ while ( my $auto_renew = $auto_renews->next ) { # CanBookBeRenewed returns 'auto_renew' when the renewal should be done by this script my ( $ok, $error ) = CanBookBeRenewed( $auto_renew->borrowernumber, $auto_renew->itemnumber, undef, 1 ); + my $updated; if ( $error eq 'auto_renew' ) { + $updated = 1; if ($verbose) { say sprintf "Issue id: %s for borrower: %s and item: %s %s be renewed.", $auto_renew->issue_id, $auto_renew->borrowernumber, $auto_renew->itemnumber, $confirm ? 'will' : 'would'; @@ -172,22 +174,25 @@ while ( my $auto_renew = $auto_renews->next ) { say sprintf "Issue id: %s for borrower: %s and item: %s %s not be renewed. (%s)", $auto_renew->issue_id, $auto_renew->borrowernumber, $auto_renew->itemnumber, $confirm ? 'will' : 'would', $error; } - if ( not $auto_renew->auto_renew_error or $error ne $auto_renew->auto_renew_error ) { + $updated = 1 if ($error ne $auto_renew->auto_renew_error); + if ( not $auto_renew->auto_renew_error or $updated ) { $auto_renew->auto_renew_error($error)->store if $confirm; push @{ $report{ $auto_renew->borrowernumber } }, $auto_renew if $error ne 'auto_too_soon' && ($send_notices_pref eq 'cron' || ($borrower_preferences && $borrower_preferences->{transports} && $borrower_preferences->{transports}->{email} && !$borrower_preferences->{'wants_digest'})); # Do not notify if it's too soon } } - if ( $error ne 'auto_too_soon' && $borrower_preferences && $borrower_preferences->{transports} && $borrower_preferences->{transports}->{email} && $borrower_preferences->{'wants_digest'} ) { + if ( $borrower_preferences && $borrower_preferences->{transports} && $borrower_preferences->{transports}->{email} && $borrower_preferences->{'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 $error eq 'auto_renew'; $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{error}++ unless $error eq '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 ne 'auto_too_soon'; } else { $renew_digest->{ $auto_renew->borrowernumber }->{success} ++ if $error eq 'auto_renew'; $renew_digest->{ $auto_renew->borrowernumber }->{error}++ unless $error eq 'auto_renew'; + $renew_digest->{ $auto_renew->borrowernumber }->{updated} = 1 if $updated && $error ne 'auto_too_soon'; push @{$renew_digest->{ $auto_renew->borrowernumber }->{issues}}, $auto_renew->itemnumber; } } @@ -271,6 +276,7 @@ sub send_digests { my $params = shift; PATRON: while ( my ( $borrowernumber, $digest ) = each %{$params->{digests}} ) { + next unless defined $digest->{updated} && $digest->{updated} == 1; my $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences( { -- 2.39.5