Attachment 55940 Details for Bug 8000 – Bug 8000 - Override emails of every message sent from Koha

[patch] Bug 8000 - Override emails of every message sent from Koha

Bug-8000---Override-emails-of-every-message-sent-f.patch (text/plain), 12.74 KB, created by Charles Farmer on 2016-09-29 19:58:26 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Charles Farmer

Created: 2016-09-29 19:58:26 UTC

Size: 12.74 KB

patch

obsolete

>From 19e5ade2a0721d8242a55696f9140aee823bc6f3 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                           | 56 ++++++++++++++++++++--
> 5 files changed, 91 insertions(+), 13 deletions(-)
> create mode 100644 installer/data/mysql/atomicupdate/bz8000_TestModeForNotices.sql
>
>diff --git a/C4/Letters.pm b/C4/Letters.pm
>index 26f3e46..8c7fec2 100644
>--- a/C4/Letters.pm
>+++ b/C4/Letters.pm
>@@ -39,6 +39,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);
> 
>@@ -403,7 +404,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;
>@@ -517,10 +522,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'}
>@@ -572,9 +586,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},
>@@ -1223,15 +1240,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'},
>@@ -1276,8 +1297,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 169665f..75d60bb 100644
>--- a/installer/data/mysql/sysprefs.sql
>+++ b/installer/data/mysql/sysprefs.sql
>@@ -444,6 +444,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 4f3fb14..75ce0de 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
>@@ -71,6 +71,11 @@ Administration:
>                   yes: "logged in library's"
>                   no: "all libraries"
>             - rules by default.
>+        -
>+            - Redirect all emails from Koha to
>+            - pref: SendAllEmailsTo
>+              class: email
>+            - instead of their intended user.
>     Login options:
>         -
>             - Automatically log out users after
>diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t
>index 39d11c7..1719714 100644
>--- a/t/db_dependent/Letters.t
>+++ b/t/db_dependent/Letters.t
>@@ -18,7 +18,7 @@
> # along with Koha; if not, see <http://www.gnu.org/licenses>.
> 
> use Modern::Perl;
>-use Test::More tests => 74;
>+use Test::More tests => 78;
> use Test::MockModule;
> use Test::Warn;
> 
>@@ -87,6 +87,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',
>@@ -111,13 +112,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' );
>@@ -128,9 +151,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

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