Bug 9103: overdue_notices.pl should use AutoEmailPrimaryAddress syspref
authorFridolyn SOMERS <fridolyn.somers@biblibre.com>
Mon, 19 Nov 2012 12:58:16 +0000 (13:58 +0100)
committerJared Camins-Esakov <jcamins@cpbibliography.com>
Wed, 20 Mar 2013 02:50:39 +0000 (22:50 -0400)
Script overdue_notices.pl creates a printed letter if borrower as no email.
Actually, unless --email option is used, first valid email of borrower is used. Email field should depend on AutoEmailPrimaryAddress syspref like in other letter creations.

Signed-off-by: MJ Ray <mjr@phonecoop.coop>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All tests and QA script pass.

Following test plan from Julien Sicot from Bugzilla:

- with patron's email address specified on "primary email"
  field AND syspref "AutoEmailPrimaryAddress" on "home"
  => notice sent to patron | OK

- with patron's email address specified on "secondary email"
  field AND syspref "AutoEmailPrimaryAddress" on "work"
  => notice sent to patron | OK

- with patron's email address specified on "alternate email"
  field AND syspref "AutoEmailPrimaryAddress" on "alternate"
  => notice sent to patron | OK

- with patron's email address specified on "secondary email"
  OR "alternate email" field AND syspref "AutoEmailPrimaryAddress" on "home"
  => no notice sent to patron, overdue notice sent to koha admin | OK

- with patron's email address specified on "primary email" OR
- with patron's email address specified on "primary email" field
  AND syspref "AutoEmailPrimaryAddress" on "home"
  => notice sent to patron | OK

- with patron's email address specified on "secondary email" field
  AND syspref "AutoEmailPrimaryAddress" on "work"
  => notice sent to patron | OK

- with patron's email address specified on "alternate email" field
  AND syspref "AutoEmailPrimaryAddress" on "alternate"
  => notice sent to patron | OK

- with patron's email address specified on "secondary email"
  OR "alternate email" field AND syspref "AutoEmailPrimaryAddress"
  on "home"
  => no notice sent to patron, overdue notice sent to koha admin | OK

- with patron's email address specified on "primary email"
  OR "secondary email" field AND syspref "AutoEmailPrimaryAddress"
  on "alternate"
  => no notice sent to patron, overdue notice sent to koha admin | OK

- with patron's email address specified on "primary email"
  OR "secondary email" OR "alternate email" field and syspref
  "AutoEmailPrimaryAddress" on "first valid" => notice sent to patron | OK"secondary email" field AND syspref "AutoEmailPrimaryAddress" on "alternate" => no notice sent to patron, overdue notice sent to koha admin | OK

- with patron's email address specified on "primary email" OR
  "secondary email" OR "alternate email" field and syspref
  "AutoEmailPrimaryAddress" on "first valid" => notice sent to patron | OK

Note: Options for AutoEmailPrimaryAddress should be like the field names  on
the patron form (primary, secondary...), but this is outside the scope of this
patch.
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
C4/Letters.pm
C4/Members.pm
C4/Reserves.pm
misc/cronjobs/overdue_notices.pl

index 8873c46..9e610c2 100644 (file)
@@ -919,13 +919,7 @@ sub _send_message_by_email {
                                    status     => 'failed' } );
             return;
         }
-        my $which_address = C4::Context->preference('AutoEmailPrimaryAddress');
-        # If the system preference is set to 'first valid' (value == OFF), look up email address
-        if ($which_address eq 'OFF') {
-            $to_address = GetFirstValidEmailAddress( $message->{'borrowernumber'} );
-        } else {
-            $to_address = $member->{$which_address};
-        }
+        $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?
index 3d468c8..61d52aa 100644 (file)
@@ -66,6 +66,7 @@ BEGIN {
         &getidcity
 
         &GetFirstValidEmailAddress
+        &GetNoticeEmailAddress
 
         &GetAge
         &GetCities
@@ -1357,6 +1358,35 @@ sub GetFirstValidEmailAddress {
     }
 }
 
+=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' ) {
+        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'} || '';
+}
+
 =head2 GetExpiryDate 
 
   $expirydate = GetExpiryDate($categorycode, $dateenrolled);
index ba12cf5..c8b0cdd 100644 (file)
@@ -1859,14 +1859,7 @@ sub _koha_notify_reserve {
     my $borrower = C4::Members::GetMember(borrowernumber => $borrowernumber);
     
     # Try to get the borrower's email address
-    my $to_address;
-    my $which_address = C4::Context->preference('AutoEmailPrimaryAddress');
-    # If the system preference is set to 'first valid' (value == OFF), look up email address
-    if ($which_address eq 'OFF') {
-        $to_address = C4::Members::GetFirstValidEmailAddress( $borrowernumber );
-    } else {
-        $to_address = $borrower->{$which_address};
-    }
+    my $to_address = C4::Members::GetNoticeEmailAddress($borrowernumber);
     
     my $letter_code;
     my $print_mode = 0;
index ba58e95..b265707 100755 (executable)
@@ -450,7 +450,7 @@ END_SQL
             # <date> <itemcount> <firstname> <lastname> <address1> <address2> <address3> <city> <postcode> <country>
 
             my $borrower_sql = <<'END_SQL';
-SELECT distinct(issues.borrowernumber), firstname, surname, address, address2, city, zipcode, country, email
+SELECT distinct(issues.borrowernumber), firstname, surname, address, address2, city, zipcode, country, email, emailpro, B_email
 FROM   issues,borrowers,categories
 WHERE  issues.borrowernumber=borrowers.borrowernumber
 AND    borrowers.categorycode=categories.categorycode
@@ -478,24 +478,26 @@ END_SQL
             $sth->execute(@borrower_parameters);
             $verbose and warn $borrower_sql . "\n $branchcode | " . $overdue_rules->{'categorycode'} . "\n ($mindays, $maxdays)\nreturns " . $sth->rows . " rows";
 
-            while ( my ( $borrowernumber, $firstname, $lastname,
-                    $address1, $address2, $city, $postcode, $country, $email
-                    ) = $sth->fetchrow )
-            {
-                $verbose and warn "borrower $firstname, $lastname ($borrowernumber) has items triggering level $i.";
+            while ( my $data = $sth->fetchrow_hashref ) {
+                my $borrowernumber = $data->{'borrowernumber'};
+                my $borr =
+                    $data->{'firstname'} . ', '
+                  . $data->{'surname'} . ' ('
+                  . $borrowernumber . ')';
+                $verbose
+                  and warn "borrower $borr has items triggering level $i.";
+
                 @emails_to_use = ();
-                if ( !$nomail) {
-                    if ( @emails ) {
-                        my $memberinfos = C4::Members::GetMember(borrowernumber => $borrowernumber);
+                my $notice_email =
+                    C4::Members::GetNoticeEmailAddress($borrowernumber);
+                unless ($nomail) {
+                    if (@emails) {
                         foreach (@emails) {
-                            push @emails_to_use, $memberinfos->{$_}
-                              if ($memberinfos->{$_} ne '');
+                            push @emails_to_use, $data->{$_} if ( $data->{$_} );
                         }
                     }
                     else {
-                        $email =
-                          C4::Members::GetFirstValidEmailAddress($borrowernumber);
-                        push @emails_to_use, $email if ($email ne '');
+                        push @emails_to_use, $notice_email if ($notice_email);
                     }
                 }
 
@@ -513,7 +515,7 @@ END_SQL
     
                     #action taken is debarring
                     C4::Members::DebarMember($borrowernumber, '9999-12-31');
-                    $verbose and warn "debarring $borrowernumber $firstname $lastname\n";
+                    $verbose and warn "debarring $borr\n";
                 }
                 my @params = ($listall ? ( $borrowernumber , 1 , $MAX ) : ( $borrowernumber, $mindays, $maxdays ));
                 $verbose and warn "STH2 PARAMS: borrowernumber = $borrowernumber, mindays = $mindays, maxdays = $maxdays";
@@ -567,57 +569,35 @@ END_SQL
                 }
                 $letter->{'content'} =~ s/\<[^<>]*?\>//g;    # Now that we've warned about them, remove them.
                 $letter->{'content'} =~ s/\<[^<>]*?\>//g;    # 2nd pass for the double nesting.
-    
-                if ($nomail) {
-    
+
+                if ( !$nomail && scalar @emails_to_use ) {
+                    C4::Letters::EnqueueLetter(
+                        {   letter                 => $letter,
+                            borrowernumber         => $borrowernumber,
+                            message_transport_type => 'email',
+                            from_address           => $admin_email_address,
+                            to_address             => join(',', @emails_to_use),
+                        }
+                    );
+                } else {
+                    # if not sent by email then print
                     push @output_chunks,
                       prepare_letter_for_printing(
                         {   letter         => $letter,
                             borrowernumber => $borrowernumber,
-                            firstname      => $firstname,
-                            lastname       => $lastname,
-                            address1       => $address1,
-                            address2       => $address2,
-                            city           => $city,
-                            postcode       => $postcode,
-                            country        => $country,
-                            email          => $email,
+                            firstname      => $data->{'firstname'},
+                            lastname       => $data->{'surname'},
+                            address1       => $data->{'address'},
+                            address2       => $data->{'address2'},
+                            city           => $data->{'city'},
+                            postcode       => $data->{'zipcode'},
+                            country        => $data->{'country'},
+                            email          => $notice_email,
                             itemcount      => $itemcount,
                             titles         => $titles,
                             outputformat   => defined $csvfilename ? 'csv' : defined $htmlfilename ? 'html' : '',
                         }
                       );
-                } else {
-                    if (scalar(@emails_to_use) > 0 ) {
-                        C4::Letters::EnqueueLetter(
-                            {   letter                 => $letter,
-                                borrowernumber         => $borrowernumber,
-                                message_transport_type => 'email',
-                                from_address           => $admin_email_address,
-                                to_address             => join(',', @emails_to_use),
-                            }
-                        );
-                    } else {
-    
-                        # If we don't have an email address for this patron, send it to the admin to deal with.
-                        push @output_chunks,
-                          prepare_letter_for_printing(
-                            {   letter         => $letter,
-                                borrowernumber => $borrowernumber,
-                                firstname      => $firstname,
-                                lastname       => $lastname,
-                                address1       => $address1,
-                                address2       => $address2,
-                                city           => $city,
-                                postcode       => $postcode,
-                                country        => $country,
-                                email          => $email,
-                                itemcount      => $itemcount,
-                                titles         => $titles,
-                                outputformat   => defined $csvfilename ? 'csv' : defined $htmlfilename ? 'html' : '',
-                            }
-                          );
-                    }
                 }
             }
             $sth->finish;