From 5a95029b6c028ff55ec85f8ad2fc54a9e08cb7e0 Mon Sep 17 00:00:00 2001 From: mxbeaulieu 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 --- 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 | 44 +++++++++++++++++ 8 files changed, 125 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 3ff39b1..df519a6 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -1288,18 +1288,18 @@ sub _send_message_by_email { my ($username, $password, $method) = @_; my $member = C4::Members::GetMember( 'borrowernumber' => $message->{'borrowernumber'} ); + my $useGurantor = $member && $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 a501041..4ef53be 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -1001,22 +1001,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 GetBorrowersToExpunge 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 8329720..9b54e5b 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -145,6 +145,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 e0e36d8..6db4b7e 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 @@ -191,6 +191,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 ff3ef68..cc68198 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 8219e5b..e5c2d61 --- a/t/db_dependent/Letters.t +++ b/t/db_dependent/Letters.t @@ -148,6 +148,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 3d24e44..6d2f58a 100755 --- a/t/db_dependent/Members.t +++ b/t/db_dependent/Members.t @@ -139,6 +139,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'}); @@ -150,6 +153,47 @@ 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'}); +ok($member->{is_expired}, "GetMemberDetails() indicates that patron is expired"); + +# Create a reserve for the patron +$builder->build({ + source => 'Reserve', + value => { + borrowernumber => $member->{ borrowernumber } + } +}); +is( Koha::Holds->search({ borrowernumber => $member->{borrowernumber} })->count, + 1, 'Hold created correctly' ); +DelMember($member->{borrowernumber}); +my $borrower = GetMember( cardnumber => $CARDNUMBER ); +is( $borrower, undef, 'DelMember should remove the patron' ); +is( Koha::Holds->search({ borrowernumber => $member->{borrowernumber} })->count, + 0, 'Hold deleted correctly' ); + # Check_Userid tests %data = ( cardnumber => "123456789", -- 1.9.1