From 0530acc63fb986bcae47244f80183cc192bd8a16 Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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). This patch also adds a new routine C4::Branch::GetBranchEmailFromBorrowernumber (UT provided). 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. --- C4/Branch.pm | 8 +++ C4/Letters.pm | 136 ++++++++++++++++++--------------------- C4/Members.pm | 45 +++++++------ C4/Message.pm | 28 +------- C4/Reserves.pm | 14 ++-- misc/cronjobs/overdue_notices.pl | 9 ++- opac/opac-memberentry.pl | 4 +- opac/opac-shareshelf.pl | 10 ++- t/db_dependent/Branch.t | 26 ++++++-- t/db_dependent/Letters.t | 76 +++++++++++++++++++--- t/db_dependent/Members.t | 18 ++++-- 11 files changed, 211 insertions(+), 163 deletions(-) diff --git a/C4/Branch.pm b/C4/Branch.pm index 83d2010..bce29f4 100644 --- a/C4/Branch.pm +++ b/C4/Branch.pm @@ -20,6 +20,7 @@ use strict; #use warnings; FIXME - Bug 2505 require Exporter; use C4::Context; +use Koha::Database; use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS); @@ -564,6 +565,13 @@ sub GetBranchesCount { return $row->{'branches_count'}; } +sub GetBranchEmailFromBorrowernumber { + my ( $borrowernumber ) = @_; + return unless $borrowernumber; + my $rs = Koha::Database->new->schema->resultset('Borrower')->search({ borrowernumber => $borrowernumber }, {join => 'branchcode'}); + return $rs->next->branchcode->branchemail; +} + 1; __END__ diff --git a/C4/Letters.pm b/C4/Letters.pm index a3fcbb7..8501779 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -31,6 +31,7 @@ use C4::Log; use C4::SMS; use C4::Debug; use Koha::DateUtils; +use Koha::Database; use Date::Calc qw( Add_Delta_Days ); use Encode; use Carp; @@ -676,7 +677,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}; @@ -696,28 +696,40 @@ 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 = @{ 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} ) { + $from_address = C4::Branch::GetBranchEmailFromBorrowernumber( $params->{borrowernumber} ); + } + + $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]) @@ -742,6 +754,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'} ); } @@ -803,31 +816,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 @@ -933,22 +939,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'} ); @@ -958,11 +956,10 @@ sub _send_message_by_email { my $content_type = $message->{'content_type'} || 'text/plain; charset="UTF-8"'; my $is_html = $content_type =~ m/html/io; - my $branch_email = ( $member ) ? GetBranchDetail( $member->{'branchcode'} )->{'branchemail'} : undef; my %sendmail_params = ( - To => $to_address, - From => $message->{'from_address'} || $branch_email || C4::Context->preference('KohaAdminEmailAddress'), + To => $message->{to_address}, + From => $message->{from_address}, Subject => $subject, charset => 'utf8', Message => $is_html ? _wrap_html($content, $subject) : $content, @@ -973,7 +970,6 @@ sub _send_message_by_email { $sendmail_params{ Bcc } = $bcc; } - _update_message_to_address($message->{'message_id'},$to_address) unless $message->{to_address}; #if initial message address was empty, coming here means that a to address was found and queue should be updated if ( sendmail( %sendmail_params ) ) { _set_message_status( { message_id => $message->{'message_id'}, status => 'sent' } ); @@ -1047,12 +1043,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 326344d..f6429e9 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -67,7 +67,7 @@ BEGIN { &getidcity &GetFirstValidEmailAddress - &GetNoticeEmailAddress + &GetNoticeEmailAddresses &GetAge &GetCities @@ -1489,33 +1489,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 GetExpiryDate diff --git a/C4/Message.pm b/C4/Message.pm index a63b3ca..77819c9 100644 --- a/C4/Message.pm +++ b/C4/Message.pm @@ -158,41 +158,15 @@ the message. sub enqueue { my ($class, $letter, $borrower, $transport) = @_; my $metadata = _metadata($letter); - my $to_address = _to_address($borrower, $transport); $letter->{metadata} = Dump($metadata); - #carp "enqueuing... to $to_address"; + C4::Letters::EnqueueLetter({ 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 a98eed4..bbc9fc5 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -225,8 +225,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, } ); } @@ -1887,9 +1886,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, @@ -1906,8 +1902,6 @@ sub _koha_notify_reserve { my $reserve = $sth->fetchrow_hashref; my $branch_details = GetBranchDetail( $reserve->{'branchcode'} ); - my $admin_email_address = $branch_details->{'branchemail'} || C4::Context->preference('KohaAdminEmailAddress'); - my %letter_params = ( module => 'reserves', branchcode => $reserve->{branchcode}, @@ -1936,13 +1930,15 @@ 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} } ) { - if ( ($mtt eq 'email' and not $to_address) or ($mtt eq 'sms' and not $borrower->{smsalertnumber}) ) { + if ( ($mtt eq 'email' and not @$to_addresses) or ($mtt eq 'sms' and not $borrower->{smsalertnumber}) ) { # email or sms is requested but not exist next; } diff --git a/misc/cronjobs/overdue_notices.pl b/misc/cronjobs/overdue_notices.pl index a0a1254..302e06b 100755 --- a/misc/cronjobs/overdue_notices.pl +++ b/misc/cronjobs/overdue_notices.pl @@ -551,8 +551,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) { @@ -560,7 +558,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; } } @@ -682,7 +682,7 @@ END_SQL city => $data->{'city'}, 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' : '', @@ -699,7 +699,6 @@ END_SQL { letter => $letter, borrowernumber => $borrowernumber, message_transport_type => $mtt, - from_address => $admin_email_address, to_address => join(',', @emails_to_use), } ); diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl index ba9eca4..55cde8c 100755 --- a/opac/opac-memberentry.pl +++ b/opac/opac-memberentry.pl @@ -122,11 +122,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 669629b..9445a14 100755 --- a/opac/opac-shareshelf.pl +++ b/opac/opac-shareshelf.pl @@ -133,8 +133,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( @@ -148,10 +149,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, } ); } @@ -177,7 +177,6 @@ sub process_addrlist { sub send_invitekey { my ($param) = @_; - my $fromaddr = C4::Context->preference('KohaAdminEmailAddress'); my $url = 'http://' . C4::Context->preference('OPACBaseURL') @@ -215,7 +214,6 @@ sub send_invitekey { { letter => $letter, message_transport_type => 'email', - from_address => $fromaddr, to_address => $a, } ); diff --git a/t/db_dependent/Branch.t b/t/db_dependent/Branch.t index 338ffe4..4d4ee8c 100644 --- a/t/db_dependent/Branch.t +++ b/t/db_dependent/Branch.t @@ -21,9 +21,10 @@ use Modern::Perl; use C4::Context; use Data::Dumper; -use Test::More tests => 36; +use Test::More tests => 38; use C4::Branch; +use C4::Members qw( AddMember ); BEGIN { use FindBin; @@ -297,6 +298,7 @@ my $brCat1 = GetBranchesInCategory( $cat1->{categorycode} ); my @b = ( $b2->{branchcode} ); is_deeply( $brCat1, \@b, 'CAT1 has branch BRB' ); +my $b3_email = 'email_b3@example.org'; my $b3 = { add => 1, branchcode => 'BRC', @@ -310,7 +312,7 @@ my $b3 = { branchcountry => 'countryC', branchphone => 'phoneC', branchfax => 'faxC', - branchemail => 'emailC', + branchemail => $b3_email, branchurl => 'urlC', branchip => 'ipC', branchprinter => undef, @@ -347,6 +349,20 @@ is_deeply($categories, [ {%$cat1}, {%$cat2},{ %new_category, selected => 1 } ], my $loop = GetBranchesLoop; is( scalar(@$loop), GetBranchesCount(), 'There is the right number of branches' ); -# End transaction -$dbh->rollback; - +# Test GetBranchEmailFromBorrowernumber +my $borrowernumber = C4::Members::AddMember( + cardnumber => 'CARDNUMBER42_TEST', + firstname => 'fn', + surname => 'surname', + categorycode => 'S', + branchcode => $b3->{branchcode}, + dateofbirth => '', + dateexpiry => '9999-12-31', + userid => 'userid' +); +is( C4::Branch::GetBranchEmailFromBorrowernumber($borrowernumber), $b3_email, +'GetBranchEmailFromBorrowernumber returns the branchemail from a borrowernumber' +); +is( C4::Branch::GetBranchEmailFromBorrowernumber(), undef, +'GetBranchEmailFromBorrowernumber returns undef if no parameter is given' +); diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t index 89e511a..29edb35 100644 --- a/t/db_dependent/Letters.t +++ b/t/db_dependent/Letters.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 45; +use Test::More tests => 63; use MARC::Record; use C4::Biblio qw( AddBiblio ); @@ -41,12 +41,15 @@ $dbh->do(q|DELETE FROM message_queue|); $dbh->do(q|DELETE FROM message_transport_types|); my $date = dt_from_string; +my ( $email, $emailpro ) = ( 'email@example.org', 'emailpro@example.org' ); my $borrowernumber = AddMember( firstname => 'Jane', surname => 'Smith', categorycode => 'PT', branchcode => 'CPL', dateofbirth => $date, + email => $email, + emailpro => $emailpro, ); my $marc_record = MARC::Record->new; @@ -72,8 +75,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} = { @@ -83,13 +85,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(); @@ -97,7 +97,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' ); @@ -260,4 +259,61 @@ $prepared_letter = GetPreparedLetter(( $my_content_letter = qq|This is a SMS for an $substitute->{status}|; is( $prepared_letter->{content}, $my_content_letter, 'GetPreparedLetter returns the content correctly' ); -$dbh->rollback; + +# 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' ); diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t index 36f7ccc..2412ce0 100755 --- a/t/db_dependent/Members.t +++ b/t/db_dependent/Members.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 55; +use Test::More tests => 57; use Data::Dumper; use C4::Context; @@ -189,15 +189,23 @@ is ($age, "-19", "Birthday In the Future"); C4::Context->set_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" ); C4::Context->set_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'); -- 2.0.0.rc2