From 0e2f18bd067fedc6c9aed8b1a55adeba76993bb6 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Thu, 5 Aug 2021 10:17:29 +0100 Subject: [PATCH] Bug 28813: Rename delivery_note to failure_code We now use the codes from the half implimented error_code in place of the plain text that was being added to the delivery_note field. As part of that we rename the field to failure_code to clarify it's intended content. Test plan 1/ Confirm t/db_dependent/Letters.t passes --- C4/Letters.pm | 99 ++++++++++++------- .../prog/en/modules/members/notices.tt | 14 +-- 2 files changed, 72 insertions(+), 41 deletions(-) diff --git a/C4/Letters.pm b/C4/Letters.pm index 177a4f51f6..a06f006ee1 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -963,7 +963,7 @@ sub EnqueueLetter { my $dbh = C4::Context->dbh(); my $statement = << 'ENDSQL'; INSERT INTO message_queue -( borrowernumber, subject, content, metadata, letter_code, message_transport_type, status, time_queued, to_address, from_address, reply_address, content_type, delivery_note ) +( borrowernumber, subject, content, metadata, letter_code, message_transport_type, status, time_queued, to_address, from_address, reply_address, content_type, failure_code ) VALUES ( ?, ?, ?, ?, ?, ?, ?, CAST(NOW() AS DATETIME), ?, ?, ?, ?, ? ) ENDSQL @@ -981,7 +981,7 @@ ENDSQL $params->{'from_address'}, # from_address $params->{'reply_address'}, # reply_address $params->{'letter'}->{'content-type'}, # content_type - $params->{'delivery_note'} || '', # delivery_note + $params->{'failure_code'} || '', # failure_code ); return $dbh->last_insert_id(undef,undef,'message_queue', undef); } @@ -1124,7 +1124,7 @@ sub GetQueuedMessages { my $dbh = C4::Context->dbh(); my $statement = << 'ENDSQL'; -SELECT message_id, borrowernumber, subject, content, message_transport_type, status, time_queued, updated_on, delivery_note +SELECT message_id, borrowernumber, subject, content, message_transport_type, status, time_queued, updated_on, failure_code FROM message_queue ENDSQL @@ -1178,7 +1178,7 @@ sub GetMessage { return unless $message_id; my $dbh = C4::Context->dbh; return $dbh->selectrow_hashref(q| - SELECT message_id, borrowernumber, subject, content, metadata, letter_code, message_transport_type, status, time_queued, updated_on, to_address, from_address, reply_address, content_type, delivery_note + SELECT message_id, borrowernumber, subject, content, metadata, letter_code, message_transport_type, status, time_queued, updated_on, to_address, from_address, reply_address, content_type, failure_code FROM message_queue WHERE message_id = ? |, {}, $message_id ); @@ -1283,7 +1283,7 @@ sub _get_unsent_messages { my $dbh = C4::Context->dbh(); my $statement = qq{ - SELECT mq.message_id, mq.borrowernumber, mq.subject, mq.content, mq.message_transport_type, mq.status, mq.time_queued, mq.from_address, mq.reply_address, mq.to_address, mq.content_type, b.branchcode, mq.letter_code, mq.delivery_note + SELECT mq.message_id, mq.borrowernumber, mq.subject, mq.content, mq.message_transport_type, mq.status, mq.time_queued, mq.from_address, mq.reply_address, mq.to_address, mq.content_type, b.branchcode, mq.letter_code, mq.failure_code FROM message_queue mq LEFT JOIN borrowers b ON b.borrowernumber = mq.borrowernumber WHERE status = ? @@ -1331,20 +1331,26 @@ sub _send_message_by_email { unless ($to_address) { unless ($patron) { warn "FAIL: No 'to_address' and INVALID borrowernumber ($message->{borrowernumber})"; - _set_message_status( { message_id => $message->{'message_id'}, - status => 'failed', - delivery_note => 'Invalid borrowernumber '.$message->{borrowernumber}, - error_code => 'INVALID_BORNUMBER' } ); + _set_message_status( + { + message_id => $message->{'message_id'}, + status => 'failed', + failure_code => 'INVALID_BORNUMBER' + } + ); return; } $to_address = $patron->notice_email_address; unless ($to_address) { # warn "FAIL: No 'to_address' and no email for " . ($member->{surname} ||'') . ", borrowernumber ($message->{borrowernumber})"; # warning too verbose for this more common case? - _set_message_status( { message_id => $message->{'message_id'}, - status => 'failed', - delivery_note => 'Unable to find an email address for this borrower', - error_code => 'NO_EMAIL' } ); + _set_message_status( + { + message_id => $message->{'message_id'}, + status => 'failed', + failure_code => 'NO_EMAIL' + } + ); return; } } @@ -1374,11 +1380,13 @@ sub _send_message_by_email { || $branch_email || C4::Context->preference('KohaAdminEmailAddress'); if( !$from_address ) { - _set_message_status({ - message_id => $message->{'message_id'}, - status => 'failed', - delivery_note => 'No from address', - }); + _set_message_status( + { + message_id => $message->{'message_id'}, + status => 'failed', + failure_code => 'NO_FROM', + } + ); return; }; my $email = Koha::Email->create( @@ -1435,7 +1443,7 @@ sub _send_message_by_email { { message_id => $message->{'message_id'}, status => 'sent', - delivery_note => '' + failure_code => '' } ); return 1; @@ -1445,10 +1453,11 @@ sub _send_message_by_email { { message_id => $message->{'message_id'}, status => 'failed', - delivery_note => $Mail::Sendmail::error + failure_code => 'SENDMAIL' } ); carp "$_"; + carp "$Mail::Sendmail::error"; return; }; } @@ -1497,26 +1506,46 @@ sub _send_message_by_sms { unless ( $patron and $patron->smsalertnumber ) { _set_message_status( { message_id => $message->{'message_id'}, status => 'failed', - delivery_note => 'Missing SMS number', - error_code => 'MISSING_SMS' } ); + failure_code => 'MISSING_SMS' } ); return; } if ( _is_duplicate( $message ) ) { - _set_message_status( { message_id => $message->{'message_id'}, - status => 'failed', - delivery_note => 'Message is duplicate', - error_code => 'DUPLICATE_MESSAGE' } ); + _set_message_status( + { + message_id => $message->{'message_id'}, + status => 'failed', + failure_code => 'DUPLICATE_MESSAGE' + } + ); return; } - my $success = C4::SMS->send_sms( { destination => $patron->smsalertnumber, - message => $message->{'content'}, - } ); - _set_message_status( { message_id => $message->{'message_id'}, - status => ($success ? 'sent' : 'failed'), - delivery_note => ($success ? '' : 'No notes from SMS driver'), - error_code => 'NO_NOTES' } ); + my $success = C4::SMS->send_sms( + { + destination => $patron->smsalertnumber, + message => $message->{'content'}, + } + ); + + if ($success) { + _set_message_status( + { + message_id => $message->{'message_id'}, + status => 'sent', + failure_code => '' + } + ); + } + else { + _set_message_status( + { + message_id => $message->{'message_id'}, + status => 'failed', + failure_code => 'NO_NOTES' + } + ); + } return $success; } @@ -1541,10 +1570,10 @@ sub _set_message_status { } my $dbh = C4::Context->dbh(); - my $statement = 'UPDATE message_queue SET status= ?, delivery_note= ? WHERE message_id = ?'; + my $statement = 'UPDATE message_queue SET status= ?, failure_code= ? WHERE message_id = ?'; my $sth = $dbh->prepare( $statement ); my $result = $sth->execute( $params->{'status'}, - $params->{'delivery_note'} || '', + $params->{'failure_code'} || '', $params->{'message_id'} ); return $result; } 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 79b6d6713b..31bd2dbb6f 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt @@ -92,12 +92,14 @@ [% QUEUED_MESSAGE.updated_on | $KohaDates with_hours => 1 %] [% QUEUED_MESSAGE.time_queued | $KohaDates with_hours => 1 %] - [% IF ( QUEUED_MESSAGE.error_code ) %] - [% IF ( QUEUED_MESSAGE.error_code == "INVALID_BORNUMBER" ) %]Invalid borrowernumber [% borrowernumber | html %] - [% ELSIF ( QUEUED_MESSAGE.error_code == 'NO_EMAIL' ) %]Unable to find an email address for this borrower - [% ELSIF ( QUEUED_MESSAGE.error_code == 'MISSING_SMS' ) %]Missing SMS number - [% ELSIF ( QUEUED_MESSAGE.error_code == 'DUPLICATE_MESSAGE' ) %]Message is duplicate - [% ELSIF ( QUEUED_MESSAGE.error_code == 'NO_NOTES' ) %]No notes from SMS driver + [% 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 == '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 + [% ELSIF ( QUEUED_MESSAGE.failure_code == 'NO_NOTES' ) %]No notes from SMS driver + [% ELSIF ( QUEUED_MESSAGE.failure_code == 'SENDMAIL' ) %]Unhandled email failure, check the logs for further details [% ELSE %]Error occurred while sending email. [% END %] [% END %] -- 2.20.1