From 2eb0d0ef587566f674e72c2eed04f8e402585e7e 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 EmailFieldPrimary 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 EmailFieldPrimary 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 EmailFieldPrimary 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 EmailFieldPrimary" 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 EmailFieldPrimary (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 EmailFieldPrimary. 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 EmailFieldPrimary; 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 | 135 +++++++++++------- C4/Message.pm | 43 ++---- C4/Reserves.pm | 13 +- Koha/Patron.pm | 43 +++--- Koha/Patron/Password/Recovery.pm | 7 +- Koha/Patrons/Import.pm | 9 +- Koha/REST/V1/TwoFactorAuth.pm | 20 +-- Koha/Ticket.pm | 18 ++- 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, 356 insertions(+), 229 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug-12802.pl diff --git a/C4/Auth.pm b/C4/Auth.pm index c449c318a92..828274edf08 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -1407,7 +1407,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 422a59a45b3..f1fedd50264 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 } ) if $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 67552f3c89d..6894b75f6fb 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 } ) if $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 ac28b2e74b5..dbd23d52676 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3780,14 +3780,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 47e12526484..fbc34ceeb5f 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -31,6 +31,7 @@ use C4::SMS; use C4::Templates; use Koha::Auth::TwoFactorAuth; use Koha::DateUtils qw( dt_from_string output_pref ); +use Koha::Database; use Koha::Email; use Koha::Exceptions; use Koha::Notice::Messages; @@ -898,20 +899,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 @@ -921,7 +925,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}; @@ -940,25 +943,76 @@ sub EnqueueLetter { ); } - my $message = Koha::Notice::Message->new( - { - letter_id => $params->{letter}->{id} || undef, - borrowernumber => $params->{borrowernumber}, - subject => $params->{letter}->{title}, - content => $params->{letter}->{content}, - metadata => $params->{letter}->{metadata} || q{}, - letter_code => $params->{letter}->{code} || q{}, - message_transport_type => $params->{message_transport_type}, - status => 'pending', - time_queued => dt_from_string(), - to_address => $params->{to_address}, - from_address => $params->{from_address}, - reply_address => $params->{reply_address}, - content_type => $params->{letter}->{'content-type'}, - failure_code => $params->{failure_code} || q{}, + 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 $patron = Koha::Patrons->find( $params->borrowernumber ); + if ( $patron ) { + $from_address = $patron->library->from_email_address; + } } - )->store(); - return $message->id; + + $from_address ||= C4::Context->preference('KohaAdminEmailAddress'); + } + + my @message_ids; + + if ( $params->{message_transport_type} eq 'email' ) { + foreach my $to_address ( @to_addresses ) { + my $message = Koha::Notice::Message->new( + { + letter_id => $params->{letter}->{id} || undef, + borrowernumber => $params->{borrowernumber}, + subject => $params->{letter}->{title}, + content => $params->{letter}->{content}, + metadata => $params->{letter}->{metadata} || q{}, + letter_code => $params->{letter}->{code} || q{}, + message_transport_type => $params->{message_transport_type}, + status => 'pending', + time_queued => dt_from_string(), + to_address => $params->{to_address}, + from_address => $params->{from_address}, + reply_address => $params->{reply_address}, + content_type => $params->{letter}->{'content-type'}, + failure_code => $params->{failure_code} || q{}, + } + )->store(); + push @message_ids, $message->id(); + } + } else { + my $message = Koha::Notice::Message->new( + { + letter_id => $params->{letter}->{id} || undef, + borrowernumber => $params->{borrowernumber}, + subject => $params->{letter}->{title}, + content => $params->{letter}->{content}, + metadata => $params->{letter}->{metadata} || q{}, + letter_code => $params->{letter}->{code} || q{}, + message_transport_type => $params->{message_transport_type}, + status => 'pending', + time_queued => dt_from_string(), + from_address => $params->{from_address}, + reply_address => $params->{reply_address}, + content_type => $params->{letter}->{'content-type'}, + failure_code => $params->{failure_code} || q{}, + } + )->store(); + push @message_ids, $message->id(); + } + + return @message_ids; } =head2 SendQueuedMessages ([$hashref]) @@ -1114,31 +1168,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 @@ -1202,9 +1240,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; } @@ -1337,6 +1372,7 @@ sub _send_message_by_email { my $cc_address; my @guarantor_address; my $count_guarantor_address; + my $to_addresses; if (C4::Context->preference('RedirectGuaranteeEmail') && $patron) { # Get guarantor addresses my $guarantor_relationships = $patron->guarantor_relationships; @@ -1361,8 +1397,9 @@ sub _send_message_by_email { } if ($patron) { $to_address = $patron->notice_email_address; + $to_addresses = $patron->notice_email_addresses; } - if (!$to_address && !$count_guarantor_address) { + if (!$to_address && !$count_guarantor_address && !$to_addresses) { warn "FAIL: No 'to_address', email address or guarantors email address for borrowernumber ($message->{borrowernumber})"; _set_message_status( { diff --git a/C4/Message.pm b/C4/Message.pm index 68205083d59..ee8b75405a5 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 872abfa13b7..ffdb1aa024e 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -842,12 +842,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) = @_; @@ -1854,9 +1855,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' @@ -1916,9 +1914,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 1a4a9205806..6f4d70baaba 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -1014,10 +1014,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', @@ -1031,15 +1031,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 } ) if $message_id; + foreach my $message_id (@message_ids) { + C4::Letters::SendQueuedMessages( { message_id => $message_id } ); + } } } } @@ -1603,25 +1604,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 @@ -2517,7 +2524,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 ) ) { @@ -2661,17 +2669,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 f18b235c27f..6c8565b72e5 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, @@ -159,7 +159,10 @@ sub SendPasswordRecoveryEmail { ); my $num_letters_attempted = 0; - $num_letters_attempted = C4::Letters::SendQueuedMessages( { message_id => $message_id } ) if $message_id; + 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 d6e0846cad2..56a773468aa 100644 --- a/Koha/Patrons/Import.pm +++ b/Koha/Patrons/Import.pm @@ -454,10 +454,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', @@ -471,11 +471,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' } ); @@ -488,7 +487,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 96dc59e9c63..b0dbecb09bd 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 } ) if $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 83b84bc97e3..782cc503d3a 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 } ) - if $acknowledgement_message_id; + foreach my $acknowledgement_message_id (@acknowledgement_message_ids) { + C4::Letters::SendQueuedMessages( + { message_id => $acknowledgement_message_id } ); + } } # Notify cataloger by email @@ -147,16 +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 } ) if $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 e863497d32b..69952abde16 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 21016bc3541..d5aef07039f 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 00000000000..55ca7e01ca7 --- /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 b58114d802e..2ad55fbd2f2 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -225,7 +225,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 484f1d609be..69b1b3f6791 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 c2043b85dd3..e93074a3ef4 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 @@ -194,7 +194,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 14e30e5e6e5..bd7c4820d2f 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt @@ -254,7 +254,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 dd410b59f2b..55b76977585 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/transferstoreceive.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/transferstoreceive.tt @@ -77,11 +77,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 25c1e0363b2..e53f3ca18f2 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 57e780cfecd..2d7c7ace3f9 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -454,9 +454,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', @@ -470,15 +470,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 } ) if $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 11f3faa917a..548eca2170f 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 81220c4b274..3be925c475a 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 6567c35d752..510f390631a 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 6f78b2838a1..8f146931bc1 100755 --- a/misc/cronjobs/overdue_notices.pl +++ b/misc/cronjobs/overdue_notices.pl @@ -617,7 +617,6 @@ END_SQL || $library->inbound_email_address; } @emails_to_use = (); - my $notice_email = $patron->notice_email_address; unless ($nomail) { if (@emails) { foreach (@emails) { @@ -625,7 +624,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; } } @@ -784,7 +784,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' : '', @@ -803,7 +803,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 5f679a99321..1cbdd40882f 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 a0f5a3ace28..7a47542df4a 100755 --- a/opac/opac-memberentry.pl +++ b/opac/opac-memberentry.pl @@ -213,16 +213,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 } ) if $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})); @@ -266,9 +266,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', @@ -282,15 +282,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 } ) if $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 60a004e18fd..504fbd72266 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 } ) if $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 d0ff9fc53a6..6afa1f77125 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 e08b474cb15..b801d94409c 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 29edd822931..2f7fdc465cd 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 5cecfc7b969..a67236d1ce7 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 495ffcdeec3..73b04dda561 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 } ) if $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 cb6da835f30..41feafb7ced 100755 --- a/suggestion/suggestion.pl +++ b/suggestion/suggestion.pl @@ -202,9 +202,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 e07e90eaf82..223ead2f47c 100755 --- a/t/db_dependent/Koha/Patron.t +++ b/t/db_dependent/Koha/Patron.t @@ -1789,7 +1789,7 @@ subtest 'notify_library_of_registration()' => sub { $schema->storage->txn_rollback; }; -subtest 'notice_email_address' => sub { +subtest 'notice_email_addresses' => sub { plan tests => 2; $schema->storage->txn_begin; @@ -1797,10 +1797,10 @@ subtest 'notice_email_address' => sub { 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; $schema->storage->txn_rollback; diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index 85a1159fcda..a5a9ee267c8 100755 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -2018,7 +2018,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 1b4f8f70ebb..b555169a1a6 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 ade28f89ad5..faab8c53959 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' ); @@ -951,8 +948,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|, @@ -975,7 +972,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)"; @@ -993,7 +990,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 @@ -1007,6 +1004,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' ); @@ -1391,7 +1389,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' ); @@ -1400,7 +1398,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"; @@ -1410,3 +1408,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 95673c36c9e..4cc116d6ee5 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 e50f3c3cd35..4ad4987e8e7 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