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

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/C4/Letters.pm b/C4/Letters.pm
index 177a4f5..a06f006 100644 (file)
--- 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 79b6d67..31bd2db 100644 (file)
--- 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 %]