From be5ed9d1d961027a99a53f1f81fdb17b58e83d60 Mon Sep 17 00:00:00 2001 From: Charles Farmer Date: Thu, 29 Sep 2016 15:49:14 -0400 Subject: [PATCH] Bug 8000 - Override emails of every message sent from Koha For testing purposes, we do not want emails sent to (legitimate) users. And sometime we also like to actually see what would be generated for the users. This preference will allow to override every message sent by koha with a new (temporary) To address. Leave it empty for normal usage. C4/Letters.pm installer/data/mysql/sysprefs.sql installer/data/mysql/updatedatabase.pl koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref TESTING: 0) Run updatedatabase.pl 1) Enter a valid address for new preference SendAllEmailsTo, something that will be easily identified as NOT the normal destination for the Koha emails. 2) Have koha generate an email to a user. Or any other mean prefered that cause Koha to send email. 3) Validate that the email is NOT sent to the user. 4) Validate that the email IS sent to the overriding address. 5) Clean the preference 6) Redo the test, validate that the email is going to the right address. PS Not sure if those steps are precise enough. Here's one way: a) set AutoEmailOpacUser to true b) create a new user, with an email address c) Normally, a confirmation email is sent to the user. Validate that it goes to the SendAllEmailsTo one. --- C4/Letters.pm | 39 ++++++++++++---- .../atomicupdate/bz8000_TestModeForNotices.sql | 3 ++ installer/data/mysql/sysprefs.sql | 1 + .../prog/en/modules/admin/preferences/admin.pref | 5 ++ t/db_dependent/Letters.t | 54 ++++++++++++++++++++-- 5 files changed, 90 insertions(+), 12 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bz8000_TestModeForNotices.sql diff --git a/C4/Letters.pm b/C4/Letters.pm index a981e8a..3d84476 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -37,6 +37,7 @@ use Koha::SMS::Providers; use Koha::Email; use Koha::DateUtils qw( format_sqldatetime dt_from_string ); +use Email::Valid; use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS); @@ -413,7 +414,11 @@ sub SendAlerts { my $alerts = getalert( '', 'issue', $subscriptionid ); foreach (@$alerts) { my $borinfo = C4::Members::GetMember('borrowernumber' => $_->{'borrowernumber'}); - my $email = $borinfo->{email} or next; + my $sendAllEmailsTo = C4::Context->preference('SendAllEmailsTo'); + my $email = $sendAllEmailsTo; + if (!($sendAllEmailsTo && Email::Valid->address($sendAllEmailsTo))){ + $email = $borinfo->{email} or next; + } # warn "sending issues..."; my $userenv = C4::Context->userenv; @@ -571,10 +576,19 @@ sub SendAlerts { # Remove the order tag $letter->{content} =~ s/(.*?)<\/order>/$1/gxms; + # SendAllEmailsTo system preference to override for testing + my $emailsTo = join( ',', @email); + my $emailsCC = join( ',', @cc); + my $sendAllEmailsTo = C4::Context->preference('SendAllEmailsTo'); + if ($sendAllEmailsTo && Email::Valid->address($sendAllEmailsTo) ) { + $emailsTo = $sendAllEmailsTo; + $emailsCC = ''; + } + # ... then send mail my %mail = ( - To => join( ',', @email), - Cc => join( ',', @cc), + To => $emailsTo, + Cc => $emailsCC, From => $userenv->{emailaddress}, Subject => Encode::encode( "UTF-8", "" . $letter->{title} ), Message => $letter->{'is_html'} @@ -628,9 +642,12 @@ sub SendAlerts { ) or return; return { error => "no_email" } unless $externalid->{'emailaddr'}; my $email = Koha::Email->new(); + my $emailaddr = $externalid->{'emailaddr'}; + my $sendAllEmailsTo = C4::Context->preference('SendAllEmailsTo'); + $emailaddr = $sendAllEmailsTo if ($sendAllEmailsTo && Email::Valid->address($sendAllEmailsTo) ); my %mail = $email->create_message_headers( { - to => $externalid->{'emailaddr'}, + to => $emailaddr, from => $library->{branchemail}, replyto => $library->{branchreplyto}, sender => $library->{branchreturnpath}, @@ -1289,15 +1306,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 && Email::Valid->address($sendAllEmailsTo) ); 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'}, @@ -1342,8 +1363,8 @@ 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' } ); return 1; } else { _set_message_status( { message_id => $message->{'message_id'}, diff --git a/installer/data/mysql/atomicupdate/bz8000_TestModeForNotices.sql b/installer/data/mysql/atomicupdate/bz8000_TestModeForNotices.sql new file mode 100644 index 0000000..8dbeb08 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bz8000_TestModeForNotices.sql @@ -0,0 +1,3 @@ +-- SYSPREF +INSERT IGNORE INTO systempreferences (variable,value,options,explanation,type) +VALUES ('SendAllEmailsTo','',NULL,'This email will replace the To address in every message sent by the system. This is meant to help testing and validation only.','Textarea'); diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index ef83c3d..0b5f9a0 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -454,6 +454,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('SelfCheckHelpMessage','','70|10','Enter HTML to include under the basic Web-based Self Checkout instructions on the Help page','Textarea'), ('SelfCheckReceiptPrompt','1','NULL','If ON, print receipt dialog pops up when self checkout is finished','YesNo'), ('SelfCheckTimeout','120','','Define the number of seconds before the Web-based Self Checkout times out a patron','Integer'), +('SendAllEmailsTo','',NULL,'This email will replace the To address in every message sent by the system. This is meant to help testing only.','Textarea'), ('SeparateHoldings','0',NULL,'Separate current branch holdings from other holdings','YesNo'), ('SeparateHoldingsBranch','homebranch','homebranch|holdingbranch','Branch used to separate holdings','Choice'), ('SessionRestrictionByIP','1','Check for change in remote IP address for session security. Disable only when remote IP address changes frequently.','','YesNo'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref index 5d8091c..dd583f1 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref @@ -16,6 +16,11 @@ Administration: class: email - "If you leave this empty, the From address will be used (often defaulting to the admin address)." - + - "Redirect all emails from Koha to" + - pref: SendAllEmailsTo + class: email + - "instead of their intended user." + - - "How much debugging information to show in the browser when an internal error occurs: " - pref: DebugLevel default: 0 diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t index 9b5c469..6dba58f 100644 --- a/t/db_dependent/Letters.t +++ b/t/db_dependent/Letters.t @@ -88,6 +88,7 @@ is_deeply( $mtts, ['email', 'phone', 'print', 'sms'], 'GetMessageTransportTypes # EnqueueLetter is( C4::Letters::EnqueueLetter(), undef, 'EnqueueLetter without argument returns undef' ); +# used to trigger _send_message_by_sms in C4::Letter my $my_message = { borrowernumber => $borrowernumber, message_transport_type => 'sms', @@ -112,13 +113,35 @@ $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', + 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', + }, +}); + +ok(defined $message_id2 && $message_id2 > 0, 'new message successfully queued'); + # GetQueuedMessages my $messages = C4::Letters::GetQueuedMessages(); -is( @$messages, 1, 'GetQueuedMessages without argument returns all the entries' ); +is( @$messages, 2, 'GetQueuedMessages without argument returns all the entries' ); $messages = C4::Letters::GetQueuedMessages({ borrowernumber => $borrowernumber }); -is( @$messages, 1, 'one message stored for the borrower' ); +is( @$messages, 2, 'two messages stored for the borrower' ); is( $messages->[0]->{message_id}, $message_id, 'EnqueueLetter returns the message id correctly' ); is( $messages->[0]->{borrowernumber}, $borrowernumber, 'EnqueueLetter stores the borrower number correctly' ); is( $messages->[0]->{subject}, $my_message->{letter}->{title}, 'EnqueueLetter stores the subject correctly' ); @@ -129,9 +152,34 @@ is( $messages->[0]->{status}, 'pending', 'EnqueueLetter stores the status pendin # SendQueuedMessages my $messages_processed = C4::Letters::SendQueuedMessages(); -is($messages_processed, 1, 'all queued messages processed'); +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'); $messages = C4::Letters::GetQueuedMessages({ borrowernumber => $borrowernumber }); +is(scalar(@$messages), 3, 'three messages stored for the borrower'); + is( $messages->[0]->{status}, 'failed', -- 1.9.1