From a7aab6d30b0a14c58bc0ab355ff284bd60750103 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Fri, 28 Jul 2017 11:58:46 +0000 Subject: [PATCH] Bug 18990: Overdue Notices are not sending through SMS correctly Bug 18478 fixed sms via email problems under the assumption that to_address was either smsalertnumber or blank. It seems overdues set the to_address to email. This patch changes the code to enforce that an sms sent with emial driver will use the smsalertnumebr and provider defined for the borrower, regardless of what is set in the queue To test: 1 - Define a messaging prefs for a patron to recieve hold notices via SMS 2 - Ensure you have defined an SMS message for an overdue letter 3 - Set an SMS alert number for patron 4 - Set the SMS::Send driver to 'Email' 5 - Checkout an item as overdue to trigger notice above 6 - Run overdue_notices.pl 6 - Check the db and note the address is email 7 - run process_message_queue.pl 8 - Check db - address is email followed by service provider 9 - Apply patch 10 - repeat 11 - Message to_address should be populated with smsalertnumber Signed-off-by: Josef Moravec Signed-off-by: Marcel de Rooy Signed-off-by: Jonathan Druart Signed-off-by: Nick Clemens --- C4/Letters.pm | 4 ++-- t/db_dependent/Letters.t | 14 ++++++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/C4/Letters.pm b/C4/Letters.pm index d7971af4e0..acf4b65717 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -1056,12 +1056,12 @@ sub SendQueuedMessages { _set_message_status( { message_id => $message->{'message_id'}, status => 'failed' } ); next MESSAGE; } - $message->{to_address} ||= $patron->smsalertnumber; - unless ( $message->{to_address} && $patron->smsalertnumber ) { + unless ( $patron->smsalertnumber ) { _set_message_status( { message_id => $message->{'message_id'}, status => 'failed' } ); warn sprintf( "No smsalertnumber found for patron %s!", $message->{'borrowernumber'} ) if $params->{'verbose'} or $debug; next MESSAGE; } + $message->{to_address} = $patron->smsalertnumber; #Sometime this is set to email - sms should always use smsalertnumber $message->{to_address} .= '@' . $sms_provider->domain(); _update_message_to_address($message->{'message_id'},$message->{to_address}); _send_message_by_email( $message, $params->{'username'}, $params->{'password'}, $params->{'method'} ); diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t index 26716c5618..e20cefb8b9 100644 --- a/t/db_dependent/Letters.t +++ b/t/db_dependent/Letters.t @@ -633,7 +633,7 @@ subtest 'TranslateNotices' => sub { subtest 'SendQueuedMessages' => sub { - plan tests => 2; + plan tests => 3; t::lib::Mocks::mock_preference( 'SMSSendDriver', 'Email' ); my $patron = Koha::Patrons->find($borrowernumber); $dbh->do(q| @@ -652,7 +652,17 @@ subtest 'SendQueuedMessages' => sub { borrowernumber => $borrowernumber, status => 'sent' })->next()->to_address(); - is( $sms_message_address, '5555555555@kidclamp.rocks', 'SendQueuedMessages populates the to address correctly for SMS by email' ); + is( $sms_message_address, '5555555555@kidclamp.rocks', 'SendQueuedMessages populates the to address correctly for SMS by email when to_address not set' ); + $schema->resultset('MessageQueue')->search({borrowernumber => $borrowernumber,status => 'sent'})->delete(); #clear borrower queue + $my_message->{to_address} = 'fixme@kidclamp.iswrong'; + $message_id = C4::Letters::EnqueueLetter($my_message); + C4::Letters::SendQueuedMessages(); + $sms_message_address = $schema->resultset('MessageQueue')->search({ + borrowernumber => $borrowernumber, + status => 'sent' + })->next()->to_address(); + is( $sms_message_address, '5555555555@kidclamp.rocks', 'SendQueuedMessages populates the to address correctly for SMS by email when to_address is set incorrectly' ); + }; subtest 'get_item_content' => sub { -- 2.39.5