From 7061e74676d17c4a306a9bec07bdb02597850817 Mon Sep 17 00:00:00 2001 From: Fridolyn SOMERS Date: Mon, 19 Nov 2012 13:58:16 +0100 Subject: [PATCH] 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 Signed-off-by: Katrin Fischer 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 --- 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 8873c46bef..9e610c2efc 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 3d468c8240..61d52aa2d7 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -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); diff --git a/C4/Reserves.pm b/C4/Reserves.pm index ba12cf57da..c8b0cdda47 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 ba58e959d1..b2657072ff 100755 --- a/misc/cronjobs/overdue_notices.pl +++ b/misc/cronjobs/overdue_notices.pl @@ -450,7 +450,7 @@ END_SQL # 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; -- 2.39.5