Bugzilla – Attachment 57296 Details for
Bug 12802
Send notices using several email addresses
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 12802: Sent notices using several email addresses
Bug-12802-Sent-notices-using-several-email-address.patch (text/plain), 30.27 KB, created by
Matthias Meusburger
on 2016-11-08 08:22:38 UTC
(
hide
)
Description:
Bug 12802: Sent notices using several email addresses
Filename:
MIME Type:
Creator:
Matthias Meusburger
Created:
2016-11-08 08:22:38 UTC
Size:
30.27 KB
patch
obsolete
>From b2f88c428876a64abcbe47049cc010aca1460a46 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@biblibre.com> >Date: Thu, 21 Aug 2014 16:48:55 +0200 >Subject: [PATCH] Bug 12802: Sent notices using several email addresses > >Currently it is not possible to select several email addresses to notify >a patron. The only place where the mechanism exists is in the >overdue_notices.pl script. > >This patch reuses the AutoEmailPrimaryAddress syspref and changes its >type to "multiple". Like that it is now possible to select several email >addresses. Note that there is no sense to select OFF and another value >for this pref. So if the 'OFF' (first valid) value exist, it takes the >priority. > >It will add the ability to choose the email addresses to use to notify >patrons. The option is "email", "emailpro" and "B_email". >If "OFF" is selected, the first valid address will be returned (same >behavior as before this patch). > >Note for the QA step: >I found a possible regression, but IMO it's not a big deal: >Before this patch if a letter did not contain a "to_address", the >C4::Letters::_send_message_by_email subroutine retrieve the email from the >given borrowernumber. >This is now done in the EnqueueMessage subroutine. >What it means: If a borrower didn't have an email address when the >notice was sent to the queue, the email address won't be check again when the >notice will really sent to the patron. >This change permits to sent a letter to the queue (EnqueueLetter) >without any information (from_address, to_address), only the borrowernumber is >mandatory to retrieve them. >The _send_message_by_email subroutine should not contain any useful code, >only sent the letter we ask it to sent. >The _update_message_to_address becomes useless since the to_address is >retrieved previously. > >What this patch does: >The GetNoticeEmailAddress subroutine has been renamed to >GetNoticeEmailAddresses (reflect the plural). It only gets the patron' emails >defined in the AutoEmailPrimaryAddress pref. > >If no 'to_address' parameter is given to EnqueueMessage, the emails >will be retrieved at this moment. > >In C4::Message: An old form was found. The AutoEmailPrimaryAddress was >not check. The smsalertnumber was sent for the to_address param, which >is wrong. > >C4::Reserves: AddReserve and _koha_notify_reserve: >The from address is built in the QueueLetter. It is useless to do it >here. > >overdue_notices.pl: The script could be cleaned a little bit if we >remove the --email parameter. Indeed it is redundant with this new >enhancement. I can propose another patch with a die statement and a >message: "you should use the pref AutoEmailPrimaryAddress" if the >--email is provided. > >opac/opac-shareshelf.pl and opac/opac-memberentry.pl: >just remove the from and to address. They will be filled on sending to the queue >(because of the borrowernumber). > >Test plan: >1/ Apply this patch without updating the pref AutoEmailPrimaryAddress (or >fill it with a single value if it is not done yet). >2/ Try the different way to sent notices to a patron (check the >following letter code: HOLD, CHECKIN, CHECKOUT, PREDUE, RENEW, DUE). >3/ Verify the email is correctly sent to the message_queue. >4/ Fill the pref with email and emailpro (for instance) >5/ Verify that 2 notices are sent to the message_queue: 1 with the email >and 1 with the emailpro. >6/ You can also verify that only 1 notice is generated if the emailpro >is empty. > >Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> >--- > C4/Letters.pm | 136 ++++++++++++++++++-------------------- > C4/Members.pm | 45 +++++++------ > C4/Message.pm | 26 -------- > C4/Reserves.pm | 12 ++-- > misc/cronjobs/overdue_notices.pl | 10 +-- > opac/opac-memberentry.pl | 4 +- > opac/opac-shareshelf.pl | 10 ++- > t/db_dependent/Letters.t | 75 ++++++++++++++++++--- > t/db_dependent/Members.t | 16 +++-- > 9 files changed, 183 insertions(+), 151 deletions(-) > >diff --git a/C4/Letters.pm b/C4/Letters.pm >index a981e8a..9bb6eba 100644 >--- a/C4/Letters.pm >+++ b/C4/Letters.pm >@@ -34,7 +34,10 @@ use C4::SMS; > use C4::Debug; > use Koha::DateUtils; > use Koha::SMS::Providers; >- >+use Koha::Database; >+use Date::Calc qw( Add_Delta_Days ); >+use Encode; >+use Carp; > use Koha::Email; > use Koha::DateUtils qw( format_sqldatetime dt_from_string ); > >@@ -974,7 +977,6 @@ sub EnqueueLetter { > my $params = shift or return; > > return unless exists $params->{'letter'}; >-# return unless exists $params->{'borrowernumber'}; > return unless exists $params->{'message_transport_type'}; > > my $content = $params->{letter}->{content}; >@@ -994,28 +996,41 @@ sub EnqueueLetter { > ); > } > >- my $dbh = C4::Context->dbh(); >- my $statement = << 'ENDSQL'; >-INSERT INTO message_queue >-( borrowernumber, subject, content, metadata, letter_code, message_transport_type, status, time_queued, to_address, from_address, content_type ) >-VALUES >-( ?, ?, ?, ?, ?, ?, ?, NOW(), ?, ?, ? ) >-ENDSQL >+ my @to_addresses; >+ if ( $params->{to_address} ) { >+ @to_addresses = ( $params->{to_address} ); >+ } elsif ( $params->{borrowernumber} and $params->{message_transport_type} eq 'email' ) { >+ @to_addresses = @{ C4::Members::GetNoticeEmailAddresses( $params->{borrowernumber} ) }; >+ } > >- my $sth = $dbh->prepare($statement); >- my $result = $sth->execute( >- $params->{'borrowernumber'}, # borrowernumber >- $params->{'letter'}->{'title'}, # subject >- $params->{'letter'}->{'content'}, # content >- $params->{'letter'}->{'metadata'} || '', # metadata >- $params->{'letter'}->{'code'} || '', # letter_code >- $params->{'message_transport_type'}, # message_transport_type >- 'pending', # status >- $params->{'to_address'}, # to_address >- $params->{'from_address'}, # from_address >- $params->{'letter'}->{'content-type'}, # content_type >- ); >- return $dbh->last_insert_id(undef,undef,'message_queue', undef); >+ my $from_address; >+ if ( $params->{from_address} ) { >+ $from_address = $params->{from_address}; >+ } elsif ( $params->{message_transport_type} eq 'email' ) { >+ if ( $params->{borrowernumber} ) { >+ my $rs = Koha::Database->new->schema->resultset('Borrower')->search({ borrowernumber => $params->{borrowernumber} }, {join => 'branchcode'}); >+ $from_address = $rs->next->branchcode->branchemail; >+ } >+ >+ $from_address ||= C4::Context->preference('KohaAdminEmailAddress'); >+ } >+ >+ my $rs = Koha::Database->new->schema->resultset('MessageQueue'); >+ for my $to_address ( @to_addresses ) { >+ $rs->create({ >+ borrowernumber => $params->{borrowernumber}, >+ subject => $params->{letter}{title}, >+ content => $params->{letter}{content}, >+ metadata => ( $params->{letter}{metadata} || '' ), >+ letter_code => ( $params->{letter}{code} || ''), >+ message_transport_type => $params->{message_transport_type}, >+ status => 'pending', >+ to_address => $to_address, >+ from_address => $from_address, >+ content_type => $params->{letter}{'content-type'}, >+ }); >+ } >+ return scalar( @to_addresses ); > } > > =head2 SendQueuedMessages ([$hashref]) >@@ -1040,6 +1055,7 @@ sub SendQueuedMessages { > if $params->{'verbose'} or $debug; > # This is just begging for subclassing > next MESSAGE if ( lc($message->{'message_transport_type'}) eq 'rss' ); >+ > if ( lc( $message->{'message_transport_type'} ) eq 'email' ) { > _send_message_by_email( $message, $params->{'username'}, $params->{'password'}, $params->{'method'} ); > } >@@ -1108,31 +1124,24 @@ list of hashes, each has represents a message in the message queue. > sub GetQueuedMessages { > my $params = shift; > >- my $dbh = C4::Context->dbh(); >- my $statement = << 'ENDSQL'; >-SELECT message_id, borrowernumber, subject, content, message_transport_type, status, time_queued >-FROM message_queue >-ENDSQL >- >- my @query_params; >- my @whereclauses; >- if ( exists $params->{'borrowernumber'} ) { >- push @whereclauses, ' borrowernumber = ? '; >- push @query_params, $params->{'borrowernumber'}; >- } >- >- if ( @whereclauses ) { >- $statement .= ' WHERE ' . join( 'AND', @whereclauses ); >- } >- >- if ( defined $params->{'limit'} ) { >- $statement .= ' LIMIT ? '; >- push @query_params, $params->{'limit'}; >- } >- >- my $sth = $dbh->prepare( $statement ); >- my $result = $sth->execute( @query_params ); >- return $sth->fetchall_arrayref({}); >+ my $rs = Koha::Database->new->schema->resultset('MessageQueue')->search( >+ { >+ ( >+ $params->{borrowernumber} >+ ? ( borrowernumber => $params->{borrowernumber} ) >+ : () >+ ), >+ }, >+ { >+ ( >+ $params->{limit} >+ ? ( rows => $params->{limit} ) >+ : () >+ ), >+ }, >+ ); >+ $rs->result_class('DBIx::Class::ResultClass::HashRefInflator'); >+ return [ $rs->all ]; > } > > =head2 GetMessageTransportTypes >@@ -1288,22 +1297,14 @@ sub _send_message_by_email { > my ($username, $password, $method) = @_; > > my $member = C4::Members::GetMember( 'borrowernumber' => $message->{'borrowernumber'} ); >- my $to_address = $message->{'to_address'}; >- unless ($to_address) { >- unless ($member) { >- warn "FAIL: No 'to_address' and INVALID borrowernumber ($message->{borrowernumber})"; >- _set_message_status( { message_id => $message->{'message_id'}, >- status => 'failed' } ); >- return; >- } >- $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? >- _set_message_status( { message_id => $message->{'message_id'}, >- status => 'failed' } ); >- return; >- } >+ >+ unless ( $member and $message->{to_address} ) { >+ warn "FAIL: No 'to_address' and INVALID borrowernumber ($message->{borrowernumber})" unless $member; >+ _set_message_status({ >+ message_id => $message->{'message_id'}, >+ status => 'failed' >+ }); >+ return; > } > > my $utf8 = decode('MIME-Header', $message->{'subject'} ); >@@ -1324,7 +1325,6 @@ sub _send_message_by_email { > my $email = Koha::Email->new(); > my %sendmail_params = $email->create_message_headers( > { >- to => $to_address, > from => $message->{'from_address'} || $branch_email, > replyto => $branch_replyto, > sender => $branch_returnpath, >@@ -1414,12 +1414,6 @@ sub _send_message_by_sms { > return $success; > } > >-sub _update_message_to_address { >- my ($id, $to)= @_; >- my $dbh = C4::Context->dbh(); >- $dbh->do('UPDATE message_queue SET to_address=? WHERE message_id=?',undef,($to,$id)); >-} >- > sub _set_message_status { > my $params = shift or return; > >diff --git a/C4/Members.pm b/C4/Members.pm >index 8953d85..f3eb147 100644 >--- a/C4/Members.pm >+++ b/C4/Members.pm >@@ -68,7 +68,7 @@ BEGIN { > &GetAllIssues > > &GetFirstValidEmailAddress >- &GetNoticeEmailAddress >+ &GetNoticeEmailAddresses > > &GetAge > &GetTitles >@@ -1135,33 +1135,38 @@ sub GetFirstValidEmailAddress { > } > } > >-=head2 GetNoticeEmailAddress >+=head2 GetNoticeEmailAddresses > >- $email = GetNoticeEmailAddress($borrowernumber); >+ $emails = GetNoticeEmailAddresses( borrowernumber ); > >-Return the email address of borrower used for notices, given the borrowernumber. >-Returns the empty string if no email address. >+Return the email addresses of borrower used for notices, given the borrowernumber. >+Returns an empty arrayref if no email address. > > =cut > >-sub GetNoticeEmailAddress { >+sub GetNoticeEmailAddresses { > 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); >+ my $which_addresses = C4::Context->preference("AutoEmailPrimaryAddress"); >+ my @email_addresses; >+ for my $email_address_field ( split /,/, $which_addresses ) { >+ >+ # if syspref is set to 'first valid' (value == OFF), look up email address >+ if ( $email_address_field eq 'OFF' ) { >+ return [ GetFirstValidEmailAddress($borrowernumber) ]; >+ } >+ >+ # specified email address field >+ my $email_address = C4::Context->dbh->selectcol_arrayref( >+ qq| >+ SELECT $email_address_field AS email >+ FROM borrowers >+ WHERE borrowernumber = ? >+ |, {}, $borrowernumber >+ ); >+ push @email_addresses, $email_address->[0] if $email_address->[0]; > } >- # 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'} || ''; >+ return \@email_addresses; > } > > =head2 GetUpcomingMembershipExpires >diff --git a/C4/Message.pm b/C4/Message.pm >index 30b71c8..800b82f 100644 >--- a/C4/Message.pm >+++ b/C4/Message.pm >@@ -158,7 +158,6 @@ the message. > sub enqueue { > my ($class, $letter, $borrower, $transport) = @_; > my $metadata = _metadata($letter); >- my $to_address = _to_address($borrower, $transport); > > # Same as render_metadata > my $format ||= sub { $_[0] || "" }; >@@ -170,34 +169,9 @@ sub enqueue { > letter => $letter, > borrowernumber => $borrower->{borrowernumber}, > message_transport_type => $transport, >- to_address => $to_address, > }); > } > >-# based on message $transport, pick an appropriate address to send to >-sub _to_address { >- my ($borrower, $transport) = @_; >- my $address; >- if ($transport eq 'email') { >- $address = $borrower->{email} >- || $borrower->{emailpro} >- || $borrower->{B_email}; >- } elsif ($transport eq 'sms') { >- $address = $borrower->{smsalertnumber} >- || $borrower->{phone} >- || $borrower->{phonepro} >- || $borrower->{B_phone}; >- } else { >- warn "'$transport' is an unknown message transport."; >- } >- if (not defined $address) { >- warn "An appropriate $transport address " >- . "for borrower $borrower->{userid} " >- . "could not be found."; >- } >- return $address; >-} >- > # _metadata($letter) -- return the letter split into head/body/footer > sub _metadata { > my ($letter) = @_; >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index ae1c7b7..aeace61 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -248,8 +248,7 @@ sub AddReserve { > { letter => $letter, > borrowernumber => $borrowernumber, > message_transport_type => 'email', >- from_address => $admin_email_address, >- to_address => $admin_email_address, >+ to_address => $admin_email_address, > } > ); > } >@@ -1990,9 +1989,6 @@ sub _koha_notify_reserve { > my $dbh = C4::Context->dbh; > my $borrower = C4::Members::GetMember(borrowernumber => $borrowernumber); > >- # Try to get the borrower's email address >- my $to_address = C4::Members::GetNoticeEmailAddress($borrowernumber); >- > my $messagingprefs = C4::Members::Messaging::GetMessagingPreferences( { > borrowernumber => $borrowernumber, > message_name => 'Hold_Filled' >@@ -2039,14 +2035,16 @@ sub _koha_notify_reserve { > C4::Letters::EnqueueLetter( { > letter => $letter, > borrowernumber => $borrowernumber, >- from_address => $admin_email_address, > message_transport_type => $mtt, > } ); > }; > >+ # Try to get the borrower's email addresses >+ my $to_addresses = C4::Members::GetNoticeEmailAddresses($borrowernumber); >+ > while ( my ( $mtt, $letter_code ) = each %{ $messagingprefs->{transports} } ) { > next if ( >- ( $mtt eq 'email' and not $to_address ) # No email address >+ ( $mtt eq 'email' and not @$to_addresses ) # No email address > or ( $mtt eq 'sms' and not $borrower->{smsalertnumber} ) # No SMS number > or ( $mtt eq 'phone' and C4::Context->preference('TalkingTechItivaPhoneNotification') ) # Notice is handled by TalkingTech_itiva_outbound.pl > ); >diff --git a/misc/cronjobs/overdue_notices.pl b/misc/cronjobs/overdue_notices.pl >index 7504c7a..c1d0eba 100755 >--- a/misc/cronjobs/overdue_notices.pl >+++ b/misc/cronjobs/overdue_notices.pl >@@ -578,8 +578,6 @@ END_SQL > and warn "borrower $borr has items triggering level $i."; > > @emails_to_use = (); >- my $notice_email = >- C4::Members::GetNoticeEmailAddress($borrowernumber); > unless ($nomail) { > if (@emails) { > foreach (@emails) { >@@ -587,7 +585,9 @@ END_SQL > } > } > else { >- push @emails_to_use, $notice_email if ($notice_email); >+ my $notice_emails = >+ C4::Members::GetNoticeEmailAddresses($borrowernumber); >+ push @emails_to_use, @$notice_emails; > } > } > >@@ -732,7 +732,7 @@ END_SQL > letternumber => $i, > postcode => $data->{'zipcode'}, > country => $data->{'country'}, >- email => $notice_email, >+ email => $notice_emails, > itemcount => $itemcount, > titles => $titles, > outputformat => defined $csvfilename ? 'csv' : defined $htmlfilename ? 'html' : defined $text_filename ? 'text' : '', >@@ -751,7 +751,7 @@ END_SQL > city => $data->{'city'}, > postcode => $data->{'zipcode'}, > country => $data->{'country'}, >- email => $notice_email, >+ email => ( scalar( @emails_to_use ) ? $emails_to_use[0] : undef ), > itemcount => $itemcount, > titles => $titles, > outputformat => defined $csvfilename ? 'csv' : defined $htmlfilename ? 'html' : defined $text_filename ? 'text' : '', >diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl >index 685d1ca..49eb256 100755 >--- a/opac/opac-memberentry.pl >+++ b/opac/opac-memberentry.pl >@@ -156,11 +156,9 @@ if ( $action eq 'create' ) { > > C4::Letters::EnqueueLetter( > { >+ borrowernumber => $borrower{borrowernumber}, > letter => $letter, > message_transport_type => 'email', >- to_address => $borrower{'email'}, >- from_address => >- C4::Context->preference('KohaAdminEmailAddress'), > } > ); > } >diff --git a/opac/opac-shareshelf.pl b/opac/opac-shareshelf.pl >index b2f5c8b..191cfe0 100755 >--- a/opac/opac-shareshelf.pl >+++ b/opac/opac-shareshelf.pl >@@ -166,8 +166,9 @@ sub show_accept { > sub notify_owner { > my ($param) = @_; > >- my $toaddr = C4::Members::GetNoticeEmailAddress( $param->{owner} ); >- return if !$toaddr; >+ my $toaddr = C4::Members::GetNoticeEmailAddresses( $param->{owner} ); >+ return unless scalar( @$toaddr ); >+ > > #prepare letter > my $letter = C4::Letters::GetPreparedLetter( >@@ -181,10 +182,9 @@ sub notify_owner { > #send letter to queue > C4::Letters::EnqueueLetter( > { >+ borrowernumber => $param->{owner}, > letter => $letter, > message_transport_type => 'email', >- from_address => C4::Context->preference('KohaAdminEmailAddress'), >- to_address => $toaddr, > } > ); > } >@@ -210,7 +210,6 @@ sub process_addrlist { > > sub send_invitekey { > my ($param) = @_; >- my $fromaddr = C4::Context->preference('KohaAdminEmailAddress'); > my $url = > C4::Context->preference('OPACBaseURL') > . "/cgi-bin/koha/opac-shareshelf.pl?shelfnumber=" >@@ -251,7 +250,6 @@ sub send_invitekey { > { > letter => $letter, > message_transport_type => 'email', >- from_address => $fromaddr, > to_address => $a, > } > ); >diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t >index 9b5c469..6e4b4c5 100644 >--- a/t/db_dependent/Letters.t >+++ b/t/db_dependent/Letters.t >@@ -18,7 +18,7 @@ > # along with Koha; if not, see <http://www.gnu.org/licenses>. > > use Modern::Perl; >-use Test::More tests => 80; >+use Test::More tests => 86; > use Test::MockModule; > use Test::Warn; > >@@ -63,12 +63,15 @@ my $library = $builder->build({ > }); > my $patron_category = $builder->build({ source => 'Category' }); > my $date = dt_from_string; >+my ( $email, $emailpro ) = ( 'email@example.org', 'emailpro@example.org' ); > my $borrowernumber = AddMember( > firstname => 'Jane', > surname => 'Smith', > categorycode => $patron_category,, > branchcode => $library->{branchcode}, > dateofbirth => $date, >+ email => $email, >+ emailpro => $emailpro, > ); > > my $marc_record = MARC::Record->new; >@@ -94,8 +97,7 @@ my $my_message = { > to_address => 'to@example.com', > from_address => 'from@example.com', > }; >-my $message_id = C4::Letters::EnqueueLetter($my_message); >-is( $message_id, undef, 'EnqueueLetter without the letter argument returns undef' ); >+is( C4::Letters::EnqueueLetter($my_message), undef, 'EnqueueLetter without the letter argument returns undef' ); > > delete $my_message->{message_transport_type}; > $my_message->{letter} = { >@@ -105,13 +107,11 @@ $my_message->{letter} = { > code => 'TEST_MESSAGE', > content_type => 'text/plain', > }; >-$message_id = C4::Letters::EnqueueLetter($my_message); >-is( $message_id, undef, 'EnqueueLetter without the message type argument argument returns undef' ); >+is( C4::Letters::EnqueueLetter($my_message), undef, 'EnqueueLetter without the message_transport_type argument returns undef' ); > > $my_message->{message_transport_type} = 'sms'; >-$message_id = C4::Letters::EnqueueLetter($my_message); >-ok(defined $message_id && $message_id > 0, 'new message successfully queued'); >- >+my $messages_queued = C4::Letters::EnqueueLetter($my_message); >+is($messages_queued, 1, 'new message successfully queued'); > > # GetQueuedMessages > my $messages = C4::Letters::GetQueuedMessages(); >@@ -119,7 +119,6 @@ is( @$messages, 1, 'GetQueuedMessages without argument returns all the entries' > > $messages = C4::Letters::GetQueuedMessages({ borrowernumber => $borrowernumber }); > is( @$messages, 1, 'one message stored for the borrower' ); >-is( $messages->[0]->{message_id}, $message_id, 'EnqueueLetter returns the message id correctly' ); > is( $messages->[0]->{borrowernumber}, $borrowernumber, 'EnqueueLetter stores the borrower number correctly' ); > is( $messages->[0]->{subject}, $my_message->{letter}->{title}, 'EnqueueLetter stores the subject correctly' ); > is( $messages->[0]->{content}, $my_message->{letter}->{content}, 'EnqueueLetter stores the content correctly' ); >@@ -453,6 +452,64 @@ is($mail{'To'}, 'testemail@mydomain.com', "mailto correct in sent claim"); > is($mail{'Message'}, 'my vendor|John Smith|Ordernumber ' . $ordernumber . ' (Silence in the library) (1 ordered)', 'Claim notice text constructed successfully'); > } > >+# EnqueueLetter and AutoEmailPrimaryAddress >+$my_message = { >+ borrowernumber => $borrowernumber, >+ message_transport_type => 'email', >+ to_address => 'to@example.com', >+ from_address => 'from@example.com', >+ letter => { >+ content => 'a message', >+ title => 'message title', >+ metadata => 'metadata', >+ code => 'TEST_MESSAGE', >+ content_type => 'text/plain', >+ }, >+}; >+ >+$dbh->do(q|DELETE FROM message_queue|); >+t::lib::Mocks::mock_preference('AutoEmailPrimaryAddress', 'email'); >+is( C4::Letters::EnqueueLetter($my_message), 1, 'message successfully queued' ); >+$messages = C4::Letters::GetQueuedMessages({ borrowernumber => $borrowernumber }); >+is( scalar( @$messages ), 1, 'The message has been queued for the expected patron' ); >+is( $messages->[0]{to_address}, 'to@example.com', 'AutoEmailPrimaryAddress=email, EnqueueLetter used the to_address given in parameter' ); >+ >+$dbh->do(q|DELETE FROM message_queue|); >+delete $my_message->{to_address}; >+is( C4::Letters::EnqueueLetter($my_message), 1, 'message successfully queued' ); >+$messages = C4::Letters::GetQueuedMessages({ borrowernumber => $borrowernumber }); >+is( scalar( @$messages ), 1, 'The message has been queued for the expected patron' ); >+is( $messages->[0]{to_address}, $email, 'AutoEmailPrimaryAddress=email, EnqueueLetter used the patron email address if no to_address is given in parameter' ); >+ >+$dbh->do(q|DELETE FROM message_queue|); >+t::lib::Mocks::mock_preference('AutoEmailPrimaryAddress', 'emailpro'); >+is( C4::Letters::EnqueueLetter($my_message), 1, 'message successfully queued' ); >+$messages = C4::Letters::GetQueuedMessages({ borrowernumber => $borrowernumber }); >+is( scalar( @$messages ), 1, 'The message has been queued for the expected patron' ); >+is( $messages->[0]{to_address}, $emailpro, 'AutoEmailPrimaryAddress=emailpro, EnqueueLetter used the patron emailpro address if no to_address is given in parameter' ); >+ >+$dbh->do(q|DELETE FROM message_queue|); >+t::lib::Mocks::mock_preference('AutoEmailPrimaryAddress', 'OFF'); >+is( C4::Letters::EnqueueLetter($my_message), 1, 'message successfully queued' ); >+$messages = C4::Letters::GetQueuedMessages({ borrowernumber => $borrowernumber }); >+is( scalar( @$messages ), 1, 'The message has been queued for the expected patron' ); >+is( $messages->[0]{to_address}, $email, 'AutoEmailPrimaryAddress=OFF, EnqueueLetter used the patron email address if no to_address is given in parameter' ); >+ >+$dbh->do(q|DELETE FROM message_queue|); >+t::lib::Mocks::mock_preference('AutoEmailPrimaryAddress', 'email|OFF|emailpro'); # This is no consistent. OFF should be alone. >+is( C4::Letters::EnqueueLetter($my_message), 1, 'message successfully queued' ); >+$messages = C4::Letters::GetQueuedMessages({ borrowernumber => $borrowernumber }); >+is( scalar( @$messages ), 1, 'The message has been queued for the expected patron' ); >+is( $messages->[0]{to_address}, $email, 'AutoEmailPrimaryAddress=email|OFF|emailpro, EnqueueLetter used the patron email address if no to_address is given in parameter' ); >+ >+$dbh->do(q|DELETE FROM message_queue|); >+t::lib::Mocks::mock_preference('AutoEmailPrimaryAddress', 'email|emailpro'); >+is( C4::Letters::EnqueueLetter($my_message), 2, 'messages successfully queued' ); >+$messages = C4::Letters::GetQueuedMessages({ borrowernumber => $borrowernumber }); >+is( scalar( @$messages ), 2, 'The messages have been queued for the expected patron' ); >+is( $messages->[0]{to_address}, $email, 'AutoEmailPrimaryAddress=email|emailpro, EnqueueLetter used the patron email address for the first letter' ); >+is( $messages->[1]{to_address}, $emailpro, 'AutoEmailPrimaryAddress=email|emailpro, EnqueueLetter used the patron emailpro address for the second letter' ); >+ > { > use C4::Serials; > >diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t >index 963d790..97c20a7 100755 >--- a/t/db_dependent/Members.t >+++ b/t/db_dependent/Members.t >@@ -139,15 +139,23 @@ is ($checkcardnum, "2", "Card number is too long"); > > t::lib::Mocks::mock_preference( 'AutoEmailPrimaryAddress', 'OFF' ); > C4::Context->clear_syspref_cache(); >+my $notice_emails = GetNoticeEmailAddresses($member->{'borrowernumber'}); >+is_deeply( $notice_emails, [ $EMAIL ], "GetNoticeEmailAddresses returns correct value when AutoEmailPrimaryAddress is off" ); > >-my $notice_email = GetNoticeEmailAddress($member->{'borrowernumber'}); >-is ($notice_email, $EMAIL, "GetNoticeEmailAddress returns correct value when AutoEmailPrimaryAddress is off"); >+C4::Context->set_preference( 'AutoEmailPrimaryAddress', 'emailpro,OFF' ); >+C4::Context->clear_syspref_cache(); >+$notice_emails = GetNoticeEmailAddresses($member->{'borrowernumber'}); >+is_deeply( $notice_emails, [ $EMAIL ], "GetNoticeEmailAddresses returns correct value when AutoEmailPrimaryAddress contains off" ); > > t::lib::Mocks::mock_preference( 'AutoEmailPrimaryAddress', 'emailpro' ); > C4::Context->clear_syspref_cache(); >+$notice_emails = GetNoticeEmailAddresses($member->{'borrowernumber'}); >+is_deeply ($notice_emails, [ $EMAILPRO ], "GetNoticeEmailAddresses returns correct value when AutoEmailPrimaryAddress is emailpro"); > >-$notice_email = GetNoticeEmailAddress($member->{'borrowernumber'}); >-is ($notice_email, $EMAILPRO, "GetNoticeEmailAddress returns correct value when AutoEmailPrimaryAddress is emailpro"); >+C4::Context->set_preference( 'AutoEmailPrimaryAddress', 'emailpro,email' ); >+C4::Context->clear_syspref_cache(); >+$notice_emails = GetNoticeEmailAddresses($member->{'borrowernumber'}); >+is_deeply ($notice_emails, [ $EMAILPRO, $EMAIL ], "GetNoticeEmailAddresses returns correct value when AutoEmailPrimaryAddress is emailpro|email"); > > ok(!$member->{is_expired}, "GetMemberDetails() indicates that patron is not expired"); > ModMember(borrowernumber => $member->{'borrowernumber'}, dateexpiry => '2001-01-1'); >-- >1.7.10.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 12802
:
31058
|
31059
|
31132
|
31382
|
31383
|
31384
|
31833
|
33864
|
33865
|
33866
|
33867
|
35890
|
35891
|
35892
|
35893
|
35926
|
35927
|
35928
|
35929
|
35930
|
36055
|
36056
|
36057
|
36058
|
36059
|
39762
|
39770
|
40167
|
45476
|
45477
|
45478
|
45479
|
45480
|
45481
|
45482
|
57296
|
61878
|
61879
|
61880
|
61881
|
61882
|
61883
|
61884
|
66934
|
66935
|
66936
|
66937
|
66938
|
66939
|
66940
|
68002
|
68003
|
68004
|
68005
|
68006
|
68007
|
68008
|
68009
|
68010
|
68011
|
68012
|
68013
|
68375
|
68376
|
68377
|
68378
|
68379
|
68380
|
119016
|
149232
|
153540
|
154076
|
159780
|
159857
|
160051
|
160052
|
160090
|
160091
|
160092
|
160103
|
160104
|
160105
|
160106
|
165494
|
165495
|
165496
|
165497