From 3a5534fcf5c945e744a86422eb03251b40ea5894 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 12 Sep 2017 15:41:56 -0300 Subject: [PATCH] Bug 19304: Move C4::Members::GetNoticeEmailAddress to Koha::Patron->notice_email_address This subroutine is quite trivial and can be replaced easily with a new method of Koha::Patron Test plan: Overdue notices and shelf sharing must be send the to an email address, according to the value of the pref AutoEmailPrimaryAddress Signed-off-by: David Bourgault Signed-off-by: Josef Moravec Signed-off-by: Jonathan Druart --- C4/Letters.pm | 2 +- C4/Members.pm | 32 ------------------- C4/Reserves.pm | 2 +- Koha/Patron.pm | 21 ++++++++++++ .../notice_unprocessed_suggestions.pl | 3 +- misc/cronjobs/overdue_notices.pl | 5 +-- opac/opac-shareshelf.pl | 7 ++-- t/db_dependent/Koha/Patrons.t | 16 +++++++++- t/db_dependent/Members.t | 13 -------- 9 files changed, 46 insertions(+), 55 deletions(-) diff --git a/C4/Letters.pm b/C4/Letters.pm index acf4b65717..42893f1bd3 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -1312,7 +1312,7 @@ sub _send_message_by_email { status => 'failed' } ); return; } - $to_address = C4::Members::GetNoticeEmailAddress( $message->{'borrowernumber'} ); + $to_address = $patron->notice_email_address; 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? diff --git a/C4/Members.pm b/C4/Members.pm index ecd9f59833..faa497f98c 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -64,8 +64,6 @@ BEGIN { &GetPendingIssues &GetAllIssues - &GetNoticeEmailAddress - &GetMemberAccountRecords &GetBorrowersToExpunge @@ -878,36 +876,6 @@ sub get_cardnumber_length { return ( $min, $max ); } -=head2 GetNoticeEmailAddress - - $email = GetNoticeEmailAddress($borrowernumber); - -Return the email address of borrower used for notices, given the borrowernumber. -Returns the empty string if no email address. - -=cut - -sub GetNoticeEmailAddress { - my $borrowernumber = shift; - - 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' ) { - my $patron = Koha::Patrons->find( $borrowernumber ); - return $patron->first_valid_email_address(); - } - # 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'} || ''; -} - =head2 GetBorrowersToExpunge $borrowers = &GetBorrowersToExpunge( diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 11af849395..216ececdba 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -1609,7 +1609,7 @@ sub _koha_notify_reserve { my $patron = Koha::Patrons->find( $borrowernumber ); # Try to get the borrower's email address - my $to_address = C4::Members::GetNoticeEmailAddress($borrowernumber); + my $to_address = $patron->notice_email_address; my $messagingprefs = C4::Members::Messaging::GetMessagingPreferences( { borrowernumber => $borrowernumber, diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 3fa5ec457c..e83eed6b55 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -619,6 +619,27 @@ sub old_holds { return Koha::Old::Holds->_new_from_dbic($old_holds_rs); } +=head3 notice_email_address + + my $email = $patron->notice_email_address; + +Return the email address of patron used for notices. +Returns the empty string if no email address. + +=cut + +sub notice_email_address{ + my ( $self ) = @_; + + 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 $self->first_valid_email_address; + } + + return $self->$which_address || ''; +} + =head3 first_valid_email_address my $first_valid_email_address = $patron->first_valid_email_address diff --git a/misc/cronjobs/notice_unprocessed_suggestions.pl b/misc/cronjobs/notice_unprocessed_suggestions.pl index f8f2e8fb51..598ef5a4b4 100755 --- a/misc/cronjobs/notice_unprocessed_suggestions.pl +++ b/misc/cronjobs/notice_unprocessed_suggestions.pl @@ -46,8 +46,7 @@ for my $number_of_days (@days) { my $budget = C4::Budgets::GetBudget( $suggestion->{budgetid} ); my $patron = Koha::Patrons->find( $budget->{budget_owner_id} ); - my $email_address = - C4::Members::GetNoticeEmailAddress( $budget->{budget_owner_id} ); + my $email_address = $patron->notice_email_address; my $library = $patron->library; my $admin_email_address = $library->branchemail || C4::Context->preference('KohaAdminEmailAddress'); diff --git a/misc/cronjobs/overdue_notices.pl b/misc/cronjobs/overdue_notices.pl index a1b477c731..8c42335c2c 100755 --- a/misc/cronjobs/overdue_notices.pl +++ b/misc/cronjobs/overdue_notices.pl @@ -43,6 +43,7 @@ use Koha::DateUtils; use Koha::Calendar; use Koha::Libraries; use Koha::Acquisition::Currencies; +use Koha::Patrons; =head1 NAME @@ -580,9 +581,9 @@ END_SQL $verbose and warn "borrower $borr has items triggering level $i."; + my $patron = Koha::Patrons->find( $borrowernumber ); @emails_to_use = (); - my $notice_email = - C4::Members::GetNoticeEmailAddress($borrowernumber); + my $notice_email = $patron->notice_email_address; unless ($nomail) { if (@emails) { foreach (@emails) { diff --git a/opac/opac-shareshelf.pl b/opac/opac-shareshelf.pl index 89d7dc9d5d..e80828a321 100755 --- a/opac/opac-shareshelf.pl +++ b/opac/opac-shareshelf.pl @@ -33,6 +33,7 @@ use C4::Letters; use C4::Members (); use C4::Output; +use Koha::Patrons; use Koha::Virtualshelves; use Koha::Virtualshelfshares; @@ -166,10 +167,10 @@ sub show_accept { sub notify_owner { my ($param) = @_; - my $toaddr = C4::Members::GetNoticeEmailAddress( $param->{owner} ); - return if !$toaddr; - my $patron = Koha::Patrons->find( $param->{owner} ); + return unless $patron; + + my $toaddr = $patron->notice_email_address or return; #prepare letter my $letter = C4::Letters::GetPreparedLetter( diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index c90a9f5a0f..d793996793 100644 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 22; +use Test::More tests => 23; use Test::Warn; use Time::Fake; use DateTime; @@ -705,6 +705,20 @@ subtest 'holds and old_holds' => sub { $patron->delete; }; +subtest 'notice_email_address' => sub { + plan tests => 2; + + my $patron = $builder->build_object({ class => 'Koha::Patrons' }); + + t::lib::Mocks::mock_preference( 'AutoEmailPrimaryAddress', 'OFF' ); + is ($patron->notice_email_address, $patron->email, "Koha::Patron->notice_email_address returns correct value when AutoEmailPrimaryAddress is off"); + + t::lib::Mocks::mock_preference( 'AutoEmailPrimaryAddress', 'emailpro' ); + is ($patron->notice_email_address, $patron->emailpro, "Koha::Patron->notice_email_address returns correct value when AutoEmailPrimaryAddress is emailpro"); + + $patron->delete; +}; + subtest 'search_patrons_to_anonymise & anonymise_issue_history' => sub { plan tests => 4; diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t index cfb314fa60..ff5a30bc2d 100755 --- a/t/db_dependent/Members.t +++ b/t/db_dependent/Members.t @@ -139,19 +139,6 @@ $checkcardnum=C4::Members::checkcardnumber($IMPOSSIBLE_CARDNUMBER, ""); is ($checkcardnum, "2", "Card number is too long"); - -t::lib::Mocks::mock_preference( 'AutoEmailPrimaryAddress', 'OFF' ); -C4::Context->clear_syspref_cache(); - -my $notice_email = GetNoticeEmailAddress($member->{'borrowernumber'}); -is ($notice_email, $EMAIL, "GetNoticeEmailAddress returns correct value when AutoEmailPrimaryAddress is off"); - -t::lib::Mocks::mock_preference( 'AutoEmailPrimaryAddress', 'emailpro' ); -C4::Context->clear_syspref_cache(); - -$notice_email = GetNoticeEmailAddress($member->{'borrowernumber'}); -is ($notice_email, $EMAILPRO, "GetNoticeEmailAddress returns correct value when AutoEmailPrimaryAddress is emailpro"); - # Check_Userid tests %data = ( cardnumber => "123456789", -- 2.39.5