@@ -, +, @@ --- 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; +} --