From 0a35ec19e1a2ce6b522e327315f37530b9a66865 Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Date: Mon, 18 Sep 2023 13:31:59 +0100
Subject: [PATCH] Bug 12532: (QA follow-up) More cleanup

Minor QA cleanup, perltidy, remove Data::Dumper and clarify warn

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
---
 C4/Letters.pm                                    | 16 +++++++---------
 .../bug_12532-RedirectGuaranteeEmail_syspref.pl  | 15 ++++++++++-----
 t/db_dependent/Letters.t                         |  2 +-
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/C4/Letters.pm b/C4/Letters.pm
index dd48978119..f1a5adfca9 100644
--- a/C4/Letters.pm
+++ b/C4/Letters.pm
@@ -40,7 +40,6 @@ use Koha::Patrons;
 use Koha::SMS::Providers;
 use Koha::SMTP::Servers;
 use Koha::Subscriptions;
-use Data::Dumper;
 
 use constant SERIALIZED_EMAIL_CONTENT_TYPE => 'message/rfc822';
 
@@ -1324,7 +1323,7 @@ sub _send_message_by_email {
     my @guarantor_address;
     my $count_guarantor_address;
     if (C4::Context->preference('RedirectGuaranteeEmail') eq 'yes' && $patron) {
-        #Get guanrantor adresses
+        #Get guanrantor addresses
         my $guarantor_relationships = $patron->guarantor_relationships;
         my @guarantors              = $guarantor_relationships->guarantors->as_list;
         foreach my $guarantor (@guarantors) {
@@ -1334,7 +1333,7 @@ sub _send_message_by_email {
         $count_guarantor_address = scalar @guarantor_address;
     }
     unless ($to_address) {
-        if (!$patron && !$count_guarantor_address) {
+        if (!$patron) {
             warn "FAIL: No 'to_address' and INVALID borrowernumber ($message->{borrowernumber})";
             _set_message_status(
                 {
@@ -1349,8 +1348,7 @@ sub _send_message_by_email {
             $to_address = $patron->notice_email_address;
         }
         if (!$to_address && !$count_guarantor_address) {
-            # warn "FAIL: No 'to_address' and no email for " . ($member->{surname} ||'') . ", borrowernumber ($message->{borrowernumber})";
-            # warning too verbose for this more common case?
+            warn "FAIL: No 'to_address', email address or guantors email address for borrowernumber ($message->{borrowernumber})";
             _set_message_status(
                 {
                     message_id   => $message->{'message_id'},
@@ -1360,7 +1358,7 @@ sub _send_message_by_email {
             );
             return;
         }
-        if (!$to_address && $count_guarantor_address) {
+        if ( !$to_address && $count_guarantor_address ) {
             $to_address = shift @guarantor_address;
         }
     }
@@ -1494,9 +1492,9 @@ sub _send_message_by_email {
     $smtp_transports->{ $smtp_server->id // 'default' } ||= $smtp_server->transport;
     my $smtp_transport = $smtp_transports->{ $smtp_server->id // 'default' };
 
-    _update_message_from_address($message->{'message_id'},$email->email->header('From') )
-      if !$message->{from_address}
-      || $message->{from_address} ne $email->email->header('From');
+    _update_message_from_address( $message->{'message_id'}, $email->email->header('From') )
+        if !$message->{from_address}
+        || $message->{from_address} ne $email->email->header('From');
 
     try {
         $email->send_or_die({ transport => $smtp_transport });
diff --git a/installer/data/mysql/atomicupdate/bug_12532-RedirectGuaranteeEmail_syspref.pl b/installer/data/mysql/atomicupdate/bug_12532-RedirectGuaranteeEmail_syspref.pl
index 270c34b3ad..36d29a5a5d 100755
--- a/installer/data/mysql/atomicupdate/bug_12532-RedirectGuaranteeEmail_syspref.pl
+++ b/installer/data/mysql/atomicupdate/bug_12532-RedirectGuaranteeEmail_syspref.pl
@@ -1,14 +1,19 @@
 use Modern::Perl;
 
 return {
-    bug_number => "12532",
+    bug_number  => "12532",
     description => "Add new system preference RedirectGuaranteeEmail",
-    up => sub {
+    up          => sub {
         my ($args) = @_;
-        my ($dbh, $out) = @$args{qw(dbh out)};
+        my ( $dbh, $out ) = @$args{qw(dbh out)};
 
-        $dbh->do(q{INSERT IGNORE INTO systempreferences (variable,value,options,explanation,type) VALUES ('RedirectGuaranteeEmail', '0', 'Enable the ability to redirect guarantee email messages to guarantor.', NULL, 'YesNo') });
+        $dbh->do(
+            q{
+            INSERT IGNORE INTO systempreferences (variable,value,options,explanation,type)
+            VALUES ('RedirectGuaranteeEmail', '0', 'Enable the ability to redirect guarantee email messages to guarantor.', NULL, 'YesNo')
+        }
+        );
 
         say $out "Added system preference 'RedirectGuaranteeEmail'";
     },
-};
\ No newline at end of file
+};
diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t
index 3373e9b6b4..0f5e798a22 100755
--- a/t/db_dependent/Letters.t
+++ b/t/db_dependent/Letters.t
@@ -894,7 +894,7 @@ subtest 'Test SMS handling in SendQueuedMessages' => sub {
     t::lib::Mocks::mock_preference( 'SMSSendDriver', 'Email' );
     t::lib::Mocks::mock_preference('EmailSMSSendDriverFromAddress', '');
 
-    t::lib::Mocks::mock_preference('RedirectGuaranteeEmail', '0');
+    t::lib::Mocks::mock_preference( 'RedirectGuaranteeEmail', '0' );
     my $patron = Koha::Patrons->find($borrowernumber);
     $dbh->do(q|
         INSERT INTO message_queue(borrowernumber, subject, content, message_transport_type, status, letter_code)
-- 
2.39.5