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 <david.bourgault@inlibro.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/C4/Letters.pm b/C4/Letters.pm
index acf4b65..42893f1 100644 (file)
--- 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 ecd9f59..faa497f 100644 (file)
--- 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 11af849..216ecec 100644 (file)
--- 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 3fa5ec4..e83eed6 100644 (file)
--- 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 f8f2e8f..598ef5a 100755 (executable)
--- 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 a1b477c..8c42335 100755 (executable)
--- 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 89d7dc9..e80828a 100755 (executable)
--- 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 c90a9f5..d793996 100644 (file)
--- 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 cfb314f..ff5a30b 100755 (executable)
--- 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",