Bugzilla – Attachment 62080 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), 10.40 KB, created by
Lari Taskula
on 2017-04-12 13:09:59 UTC
(
hide
)
Description:
Bug 14723: Additional delivery notes to messages
Filename:
MIME Type:
Creator:
Lari Taskula
Created:
2017-04-12 13:09:59 UTC
Size:
10.40 KB
patch
obsolete
>From a6873deaf967fc6c2b4bb926c0672bbf44171dc8 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 | 32 ++++++++++++++-------- > ...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, 42 insertions(+), 14 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 74263f0..5768e1c 100644 >--- a/C4/Letters.pm >+++ b/C4/Letters.pm >@@ -1000,9 +1000,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); >@@ -1017,6 +1017,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); > } >@@ -1113,7 +1114,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 > >@@ -1296,7 +1297,8 @@ sub _send_message_by_email { > unless ($member) { > 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 = C4::Members::GetNoticeEmailAddress( $message->{'borrowernumber'} ); >@@ -1304,7 +1306,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; > } > } >@@ -1346,11 +1349,13 @@ sub _send_message_by_email { > > if ( 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; > } >@@ -1399,13 +1404,15 @@ sub _send_message_by_sms { > > unless ( $member->{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; > } > >@@ -1413,7 +1420,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; > } > >@@ -1431,9 +1440,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 076d745..8fa065d 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/installer/data/mysql/kohastructure.sql >@@ -2508,6 +2508,7 @@ CREATE TABLE `message_queue` ( > `to_address` mediumtext, > `from_address` mediumtext, > `content_type` text, >+ `delivery_note` text, > 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 cea5b0d..f7b2b18 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt >@@ -12,7 +12,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" > })); > >@@ -51,6 +51,7 @@ > <th>Type</th> > <th>Status</th> > <th>Time</th> >+ <th>Delivery note</th> > </tr> > </thead> > <tbody> >@@ -89,6 +90,9 @@ > [% END %] > </td> > <td><span title="[% QUEUED_MESSAGE.time_queued %]">[% QUEUED_MESSAGE.time_queued | $KohaDates with_hours => 1 %]</span></td> >+ <td> >+ [% QUEUED_MESSAGE.delivery_note %] >+ </td> > </tr> > [% END %] > </tbody> >diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t >index cb8e6ca..c7ef291 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 => 80; >+use Test::More tests => 82; > use Test::MockModule; > use Test::Warn; > >@@ -126,7 +126,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(); >@@ -149,6 +149,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.7.4
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=62080">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