Attachment 172338 Details for Bug 33221 – Bug 33221: (QA follow-up) Tidy code

[patch] Bug 33221: (QA follow-up) Tidy code

Bug-33221-QA-follow-up-Tidy-code.patch (text/plain), 7.95 KB, created by Kyle M Hall (khall) on 2024-10-03 11:36:19 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2024-10-03 11:36:19 UTC

Size: 7.95 KB

patch

obsolete

>From b175bb37f1021675af199c88bb92d9d5b6a5c5e4 Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>Date: Thu, 3 Oct 2024 07:18:44 -0400
>Subject: [PATCH] Bug 33221: (QA follow-up) Tidy code
>
>Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
>---
> Koha/Patron.pm                   | 45 ++++++++++++++++++--------------
> members/memberentry.pl           |  6 +++--
> opac/opac-memberentry.pl         |  4 ++-
> opac/opac-registration-verify.pl |  4 ++-
> 4 files changed, 35 insertions(+), 24 deletions(-)
>
>diff --git a/Koha/Patron.pm b/Koha/Patron.pm
>index 556da6f049e..c2268082dd9 100644
>--- a/Koha/Patron.pm
>+++ b/Koha/Patron.pm
>@@ -2567,33 +2567,36 @@ Returns a hashref with the following keys:
> sub queue_notice {
>     my ( $self, $params ) = @_;
>     my $letter_params = $params->{letter_params};
>-    my $test_mode = $params->{test_mode};
>+    my $test_mode     = $params->{test_mode};
> 
>     return unless $letter_params;
>-    return unless exists $params->{message_name} xor $params->{message_transports}; # We only want one of these
>+    return unless exists $params->{message_name} xor $params->{message_transports};    # We only want one of these
> 
>-    my $library = Koha::Libraries->find( $letter_params->{branchcode} );
>+    my $library            = Koha::Libraries->find( $letter_params->{branchcode} );
>     my $from_email_address = $library->from_email_address;
> 
>     my @message_transports;
>     my $letter_code;
>     $letter_code = $letter_params->{letter_code};
>-    if( $params->{message_name} ){
>-        my $messaging_prefs = C4::Members::Messaging::GetMessagingPreferences( {
>+    if ( $params->{message_name} ) {
>+        my $messaging_prefs = C4::Members::Messaging::GetMessagingPreferences(
>+            {
>                 borrowernumber => $letter_params->{borrowernumber},
>-                message_name => $params->{message_name}
>-        } );
>+                message_name   => $params->{message_name}
>+            }
>+        );
>         @message_transports = ( keys %{ $messaging_prefs->{transports} } );
>-        $letter_code = $messaging_prefs->{transports}->{$message_transports[0]} unless $letter_code;
>+        $letter_code        = $messaging_prefs->{transports}->{ $message_transports[0] } unless $letter_code;
>     } else {
>-        @message_transports = @{$params->{message_transports}};
>+        @message_transports = @{ $params->{message_transports} };
>     }
>     return unless defined $letter_code;
>     $letter_params->{letter_code} = $letter_code;
>     my $print_sent = 0;
>     my %return;
>-    foreach my $mtt (@message_transports){
>-        next if ($mtt eq 'itiva' and C4::Context->preference('TalkingTechItivaPhoneNotification') );
>+    foreach my $mtt (@message_transports) {
>+        next if ( $mtt eq 'itiva' and C4::Context->preference('TalkingTechItivaPhoneNotification') );
>+
>         # Notice is handled by TalkingTech_itiva_outbound.pl
>         if (   ( $mtt eq 'email' and not $self->notice_email_address )
>             or ( $mtt eq 'sms'   and not $self->smsalertnumber )
>@@ -2604,17 +2607,19 @@ sub queue_notice {
>         }
>         next if $mtt eq 'print' && $print_sent;
>         $letter_params->{message_transport_type} = $mtt;
>-        my $letter = C4::Letters::GetPreparedLetter( %$letter_params );
>+        my $letter = C4::Letters::GetPreparedLetter(%$letter_params);
>         unless ($test_mode) {
>-            my $message_id = C4::Letters::EnqueueLetter({
>-                letter => $letter,
>-                borrowernumber => $self->borrowernumber,
>-                from_address   => $from_email_address,
>-                message_transport_type => $mtt
>-            });
>-            push @{$return{message_ids}}, $message_id if $message_id;
>+            my $message_id = C4::Letters::EnqueueLetter(
>+                {
>+                    letter                 => $letter,
>+                    borrowernumber         => $self->borrowernumber,
>+                    from_address           => $from_email_address,
>+                    message_transport_type => $mtt
>+                }
>+            );
>+            push @{ $return{message_ids} }, $message_id if $message_id;
>         }
>-        push @{$return{sent}}, $mtt;
>+        push @{ $return{sent} }, $mtt;
>         $print_sent = 1 if $mtt eq 'print';
>     }
>     return \%return;
>diff --git a/members/memberentry.pl b/members/memberentry.pl
>index 1da4c533679..80502180bb8 100755
>--- a/members/memberentry.pl
>+++ b/members/memberentry.pl
>@@ -447,9 +447,11 @@ if ((!$nok) and $nodouble and ($op eq 'cud-insert' or $op eq 'cud-save')){
> 
>         # If 'AutoEmailNewUser' syspref is on, email user their account details from the 'notice' that matches the user's branchcode.
>         if ( $patron && C4::Context->preference("AutoEmailNewUser") ) {
>+
>             #look for defined primary email address, if blank - attempt to use borr.email and borr.emailpro instead
>             my $emailaddr = $patron->notice_email_address;
>-            # if we manage to find a valid email address, send notice 
>+
>+            # if we manage to find a valid email address, send notice
>             if ($emailaddr) {
>                 eval {
>                     my $result = $patron->queue_notice(
>@@ -465,7 +467,7 @@ if ((!$nok) and $nodouble and ($op eq 'cud-insert' or $op eq 'cud-save')){
>                                 },
>                                 want_librarian => 1,
>                             },
>-                            message_transports => ['email', 'sms'],
>+                            message_transports => [ 'email', 'sms' ],
>                         }
>                     );
>                     foreach my $message_id ( @{ $result->{message_ids} } ) {
>diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl
>index e0a563ddfa3..7abb21049fc 100755
>--- a/opac/opac-memberentry.pl
>+++ b/opac/opac-memberentry.pl
>@@ -260,8 +260,10 @@ if ( $op eq 'cud-create' ) {
> 
>                 # If 'AutoEmailNewUser' syspref is on, email user their account details from the 'notice' that matches the user's branchcode.
>                 if ( C4::Context->preference("AutoEmailNewUser") ) {
>+
>                     #look for defined primary email address, if blank - attempt to use borr.email and borr.emailpro instead
>                     my $emailaddr = $patron->notice_email_address;
>+
>                     # if we manage to find a valid email address, send notice
>                     if ($emailaddr) {
>                         eval {
>@@ -278,7 +280,7 @@ if ( $op eq 'cud-create' ) {
>                                         },
>                                         want_librarian => 1,
>                                     },
>-                                    message_transports => ['email', 'sms'],
>+                                    message_transports => [ 'email', 'sms' ],
>                                 }
>                             );
>                             foreach my $message_id ( @{ $result->{message_ids} } ) {
>diff --git a/opac/opac-registration-verify.pl b/opac/opac-registration-verify.pl
>index 6f5eab1201f..da60a528b3e 100755
>--- a/opac/opac-registration-verify.pl
>+++ b/opac/opac-registration-verify.pl
>@@ -121,8 +121,10 @@ elsif ( $rego_found
> 
>         # If 'AutoEmailNewUser' syspref is on, email user their account details from the 'notice' that matches the user's branchcode.
>         if ( C4::Context->preference("AutoEmailNewUser") ) {
>+
>             # Look up correct email address taking EmailFieldPrimary into account
>             my $emailaddr = $patron->notice_email_address;
>+
>             # if we manage to find a valid email address, send notice
>             if ($emailaddr) {
>                 eval {
>@@ -139,7 +141,7 @@ elsif ( $rego_found
>                                 },
>                                 want_librarian => 1,
>                             },
>-                            message_transports => ['email', 'sms'],
>+                            message_transports => [ 'email', 'sms' ],
>                         }
>                     );
>                     foreach my $message_id ( @{ $result->{message_ids} } ) {
>-- 
>2.39.2

Actions: View | Diff | Splinter Review

Attachments on bug 33221: 148152 | 148251 | 171712 | 172123 | 172126 | 172336 | 172337 | 172338 | 172339