From 646ecc7763b5e7739c6e52604c07d4d82640f0e7 Mon Sep 17 00:00:00 2001
From: Nick Clemens <nick@bywatersolutions.com>
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'
---
 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 c954aa9714..6e8ad4fa51 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.30.2