From e9ec475551f2a2a0b48b9984b6914eaa851b8254 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. Signed-off-by: Mathieu Saby --- 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 | 43 +++--- Koha/Patron/Password/Recovery.pm | 9 +- Koha/Patrons/Import.pm | 9 +- Koha/REST/V1/TwoFactorAuth.pm | 20 +-- Koha/Ticket.pm | 19 ++- basket/sendbasket.pl | 2 +- circ/pendingreserves.pl | 2 +- .../data/mysql/atomicupdate/bug-12802.pl | 12 ++ installer/data/mysql/mandatory/sysprefs.sql | 2 +- .../prog/en/includes/waiting_holds.inc | 6 +- .../en/modules/admin/preferences/patrons.pref | 2 +- .../intranet-tmpl/prog/en/modules/auth.tt | 2 +- .../en/modules/circ/transferstoreceive.tt | 6 +- .../bootstrap/en/modules/clubs/clubs-tab.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-sendbasket.pl | 2 +- opac/opac-sendshelf.pl | 2 +- opac/opac-shareshelf.pl | 7 +- opac/opac-shelves.pl | 4 +- pos/pay.pl | 6 +- suggestion/suggestion.pl | 5 +- t/db_dependent/Koha/Patron.t | 6 +- t/db_dependent/Koha/Patrons.t | 2 +- t/db_dependent/Koha/Recall.t | 2 +- t/db_dependent/Letters.t | 106 +++++++++++-- t/db_dependent/Message.t | 6 +- virtualshelves/sendshelf.pl | 2 +- 41 files changed, 354 insertions(+), 239 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug-12802.pl diff --git a/C4/Auth.pm b/C4/Auth.pm index 6c308642f9..9a46e087f0 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -1391,7 +1391,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 364fd774b1..6894b75f6f 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 5a6e036685..6185f99f3b 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3742,14 +3742,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 e070697df4..6cdc234ab4 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; @@ -883,20 +883,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 @@ -906,7 +909,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}; @@ -925,30 +927,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]) @@ -1101,31 +1146,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 @@ -1189,9 +1218,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; } @@ -1334,8 +1360,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 69f81df6e1..cf067d1a3a 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -808,12 +808,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) = @_; @@ -1863,9 +1864,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' @@ -1908,9 +1906,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 6937ea904a..c923d85db2 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -907,10 +907,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', @@ -924,15 +924,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 } ); + } } } } @@ -1402,25 +1403,31 @@ 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("EmailFieldPrimary"); + my $which_addresses = C4::Context->preference("EmailFieldPrimary"); # 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 @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 @@ -2214,7 +2221,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 ) ) { @@ -2353,17 +2361,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 9681fca16d..cb5dbc1e5a 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..782cc503d3 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/basket/sendbasket.pl b/basket/sendbasket.pl index 3d5438fb42..a23a504912 100755 --- a/basket/sendbasket.pl +++ b/basket/sendbasket.pl @@ -58,7 +58,7 @@ if ($email_add) { ); my $patron = Koha::Patrons->find($borrowernumber); - my $user_email = $patron->notice_email_address; + my ($user_email) = @{ $patron->notice_email_addresses }; my $comment = $query->param('comment'); 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 30197591a8..eb62ecc6a1 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -213,7 +213,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('EmailAddressForPatronRegistrations', '', '', ' If you choose EmailAddressForPatronRegistrations you have to enter a valid email address: ', 'free'), ('EmailAddressForSuggestions','','',' If you choose EmailAddressForSuggestions you have to enter a valid email address: ','free'), ('EmailFieldPrecedence','email|emailpro|B_email','','Ordered list of patron email fields to use when AutoEmailPrimaryAddress is set to first valid','multiple'), -('EmailFieldPrimary','OFF','email|emailpro|B_email|cardnumber|OFF','Defines the default email address field where patron email notices are sent.','Choice'), +('EmailFieldPrimary','OFF','email|emailpro|B_email|cardnumber|OFF','Defines the default email address field where patron email notices are sent.','multiple'), ('emailLibrarianWhenHoldIsPlaced','0',NULL,'If ON, emails the librarian whenever a hold is placed','YesNo'), ('EmailOverduesNoEmail','1',NULL,'Send send overdues of patrons without email address to staff','YesNo'), ('EmailPatronRegistrations', '0', '0|EmailAddressForPatronRegistrations|BranchEmailAddress|KohaAdminEmailAddress', 'Choose email address that new patron registrations will be sent to: ', 'Choice'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc index 484f1d609b..69b1b3f679 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc @@ -54,9 +54,9 @@ [% INCLUDE 'patron-title.inc' patron=reserveloo.borrower invert_name=1 no_title=1 %] [% IF ( reserveloo.borrower.phone ) %]
[% reserveloo.borrower.phone | html %][% END %] - [% IF ( reserveloo.borrower.notice_email_address ) %] -
- [% reserveloo.borrower.notice_email_address | html %]
+ [% FOREACH notice_email_address IN reserveloo.borrower.notice_email_addresses %] +
+ [% notice_email_address | html %]
[% END %] [% Branches.GetName( reserveloo.item.homebranch ) | html %] 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 32cd34f706..4284862a62 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 @@ -182,7 +182,7 @@ Patrons: - "Use the patron's" - pref: EmailFieldPrimary default: "OFF" - choices: + multiple: email: primary email emailpro: secondary email B_email: alternate email diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt index e87cb438c4..09e52a778b 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/koha-tmpl/intranet-tmpl/prog/en/modules/circ/transferstoreceive.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/transferstoreceive.tt index 81b0fbadb5..5982088c78 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/transferstoreceive.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/transferstoreceive.tt @@ -72,11 +72,11 @@ [% reser.patron.surname | html %][%IF ( reser.patron.firstname ) %], [% reser.patron.firstname | html %][% END %] [% IF ( reser.patron.phone ) %]
[% reser.patron.phone | html %][% END %] - [% IF ( reser.patron.notice_email_address ) %] + [% FOREACH notice_email_address IN reser.patron.notice_email_addresses %]
[% BLOCK subject %]Hold:[% END %] - - [% reser.patron.notice_email_address | html %] + + [% notice_email_address | html %] [% END %] [% ELSIF ( reser.recall ) %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/clubs/clubs-tab.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/clubs/clubs-tab.tt index c084ae304f..393277999a 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/clubs/clubs-tab.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/clubs/clubs-tab.tt @@ -58,7 +58,7 @@ [% c.name | html %] [% c.description | html %] - [% IF !c.club_template.is_email_required || ( c.club_template.is_email_required && borrower.notice_email_address ) %] + [% IF !c.club_template.is_email_required || ( c.club_template.is_email_required && borrower.notice_email_addresses.size > 0 ) %] diff --git a/members/memberentry.pl b/members/memberentry.pl index 64d9aa2aa7..7a6d805421 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 d18e5f542b..86f1609b98 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 5cb30239da..504fbd7226 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 EmailFieldPrimary 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-sendbasket.pl b/opac/opac-sendbasket.pl index ae0ec81e1c..2893f388a6 100755 --- a/opac/opac-sendbasket.pl +++ b/opac/opac-sendbasket.pl @@ -58,7 +58,7 @@ if ( $email_add ) { ); my $patron = Koha::Patrons->find($borrowernumber); - my $user_email = $patron->notice_email_address; + my $user_email = @{ $patron->notice_email_addresses }; my $comment = $query->param('comment'); diff --git a/opac/opac-sendshelf.pl b/opac/opac-sendshelf.pl index a90d2ca5f4..2e5d1e05da 100755 --- a/opac/opac-sendshelf.pl +++ b/opac/opac-sendshelf.pl @@ -61,7 +61,7 @@ if ( $shelf and $shelf->can_be_viewed($borrowernumber) ) { my $comment = $query->param('comment'); my $patron = Koha::Patrons->find($borrowernumber); - my $user_email = $patron->notice_email_address; + my ($user_email) = @{ $patron->notice_email_addresses }; my $shelf = Koha::Virtualshelves->find($shelfid); my $contents = $shelf->get_contents; my $iso2709; 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 5cecfc7b96..a67236d1ce 100755 --- a/opac/opac-shelves.pl +++ b/opac/opac-shelves.pl @@ -273,8 +273,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 b2cbf3a639..f01e6e7486 100755 --- a/suggestion/suggestion.pl +++ b/suggestion/suggestion.pl @@ -197,9 +197,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/Patron.t b/t/db_dependent/Koha/Patron.t index 5d05a16efd..f472cb11a7 100755 --- a/t/db_dependent/Koha/Patron.t +++ b/t/db_dependent/Koha/Patron.t @@ -1393,17 +1393,17 @@ subtest 'notify_library_of_registration()' => sub { $schema->storage->txn_rollback; }; -subtest 'notice_email_address' => sub { +subtest 'notice_email_addresses' => sub { plan tests => 2; my $patron = $builder->build_object({ class => 'Koha::Patrons' }); t::lib::Mocks::mock_preference( 'EmailFieldPrecedence', 'email|emailpro' ); t::lib::Mocks::mock_preference( 'EmailFieldPrimary', 'OFF' ); - is ($patron->notice_email_address, $patron->email, "Koha::Patron->notice_email_address returns correct value when EmailFieldPrimary is off"); + is_deeply ($patron->notice_email_addresses, [$patron->email], "Koha::Patron->notice_email_addresses returns correct value when EmailFieldPrimary is off"); t::lib::Mocks::mock_preference( 'EmailFieldPrimary', 'emailpro' ); - is ($patron->notice_email_address, $patron->emailpro, "Koha::Patron->notice_email_address returns correct value when EmailFieldPrimary is emailpro"); + is_deeply ($patron->notice_email_addresses, [$patron->emailpro], "Koha::Patron->notice_email_addresses returns correct value when EmailFieldPrimary is emailpro"); $patron->delete; }; diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index 89ce426187..b3e7c2738d 100755 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -1971,7 +1971,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 3d1e34a6bb..ec13d0aefc 100755 --- a/t/db_dependent/Letters.t +++ b/t/db_dependent/Letters.t @@ -117,8 +117,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} = { @@ -129,13 +128,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(); @@ -143,7 +141,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' ); @@ -863,8 +860,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|, @@ -887,7 +884,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)"; @@ -905,7 +902,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 @@ -919,6 +916,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' ); @@ -1157,7 +1155,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); $send_or_die_count = 0; # reset my $processed = C4::Letters::SendQueuedMessages( { message_id => $message_id } ); is( $send_or_die_count, 0, 'Nothing sent when one message_id passed' ); @@ -1166,7 +1164,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"; @@ -1176,3 +1174,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 EmailFieldPrimary' => 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( 'EmailFieldPrimary', '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', 'EmailFieldPrimary=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, 'EmailFieldPrimary=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( 'EmailFieldPrimary', '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, 'EmailFieldPrimary=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( 'EmailFieldPrimary', '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, 'EmailFieldPrimary=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( 'EmailFieldPrimary', '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, 'EmailFieldPrimary=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( 'EmailFieldPrimary', '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, 'EmailFieldPrimary=email,emailpro, EnqueueLetter used the patron email address for the first letter' ); + is( $messages->[1]{to_address}, $emailpro, 'EmailFieldPrimary=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 95673c36c9..4cc116d6ee 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( 'EmailFieldPrimary', '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 EmailFieldPrimary 'off'"); @@ -71,5 +71,5 @@ like( $message->{metadata}, qr{hell❤️} ); t::lib::Mocks::mock_preference( 'EmailFieldPrimary', '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 EmailFieldPrimary 'emailpro'"); +($message) = C4::Message->find_last_messages($patron->unblessed, 'TEST_MESSAGE', 'email'); +is ($patron->notice_email_addresses, [$patron->emailpro], "To address set correctly for EmailFieldPrimary 'emailpro'"); diff --git a/virtualshelves/sendshelf.pl b/virtualshelves/sendshelf.pl index 3a2738c2d8..87cffaf34f 100755 --- a/virtualshelves/sendshelf.pl +++ b/virtualshelves/sendshelf.pl @@ -61,7 +61,7 @@ if ($to_address) { my $comment = $query->param('comment'); my $patron = Koha::Patrons->find($borrowernumber); - my $user_email = $patron->notice_email_address; + my ($user_email) = @{ $patron->notice_email_addresses }; my $contents = $shelf->get_contents; my @biblionumbers; my $iso2709; -- 2.30.2