Attachment 56052 Details for Bug 12532 – Bug 12532 - Redirect guarantee email to guarantor

[patch] Bug 12532 - Redirect guarantee email to guarantor

Bug-12532---Redirect-guarantee-email-to-guarantor.patch (text/plain), 12.92 KB, created by Bouzid Fergani on 2016-10-05 12:59:10 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Bouzid Fergani

Created: 2016-10-05 12:59:10 UTC

Size: 12.92 KB

patch

obsolete

>From 6707d49aace841e8142589c6fa3cf6957d358750 Mon Sep 17 00:00:00 2001
>From: mxbeaulieu <maxime.beaulieu@inlibro.com>
>Date: Tue, 2 Jun 2015 14:22:31 -0400
>Subject: [PATCH] Bug 12532 - Redirect guarantee email to guarantor
>
>Fixed merge conflicts.
>Both the guarantor and the guarantee will receive the email now.
>Also fixed some log errors since the "save" button is the same one use for editing the patron details.
>Changed the save button "save" id whene editing only the Patron messaging preferences so we can detect where the request is comming from.
>
>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
>
>Tested together with follow-up works as expected.
>Signed-off-by: Marc <veron@veron.ch>
>---
> C4/Letters.pm                                      |  8 ++--
> C4/Members.pm                                      | 27 +++++------
> ..._12532-EnableRedirectGuaranteeEmail_syspref.sql |  2 +
> installer/data/mysql/sysprefs.sql                  |  1 +
> .../prog/en/modules/admin/preferences/patrons.pref |  6 +++
> .../prog/en/modules/members/memberentrygen.tt      |  1 -
> t/db_dependent/Letters.t                           | 55 ++++++++++++++++++++++
> t/db_dependent/Members.t                           | 24 ++++++++++
> 8 files changed, 105 insertions(+), 19 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 26f3e46..6abd0ea 100644
>--- a/C4/Letters.pm
>+++ b/C4/Letters.pm
>@@ -1222,18 +1222,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;
>diff --git a/C4/Members.pm b/C4/Members.pm
>index 4b2d7ff..fd40ee5 100644
>--- a/C4/Members.pm
>+++ b/C4/Members.pm
>@@ -1198,22 +1198,21 @@ Returns the empty string if no email address.
> 
> sub GetNoticeEmailAddress {
>     my $borrowernumber = shift;
>+    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 $guarantor =  C4::Members::GetMember( 'borrowernumber' => $guarantorid ) if $useGuarantor;
> 
>-    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($member->{'borrowernumber'}) : $member->{ $which_address };
>+    my $guarantor_to_address = $which_address eq 'OFF' ? GetFirstValidEmailAddress($guarantor->{'borrowernumber'}) : $guarantor->{ $which_address } if $guarantor;
>+    $to_address .="," . $guarantor_to_address if $guarantor_to_address;
>+
>+   return $to_address || '';
> }
> 
> =head2 GetUpcomingMembershipExpires
>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..11fb97e
>--- /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 ('RedirectGuaranteeEmail','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 169665f..13c918c 100644
>--- a/installer/data/mysql/sysprefs.sql
>+++ b/installer/data/mysql/sysprefs.sql
>@@ -137,6 +137,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
> ('EnableAdvancedCatalogingEditor','0','','Enable the Rancor advanced cataloging editor','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',''),
>+('RedirectGuaranteeEmail', '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'),
> ('EnhancedMessagingPreferencesOPAC', '1', NULL, 'If ON, show patrons messaging setting on the OPAC.', '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 2f2faa0..9066424 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
>@@ -186,6 +186,12 @@ Patrons:
>                no: "Don't"
>          - track last patron activity.
>          - Everytime a patron will connect, the borrowers.lastseen will be updated with the current time.
>+     -
>+         - pref: RedirectGuaranteeEmail
>+           choices:
>+               yes: Enable
>+               no: "Don't enable"
>+         - redirection of guarantee email messages to the guarantor. All email messages send via the message_queue table will be affected.
>     "Norwegian patron database":
>      -
>          - pref: NorwegianPatronDBEnable
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
>index 82418ff..113a2ec 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
>@@ -77,7 +77,6 @@ $(document).ready(function() {
>             $("#entryform").submit();
>         }
>     });
>-
> });
> 
>         var MSG_SEPARATOR = _("Separator must be / in field %s");
>diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t
>old mode 100644
>new mode 100755
>index 39d11c7..bec56ef
>--- a/t/db_dependent/Letters.t
>+++ b/t/db_dependent/Letters.t
>@@ -147,6 +147,61 @@ is( $resent, 0, 'The message should not have been resent again' );
> $resent = C4::Letters::ResendMessage();
> is( $resent, undef, 'ResendMessage should return undef if not message_id given' );
> 
>+#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 8ab9069..0b8a150 100755
>--- a/t/db_dependent/Members.t
>+++ b/t/db_dependent/Members.t
>@@ -152,6 +152,9 @@ is ($checkcardnum, "2", "Card number is too long");
> 
> 
> t::lib::Mocks::mock_preference( 'AutoEmailPrimaryAddress', 'OFF' );
>+
>+C4::Context->set_preference( 'EnableRedirectGuaranteeEmail', 0 );
>+C4::Context->set_preference( 'AutoEmailPrimaryAddress', 'OFF' );
> C4::Context->clear_syspref_cache();
> 
> my $notice_email = GetNoticeEmailAddress($member->{'borrowernumber'});
>@@ -163,6 +166,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