Bugzilla – Attachment 58434 Details for
Bug 8000
Test mode for notices
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 8000 - Override emails of every message sent from Koha
Bug-8000---Override-emails-of-every-message-sent-f.patch (text/plain), 12.66 KB, created by
Charles Farmer
on 2016-12-27 14:45:55 UTC
(
hide
)
Description:
Bug 8000 - Override emails of every message sent from Koha
Filename:
MIME Type:
Creator:
Charles Farmer
Created:
2016-12-27 14:45:55 UTC
Size:
12.66 KB
patch
obsolete
>From be5ed9d1d961027a99a53f1f81fdb17b58e83d60 Mon Sep 17 00:00:00 2001 >From: Charles Farmer <charles.farmer@inlibro.com> >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>(.*?)<\/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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 8000
:
21976
|
22315
|
24784
|
25917
|
25919
|
25920
|
25923
|
25924
|
25925
|
25926
|
25927
|
25928
|
25939
|
25950
|
25951
|
26446
|
37463
|
37464
|
37465
|
37466
|
37467
|
37468
|
37469
|
37472
|
37473
|
37474
|
37475
|
37476
|
37477
|
37478
|
37481
|
37482
|
37483
|
37484
|
37485
|
37486
|
37487
|
39985
|
39986
|
39987
|
39988
|
39989
|
39990
|
39991
|
39992
|
40215
|
40216
|
40217
|
40218
|
40219
|
40220
|
40221
|
40222
|
40223
|
40224
|
40225
|
40226
|
40227
|
40228
|
40229
|
40230
|
46354
|
46355
|
46356
|
46357
|
46358
|
46359
|
46360
|
46361
|
46362
|
49991
|
49992
|
49993
|
49998
|
49999
|
50000
|
55940
|
58434
|
62863
|
67155
|
70086
|
70087
|
70484
|
74110
|
74111
|
74848
|
80370
|
81487
|
81488
|
81589
|
81590
|
81591
|
84801
|
84802
|
84803
|
84804
|
84805
|
86861
|
86862
|
86863
|
86864
|
86865
|
88874
|
88875
|
88876
|
88877
|
88878
|
88879