Attachment 39925 Details for Bug 12532 – [SIGNED OFF] Bug 12532 - Redirect guarantee email to guarantor

[patch] [SIGNED OFF] Bug 12532 - Redirect guarantee email to guarantor

SIGNED-OFF-Bug-12532---Redirect-guarantee-email-to.patch (text/plain), 12.68 KB, created by Nick Clemens (kidclamp) on 2015-06-05 20:46:20 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2015-06-05 20:46:20 UTC

Size: 12.68 KB

patch

obsolete

>From a9aeb04589f6864eca374cbfaac9181fc0d685ab Mon Sep 17 00:00:00 2001
>From: mxbeaulieu <maxime.beaulieu@inlibro.com>
>Date: Tue, 2 Jun 2015 14:22:31 -0400
>Subject: [PATCH] [SIGNED OFF] Bug 12532 - Redirect guarantee email to
> guarantor
>
>Added unit tests.
>Added an atomicupdate file.
>Reordered the sysprefs in alphabetical order.
>Refactored some code.
>
>TEST PLAN:
>
>I Apply the patch
>II Run updatedatabase.pl
>III Run db_dependent/Letters.t and db_dependant/Members.t
>
>0) Enable EnhancedMessagingPreferences preference;
>0) Enable EnableRedirectGuaranteeEmail preference;
>1) Select a patron with child;
>2) Enable Item checkout -> email in patron messaging preferences;
>3) Select the patron's child;
>4) Checkout an item;
>5) Valide child checkout message in patron's inbox;
>
>sponsored by the ville de victoriaville
>
>Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
>---
> C4/Letters.pm                                      | 11 +++--
> C4/Members.pm                                      | 27 +++++-----
> ..._12532-EnableRedirectGuaranteeEmail_syspref.sql |  2 +
> installer/data/mysql/sysprefs.sql                  |  1 +
> .../prog/en/modules/admin/preferences/patrons.pref |  6 +++
> t/db_dependent/Letters.t                           | 57 +++++++++++++++++++++-
> t/db_dependent/Members.t                           | 24 ++++++++-
> 7 files changed, 107 insertions(+), 21 deletions(-)
> create mode 100644 installer/data/mysql/atomicupdate/bug_12532-EnableRedirectGuaranteeEmail_syspref.sql
> mode change 100644 => 100755 t/db_dependent/Letters.t
>
>diff --git a/C4/Letters.pm b/C4/Letters.pm
>index c7751ca..811e13d 100644
>--- a/C4/Letters.pm
>+++ b/C4/Letters.pm
>@@ -1131,18 +1131,18 @@ sub _send_message_by_email {
>     my ($username, $password, $method) = @_;
> 
>     my $member = C4::Members::GetMember( 'borrowernumber' => $message->{'borrowernumber'} );
>+    my $useGurantor = $member->{'guarantorid'} && C4::Context->preference("EnableRedirectGuaranteeEmail");
>     my $to_address = $message->{'to_address'};
>-    unless ($to_address) {
>+    if ($useGurantor || !$to_address) {
>         unless ($member) {
>-            warn "FAIL: No 'to_address' and INVALID borrowernumber ($message->{borrowernumber})";
>+            warn "FAIL: 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) {  
>-            # warn "FAIL: No 'to_address' and no email for " . ($member->{surname} ||'') . ", borrowernumber ($message->{borrowernumber})";
>-            # warning too verbose for this more common case?
>+            warn "FAIL: No 'to_address' and no email for " . ($member->{surname} ||'') . ", borrowernumber ($message->{borrowernumber})";
>             _set_message_status( { message_id => $message->{'message_id'},
>                                    status     => 'failed' } );
>             return;
>@@ -1182,7 +1182,8 @@ sub _send_message_by_email {
>        $sendmail_params{ Bcc } = $bcc;
>     }
> 
>-    _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 initial message address was empty or modified, coming here means that a to address was found and queue should be updated
>+    _update_message_to_address($message->{'message_id'},$to_address) if !$message->{to_address} || $message->{to_address} ne $to_address;
>     if ( sendmail( %sendmail_params ) ) {
>         _set_message_status( { message_id => $message->{'message_id'},
>                 status     => 'sent' } );
>diff --git a/C4/Members.pm b/C4/Members.pm
>index c5e5453..19b0f49 100644
>--- a/C4/Members.pm
>+++ b/C4/Members.pm
>@@ -1441,22 +1441,21 @@ Returns the empty string if no email address.
> 
> sub GetNoticeEmailAddress {
>     my $borrowernumber = shift;
>+    my $dbh = C4::Context->dbh();
>+
>+    my $member = C4::Members::GetMember( 'borrowernumber' => $borrowernumber );
>+
>+    #  Determine who we send the email to.
>+    my $guarantorid = $member->{'guarantorid'};
>+    my $useGuarantor = $guarantorid && C4::Context->preference("EnableRedirectGuaranteeEmail");
>+
>+    my $recipient = $useGuarantor ? C4::Members::GetMember( 'borrowernumber' => $guarantorid ) : $member;
> 
>-    my $which_address = C4::Context->preference("AutoEmailPrimaryAddress");
>     # if syspref is set to 'first valid' (value == OFF), look up email address
>-    if ( $which_address eq 'OFF' ) {
>-        return GetFirstValidEmailAddress($borrowernumber);
>-    }
>-    # specified email address field
>-    my $dbh = C4::Context->dbh;
>-    my $sth = $dbh->prepare( qq{
>-        SELECT $which_address AS primaryemail
>-        FROM borrowers
>-        WHERE borrowernumber=?
>-    } );
>-    $sth->execute($borrowernumber);
>-    my $data = $sth->fetchrow_hashref;
>-    return $data->{'primaryemail'} || '';
>+    my $which_address = C4::Context->preference("AutoEmailPrimaryAddress");
>+    my $to_address = $which_address eq 'OFF' ? GetFirstValidEmailAddress($recipient->{'borrowernumber'}) : $recipient->{ $which_address };
>+
>+   return $to_address || '';
> }
> 
> =head2 GetExpiryDate 
>diff --git a/installer/data/mysql/atomicupdate/bug_12532-EnableRedirectGuaranteeEmail_syspref.sql b/installer/data/mysql/atomicupdate/bug_12532-EnableRedirectGuaranteeEmail_syspref.sql
>new file mode 100644
>index 0000000..2e08159
>--- /dev/null
>+++ b/installer/data/mysql/atomicupdate/bug_12532-EnableRedirectGuaranteeEmail_syspref.sql
>@@ -0,0 +1,2 @@
>+INSERT IGNORE INTO systempreferences (variable,value,explanation,options,type)
>+VALUES ('EnableRedirectGuaranteeEmail','0',NULL,'Enable the ability to redirect guarantee email messages to guarantor.','YesNo');
>diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql
>index 9620d67..2c156f7 100644
>--- a/installer/data/mysql/sysprefs.sql
>+++ b/installer/data/mysql/sysprefs.sql
>@@ -119,6 +119,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
> ('emailLibrarianWhenHoldIsPlaced','0',NULL,'If ON, emails the librarian whenever a hold is placed','YesNo'),
> ('EnableBorrowerFiles','0',NULL,'If enabled, allows librarians to upload and attach arbitrary files to a borrower record.','YesNo'),
> ('EnableOpacSearchHistory','1','YesNo','Enable or disable opac search history',''),
>+('EnableRedirectGuaranteeEmail', '0', NULL, 'Enable the ability to redirect guarantee email messages to guarantor.', 'YesNo'),
> ('EnableSearchHistory','0','','Enable or disable search history','YesNo'),
> ('EnhancedMessagingPreferences','0','','If ON, allows patrons to select to receive additional messages about items due or nearly due.','YesNo'),
> ('expandedSearchOption','0',NULL,'If ON, set advanced search to be expanded by default','YesNo'),
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
>index ee908d2..68cac9e 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
>@@ -138,6 +138,12 @@ Patrons:
>                yes: Allow
>                no: "Don't allow"
>          - librarians to discharge borrowers and borrowers to request a discharge.
>+     -
>+         - pref: EnableRedirectGuaranteeEmail
>+           choices:
>+               yes: Do
>+               no: "Don't"
>+         - enable the ability to redirect guarantee email messages to guarantor. All email message placed in message_queue table will be affected.
>     "Norwegian patron database":
>      -
>          - pref: NorwegianPatronDBEnable
>diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t
>old mode 100644
>new mode 100755
>index f53e75b..45ff367
>--- 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 => 58;
>+use Test::More tests => 64;
> use Test::MockModule;
> use Test::Warn;
> 
>@@ -132,6 +132,61 @@ is(
>     'message marked failed if tried to send SMS message for borrower with no smsalertnumber set (bug 11208)'
> );
> 
>+#Test guarantee-guarantor email redirection
>+my ($GUARANTEE_EMAIL, $GUARANTOR_EMAIL) = qw| guarantee@example.com guarantor@example.com |;
>+my $guarantor = {
>+    email        => $GUARANTOR_EMAIL,
>+    categorycode => 'PT',
>+    branchcode   => 'CPL',
>+    surname      => "Guarantor"
>+};
>+$guarantor->{'borrowernumber'} = AddMember(%$guarantor);
>+my $guarantee = {
>+    guarantorid  => $guarantor->{'borrowernumber'},
>+    email        => $GUARANTEE_EMAIL,
>+    categorycode => 'PT',
>+    branchcode   => 'CPL',
>+    surname      => "Guarantee"
>+};
>+$guarantee->{'borrowernumber'} = AddMember(%$guarantee);
>+
>+my $GUARANTEE_TO_ADDRESS = 'toto@exemple.com';
>+my $guarantee_message = {
>+    borrowernumber         => $guarantee->{'borrowernumber'},
>+    message_transport_type => 'email',
>+    to_address             => $GUARANTEE_TO_ADDRESS,
>+    from_address           => 'from@example.com',
>+    letter                 => $my_message->{'letter'}
>+};
>+
>+sub SendToGuarantor {
>+    my ($useGuarantor, $message) = @_;
>+    C4::Context->set_preference( 'AutoEmailPrimaryAddress', 'OFF' );
>+    C4::Context->set_preference( 'EnableRedirectGuaranteeEmail', $useGuarantor );
>+    C4::Context->clear_syspref_cache();
>+    my $id = C4::Letters::EnqueueLetter($message);
>+    C4::Letters::SendQueuedMessages();
>+    my $result = $dbh->selectrow_hashref( 'SELECT to_address, status FROM message_queue WHERE message_id = ?', undef, ($id) );
>+    return $result;
>+}
>+my $result = SendToGuarantor(0,$guarantee_message);
>+is($result->{'status'}, "sent", "With EnableRedirectGuaranteeEmail off, message is sent.");
>+is($result->{'to_address'}, $GUARANTEE_TO_ADDRESS, "With EnableRedirectGuaranteeEmail off, Message is sent to the specified to_address.");
>+
>+$result = SendToGuarantor(1,$guarantee_message);
>+is($result->{'status'}, "sent", "With EnableRedirectGuaranteeEmail on, message is sent.");
>+is($result->{'to_address'}, $GUARANTOR_EMAIL, "With EnableRedirectGuaranteeEmail on, Message is sent to the guarantor's email address.");
>+
>+$guarantor->{'email'} = undef;
>+ModMember(%$guarantor);
>+$result = SendToGuarantor(1,$guarantee_message);
>+is($result->{'status'}, "failed", "With EnableRedirectGuaranteeEmail on, message fails if guarantor has no address.");
>+
>+$guarantee->{'guarantorid'} = undef;
>+ModMember(%$guarantee);
>+$result = SendToGuarantor(1,$guarantee_message);
>+is($result->{'to_address'}, $GUARANTEE_TO_ADDRESS, "With EnableRedirectGuaranteeEmail on, Message is sent to specicied to_address if he has no guarantor.");
>+
> # GetLetters
> my $letters = C4::Letters::GetLetters();
> is( @$letters, 0, 'GetLetters returns the correct number of letters' );
>diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t
>index d9136fb..b8e8f26 100755
>--- a/t/db_dependent/Members.t
>+++ b/t/db_dependent/Members.t
>@@ -17,7 +17,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 61;
>+use Test::More tests => 63;
> use Test::MockModule;
> use Data::Dumper;
> use C4::Context;
>@@ -143,6 +143,7 @@ is ($checkcardnum, "2", "Card number is too long");
> 
> 
> 
>+C4::Context->set_preference( 'EnableRedirectGuaranteeEmail', 0 );
> C4::Context->set_preference( 'AutoEmailPrimaryAddress', 'OFF' );
> C4::Context->clear_syspref_cache();
> 
>@@ -155,6 +156,27 @@ C4::Context->clear_syspref_cache();
> $notice_email = GetNoticeEmailAddress($member->{'borrowernumber'});
> is ($notice_email, $EMAILPRO, "GetNoticeEmailAddress returns correct value when AutoEmailPrimaryAddress is emailpro");
> 
>+#Test guarantee-guarantor email redirection
>+my $GUARANTEE_EMAIL = 'guanrantee@email.test';
>+my $guarantee = {
>+    guarantorid  => $member->{'borrowernumber'},
>+    emailpro     => $GUARANTEE_EMAIL,
>+    categorycode => $CATEGORYCODE,
>+    branchcode   => $BRANCHCODE,
>+    surname      => "surname"
>+};
>+$guarantee->{'borrowernumber'} = AddMember(%$guarantee);
>+
>+$notice_email = GetNoticeEmailAddress($guarantee->{'borrowernumber'});
>+is ($notice_email, $GUARANTEE_EMAIL, "GetNoticeEmailAddress returns correct value when EnableRedirectGuaranteeEmail is disabled");
>+
>+C4::Context->set_preference( 'EnableRedirectGuaranteeEmail', 1 );
>+C4::Context->clear_syspref_cache();
>+
>+$notice_email = GetNoticeEmailAddress($guarantee->{'borrowernumber'});
>+is ($notice_email, $EMAILPRO, "GetNoticeEmailAddress returns correct value when EnableRedirectGuaranteeEmail is activated");
>+
>+
> ok(!$member->{is_expired}, "GetMemberDetails() indicates that patron is not expired");
> ModMember(borrowernumber => $member->{'borrowernumber'}, dateexpiry => '2001-01-1');
> $member = GetMemberDetails($member->{'borrowernumber'});
>-- 
>1.9.1

Actions: View | Diff | Splinter Review

Attachments on bug 12532: 29510 | 30747 | 35313 | 36752 | 39787 | 39925 | 48645 | 53649 | 53689 | 53690 | 53699 | 53700 | 55985 | 56052 | 56053 | 59445 | 59446 | 59447 | 63197 | 64802 | 64824 | 67115 | 68340 | 70865 | 72064 | 72617 | 75099 | 77606 | 80313 | 85134 | 85135 | 87815 | 87816 | 88208 | 88209 | 88210 | 88211 | 88212 | 88213 | 88295 | 88296 | 114455 | 114456 | 114457 | 114458 | 114459 | 114460 | 114461 | 131091 | 131092 | 131093 | 131094 | 149233 | 150071 | 150208 | 150217 | 150240 | 150256 | 151028 | 151029 | 154854 | 154855 | 154856 | 154857 | 155036 | 155037 | 155828 | 155829 | 155830 | 155831 | 155832 | 155833 | 155834 | 155835 | 155836 | 155837 | 155873 | 155874 | 155875 | 155876 | 155877 | 155878 | 155879 | 155880 | 155881 | 155882 | 155883 | 155886 | 155934 | 155935 | 155936 | 155937 | 155938 | 155939 | 155940 | 155941 | 155942 | 155943 | 155944 | 155945 | 155946