Bugzilla – Attachment 120487 Details for
Bug 14723
Additional delivery notes to messages
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 14723: Additional delivery notes to messages
Bug-14723-Additional-delivery-notes-to-messages.patch (text/plain), 11.99 KB, created by
Kyle M Hall (khall)
on 2021-05-05 11:18:00 UTC
(
hide
)
Description:
Bug 14723: Additional delivery notes to messages
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2021-05-05 11:18:00 UTC
Size:
11.99 KB
patch
obsolete
>From a7eca8ccfdf687c233529c60c08a9adf6478c92f Mon Sep 17 00:00:00 2001 >From: Lari Taskula <lari.taskula@hypernova.fi> >Date: Wed, 12 Apr 2017 11:14:44 +0000 >Subject: [PATCH] Bug 14723: Additional delivery notes to messages > >This patch adds additional delivery notes to messages in message queue as there >can be multiple reasons for a delivery to fail. > >Currently in message_queue we are given only two delivery statuses for messages, >"sent" and "failed". When the status becomes failed, we have no idea why it fails. > >This feature can be useful with SMS gateway providers. Many SMS gateways inform >the application the reason of SMS delivery failure. With this feature, this >information can now be stored. As well as for emails, instead of simply logging >failures, we can now store the reason of failure directly into the message row >of message_queue. > >Test plan: > >1. Enable EnhancedMessagingPreferences syspref >2. Find a borrower with notices at members/notices.pl >3. Observe that there is no column for Delivery notes >4. Apply patch and run the given database update >5. Repeat step 1. >6. Observe that there is now a column for Delivery notes > >Sponsored-by: Hypernova Oy > >Signed-off-by: Liz Rea <wizzyrea@gmail.com> > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >--- > C4/Letters.pm | 36 ++++++++++++------- > ...Additional_delivery_notes_to_messages.perl | 8 +++++ > installer/data/mysql/kohastructure.sql | 1 + > .../prog/en/modules/members/notices.tt | 4 ++- > t/db_dependent/Letters.t | 8 ++++- > 5 files changed, 42 insertions(+), 15 deletions(-) > create mode 100644 installer/data/mysql/atomicupdate/Bug-14723_-_Additional_delivery_notes_to_messages.perl > >diff --git a/C4/Letters.pm b/C4/Letters.pm >index 40d2f18a3c..a2c63cb2e7 100644 >--- a/C4/Letters.pm >+++ b/C4/Letters.pm >@@ -963,9 +963,9 @@ 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 ) >+( borrowernumber, subject, content, metadata, letter_code, message_transport_type, status, time_queued, to_address, from_address, reply_address, content_type, delivery_note ) > VALUES >-( ?, ?, ?, ?, ?, ?, ?, NOW(), ?, ?, ?, ? ) >+( ?, ?, ?, ?, ?, ?, ?, NOW(), ?, ?, ?, ?, ? ) > ENDSQL > > my $sth = $dbh->prepare($statement); >@@ -981,6 +981,7 @@ ENDSQL > $params->{'from_address'}, # from_address > $params->{'reply_address'}, # reply_address > $params->{'letter'}->{'content-type'}, # content_type >+ $params->{'delivery_note'} || '', # delivery_note > ); > return $dbh->last_insert_id(undef,undef,'message_queue', undef); > } >@@ -1123,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 >+SELECT message_id, borrowernumber, subject, content, message_transport_type, status, time_queued, updated_on, delivery_note > FROM message_queue > ENDSQL > >@@ -1177,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 >+ 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 > FROM message_queue > WHERE message_id = ? > |, {}, $message_id ); >@@ -1282,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 >+ 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 > FROM message_queue mq > LEFT JOIN borrowers b ON b.borrowernumber = mq.borrowernumber > WHERE status = ? >@@ -1333,7 +1334,8 @@ sub _send_message_by_email { > unless ($patron) { > warn "FAIL: No 'to_address' and INVALID borrowernumber ($message->{borrowernumber})"; > _set_message_status( { message_id => $message->{'message_id'}, >- status => 'failed' } ); >+ status => 'failed', >+ delivery_note => 'Invalid borrowernumber '.$message->{borrowernumber} } ); > return; > } > $to_address = $patron->notice_email_address; >@@ -1341,7 +1343,8 @@ sub _send_message_by_email { > # 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' } ); >+ status => 'failed', >+ delivery_note => 'Unable to find an email address for this borrower' } ); > return; > } > } >@@ -1417,7 +1420,8 @@ sub _send_message_by_email { > _set_message_status( > { > message_id => $message->{'message_id'}, >- status => 'sent' >+ status => 'sent', >+ delivery_note => '' > } > ); > return 1; >@@ -1426,7 +1430,8 @@ sub _send_message_by_email { > _set_message_status( > { > message_id => $message->{'message_id'}, >- status => 'failed' >+ status => 'failed', >+ delivery_note => $Mail::Sendmail::error > } > ); > carp "$_"; >@@ -1477,13 +1482,15 @@ sub _send_message_by_sms { > > unless ( $patron and $patron->smsalertnumber ) { > _set_message_status( { message_id => $message->{'message_id'}, >- status => 'failed' } ); >+ status => 'failed', >+ delivery_note => 'Missing SMS number' } ); > return; > } > > if ( _is_duplicate( $message ) ) { > _set_message_status( { message_id => $message->{'message_id'}, >- status => 'failed' } ); >+ status => 'failed', >+ delivery_note => 'Message is duplicate' } ); > return; > } > >@@ -1491,7 +1498,9 @@ sub _send_message_by_sms { > message => $message->{'content'}, > } ); > _set_message_status( { message_id => $message->{'message_id'}, >- status => ($success ? 'sent' : 'failed') } ); >+ status => ($success ? 'sent' : 'failed'), >+ delivery_note => ($success ? '' : 'No notes from SMS driver') } ); >+ > return $success; > } > >@@ -1515,9 +1524,10 @@ sub _set_message_status { > } > > my $dbh = C4::Context->dbh(); >- my $statement = 'UPDATE message_queue SET status= ? WHERE message_id = ?'; >+ my $statement = 'UPDATE message_queue SET status= ?, delivery_note= ? WHERE message_id = ?'; > my $sth = $dbh->prepare( $statement ); > my $result = $sth->execute( $params->{'status'}, >+ $params->{'delivery_note'} || '', > $params->{'message_id'} ); > return $result; > } >diff --git a/installer/data/mysql/atomicupdate/Bug-14723_-_Additional_delivery_notes_to_messages.perl b/installer/data/mysql/atomicupdate/Bug-14723_-_Additional_delivery_notes_to_messages.perl >new file mode 100644 >index 0000000000..5f03d8200d >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/Bug-14723_-_Additional_delivery_notes_to_messages.perl >@@ -0,0 +1,8 @@ >+$DBversion = 'XXX'; # will be replaced by the RM >+if( CheckVersion( $DBversion ) ) { >+ $dbh->do("ALTER TABLE message_queue ADD delivery_note mediumtext"); >+ >+ # Always end with this (adjust the bug info) >+ SetVersion( $DBversion ); >+ print "Upgrade to $DBversion done (Bug 14723 - Additional delivery notes to messages)\n"; >+} >diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql >index aa39f89043..e41efb653b 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/installer/data/mysql/kohastructure.sql >@@ -3625,6 +3625,7 @@ CREATE TABLE `message_queue` ( > `from_address` longtext COLLATE utf8mb4_unicode_ci DEFAULT NULL, > `reply_address` longtext COLLATE utf8mb4_unicode_ci DEFAULT NULL, > `content_type` mediumtext COLLATE utf8mb4_unicode_ci DEFAULT NULL, >+ `delivery_note` mediumtext COLLATE utf8mb4_unicode_ci DEFAULT NULL, > PRIMARY KEY (`message_id`), > KEY `borrowernumber` (`borrowernumber`), > KEY `message_transport_type` (`message_transport_type`), >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 0b7bdffd15..f14231707a 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt >@@ -51,6 +51,7 @@ > <th>Status</th> > <th>Updated on</th> > <th>Time created</th> >+ <th>Delivery note</th> > </tr> > </thead> > <tbody> >@@ -90,7 +91,8 @@ > </td> > <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> >- </tr> >+ <td>[% QUEUED_MESSAGE.delivery_note | html %]</td> >+ </tr> > [% END %] > </tbody> > </table> >diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t >index 35eb84d32e..5913e7af96 100755 >--- a/t/db_dependent/Letters.t >+++ b/t/db_dependent/Letters.t >@@ -18,7 +18,7 @@ > # along with Koha; if not, see <http://www.gnu.org/licenses>. > > use Modern::Perl; >-use Test::More tests => 84; >+use Test::More tests => 86; > use Test::MockModule; > use Test::Warn; > use Test::Exception; >@@ -146,6 +146,7 @@ is( $messages->[0]->{message_transport_type}, $my_message->{message_transport_ty > is( $messages->[0]->{status}, 'pending', 'EnqueueLetter stores the status pending correctly' ); > isnt( $messages->[0]->{time_queued}, undef, 'Time queued inserted by default in message_queue table' ); > is( $messages->[0]->{updated_on}, $messages->[0]->{time_queued}, 'Time status changed equals time queued when created in message_queue table' ); >+is( $messages->[0]->{delivery_note}, '', 'Delivery note for successful message correctly empty'); > > # Setting time_queued to something else than now > my $yesterday = dt_from_string->subtract( days => 1 ); >@@ -175,6 +176,11 @@ is( $resent, 0, 'The message should not have been resent again' ); > $resent = C4::Letters::ResendMessage(); > is( $resent, undef, 'ResendMessage should return undef if not message_id given' ); > >+# Delivery notes >+is($messages->[0]->{delivery_note}, 'Missing SMS number', >+ 'Delivery note for no smsalertnumber correctly set'); >+ >+ > # GetLetters > my $letters = C4::Letters::GetLetters(); > is( @$letters, 0, 'GetLetters returns the correct number of letters' ); >-- >2.24.3 (Apple Git-128)
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=120487">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 14723
:
42053
|
42074
|
42159
|
42170
|
42227
|
42228
|
42384
|
42453
|
42455
|
42466
|
42505
|
42553
|
42574
|
42668
|
42669
|
44367
|
44413
|
62080
|
62138
|
78313
|
87525
|
87531
|
105776
|
105777
|
116642
|
116643
|
118225
|
120252
|
120309
|
120310
|
120312
|
120314
|
120376
|
120487
|
120488
|
120489
|
120490
|
120491
|
120492
|
120667