From 82a16dd7602cb859ab5c5c71602502313aa3bd3c 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, 48 insertions(+), 11 deletions(-) diff --git a/C4/Letters.pm b/C4/Letters.pm index a2affe0..cd7a3f0 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -826,7 +826,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 @@ -937,17 +937,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'}, @@ -980,8 +982,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'}, @@ -1043,11 +1046,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 653508f..76d3ec4 100644 --- a/t/db_dependent/Letters.t +++ b/t/db_dependent/Letters.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 5; +use Test::More tests => 8; use C4::Context; use C4::Letters; @@ -55,7 +55,12 @@ my $message_id = C4::Letters::EnqueueLetter({ }, }); +# 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', @@ -76,8 +81,28 @@ ok(defined $message_id2 && $message_id2 > 0, 'new message successfully queued'); my $messages_processed = C4::Letters::SendQueuedMessages(); is($messages_processed, 2, 'all queued messages processed'); +# 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), 2, 'two messages stored for the borrower'); +is(scalar(@$messages), 3, 'three messages stored for the borrower'); is( $messages->[0]->{status}, @@ -85,4 +110,7 @@ is( '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.'); + $dbh->rollback; -- 1.7.9.5