Bugzilla – Attachment 78313 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.71 KB, created by
Nick Clemens (kidclamp)
on 2018-08-30 16:29:50 UTC
(
hide
)
Description:
Bug 14723: Additional delivery notes to messages
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2018-08-30 16:29:50 UTC
Size:
11.71 KB
patch
obsolete
>From 234b175a08d89f7dbfbd060f2b81124880dff8d6 Mon Sep 17 00:00:00 2001 >From: Lari Taskula <lari.taskula@jns.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 >--- > C4/Letters.pm | 36 ++++++++++++++-------- > ...23_-_Additional_delivery_notes_to_messages.perl | 8 +++++ > installer/data/mysql/kohastructure.sql | 1 + > .../prog/en/modules/members/notices.tt | 6 +++- > t/db_dependent/Letters.t | 9 ++++-- > 5 files changed, 44 insertions(+), 16 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 52db4d0..968378a 100644 >--- a/C4/Letters.pm >+++ b/C4/Letters.pm >@@ -906,9 +906,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, content_type ) >+( borrowernumber, subject, content, metadata, letter_code, message_transport_type, status, time_queued, to_address, from_address, content_type, delivery_note ) > VALUES >-( ?, ?, ?, ?, ?, ?, ?, NOW(), ?, ?, ? ) >+( ?, ?, ?, ?, ?, ?, ?, NOW(), ?, ?, ?, ? ) > ENDSQL > > my $sth = $dbh->prepare($statement); >@@ -923,6 +923,7 @@ ENDSQL > $params->{'to_address'}, # to_address > $params->{'from_address'}, # from_address > $params->{'letter'}->{'content-type'}, # content_type >+ $params->{'delivery_note'} || '', # delivery_note > ); > return $dbh->last_insert_id(undef,undef,'message_queue', undef); > } >@@ -1056,7 +1057,7 @@ sub GetQueuedMessages { > > my $dbh = C4::Context->dbh(); > my $statement = << 'ENDSQL'; >-SELECT message_id, borrowernumber, subject, content, message_transport_type, status, time_queued >+SELECT message_id, borrowernumber, subject, content, message_transport_type, status, time_queued, delivery_note > FROM message_queue > ENDSQL > >@@ -1110,7 +1111,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, to_address, from_address, content_type >+ SELECT message_id, borrowernumber, subject, content, metadata, letter_code, message_transport_type, status, time_queued, to_address, from_address, content_type, delivery_note > FROM message_queue > WHERE message_id = ? > |, {}, $message_id ); >@@ -1214,7 +1215,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.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.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 = ? >@@ -1261,7 +1262,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; >@@ -1269,7 +1271,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; > } > } >@@ -1311,11 +1314,13 @@ sub _send_message_by_email { > > if ( Mail::Sendmail::sendmail( %sendmail_params ) ) { > _set_message_status( { message_id => $message->{'message_id'}, >- status => 'sent' } ); >+ status => 'sent', >+ delivery_note => '' } ); > return 1; > } else { > _set_message_status( { message_id => $message->{'message_id'}, >- status => 'failed' } ); >+ status => 'failed', >+ delivery_note => $Mail::Sendmail::error } ); > carp $Mail::Sendmail::error; > return; > } >@@ -1364,13 +1369,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; > } > >@@ -1378,7 +1385,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; > } > >@@ -1396,9 +1405,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 0000000..3b1cbe1 >--- /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 TEXT"); >+ >+ # 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 45fd117..be59ade 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/installer/data/mysql/kohastructure.sql >@@ -2503,6 +2503,7 @@ CREATE TABLE `message_queue` ( > `to_address` LONGTEXT, > `from_address` LONGTEXT, > `content_type` MEDIUMTEXT, >+ `delivery_note` MEDIUMTEXT, > PRIMARY KEY `message_id` (`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 a26d5f0..4bb5bb6 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt >@@ -36,6 +36,7 @@ > <th>Type</th> > <th>Status</th> > <th>Time</th> >+ <th>Delivery note</th> > </tr> > </thead> > <tbody> >@@ -74,6 +75,9 @@ > [% END %] > </td> > <td><span title="[% QUEUED_MESSAGE.time_queued | html %]">[% QUEUED_MESSAGE.time_queued | $KohaDates with_hours => 1 | html %]</span></td> >+ <td> >+ [% QUEUED_MESSAGE.delivery_note | html %] >+ </td> > </tr> > [% END %] > </tbody> >@@ -97,7 +101,7 @@ > $(document).ready(function() { > $("#noticestable").dataTable($.extend(true, {}, dataTablesDefaults, { > "aaSorting": [[ 3, "desc" ]], >- "aoColumns": [ null,null,null,{ "sType": "title-string" } ], >+ "aoColumns": [ null,null,null,{ "sType": "title-string" }, null ], > "sPaginationType": "four_button" > })); > >diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t >index 344473c..ec3b9c1 100644 >--- 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 => 64; >+use Test::More tests => 66; > use Test::MockModule; > use Test::Warn; > >@@ -132,7 +132,7 @@ is( $messages->[0]->{subject}, $my_message->{letter}->{title}, 'EnqueueLetter st > is( $messages->[0]->{content}, $my_message->{letter}->{content}, 'EnqueueLetter stores the content correctly' ); > is( $messages->[0]->{message_transport_type}, $my_message->{message_transport_type}, 'EnqueueLetter stores the message type correctly' ); > is( $messages->[0]->{status}, 'pending', 'EnqueueLetter stores the status pending correctly' ); >- >+is( $messages->[0]->{delivery_note}, '', 'Delivery note for successful message correctly empty'); > > # SendQueuedMessages > my $messages_processed = C4::Letters::SendQueuedMessages( { type => 'email' }); >@@ -156,6 +156,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.1.4
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=78313">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