Bugzilla – Attachment 124888 Details for
Bug 28803
process_message_queue.pl dies if any messsages in the message queue contain an invalid to_address
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 28803: (follow-up) Error details improvement
Bug-28803-follow-up-Error-details-improvement.patch (text/plain), 6.74 KB, created by
Martin Renvoize (ashimema)
on 2021-09-15 13:15:53 UTC
(
hide
)
Description:
Bug 28803: (follow-up) Error details improvement
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2021-09-15 13:15:53 UTC
Size:
6.74 KB
patch
obsolete
>From cb68b3bced03cf9c2cf04217c0ba19b12dada254 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Mon, 13 Sep 2021 10:18:46 +0100 >Subject: [PATCH] Bug 28803: (follow-up) Error details improvement > >This patch updates the exceptions thrown by Koha::Email to include the >parameter that failed email validation and then updates the failure code >to include this parameter and finally display this field in the template. >--- > C4/Letters.pm | 24 +++++++++++++------ > Koha/Email.pm | 18 +++++++++----- > .../prog/en/modules/members/notices.tt | 2 +- > t/db_dependent/Letters.t | 10 ++++---- > 4 files changed, 35 insertions(+), 19 deletions(-) > >diff --git a/C4/Letters.pm b/C4/Letters.pm >index 07a47380b1..a02c0d51a9 100644 >--- a/C4/Letters.pm >+++ b/C4/Letters.pm >@@ -1406,13 +1406,23 @@ sub _send_message_by_email { > ); > } > catch { >- _set_message_status( >- { >- message_id => $message->{'message_id'}, >- status => 'failed', >- failure_code => 'INVALID_EMAIL' >- } >- ); >+ if ( ref($_) eq 'Koha::Exceptions::BadParameter' ) { >+ _set_message_status( >+ { >+ message_id => $message->{'message_id'}, >+ status => 'failed', >+ failure_code => "INVALID_EMAIL:".$_->parameter >+ } >+ ); >+ } else { >+ _set_message_status( >+ { >+ message_id => $message->{'message_id'}, >+ status => 'failed', >+ failure_code => 'UNKNOWN_ERROR' >+ } >+ ); >+ } > return 0; > }; > return unless $email; >diff --git a/Koha/Email.pm b/Koha/Email.pm >index 4806126217..cac63763ab 100644 >--- a/Koha/Email.pm >+++ b/Koha/Email.pm >@@ -77,8 +77,10 @@ sub create { > > my $args = {}; > $args->{from} = $params->{from} || C4::Context->preference('KohaAdminEmailAddress'); >- Koha::Exceptions::BadParameter->throw("Invalid 'from' parameter: ".$args->{from}) >- unless Koha::Email->is_valid($args->{from}); # from is mandatory >+ Koha::Exceptions::BadParameter->throw( >+ error => "Invalid 'from' parameter: " . $args->{from}, >+ parameter => 'from' >+ ) unless Koha::Email->is_valid( $args->{from} ); # from is mandatory > > $args->{subject} = $params->{subject} // ''; > >@@ -89,8 +91,10 @@ sub create { > $args->{to} = $params->{to}; > } > >- Koha::Exceptions::BadParameter->throw("Invalid 'to' parameter: ".$args->{to}) >- unless Koha::Email->is_valid($args->{to}); # to is mandatory >+ Koha::Exceptions::BadParameter->throw( >+ error => "Invalid 'to' parameter: " . $args->{to}, >+ parameter => 'to' >+ ) unless Koha::Email->is_valid( $args->{to} ); # to is mandatory > > my $addresses = {}; > $addresses->{reply_to} = $params->{reply_to}; >@@ -110,9 +114,11 @@ sub create { > > foreach my $address ( keys %{$addresses} ) { > Koha::Exceptions::BadParameter->throw( >- "Invalid '$address' parameter: " . $addresses->{$address} ) >+ error => "Invalid '$address' parameter: " . $addresses->{$address}, >+ parameter => $address >+ ) > if $addresses->{$address} >- and !Koha::Email->is_valid($addresses->{$address}); >+ and !Koha::Email->is_valid( $addresses->{$address} ); > } > > $args->{cc} = $addresses->{cc} >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt >index 5503654b66..f9d504226e 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt >@@ -95,7 +95,7 @@ > [% IF ( QUEUED_MESSAGE.failure_code ) %] > [% IF ( QUEUED_MESSAGE.failure_code == "INVALID_BORNUMBER" ) %]Invalid borrowernumber [% borrowernumber | html %] > [% ELSIF ( QUEUED_MESSAGE.failure_code == 'NO_EMAIL' ) %]Unable to find an email address for this borrower >- [% ELSIF ( QUEUED_MESSAGE.failure_code == "INVALID_EMAIL" ) %]Invalid email address found [% borrowernumber | html %] >+ [% ELSIF (matches = QUEUED_MESSAGE.failure_code.match('INVALID_EMAIL:(\w+)') ) %]Invalid [% matches.0 | html %] email address found [% borrowernumber | html %] > [% ELSIF ( QUEUED_MESSAGE.failure_code == 'NO_FROM' ) %]Missing from email address > [% ELSIF ( QUEUED_MESSAGE.failure_code == 'MISSING_SMS' ) %]Missing SMS number > [% ELSIF ( QUEUED_MESSAGE.failure_code == 'DUPLICATE_MESSAGE' ) %]Message is duplicate >diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t >index f6621d2bb6..9a37118503 100755 >--- a/t/db_dependent/Letters.t >+++ b/t/db_dependent/Letters.t >@@ -949,16 +949,16 @@ subtest 'Test message_id parameter for SendQueuedMessages' => sub { > 'borrowernumber' => $borrowernumber, > 'to_address' => 'to@example.org', > 'message_transport_type' => 'email', >- 'from_address' => 'root@localhost.' # invalid KohaAdminEmailAddress >+ 'from_address' => '@example.com' # invalid from_address > }; > my $message_id = C4::Letters::EnqueueLetter($my_message); > my $processed = C4::Letters::SendQueuedMessages( { message_id => $message_id } ); > is( $processed, 1, 'Processed 1 message when one message_id passed' ); > my $message_1 = C4::Letters::GetMessage($message_id); >- is( $message_1->{status}, 'failed', 'Invalid KohaAdminEmailAddress => status failed' ); >- is( $message_1->{failure_code}, 'INVALID_EMAIL', 'Failure code set correctly for invalid email parameter'); >+ is( $message_1->{status}, 'failed', 'Invalid from_address => status failed' ); >+ is( $message_1->{failure_code}, 'INVALID_EMAIL:from', 'Failure code set correctly for invalid email parameter'); > >- $my_message->{from_address} = 'root@example.org'; # valid KohaAdminEmailAddress >+ $my_message->{from_address} = 'root@example.org'; # valid from_address > $message_id = C4::Letters::EnqueueLetter($my_message); > warning_like { C4::Letters::SendQueuedMessages( { message_id => $message_id } ); } > qr|Fake send_or_die|, >@@ -966,5 +966,5 @@ subtest 'Test message_id parameter for SendQueuedMessages' => sub { > $message_1 = C4::Letters::GetMessage($message_1->{message_id}); > my $message_2 = C4::Letters::GetMessage($message_id); > is( $message_1->{status}, 'failed', 'Message 1 status is unchanged' ); >- is( $message_2->{status}, 'sent', 'Valid KohaAdminEmailAddress => status sent' ); >+ is( $message_2->{status}, 'sent', 'Valid from_address => status sent' ); > }; >-- >2.20.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 28803
:
123469
|
123470
|
123475
|
123476
|
124000
|
124001
|
124131
|
124132
|
124746
|
124829
|
124878
|
124879
|
124880
|
124886
|
124887
|
124888
|
124900
|
124901
|
124902
|
125114
|
125115
|
125343
|
125344
|
125345
|
125346