Attachment 123474 Details for Bug 28813 – Bug 28813: Rename delivery_note to failure_code

[patch] Bug 28813: Rename delivery_note to failure_code

Bug-28813-Rename-deliverynote-to-failurecode.patch (text/plain), 11.31 KB, created by Martin Renvoize (ashimema) on 2021-08-05 09:25:57 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2021-08-05 09:25:57 UTC

Size: 11.31 KB

patch

obsolete

>From 0e2f18bd067fedc6c9aed8b1a55adeba76993bb6 Mon Sep 17 00:00:00 2001
>From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>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 @@
>         <td data-order="[% QUEUED_MESSAGE.updated_on | html %]">[% QUEUED_MESSAGE.updated_on | $KohaDates  with_hours => 1 %]</td>
>         <td data-order="[% QUEUED_MESSAGE.time_queued | html %]">[% QUEUED_MESSAGE.time_queued | $KohaDates  with_hours => 1 %]</td>
>         <td>
>-        [% 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

Actions: View | Diff | Splinter Review

Attachments on bug 28813: 123471 | 123472 | 123473 | 123474 | 123536 | 123537 | 123538 | 123539 | 123667 | 123668 | 123669 | 123670