From 22bce346d197172f56df38a55aea41e4a9c855ca Mon Sep 17 00:00:00 2001
From: Nick Clemens <nick@bywatersolutions.com>
Date: Fri, 11 Mar 2022 13:24:33 +0000
Subject: [PATCH] Bug 30222: Simplify logic for reporting

This patch adds two new variables:
$wants_email and $wants digest

These are used to simplify checks on whether notices should be sent

To test:
1 - Apply patch
2 - Confirm notices are not sent of pref is 'cron' and send_notcies flag not set
3 - Confirm notices are sent if pref is cron and send_notices flag is set and borrower
    does not have preference set for auto renewals
4 - Confirm notices not sent if pref is set to follow messaging preference and borrower
    does not have preferences set for auto_renewals
5 - Confirm regular notices sent if pref is set to follow messaging preferences and borrower
    does have the preference set but not digest
6 - Confirm digest notices sent if pref is set to follow messaging preferences and borrower
    does have the preference set and wants digest

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 17f1793ba952e3a95f98ec777d56486611b7036d)

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
---
 misc/cronjobs/automatic_renewals.pl | 30 ++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/misc/cronjobs/automatic_renewals.pl b/misc/cronjobs/automatic_renewals.pl
index 43e41ce012..b5c7426d46 100755
--- a/misc/cronjobs/automatic_renewals.pl
+++ b/misc/cronjobs/automatic_renewals.pl
@@ -140,11 +140,26 @@ my %report;
 while ( my $auto_renew = $auto_renews->next ) {
     print "examining item '" . $auto_renew->itemnumber . "' to auto renew\n" if $verbose;
 
-    my $borrower_preferences;
-    $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences( { borrowernumber => $auto_renew->borrowernumber,
-                                                                                   message_name   => 'auto_renewals' } ) if $send_notices_pref eq 'preferences';
+    my ( $borrower_preferences, $wants_email, $wants_digest );
+    if ( $send_notices_pref eq 'preferences' ){
+        $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences(
+            {
+                borrowernumber => $auto_renew->borrowernumber,
+                message_name   => 'auto_renewals'
+            }
+        );
+        $wants_email = 1
+            if $borrower_preferences
+            && $borrower_preferences->{transports}
+            && $borrower_preferences->{transports}->{email};
+        $wants_digest = 1
+            if $wants_email
+            && $borrower_preferences->{wants_digest};
+    }
 
-    $send_notices = 1 if !$send_notices && $send_notices_pref eq 'preferences' && $borrower_preferences && $borrower_preferences->{transports} && $borrower_preferences->{transports}->{email};
+    $send_notices = 1
+      if !$send_notices
+      && $wants_email;
 
     # 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 );
@@ -159,7 +174,8 @@ while ( my $auto_renew = $auto_renews->next ) {
             my $date_due = AddRenewal( $auto_renew->borrowernumber, $auto_renew->itemnumber, $auto_renew->branchcode, undef, undef, undef, 0 );
             $auto_renew->auto_renew_error(undef)->store;
         }
-        push @{ $report{ $auto_renew->borrowernumber } }, $auto_renew unless $send_notices_pref ne 'cron' && (!$borrower_preferences || !$borrower_preferences->{transports} || !$borrower_preferences->{transports}->{email} || $borrower_preferences->{'wants_digest'});
+        push @{ $report{ $auto_renew->borrowernumber } }, $auto_renew
+            if $send_notices && !$wants_digest;
     } elsif ( $error eq 'too_many'
         or $error eq 'on_reserve'
         or $error eq 'restriction'
@@ -178,11 +194,11 @@ while ( my $auto_renew = $auto_renews->next ) {
         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' && ( $send_notices && !$wants_digest );    # Do not notify if it's too soon
         }
     }
 
-    if ( $borrower_preferences && $borrower_preferences->{transports} && $borrower_preferences->{transports}->{email} && $borrower_preferences->{'wants_digest'} ) {
+    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 $error eq 'auto_renew';
-- 
2.39.5