Bugzilla – Attachment 16248 Details for
Bug 9103
overdue_notices.pl should use AutoEmailPrimaryAddress syspref
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
[PASSED QA] Bug 9103: overdue_notices.pl should use AutoEmailPrimaryAddress syspref
PASSED-QA-Bug-9103-overduenoticespl-should-use-Aut.patch (text/plain), 13.63 KB, created by
Katrin Fischer
on 2013-03-18 14:30:02 UTC
(
hide
)
Description:
[PASSED QA] Bug 9103: overdue_notices.pl should use AutoEmailPrimaryAddress syspref
Filename:
MIME Type:
Creator:
Katrin Fischer
Created:
2013-03-18 14:30:02 UTC
Size:
13.63 KB
patch
obsolete
>From 8d9f5576af62c1caa8f12caf40a41cd0c6318486 Mon Sep 17 00:00:00 2001 >From: Fridolyn SOMERS <fridolyn.somers@biblibre.com> >Date: Mon, 19 Nov 2012 13:58:16 +0100 >Subject: [PATCH] [PASSED QA] Bug 9103: overdue_notices.pl should use > AutoEmailPrimaryAddress syspref > >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. >--- > C4/Letters.pm | 8 +--- > C4/Members.pm | 30 ++++++++++++ > C4/Reserves.pm | 9 +--- > misc/cronjobs/overdue_notices.pl | 94 +++++++++++++++----------------------- > 4 files changed, 69 insertions(+), 72 deletions(-) > >diff --git a/C4/Letters.pm b/C4/Letters.pm >index 8873c46..9e610c2 100644 >--- a/C4/Letters.pm >+++ b/C4/Letters.pm >@@ -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? >diff --git a/C4/Members.pm b/C4/Members.pm >index 8042840..f3334f0 100644 >--- a/C4/Members.pm >+++ b/C4/Members.pm >@@ -66,6 +66,7 @@ BEGIN { > &getidcity > > &GetFirstValidEmailAddress >+ &GetNoticeEmailAddress > > &GetAge > &GetCities >@@ -1359,6 +1360,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); >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index ba12cf5..c8b0cdd 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -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; >diff --git a/misc/cronjobs/overdue_notices.pl b/misc/cronjobs/overdue_notices.pl >index ba58e95..b265707 100755 >--- a/misc/cronjobs/overdue_notices.pl >+++ b/misc/cronjobs/overdue_notices.pl >@@ -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; >-- >1.7.9.5
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=16248">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 9103
:
13511
|
15931
|
16110
| 16248 |
16452