From ad63540107ae74cf6330996cdaa1e8d0c01f335e Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 21 Aug 2014 13:15:36 +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. What this patch does: The Koha::Patron::notice_email_address subroutine has been renamed to Koha::Patron::notice_email_addresses (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. Important note for testers: Pending messages must be removed each time you change the value of AutoEmailPrimaryAddress. When there are already existing pending messages for a borrower, Koha tries to update them rather than enqueuing new ones, but it will not update the recipient's email address, nor add/remove messages depending on the new value of AutoEmailPrimaryAddress; not sure if this is a bug or not. --- C4/Auth.pm | 2 +- C4/Auth_with_ldap.pm | 11 +- C4/Auth_with_shibboleth.pm | 11 +- C4/Circulation.pm | 10 +- C4/Letters.pm | 140 +++++++++++------- C4/Message.pm | 43 +----- C4/Reserves.pm | 13 +- Koha/Patron.pm | 44 +++--- Koha/Patron/Password/Recovery.pm | 9 +- Koha/Patrons/Import.pm | 9 +- Koha/REST/V1/TwoFactorAuth.pm | 20 +-- Koha/Ticket.pm | 19 ++- circ/pendingreserves.pl | 2 +- .../data/mysql/atomicupdate/bug-12802.pl | 12 ++ installer/data/mysql/mandatory/sysprefs.sql | 2 +- .../en/modules/admin/preferences/patrons.pref | 2 +- .../intranet-tmpl/prog/en/modules/auth.tt | 2 +- members/memberentry.pl | 11 +- members/notices.pl | 13 +- members/two_factor_auth.pl | 2 +- .../notice_unprocessed_suggestions.pl | 4 +- misc/cronjobs/overdue_notices.pl | 8 +- .../thirdparty/TalkingTech_itiva_outbound.pl | 7 +- opac/opac-memberentry.pl | 21 +-- opac/opac-registration-verify.pl | 11 +- opac/opac-shareshelf.pl | 7 +- opac/opac-shelves.pl | 4 +- pos/pay.pl | 6 +- suggestion/suggestion.pl | 5 +- t/db_dependent/Koha/Patrons.t | 17 ++- t/db_dependent/Koha/Recall.t | 2 +- t/db_dependent/Letters.t | 106 +++++++++++-- t/db_dependent/Message.t | 6 +- 33 files changed, 351 insertions(+), 230 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug-12802.pl diff --git a/C4/Auth.pm b/C4/Auth.pm index adc1b5dd0d..655e85863b 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -1390,7 +1390,7 @@ sub checkauth { $template->param( TwoFA_prompt => 1, invalid_otp_token => $invalid_otp_token, - notice_email_address => $patron->notice_email_address, # We could also pass logged_in_user if necessary + notice_email_addresses => $patron->notice_email_addresses, # We could also pass logged_in_user if necessary ); } diff --git a/C4/Auth_with_ldap.pm b/C4/Auth_with_ldap.pm index f13dbdcf8e..f1fedd5026 100644 --- a/C4/Auth_with_ldap.pm +++ b/C4/Auth_with_ldap.pm @@ -236,10 +236,10 @@ sub checkpw_ldap { # Send welcome email if enabled if ( $config{welcome} ) { - my $emailaddr = $patron->notice_email_address; + my $emailaddrs = $patron->notice_email_addresses; # if we manage to find a valid email address, send notice - if ($emailaddr) { + if (@$emailaddrs) { my $letter = C4::Letters::GetPreparedLetter( module => 'members', letter_code => 'WELCOME', @@ -252,16 +252,17 @@ sub checkpw_ldap { want_librarian => 1, ) or return; - my $message_id = C4::Letters::EnqueueLetter( + my @message_ids = C4::Letters::EnqueueLetter( { letter => $letter, borrowernumber => $patron->id, - to_address => $emailaddr, message_transport_type => 'email' } ); - C4::Letters::SendQueuedMessages( { message_id => $message_id } ); + foreach my $message_id (@message_ids) { + C4::Letters::SendQueuedMessages( { message_id => $message_id } ); + } } } } else { diff --git a/C4/Auth_with_shibboleth.pm b/C4/Auth_with_shibboleth.pm index 9474fdb111..6bf8d77a66 100644 --- a/C4/Auth_with_shibboleth.pm +++ b/C4/Auth_with_shibboleth.pm @@ -144,10 +144,10 @@ sub _autocreate { # Send welcome email if enabled if ( $config->{welcome} ) { - my $emailaddr = $patron->notice_email_address; + my $emailaddrs = $patron->notice_email_addresses; # if we manage to find a valid email address, send notice - if ($emailaddr) { + if (@$emailaddrs) { my $letter = C4::Letters::GetPreparedLetter( module => 'members', letter_code => 'WELCOME', @@ -161,15 +161,16 @@ sub _autocreate { want_librarian => 1, ) or return; - my $message_id = C4::Letters::EnqueueLetter( + my @message_ids = C4::Letters::EnqueueLetter( { letter => $letter, borrowernumber => $patron->id, - to_address => $emailaddr, message_transport_type => 'email' } ); - C4::Letters::SendQueuedMessages( { message_id => $message_id } ); + foreach my $message_id (@message_ids) { + C4::Letters::SendQueuedMessages( { message_id => $message_id } ); + } } } return ( 1, $patron->cardnumber, $patron->userid ); diff --git a/C4/Circulation.pm b/C4/Circulation.pm index fe1991f2d1..864fb68c29 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3654,14 +3654,16 @@ sub SendCirculationAlert { C4::Context->dbh->do(q|LOCK TABLE message_queue READ|) unless $do_not_lock; C4::Context->dbh->do(q|LOCK TABLE message_queue WRITE|) unless $do_not_lock; - my $message = C4::Message->find_last_message($borrower, $type, $mtt); - unless ( $message ) { + my @messages = C4::Message->find_last_messages($borrower, $type, $mtt); + unless ( @messages ) { C4::Context->dbh->do(q|UNLOCK TABLES|) unless $do_not_lock; my $patron = Koha::Patrons->find($borrower->{borrowernumber}); C4::Message->enqueue($letter, $patron, $mtt); } else { - $message->append($letter); - $message->update; + foreach my $message (@messages) { + $message->append($letter); + $message->update; + } } C4::Context->dbh->do(q|UNLOCK TABLES|) unless $do_not_lock; } diff --git a/C4/Letters.pm b/C4/Letters.pm index 36d3f5d77e..15376fabd4 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -30,7 +30,7 @@ use C4::Log qw( logaction ); use C4::SMS; use C4::Templates; use Koha::SMS::Providers; - +use Koha::Database; use Koha::Email; use Koha::Notice::Messages; use Koha::Notice::Templates; @@ -880,20 +880,23 @@ sub _parseletter { =head2 EnqueueLetter - my $success = EnqueueLetter( { letter => $letter, - borrowernumber => '12', message_transport_type => 'email' } ) + my @message_ids = EnqueueLetter({ + letter => $letter, + borrowernumber => '12', + message_transport_type => 'email' + }); Places a letter in the message_queue database table, which will eventually get processed (sent) by the process_message_queue.pl cronjob when it calls SendQueuedMessages. -Return message_id on success +return the list of message ids Parameters * letter - required; A letter hashref as returned from GetPreparedLetter * message_transport_type - required; One of the available mtts * borrowernumber - optional if 'to_address' is passed; The borrowernumber of the patron we enqueuing the notice for -* to_address - optional if 'borrowernumber' is passed; The destination email address for the notice (defaults to patron->notice_email_address) +* to_address - optional if 'borrowernumber' is passed; The destination email address for the notice (defaults to patron->notice_email_addresses) * from_address - optional; The from address for the notice, defaults to patron->library->from_email_address * reply_address - optional; The reply address for the notice, defaults to patron->library->reply_to @@ -903,7 +906,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}; @@ -922,30 +924,73 @@ sub EnqueueLetter { ); } - my $dbh = C4::Context->dbh(); - my $statement = << 'ENDSQL'; -INSERT INTO message_queue -( letter_id, borrowernumber, subject, content, metadata, letter_code, message_transport_type, status, time_queued, to_address, from_address, reply_address, content_type, failure_code ) -VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, CAST(NOW() AS DATETIME), ?, ?, ?, ?, ? ) -ENDSQL - - my $sth = $dbh->prepare($statement); - my $result = $sth->execute( - $params->{letter}->{id} || undef, # letter.id - $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->{'reply_address'}, # reply_address - $params->{'letter'}->{'content-type'}, # content_type - $params->{'failure_code'} || '', # failure_code - ); - return $dbh->last_insert_id(undef,undef,'message_queue', undef); + my @to_addresses; + if ( $params->{to_address} ) { + @to_addresses = ( $params->{to_address} ); + } elsif ( $params->{borrowernumber} and $params->{message_transport_type} eq 'email' ) { + my $patron = Koha::Patrons->find( $params->{borrowernumber} ); + if ($patron) { + @to_addresses = @{ $patron->notice_email_addresses }; + } + } + + 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->schema->resultset('Borrower')->search( { borrowernumber => $params->{borrowernumber} }, { join => 'branchcode' } ); + $from_address = $rs->next->branchcode->branchemail; + } + + $from_address ||= C4::Context->preference('KohaAdminEmailAddress'); + } + + my @message_ids; + + my $rs = Koha::Database->schema->resultset('MessageQueue'); + if ( $params->{message_transport_type} eq 'email' ) { + for my $to_address (@to_addresses) { + my $message = $rs->create( + { letter_id => $params->{letter}->{id} || undef, + 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', + time_queued => \'NOW()', + to_address => $to_address, + from_address => $from_address, + reply_address => $params->{reply_address}, + content_type => $params->{letter}->{'content-type'}, + failure_code => $params->{failure_code} || '', + } + ); + push @message_ids, $message->id(); + } + } else { + my $message = $rs->create( + { letter_id => $params->{letter}->{id} || undef, + 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', + time_queued => \'NOW()', + from_address => $from_address, + reply_address => $params->{reply_address}, + content_type => $params->{letter}{'content-type'}, + failure_code => $params->{failure_code} || '', + } + ); + push @message_ids, $message->id(); + } + + return @message_ids; } =head2 SendQueuedMessages ([$hashref]) @@ -1085,31 +1130,15 @@ Return is an arrayref of hashes, each has represents a message in the message qu 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, updated_on, failure_code -FROM message_queue -ENDSQL - - my @query_params; - my @whereclauses; - if ( exists $params->{'borrowernumber'} ) { - push @whereclauses, ' borrowernumber = ? '; - push @query_params, $params->{'borrowernumber'}; - } + my ( %cond, %attrs ); - if ( @whereclauses ) { - $statement .= ' WHERE ' . join( 'AND', @whereclauses ); - } + $cond{borrowernumber} = $params->{borrowernumber} if $params->{borrowernumber}; - if ( defined $params->{'limit'} ) { - $statement .= ' LIMIT ? '; - push @query_params, $params->{'limit'}; - } + $attrs{rows} = $params->{limit} if $params->{limit}; - my $sth = $dbh->prepare( $statement ); - my $result = $sth->execute( @query_params ); - return $sth->fetchall_arrayref({}); + my $rs = Koha::Database->schema->resultset('MessageQueue')->search( \%cond, \%attrs ); + $rs->result_class('DBIx::Class::ResultClass::HashRefInflator'); + return [ $rs->all ]; } =head2 GetMessageTransportTypes @@ -1173,9 +1202,6 @@ sub ResendMessage { status => 'pending', }); $rv = $rv > 0? 1: 0; - # Clear destination email address to force address update - _update_message_to_address( $message_id, undef ) if $rv && - $message->{message_transport_type} eq 'email'; } return $rv; } @@ -1317,8 +1343,8 @@ sub _send_message_by_email { ); return; } - $to_address = $patron->notice_email_address; - unless ($to_address) { + my $to_addresses = $patron->notice_email_addresses; + unless (@$to_addresses) { # 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( diff --git a/C4/Message.pm b/C4/Message.pm index 68205083d5..ee8b75405a 100644 --- a/C4/Message.pm +++ b/C4/Message.pm @@ -54,7 +54,7 @@ How to update a borrower's last checkout message: use C4::Message; my $borrower = { borrowernumber => 1 }; - my $message = C4::Message->find_last_message($borrower, 'CHECKOUT', 'email'); + my $message = C4::Message->find_last_messages($borrower, 'CHECKOUT', 'email'); $message->append("you also checked out some other book...."); $message->update; @@ -111,17 +111,17 @@ sub find { } } -=head3 C4::Message->find_last_message($borrower, $letter_code, $transport) +=head3 C4::Message->find_last_messages($borrower, $letter_code, $transport) This method is used to get the borrower's most recent, pending, check-in or -checkout message. (This makes it possible to add more information to the +checkout messages. (This makes it possible to add more information to the message before it gets sent out.) =cut -# C4::Message->find_last_message($borrower, $letter_code, $transport) -# -- get the borrower's most recent pending checkin or checkout notification -sub find_last_message { +# C4::Message->find_last_messages($borrower, $letter_code, $transport) +# -- get the borrower's most recent pending checkin or checkout notifications +sub find_last_messages { my ($class, $borrower, $letter_code, $transport) = @_; # $type is the message_transport_type $transport ||= 'email'; @@ -140,11 +140,8 @@ sub find_last_message { $letter_code, $transport, ); - if (@$msgs) { - return $class->new($msgs->[0]); - } else { - return; - } + + return map { $class->new($_) } @$msgs; } @@ -159,7 +156,6 @@ the message. sub enqueue { my ( $class, $letter, $patron, $transport ) = @_; my $metadata = _metadata($letter); - my $to_address = _to_address( $patron, $transport ); # Same as render_metadata my $format ||= sub { $_[0] || "" }; @@ -172,33 +168,10 @@ sub enqueue { letter => $letter, borrowernumber => $patron->id, message_transport_type => $transport, - to_address => $to_address, } ); } -# based on message $transport, pick an appropriate address to send to -sub _to_address { - my ( $patron, $transport ) = @_; - my $address; - if ( $transport eq 'email' ) { - $address = $patron->notice_email_address; - } - elsif ( $transport eq 'sms' ) { - $address = $patron->smsalertnumber; - } - else { - warn "'$transport' is an unknown message transport."; - } - if ( not defined $address ) { - warn "An appropriate $transport address " - . "for borrower " - . $patron->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 987543e1cd..65c668c178 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -791,12 +791,13 @@ SELECT COUNT(*) FROM reserves WHERE biblionumber=? AND borrowernumber<>? Takes an itemnumber and returns the status of the reserve placed on it. If several reserves exist, the reserve with the lower priority is given. -=cut - ## FIXME: I don't think this does what it thinks it does. ## It only ever checks the first reserve result, even though ## multiple reserves for that bib can have the itemnumber set ## the sub is only used once in the codebase. + +=cut + sub GetReserveStatus { my ($itemnumber) = @_; @@ -1879,9 +1880,6 @@ sub _koha_notify_reserve { my $patron = Koha::Patrons->find( $borrowernumber ); - # Try to get the borrower's email address - my $to_address = $patron->notice_email_address; - my $messagingprefs = C4::Members::Messaging::GetMessagingPreferences( { borrowernumber => $borrowernumber, message_name => 'Hold_Filled' @@ -1924,9 +1922,12 @@ sub _koha_notify_reserve { } ); }; + # Try to get the borrower's email addresses + my $to_addresses = $patron->notice_email_addresses; + 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 $patron->smsalertnumber ) # No SMS number or ( $mtt eq 'itiva' and C4::Context->preference('TalkingTechItivaPhoneNotification') ) # Notice is handled by TalkingTech_itiva_outbound.pl or ( $mtt eq 'phone' and not $patron->phone ) # No phone number to call diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 9a532af2f6..e1f5e40c55 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -894,10 +894,10 @@ sub set_password { if ( C4::Context->preference('NotifyPasswordChange') ) { my $self_from_storage = $self->get_from_storage; if ( !C4::Auth::checkpw_hash( $password, $self_from_storage->password ) ) { - my $emailaddr = $self_from_storage->notice_email_address; + my $emailaddrs = $self_from_storage->notice_email_addresses; # if we manage to find a valid email address, send notice - if ($emailaddr) { + if (@$emailaddrs) { my $letter = C4::Letters::GetPreparedLetter( module => 'members', letter_code => 'PASSWORD_CHANGE', @@ -911,15 +911,16 @@ sub set_password { want_librarian => 1, ) or return; - my $message_id = C4::Letters::EnqueueLetter( + my @message_ids = C4::Letters::EnqueueLetter( { letter => $letter, borrowernumber => $self_from_storage->id, - to_address => $emailaddr, message_transport_type => 'email' } ); - C4::Letters::SendQueuedMessages( { message_id => $message_id } ); + foreach my $message_id (@message_ids) { + C4::Letters::SendQueuedMessages( { message_id => $message_id } ); + } } } } @@ -1388,25 +1389,30 @@ sub return_claims { return Koha::Checkouts::ReturnClaims->_new_from_dbic( $return_claims ); } -=head3 notice_email_address +=head3 notice_email_addresses - my $email = $patron->notice_email_address; + my $emails = $patron->notice_email_addresses; -Return the email address of patron used for notices. -Returns the empty string if no email address. +Return an arrayref containing the email addresses of patron used for notices. =cut -sub notice_email_address{ +sub notice_email_addresses { my ( $self ) = @_; - 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 $self->first_valid_email_address; + 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 [ $self->first_valid_email_address ]; + } + + my $email_address = $self->$email_address_field; + push @email_addresses, $email_address if $email_address; } - return $self->$which_address || ''; + return \@email_addresses; } =head3 first_valid_email_address @@ -2191,7 +2197,8 @@ sub queue_notice { foreach my $mtt (@message_transports){ next if ($mtt eq 'itiva' and C4::Context->preference('TalkingTechItivaPhoneNotification') ); # Notice is handled by TalkingTech_itiva_outbound.pl - if ( ( $mtt eq 'email' and not $self->notice_email_address ) + my $email_addresses = $self->notice_email_addresses; + if ( ( $mtt eq 'email' and not @$email_addresses ) or ( $mtt eq 'sms' and not $self->smsalertnumber ) or ( $mtt eq 'phone' and not $self->phone ) ) { @@ -2330,17 +2337,18 @@ sub notify_library_of_registration { || C4::Context->preference('KohaAdminEmailAddress'); } - my $message_id = C4::Letters::EnqueueLetter( + my ($message_id) = C4::Letters::EnqueueLetter( { letter => $letter, borrowernumber => $self->borrowernumber, to_address => $to_address, message_transport_type => 'email' } - ) or warn "can't enqueue letter $letter"; + ); if ( $message_id ) { return 1; } + warn "can't enqueue letter $letter"; } } diff --git a/Koha/Patron/Password/Recovery.pm b/Koha/Patron/Password/Recovery.pm index 1e462be368..6c8565b72e 100644 --- a/Koha/Patron/Password/Recovery.pm +++ b/Koha/Patron/Password/Recovery.pm @@ -148,7 +148,7 @@ sub SendPasswordRecoveryEmail { my $library = $borrower->library; my $kohaEmail = $library->from_email_address; # send from patron's branch or Koha Admin - my $message_id = C4::Letters::EnqueueLetter( + my @message_ids = C4::Letters::EnqueueLetter( { letter => $letter, borrowernumber => $borrower->borrowernumber, @@ -158,8 +158,11 @@ sub SendPasswordRecoveryEmail { } ); - my $num_letters_attempted = - C4::Letters::SendQueuedMessages( { message_id => $message_id } ); + my $num_letters_attempted = 0; + foreach my $message_id (@message_ids) { + $num_letters_attempted += + C4::Letters::SendQueuedMessages( { message_id => $message_id } ); + } return ($num_letters_attempted > 0); } diff --git a/Koha/Patrons/Import.pm b/Koha/Patrons/Import.pm index 3ba184d1e2..b7fdf41861 100644 --- a/Koha/Patrons/Import.pm +++ b/Koha/Patrons/Import.pm @@ -448,10 +448,10 @@ sub import_patrons { # Send WELCOME welcome email is the user is new and we're set to send mail if ($send_welcome && $is_new) { - my $emailaddr = $patron->notice_email_address; + my $emailaddrs = $patron->notice_email_addresses; # if we manage to find a valid email address, send notice - if ($emailaddr) { + if (@$emailaddrs) { eval { my $letter = GetPreparedLetter( module => 'members', @@ -465,11 +465,10 @@ sub import_patrons { want_librarian => 1, ) or return; - my $message_id = EnqueueLetter( + EnqueueLetter( { letter => $letter, borrowernumber => $patron->id, - to_address => $emailaddr, message_transport_type => 'email' } ); @@ -482,7 +481,7 @@ sub import_patrons { { feedback => 1, name => 'welcome_sent', - value => $borrower{'surname'} . ' / ' . $borrowernumber . ' / ' . $emailaddr + value => $borrower{'surname'} . ' / ' . $borrowernumber . ' / ' . join(', ', @$emailaddrs) } ); } diff --git a/Koha/REST/V1/TwoFactorAuth.pm b/Koha/REST/V1/TwoFactorAuth.pm index f9f199f74b..3c36477c4e 100644 --- a/Koha/REST/V1/TwoFactorAuth.pm +++ b/Koha/REST/V1/TwoFactorAuth.pm @@ -55,22 +55,24 @@ sub send_otp_token { borrowers => $patron->unblessed, } ); - my $message_id = C4::Letters::EnqueueLetter( + my @message_ids = C4::Letters::EnqueueLetter( { letter => $letter, borrowernumber => $patron->borrowernumber, message_transport_type => 'email' } ); - C4::Letters::SendQueuedMessages({message_id => $message_id}); + foreach my $message_id (@message_ids) { + C4::Letters::SendQueuedMessages( { message_id => $message_id } ); - my $message = C4::Letters::GetMessage($message_id); - - if ( $message->{status} eq 'sent' ) { - return $c->render(status => 200, openapi => {}); - } elsif ( $message->{status} eq 'failed' ) { - return $c->render(status => 400, openapi => { error => 'email_not_sent'}); + my $message = C4::Letters::GetMessage($message_id); + if ( $message->{status} ne 'sent' ) { + return $c->render( status => 400, openapi => { error => 'email_not_sent' } ); + } } + + return $c->render( status => 200, openapi => {} ); + } catch { $c->unhandled_exception($_); @@ -155,7 +157,7 @@ sub verification { # FIXME Generate a (new?) secret $patron->encode_secret($secret32); $patron->auth_method('two-factor')->store; - if ( $patron->notice_email_address ) { + if ( @{ $patron->notice_email_addresses } ) { $patron->queue_notice( { letter_params => { diff --git a/Koha/Ticket.pm b/Koha/Ticket.pm index bec743b783..34dee1ac36 100644 --- a/Koha/Ticket.pm +++ b/Koha/Ticket.pm @@ -124,15 +124,17 @@ sub store { ); if ($acknowledgement_letter) { - my $acknowledgement_message_id = C4::Letters::EnqueueLetter( + my @acknowledgement_message_ids = C4::Letters::EnqueueLetter( { letter => $acknowledgement_letter, message_transport_type => 'email', borrowernumber => $self->reporter_id, } ); - C4::Letters::SendQueuedMessages( - { message_id => $acknowledgement_message_id } ); + foreach my $acknowledgement_message_id (@acknowledgement_message_ids) { + C4::Letters::SendQueuedMessages( + { message_id => $acknowledgement_message_id } ); + } } # Notify cataloger by email @@ -147,17 +149,20 @@ sub store { ); if ($notify_letter) { - my $message_id = C4::Letters::EnqueueLetter( + my ($reporter_first_notice_email_address) = $self->reporter->notice_email_addresses; + my @message_ids = C4::Letters::EnqueueLetter( { letter => $notify_letter, message_transport_type => 'email', to_address => C4::Context->preference('CatalogerEmails'), - reply_address => $self->reporter->notice_email_address, + reply_address => $reporter_first_notice_email_address, } ); - C4::Letters::SendQueuedMessages( - { message_id => $message_id } ); + foreach my $message_id (@message_ids) { + C4::Letters::SendQueuedMessages( + { message_id => $message_id } ); + } } } } diff --git a/circ/pendingreserves.pl b/circ/pendingreserves.pl index 21016bc354..d5aef07039 100755 --- a/circ/pendingreserves.pl +++ b/circ/pendingreserves.pl @@ -93,7 +93,7 @@ if ( $op eq 'cancel_reserve' and $reserve_id ) { from_address => $from_address, } ); - unless ( $patron->notice_email_address ) { + unless ( @{ $patron->notice_email_addresses } ) { push @messages, {type => 'alert', code => 'no_email_address', }; } push @messages, { type => 'message', code => 'letter_enqueued' }; diff --git a/installer/data/mysql/atomicupdate/bug-12802.pl b/installer/data/mysql/atomicupdate/bug-12802.pl new file mode 100755 index 0000000000..55ca7e01ca --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug-12802.pl @@ -0,0 +1,12 @@ +use Modern::Perl; + +return { + bug_number => '12802', + description => 'Change type of system preference AutoEmailPrimaryAddress to multiple', + up => sub { + my ($args) = @_; + my ( $dbh, $out ) = @$args{qw(dbh out)}; + + $dbh->do("UPDATE systempreferences SET type='multiple' WHERE variable='AutoEmailPrimaryAddress'"); + }, +}; diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql index 98eced6bc0..6628e2859c 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -80,7 +80,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('AutoCreateAuthorities','0',NULL,'Automatically create authorities that do not exist when cataloging records.','YesNo'), ('AutoCreditNumber', '', '', 'Automatically generate a number for account credits', 'Choice'), ('AutoEmailNewUser','0',NULL,'Send an email to newly created patrons.','YesNo'), -('AutoEmailPrimaryAddress','OFF','email|emailpro|B_email|cardnumber|OFF','Defines the default email address where \'Account Details\' emails are sent.','Choice'), +('AutoEmailPrimaryAddress','OFF','email|emailpro|B_email|cardnumber|OFF','Defines the default email address where \'Account Details\' emails are sent.','multiple'), ('AutoShareWithMana','subscription','','defines datas automatically shared with mana','multiple'), ('AutoLocation','0',NULL,'If ON, IP authentication is enabled, blocking access to the staff interface from unauthorized IP addresses','YesNo'), ('AutomaticItemReturn','1',NULL,'If ON, Koha will automatically set up a transfer of this item to its homebranch','YesNo'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref index b0638195a8..e1cfd440a0 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref @@ -176,7 +176,7 @@ Patrons: - "Use" - pref: AutoEmailPrimaryAddress default: "OFF" - choices: + multiple: email: home emailpro: work B_email: alternate diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt index bee28f8274..d396e9b1cc 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt @@ -226,7 +226,7 @@ $("#send_otp").on("click", function(e){ e.preventDefault(); - [% UNLESS notice_email_address %] + [% UNLESS notice_email_addresses.size > 0 %] alert("Cannot send the notice, you don't have an email address defined.") [% ELSE %] $("#email_success").hide(); diff --git a/members/memberentry.pl b/members/memberentry.pl index 7d58011f62..985d25fb97 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -426,9 +426,9 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ # If 'AutoEmailNewUser' syspref is on, email user their account details from the 'notice' that matches the user's branchcode. if ( $patron && C4::Context->preference("AutoEmailNewUser") ) { #look for defined primary email address, if blank - attempt to use borr.email and borr.emailpro instead - my $emailaddr = $patron->notice_email_address; + my $emailaddrs = $patron->notice_email_addresses; # if we manage to find a valid email address, send notice - if ($emailaddr) { + if (@$emailaddrs) { eval { my $letter = GetPreparedLetter( module => 'members', @@ -442,15 +442,16 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ want_librarian => 1, ) or return; - my $message_id = EnqueueLetter( + my @message_ids = EnqueueLetter( { letter => $letter, borrowernumber => $patron->id, - to_address => $emailaddr, message_transport_type => 'email' } ); - SendQueuedMessages({ message_id => $message_id }); + foreach my $message_id (@message_ids) { + SendQueuedMessages({ message_id => $message_id }); + } }; if ($@) { $template->param( error_alert => $@ ); diff --git a/members/notices.pl b/members/notices.pl index 11f3faa917..548eca2170 100755 --- a/members/notices.pl +++ b/members/notices.pl @@ -62,10 +62,10 @@ if ( $op eq 'resend_notice' ) { } if ( $op eq 'send_welcome' ) { - my $emailaddr = $patron->notice_email_address; + my $emailaddrs = $patron->notice_email_addresses; # if we manage to find a valid email address, send notice - if ($emailaddr) { + if (@$emailaddrs) { eval { my $letter = GetPreparedLetter( module => 'members', @@ -79,11 +79,10 @@ if ( $op eq 'send_welcome' ) { want_librarian => 1, ) or return; - my $message_id = EnqueueLetter( + EnqueueLetter( { letter => $letter, borrowernumber => $patron->id, - to_address => $emailaddr, message_transport_type => 'email' } ); @@ -96,12 +95,12 @@ if ( $op eq 'send_welcome' ) { if ( $op eq 'send_password_reset' ) { - my $emailaddr = $patron->notice_email_address; + my $emailaddrs = $patron->notice_email_addresses; - if ($emailaddr) { + if (@$emailaddrs) { # send staff initiated password recovery - SendPasswordRecoveryEmail( $patron, $emailaddr, 1 ); + SendPasswordRecoveryEmail( $patron, undef, 1 ); } # redirect to self to avoid form submission on refresh diff --git a/members/two_factor_auth.pl b/members/two_factor_auth.pl index 81220c4b27..3be925c475 100755 --- a/members/two_factor_auth.pl +++ b/members/two_factor_auth.pl @@ -63,7 +63,7 @@ else { Koha::Auth::TwoFactorAuth->new( { patron => $logged_in_user } ); $logged_in_user->secret(undef); $logged_in_user->auth_method('password')->store; - if ( $logged_in_user->notice_email_address ) { + if ( @{ $logged_in_user->notice_email_addresses } ) { $logged_in_user->queue_notice( { letter_params => { diff --git a/misc/cronjobs/notice_unprocessed_suggestions.pl b/misc/cronjobs/notice_unprocessed_suggestions.pl index 6567c35d75..510f390631 100755 --- a/misc/cronjobs/notice_unprocessed_suggestions.pl +++ b/misc/cronjobs/notice_unprocessed_suggestions.pl @@ -47,9 +47,9 @@ for my $number_of_days (@days) { my $budget = C4::Budgets::GetBudget( $suggestion->{budgetid} ); my $patron = Koha::Patrons->find( $budget->{budget_owner_id} ); - my $email_address = $patron->notice_email_address; + my $email_addresses = $patron->notice_email_addresses; - if ($email_address) { + if (@$email_addresses) { say "Patron " . $patron->borrowernumber . " is going to be notified" if $verbose; my $letter = C4::Letters::GetPreparedLetter( module => 'suggestions', diff --git a/misc/cronjobs/overdue_notices.pl b/misc/cronjobs/overdue_notices.pl index 19e6aef27b..373206bc49 100755 --- a/misc/cronjobs/overdue_notices.pl +++ b/misc/cronjobs/overdue_notices.pl @@ -604,7 +604,6 @@ END_SQL my $patron = Koha::Patrons->find( $borrowernumber ); @emails_to_use = (); - my $notice_email = $patron->notice_email_address; unless ($nomail) { if (@emails) { foreach (@emails) { @@ -612,7 +611,8 @@ END_SQL } } else { - push @emails_to_use, $notice_email if ($notice_email); + my $notice_emails = $patron->notice_email_addresses; + push @emails_to_use, @$notice_emails; } } @@ -770,7 +770,7 @@ END_SQL letternumber => $i, 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' : '', @@ -789,7 +789,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/misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl b/misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl index 5f679a9932..1cbdd40882 100755 --- a/misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl +++ b/misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl @@ -122,7 +122,10 @@ foreach my $type (@types) { my $patrons; foreach my $issues (@loop) { $patrons->{$issues->{borrowernumber}} ||= Koha::Patrons->find( $issues->{borrowernumber} ) if $skip_patrons_with_email; - next if $skip_patrons_with_email && $patrons->{$issues->{borrowernumber}}->notice_email_address; + if ($skip_patrons_with_email) { + my $email_addresses = $patrons->{$issues->{borrowernumber}}->notice_email_addresses; + next if @$email_addresses; + } my $date_dt = dt_from_string ( $issues->{'date_due'} ); my $due_date = output_pref( { dt => $date_dt, dateonly => 1, dateformat =>'metric' } ); @@ -143,7 +146,7 @@ foreach my $type (@types) { my $message_id = 0; if ($outfile) { - $message_id = C4::Letters::EnqueueLetter( + ($message_id) = C4::Letters::EnqueueLetter( { letter => $letter, borrowernumber => $issues->{'borrowernumber'}, message_transport_type => 'itiva', diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl index a09bcb8bc1..14689aa132 100755 --- a/opac/opac-memberentry.pl +++ b/opac/opac-memberentry.pl @@ -202,16 +202,16 @@ if ( $action eq 'create' ) { }, ); - my $message_id = C4::Letters::EnqueueLetter( + my @message_ids = C4::Letters::EnqueueLetter( { + borrowernumber => $borrower{borrowernumber}, letter => $letter, message_transport_type => 'email', - to_address => $borrower{'email'}, - from_address => - C4::Context->preference('KohaAdminEmailAddress'), } ); - C4::Letters::SendQueuedMessages({ message_id => $message_id }); + for my $message_id (@message_ids) { + C4::Letters::SendQueuedMessages({ message_id => $message_id }); + } } else { $borrower{password} ||= Koha::AuthUtils::generate_password(Koha::Patron::Categories->find($borrower{categorycode})); @@ -255,9 +255,9 @@ if ( $action eq 'create' ) { # If 'AutoEmailNewUser' syspref is on, email user their account details from the 'notice' that matches the user's branchcode. if ( C4::Context->preference("AutoEmailNewUser") ) { #look for defined primary email address, if blank - attempt to use borr.email and borr.emailpro instead - my $emailaddr = $patron->notice_email_address; + my $emailaddrs = $patron->notice_email_addresses; # if we manage to find a valid email address, send notice - if ($emailaddr) { + if (@$emailaddrs) { eval { my $letter = GetPreparedLetter( module => 'members', @@ -271,15 +271,16 @@ if ( $action eq 'create' ) { want_librarian => 1, ) or return; - my $message_id = EnqueueLetter( + my @message_ids = EnqueueLetter( { letter => $letter, borrowernumber => $patron->id, - to_address => $emailaddr, message_transport_type => 'email' } ); - SendQueuedMessages({ message_id => $message_id }); + foreach my $message_id (@message_ids) { + SendQueuedMessages({ message_id => $message_id }); + } }; } } diff --git a/opac/opac-registration-verify.pl b/opac/opac-registration-verify.pl index 4af265c110..d9f2a57a55 100755 --- a/opac/opac-registration-verify.pl +++ b/opac/opac-registration-verify.pl @@ -96,9 +96,9 @@ if ( # If 'AutoEmailNewUser' syspref is on, email user their account details from the 'notice' that matches the user's branchcode. if ( C4::Context->preference("AutoEmailNewUser") ) { # Look up correct email address taking AutoEmailPrimaryAddress into account - my $emailaddr = $patron->notice_email_address; + my $emailaddrs = $patron->notice_email_addresses; # if we manage to find a valid email address, send notice - if ($emailaddr) { + if (@$emailaddrs) { eval { my $letter = GetPreparedLetter( module => 'members', @@ -112,15 +112,16 @@ if ( want_librarian => 1, ) or return; - my $message_id = EnqueueLetter( + my @message_ids = EnqueueLetter( { letter => $letter, borrowernumber => $patron->id, - to_address => $emailaddr, message_transport_type => 'email' } ); - SendQueuedMessages({ message_id => $message_id }); + foreach my $message_id (@message_ids) { + SendQueuedMessages({ message_id => $message_id }); + } }; } } diff --git a/opac/opac-shareshelf.pl b/opac/opac-shareshelf.pl index 29edd82293..2f7fdc465c 100755 --- a/opac/opac-shareshelf.pl +++ b/opac/opac-shareshelf.pl @@ -164,7 +164,7 @@ sub notify_owner { my $patron = Koha::Patrons->find( $param->{owner} ); return unless $patron; - my $toaddr = $patron->notice_email_address or return; + return unless @{ $patron->notice_email_addresses }; #prepare letter my $letter = C4::Letters::GetPreparedLetter( @@ -179,10 +179,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, } ); } @@ -208,7 +207,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=" @@ -250,7 +248,6 @@ sub send_invitekey { { letter => $letter, message_transport_type => 'email', - from_address => $fromaddr, to_address => $a, } ); diff --git a/opac/opac-shelves.pl b/opac/opac-shelves.pl index a7be90ba58..6d9eccc9ee 100755 --- a/opac/opac-shelves.pl +++ b/opac/opac-shelves.pl @@ -270,8 +270,8 @@ if ( $op eq 'add_form' ) { my $patrons = []; my $shares = $shelf->get_shares->search({ borrowernumber => { '!=' => undef } }); while( my $share = $shares->next ) { - my $email = $share->sharee->notice_email_address; - push @$patrons, { email => $email, borrowernumber => $share->get_column('borrowernumber') } if $email; + my $emails = $share->sharee->notice_email_addresses; + push @$patrons, { email => $emails->[0], borrowernumber => $share->get_column('borrowernumber') } if @$emails; } if( @$patrons ) { $template->param( shared_users => $patrons ); diff --git a/pos/pay.pl b/pos/pay.pl index 871ffd6c19..73b04dda56 100755 --- a/pos/pay.pl +++ b/pos/pay.pl @@ -106,7 +106,7 @@ if ( $action eq 'send' ) { ); # Add letter to the queue - my $message_id = EnqueueLetter( + my @message_ids = EnqueueLetter( { letter => $letter, message_transport_type => 'email', @@ -116,7 +116,9 @@ if ( $action eq 'send' ) { ); # Send immediately - SendQueuedMessages( { message_id => $message_id } ); + foreach my $message_id (@message_ids) { + SendQueuedMessages( { message_id => $message_id } ); + } # Set variables for template to allow printing still $template->param( diff --git a/suggestion/suggestion.pl b/suggestion/suggestion.pl index f85b4366ea..0fc275f2d1 100755 --- a/suggestion/suggestion.pl +++ b/suggestion/suggestion.pl @@ -196,9 +196,8 @@ if ( $op =~ /save/i ) { if ( $notify ) { my $patron = Koha::Patrons->find( $suggestion_only->{managedby} ); - my $email_address = $patron->notice_email_address; - if ($patron->notice_email_address) { - + my $email_addresses = $patron->notice_email_addresses; + if (@$email_addresses) { my $letter = C4::Letters::GetPreparedLetter( module => 'suggestions', letter_code => 'NOTIFY_MANAGER', diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index afb0dcb461..8c6cc926c2 100755 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -1061,16 +1061,23 @@ subtest 'holds and old_holds' => sub { $patron->delete; }; -subtest 'notice_email_address' => sub { - plan tests => 2; +subtest 'notice_email_addresses' => sub { + plan tests => 3; my $patron = $builder->build_object({ class => 'Koha::Patrons' }); t::lib::Mocks::mock_preference( 'AutoEmailPrimaryAddress', 'OFF' ); - is ($patron->notice_email_address, $patron->email, "Koha::Patron->notice_email_address returns correct value when AutoEmailPrimaryAddress is off"); + is_deeply( $patron->notice_email_addresses, [ $patron->email ], "Koha::Patron->notice_email_addresses returns correct value when AutoEmailPrimaryAddress is off" ); t::lib::Mocks::mock_preference( 'AutoEmailPrimaryAddress', 'emailpro' ); - is ($patron->notice_email_address, $patron->emailpro, "Koha::Patron->notice_email_address returns correct value when AutoEmailPrimaryAddress is emailpro"); + is_deeply( $patron->notice_email_addresses, [ $patron->emailpro ], "Koha::Patron->notice_email_addresses returns correct value when AutoEmailPrimaryAddress is emailpro" ); + + t::lib::Mocks::mock_preference( 'AutoEmailPrimaryAddress', 'email,emailpro' ); + is_deeply( + $patron->notice_email_addresses, + [ $patron->email, $patron->emailpro ], + "Koha::Patron->notice_email_addresses returns correct value when AutoEmailPrimaryAddress is email,emailpro" + ); $patron->delete; }; @@ -1983,7 +1990,7 @@ subtest '->set_password' => sub { t::lib::Mocks::mock_preference( 'NotifyPasswordChange', 1 ); $patron->set_password({ password => 'abcd c' }); my $queued_notices = Koha::Notice::Messages->search({ borrowernumber => $patron->borrowernumber }); - is( $queued_notices->count, 1, "One notice queued when NotifyPasswordChange enabled" ); + is( $queued_notices->count, scalar @{ $patron->notice_email_addresses }, "One notice queued for each notice email address when NotifyPasswordChange enabled" ); my $THE_notice = $queued_notices->next; is( $THE_notice->status, 'failed', "The notice was handled immediately and failed on wrong email address."); #FIXME Mock sending mail $schema->storage->txn_rollback; diff --git a/t/db_dependent/Koha/Recall.t b/t/db_dependent/Koha/Recall.t index f36e328ce5..01ed730b49 100755 --- a/t/db_dependent/Koha/Recall.t +++ b/t/db_dependent/Koha/Recall.t @@ -129,7 +129,7 @@ is( t::lib::Dates::compare( $expirationdate, $expected_expirationdate ), 0, "Exp $recall2->set_waiting({ expirationdate => $expirationdate }); is( $recall2->waiting, 1, "Recall is waiting" ); -my $notice = C4::Message->find_last_message( $patron1->unblessed, 'PICKUP_RECALLED_ITEM', 'email' ); +my ($notice) = C4::Message->find_last_messages( $patron1->unblessed, 'PICKUP_RECALLED_ITEM', 'email' ); ok( defined $notice, "Patron was notified to pick up waiting recall" ); $recall2->set_expired({ interface => 'COMMANDLINE' }); diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t index 33a518fc09..58e15cc0c4 100755 --- a/t/db_dependent/Letters.t +++ b/t/db_dependent/Letters.t @@ -112,8 +112,7 @@ my $my_message = { to_address => undef, 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} = { @@ -124,13 +123,12 @@ $my_message->{letter} = { content_type => 'text/plain', }; -$message_id = C4::Letters::EnqueueLetter($my_message); -is( $message_id, undef, 'EnqueueLetter without the message type argument argument returns undef' ); +my @message_ids = C4::Letters::EnqueueLetter($my_message); +is( scalar @message_ids, 0, 'EnqueueLetter without the message type argument argument returns an empty list' ); $my_message->{message_transport_type} = 'sms'; -$message_id = C4::Letters::EnqueueLetter($my_message); -ok(defined $message_id && $message_id > 0, 'new message successfully queued'); - +@message_ids = C4::Letters::EnqueueLetter($my_message); +is(scalar @message_ids, 1, 'new message successfully queued'); # GetQueuedMessages my $messages = C4::Letters::GetQueuedMessages(); @@ -138,7 +136,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' ); @@ -847,8 +844,8 @@ subtest 'Test SMS handling in SendQueuedMessages' => sub { is( $@, '', 'SendQueuedMessages should not explode if the patron does not have a sms provider set' ); my $sms_pro = $builder->build_object({ class => 'Koha::SMS::Providers', value => { domain => 'kidclamp.rocks' } }); - $patron->set( { smsalertnumber => '5555555555', sms_provider_id => $sms_pro->id() } )->store; - $message_id = C4::Letters::EnqueueLetter($my_message); #using datas set around line 95 and forward + $patron->set( { smsalertnumber => '5555555555', sms_provider_id => $sms_pro->id(), email => '', emailpro => '' } )->store; + C4::Letters::EnqueueLetter($my_message); #using datas set around line 95 and forward warning_like { C4::Letters::SendQueuedMessages(); } qr|Fake send_or_die|, @@ -871,7 +868,7 @@ subtest 'Test SMS handling in SendQueuedMessages' => sub { t::lib::Mocks::mock_preference('EmailSMSSendDriverFromAddress', 'override@example.com'); - $message_id = C4::Letters::EnqueueLetter($my_message); + C4::Letters::EnqueueLetter($my_message); warning_like { C4::Letters::SendQueuedMessages(); } qr|Fake send_or_die|, "SendAlerts is using the mocked send_or_die routine (claimissues)"; @@ -889,7 +886,7 @@ subtest 'Test SMS handling in SendQueuedMessages' => sub { $schema->resultset('MessageQueue')->search({borrowernumber => $borrowernumber,status => 'sent'})->delete(); #clear borrower queue $my_message->{to_address} = 'fixme@kidclamp.iswrong'; - $message_id = C4::Letters::EnqueueLetter($my_message); + C4::Letters::EnqueueLetter($my_message); my $number_attempted = C4::Letters::SendQueuedMessages({ borrowernumber => -1, # -1 still triggers the borrowernumber condition @@ -903,6 +900,7 @@ subtest 'Test SMS handling in SendQueuedMessages' => sub { my $sms_message_address = $schema->resultset('MessageQueue')->search({ borrowernumber => $borrowernumber, + message_transport_type => 'sms', status => 'sent' })->next()->to_address(); is( $sms_message_address, '5555555555@kidclamp.rocks', 'SendQueuedMessages populates the to address correctly for SMS by email when to_address is set incorrectly' ); @@ -1091,7 +1089,7 @@ subtest 'Test message_id parameter for SendQueuedMessages' => sub { 'message_transport_type' => 'email', 'from_address' => '@example.com' # invalid from_address }; - my $message_id = C4::Letters::EnqueueLetter($my_message); + my ($message_id) = C4::Letters::EnqueueLetter($my_message); my $processed = C4::Letters::SendQueuedMessages( { message_id => $message_id } ); is( $processed, 1, 'Processed 1 message when one message_id passed' ); my $message_1 = C4::Letters::GetMessage($message_id); @@ -1099,7 +1097,7 @@ subtest 'Test message_id parameter for SendQueuedMessages' => sub { is( $message_1->{failure_code}, 'INVALID_EMAIL:from', 'Failure code set correctly for invalid email parameter'); $my_message->{from_address} = 'root@example.org'; # valid from_address - $message_id = C4::Letters::EnqueueLetter($my_message); + ($message_id) = C4::Letters::EnqueueLetter($my_message); warning_like { C4::Letters::SendQueuedMessages( { message_id => $message_id } ); } qr|Fake send_or_die|, "SendQueuedMessages is using the mocked send_or_die routine"; @@ -1108,3 +1106,83 @@ subtest 'Test message_id parameter for SendQueuedMessages' => sub { is( $message_1->{status}, 'failed', 'Message 1 status is unchanged' ); is( $message_2->{status}, 'sent', 'Valid from_address => status sent' ); }; + +subtest 'EnqueueLetter and AutoEmailPrimaryAddress' => sub { + plan tests => 19; + + my ( $email, $emailpro ) = ( 'email@example.org', 'emailpro@example.org' ); + my $borrowernumber = Koha::Patron->new( + { firstname => 'Jane', + surname => 'Smith', + categorycode => $patron_category, + branchcode => $library->{branchcode}, + dateofbirth => $date, + smsalertnumber => undef, + email => $email, + emailpro => $emailpro, + } + )->store->borrowernumber; + + my $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' ); + @message_ids = C4::Letters::EnqueueLetter($my_message); + is( scalar @message_ids, 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}; + @message_ids = C4::Letters::EnqueueLetter($my_message); + is( scalar @message_ids, 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' ); + @message_ids = C4::Letters::EnqueueLetter($my_message); + is( scalar @message_ids, 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' ); + @message_ids = C4::Letters::EnqueueLetter($my_message); + is( scalar @message_ids, 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. + @message_ids = C4::Letters::EnqueueLetter($my_message); + is( scalar @message_ids, 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' ); + @message_ids = C4::Letters::EnqueueLetter($my_message); + is( scalar @message_ids, 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/Message.t b/t/db_dependent/Message.t index b223c56505..9928e07cdc 100755 --- a/t/db_dependent/Message.t +++ b/t/db_dependent/Message.t @@ -52,7 +52,7 @@ my $letter = C4::Letters::GetPreparedLetter( t::lib::Mocks::mock_preference( 'AutoEmailPrimaryAddress', 'OFF' ); C4::Message->enqueue($letter, $patron, 'email'); -my $message = C4::Message->find_last_message($patron->unblessed, 'TEST_MESSAGE', 'email'); +my ($message) = C4::Message->find_last_messages($patron->unblessed, 'TEST_MESSAGE', 'email'); like( $message->{metadata}, qr{heÄllo} ); is ($message->{to_address}, $patron->email, "To address set correctly for AutoEmailPrimaryAddress 'off'"); @@ -71,5 +71,5 @@ like( $message->{metadata}, qr{hell❤️} ); t::lib::Mocks::mock_preference( 'AutoEmailPrimaryAddress', 'emailpro' ); C4::Message->enqueue($letter, $patron, 'email'); -$message = C4::Message->find_last_message($patron->unblessed, 'TEST_MESSAGE', 'email'); -is ($patron->notice_email_address, $patron->emailpro, "To address set correctly for AutoEmailPrimaryAddress 'emailpro'"); +($message) = C4::Message->find_last_messages($patron->unblessed, 'TEST_MESSAGE', 'email'); +is_deeply ($patron->notice_email_addresses, [$patron->emailpro], "To address set correctly for AutoEmailPrimaryAddress 'emailpro'"); -- 2.30.2