Bugzilla – Attachment 87531 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.78 KB, created by
Liz Rea
on 2019-04-08 15:27:56 UTC
(
hide
)
Description:
Bug 14723: Additional delivery notes to messages
Filename:
MIME Type:
Creator:
Liz Rea
Created:
2019-04-08 15:27:56 UTC
Size:
11.78 KB
patch
obsolete
>From a1a26e97023fdf6175bfcc0f51cad3b7b6c688d3 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> >--- > C4/Letters.pm | 36 ++++++++++++++-------- > ...23_-_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 | 9 ++++-- > 5 files changed, 42 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 cbe6559e82..fc3173a2d0 100644 >--- a/C4/Letters.pm >+++ b/C4/Letters.pm >@@ -908,9 +908,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); >@@ -925,6 +925,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); > } >@@ -1066,7 +1067,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 > >@@ -1120,7 +1121,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 ); >@@ -1224,7 +1225,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 = ? >@@ -1271,7 +1272,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; >@@ -1279,7 +1281,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; > } > } >@@ -1322,11 +1325,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; > } >@@ -1375,13 +1380,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; > } > >@@ -1389,7 +1396,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; > } > >@@ -1413,9 +1422,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..3b1cbe1f7c >--- /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 6d7a4d28e1..5ce8f31249 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/installer/data/mysql/kohastructure.sql >@@ -2490,6 +2490,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 d774136f28..6cfbebc303 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,7 @@ > [% END %] > </td> > <td><span title="[% QUEUED_MESSAGE.time_queued | html %]">[% QUEUED_MESSAGE.time_queued | $KohaDates with_hours => 1 %]</span></td> >+ <td>[% QUEUED_MESSAGE.delivery_note | html %]</td> > </tr> > [% END %] > </tbody> >@@ -98,7 +100,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 a14df42344..cf0a342012 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 => 67; >+use Test::More tests => 69; > 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.11.0
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=87531">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