@@ -, +, @@
---
C4/Letters.pm | 28 ++-
...4723_-_Additional_delivery_notes_to_messages.pl | 8 +
.../prog/en/modules/members/notices.tt | 4 +
t/db_dependent/MessageDeliveryNote.t | 208 +++++++++++++++++++++
4 files changed, 239 insertions(+), 9 deletions(-)
create mode 100644 installer/data/mysql/atomicupdate/Bug-14723_-_Additional_delivery_notes_to_messages.pl
create mode 100644 t/db_dependent/MessageDeliveryNote.t
--- a/C4/Letters.pm
+++ a/C4/Letters.pm
@@ -1004,7 +1004,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
@@ -1137,7 +1137,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'} );
@@ -1145,7 +1146,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;
}
}
@@ -1186,11 +1188,13 @@ sub _send_message_by_email {
_update_message_to_address($message->{'message_id'},$to_address) unless $message->{to_address}; #if initial message address was empty, coming here means that a to address was found and queue should be updated
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;
}
@@ -1239,21 +1243,26 @@ 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;
}
my $success = C4::SMS->send_sms( { destination => $member->{'smsalertnumber'},
message => $message->{'content'},
+ message_id => $message->{'message_id'}
} );
_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;
}
@@ -1271,9 +1280,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;
}
--- a/installer/data/mysql/atomicupdate/Bug-14723_-_Additional_delivery_notes_to_messages.pl
+++ a/installer/data/mysql/atomicupdate/Bug-14723_-_Additional_delivery_notes_to_messages.pl
@@ -0,0 +1,8 @@
+#! /usr/bin/perl
+use strict;
+use warnings;
+use C4::Context;
+my $dbh = C4::Context->dbh;
+
+$dbh->do("ALTER TABLE message_queue ADD delivery_note TEXT");
+print "Upgrade to done (Bug 14723 - Additional delivery notes to messages)\n";
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt
+++ a/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt
@@ -49,6 +49,7 @@
Type |
Status |
Time |
+ Delivery note |
@@ -75,6 +76,9 @@
[% ELSE %][% QUEUED_MESSAGE.status %][% END %]
[% QUEUED_MESSAGE.time_queued | $KohaDates with_hours => 1 %] |
+
+ [% QUEUED_MESSAGE.delivery_note %]
+ |
[% END %]
--- a/t/db_dependent/MessageDeliveryNote.t
+++ a/t/db_dependent/MessageDeliveryNote.t
@@ -0,0 +1,208 @@
+#!/usr/bin/env perl
+
+# Copyright 2015 Open Source Freedom Fighters
+#
+# This file is part of Koha.
+#
+# Koha is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# Koha is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Koha; if not, see .
+$ENV{KOHA_PAGEOBJECT_DEBUG} = 1;
+use Modern::Perl;
+
+use Test::More;
+use Try::Tiny; #Even Selenium::Remote::Driver uses Try::Tiny :)
+
+use Koha::Auth::PermissionManager;
+
+use t::lib::Page::Mainpage;
+use t::lib::Page::Opac::OpacMain;
+use t::lib::Page::Opac::OpacMemberentry;
+use t::lib::Page::Members::Memberentry;
+use t::lib::Page::Members::Moremember;
+use t::lib::Page::Members::Notices;
+
+use t::lib::TestObjects::BorrowerFactory;
+use t::lib::TestObjects::MessageQueueFactory;
+use t::lib::TestObjects::SystemPreferenceFactory;
+
+use C4::Context;
+use C4::Members;
+
+##Setting up the test context
+my $testContext = {};
+
+my $password = '1234';
+my $invalidBorrowerNumber = '000';
+my $borrowerFactory = t::lib::TestObjects::BorrowerFactory->new();
+my $borrowers = $borrowerFactory->createTestGroup([
+ {firstname => 'Testthree',
+ surname => 'Testfour',
+ cardnumber => 'superuberadmin',
+ branchcode => 'CPL',
+ userid => 'god',
+ address => 'testi',
+ city => 'joensuu',
+ zipcode => '80100',
+ password => $password,
+ },
+ ], undef, $testContext);
+my $messages = t::lib::TestObjects::MessageQueueFactory->createTestGroup([
+ {letter => {
+ title => "Test title",
+ content => "Tessst content",
+ },
+ cardnumber => $borrowers->{'superuberadmin'}->cardnumber,
+ message_transport_type => 'sms',
+ from_address => 'A001@example.com',
+ },
+ {letter => {
+ title => "Test title",
+ content => "Tessst content",
+ },
+ cardnumber => $borrowers->{'superuberadmin'}->cardnumber,
+ message_transport_type => 'sms',
+ from_address => 'A002@example.com',
+ },
+ {letter => {
+ title => "Test title",
+ content => "Tessst content",
+ },
+ cardnumber => $borrowers->{'superuberadmin'}->cardnumber,
+ message_transport_type => 'email',
+ to_address => '',
+ from_address => 'B001@example.com',
+ },
+ {letter => {
+ title => "Test title",
+ content => "Tessst content",
+ },
+ cardnumber => $borrowers->{'superuberadmin'}->cardnumber,
+ message_transport_type => 'email',
+ to_address => 'nobody@example.com',
+ from_address => 'B002@example.com',
+ },
+ {letter => {
+ title => "INVALID USER",
+ content => "INVALID USER",
+ },
+ cardnumber => $borrowers->{'superuberadmin'}->cardnumber,
+ message_transport_type => 'email',
+ to_address => '',
+ from_address => 'B003@example.com',
+ },
+ ], undef, $testContext);
+my $systempreferences = t::lib::TestObjects::SystemPreferenceFactory->createTestGroup([
+ {preference => 'SMSSendDriver',
+ value => 'nonexistentdriver'
+ },
+ ], undef, $testContext);
+
+my $permissionManager = Koha::Auth::PermissionManager->new();
+$permissionManager->grantPermissions($borrowers->{'superuberadmin'}, {superlibrarian => 'superlibrarian'});
+
+eval {
+
+ # staff client
+ my $notices = t::lib::Page::Members::Notices->new({borrowernumber => $borrowers->{'superuberadmin'}->borrowernumber});
+
+ # let's send the messages
+
+ # first, send one email message with invalid borrower number
+ $messages->{'B003@example.com'}->{borrowernumber} = $invalidBorrowerNumber;
+ C4::Letters::_send_message_by_email($messages->{'B003@example.com'});
+
+ # the rest should produce errors automatically without our modificaitons
+ C4::Letters::SendQueuedMessages();
+
+ # then, login to intranet
+ $notices->doPasswordLogin($borrowers->{'superuberadmin'}->userid(), $password);
+
+ # test delivery notes for SMS messaging
+ DoSMSTests($notices);
+ # check delivery notes for email messaging
+ CheckEmailMessages($notices);
+
+};
+if ($@) { #Catch all leaking errors and gracefully terminate.
+ warn $@;
+ tearDown();
+ exit 1;
+}
+
+##All tests done, tear down test context
+tearDown();
+done_testing;
+
+sub tearDown {
+ t::lib::TestObjects::ObjectFactory->tearDownTestContext($testContext);
+}
+
+
+
+
+
+
+
+
+######################################################
+ ### STARTING TEST IMPLEMENTATIONS ###
+######################################################
+
+sub DoSMSTests {
+ my ($notices) = @_;
+
+ # login and check that our table is displayed correctly
+ $notices->
+ hasDeliveryNoteColumnInNoticesTable()->
+ hasTextInTableCell("Missing SMS number");
+
+ # now let's give the user a SMS number so we proceed to next step of failure
+ ModMember( borrowernumber => $borrowers->{superuberadmin}->borrowernumber,
+ smsalertnumber => 'just_some_number' );
+
+ # set first to sent and second to pending to check "duplicate" (Letters::_is_duplicate())
+ setMessageStatus($messages->{'A001@example.com'}->{message_id}, "sent");
+ setMessageStatus($messages->{'A002@example.com'}->{message_id}, "pending");
+
+ C4::Letters::SendQueuedMessages();
+ # A002 should go into failed: message is duplicate status
+
+ # set A001 back to pending to check that we get no reply from driver
+ setMessageStatus($messages->{'A001@example.com'}->{message_id}, "pending");
+
+ # let's send the message and check for delivery note, we should get a warning
+ # for non-installed driver.
+ C4::Letters::SendQueuedMessages();
+
+ $notices->navigateToNotices()->
+ hasTextInTableCell("Message is duplicate")->
+ hasTextInTableCell("No notes from SMS driver");
+}
+
+sub CheckEmailMessages {
+ my ($notices) = @_;
+
+ $notices->
+ hasTextInTableCell("Unable to find an email address")->
+ hasTextInTableCell("Invalid borrowernumber")->
+ hasTextInTableCell("Connection refused"); #sendmail should also fail
+}
+sub setMessageStatus {
+ my ($message_id, $status) = @_;
+ my $dbh = C4::Context->dbh;
+
+ my $sth = $dbh->prepare("UPDATE message_queue SET status=?, delivery_note='' WHERE message_id=?");
+ my $result = $sth->execute($status, $message_id);
+
+ return $result;
+}
--