Attachment 155832 Details for Bug 12532 – Bug 12532: (QA follow-up) More cleanup

[patch] Bug 12532: (QA follow-up) More cleanup

Bug-12532-QA-follow-up-More-cleanup.patch (text/plain), 5.35 KB, created by Martin Renvoize (ashimema) on 2023-09-18 15:37:51 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2023-09-18 15:37:51 UTC

Size: 5.35 KB

patch

obsolete

>From 1f9215abcfbc6231d9111ad75e78fb9473d28924 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>
>---
> 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 dd489781193..f1a5adfca96 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 270c34b3ada..36d29a5a5d7 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 59b00edbf5b..c1593f1806d 100755
>--- a/t/db_dependent/Letters.t
>+++ b/t/db_dependent/Letters.t
>@@ -869,7 +869,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.41.0

Actions: View | Diff | Splinter Review

Attachments on bug 12532: 29510 | 30747 | 35313 | 36752 | 39787 | 39925 | 48645 | 53649 | 53689 | 53690 | 53699 | 53700 | 55985 | 56052 | 56053 | 59445 | 59446 | 59447 | 63197 | 64802 | 64824 | 67115 | 68340 | 70865 | 72064 | 72617 | 75099 | 77606 | 80313 | 85134 | 85135 | 87815 | 87816 | 88208 | 88209 | 88210 | 88211 | 88212 | 88213 | 88295 | 88296 | 114455 | 114456 | 114457 | 114458 | 114459 | 114460 | 114461 | 131091 | 131092 | 131093 | 131094 | 149233 | 150071 | 150208 | 150217 | 150240 | 150256 | 151028 | 151029 | 154854 | 154855 | 154856 | 154857 | 155036 | 155037 | 155828 | 155829 | 155830 | 155831 | 155832 | 155833 | 155834 | 155835 | 155836 | 155837 | 155873 | 155874 | 155875 | 155876 | 155877 | 155878 | 155879 | 155880 | 155881 | 155882 | 155883 | 155886 | 155934 | 155935 | 155936 | 155937 | 155938 | 155939 | 155940 | 155941 | 155942 | 155943 | 155944 | 155945 | 155946