From b5d536abeee3c379ddf6997e5f0c490194c5be34 Mon Sep 17 00:00:00 2001 From: Mark Tompsett Date: Thu, 6 Mar 2014 22:26:23 -0500 Subject: [PATCH] Bug 8000 - Correct inability to know where message was sent The added column in the previous patch is used to determine where the message was actually sent when using email. This include a modification to the C4/Letters.pm libraries to appropriate track and update the column and revising the test file to check that the system preference actually does override or not the destination. --- C4/Letters.pm | 27 ++++++++++++++++++--------- t/db_dependent/Letters.t | 32 +++++++++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/C4/Letters.pm b/C4/Letters.pm index 62f49ba..b13ebb2 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -1016,7 +1016,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, sentto_address FROM message_queue ENDSQL @@ -1145,17 +1145,19 @@ sub _send_message_by_email { my $member = C4::Members::GetMember( 'borrowernumber' => $message->{'borrowernumber'} ); my $to_address = $message->{'to_address'}; + if (!$to_address && $member) { + $to_address = C4::Members::GetNoticeEmailAddress( $message->{'borrowernumber'} ); + } my $sendAllEmailsTo = C4::Context->preference('SendAllEmailsTo'); $to_address = $sendAllEmailsTo if ($sendAllEmailsTo && $sendAllEmailsTo =~ /@/ ); # some validation. This could be improved. unless ($to_address) { - unless ($member) { + if (!$member) { warn "FAIL: No 'to_address' and INVALID borrowernumber ($message->{borrowernumber})"; _set_message_status( { message_id => $message->{'message_id'}, status => 'failed' } ); return; } - $to_address = C4::Members::GetNoticeEmailAddress( $message->{'borrowernumber'} ); - unless ($to_address) { + else { # 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'}, @@ -1199,8 +1201,9 @@ 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' } ); + _set_message_status( { message_id => $message->{'message_id'}, + status => 'sent', + sentto_address => $to_address } ); return 1; } else { _set_message_status( { message_id => $message->{'message_id'}, @@ -1284,11 +1287,17 @@ sub _set_message_status { return unless exists $params->{ $required_parameter }; } - my $dbh = C4::Context->dbh(); my $statement = 'UPDATE message_queue SET status= ? WHERE message_id = ?'; + my @values; + push @values,$params->{'status'}; + if (exists($params->{'sentto_address'})) { + $statement = 'UPDATE message_queue SET status=?, sentto_address=? WHERE message_id = ?'; + push @values,$params->{'sentto_address'}; + } + push @values,$params->{'message_id'}; + my $dbh = C4::Context->dbh(); my $sth = $dbh->prepare( $statement ); - my $result = $sth->execute( $params->{'status'}, - $params->{'message_id'} ); + my $result = $sth->execute( @values ); return $result; } diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t index b0143e2..a4d5e70 100644 --- a/t/db_dependent/Letters.t +++ b/t/db_dependent/Letters.t @@ -18,7 +18,7 @@ # along with Koha; if not, see . use Modern::Perl; -use Test::More tests => 60; +use Test::More tests => 64; use Test::MockModule; use Test::Warn; @@ -107,7 +107,12 @@ $my_message->{message_transport_type} = 'sms'; $message_id = C4::Letters::EnqueueLetter($my_message); ok(defined $message_id && $message_id > 0, 'new message successfully queued'); +# This is will force C4::Context->preference() to read the DB. +C4::Context->disable_syspref_cache(); + # used to trigger _send_message_by_email in C4::Letter +# first override. +$dbh->do("UPDATE systempreferences SET value='override\@example.com' WHERE variable='SendAllEmailsTo';"); my $message_id2 = C4::Letters::EnqueueLetter({ borrowernumber => $borrowernumber, message_transport_type => 'email', @@ -146,11 +151,36 @@ is($messages_processed, 2, 'all queued messages processed'); $messages = C4::Letters::GetQueuedMessages({ borrowernumber => $borrowernumber }); is(scalar(@$messages), 2, 'two messages stored for the borrower'); +# used to trigger _send_message_by_email in C4::Letter +# second no override. +$dbh->do("UPDATE systempreferences SET value='' WHERE variable='SendAllEmailsTo';"); +my $message_id3 = C4::Letters::EnqueueLetter({ + borrowernumber => $borrowernumber, + message_transport_type => 'email', + to_address => 'to@example.com', + from_address => 'from@example.com', + letter => { + content => 'a message', + title => 'message title', + metadata => 'metadata', + code => 'TEST_MESSAGE', + content_type => 'text/plain', + }, +}); + +$messages_processed = C4::Letters::SendQueuedMessages(); +is($messages_processed, 1, 'third queued message processed'); + +my $messages = C4::Letters::GetQueuedMessages({ borrowernumber => $borrowernumber }); +is(scalar(@$messages), 3, 'three messages stored for the borrower'); + is( $messages->[0]->{status}, 'failed', 'message marked failed if tried to send SMS message for borrower with no smsalertnumber set (bug 11208)' ); +is ( $messages->[1]->{sentto_address},'override@example.com','Second message sent to override.'); +is ( $messages->[2]->{sentto_address},'to@example.com','Third message sent to default.'); # GetLetters my $letters = C4::Letters::GetLetters(); -- 1.9.1