@@ -, +, @@ --email is provided. (or fill it with a single value if it is not done yet). following letter code: HOLD, CHECKIN, CHECKOUT, PREDUE, RENEW, DUE). and 1 with the emailpro. is empty. --- 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 --- a/C4/Auth.pm +++ a/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 ); } --- a/C4/Auth_with_ldap.pm +++ a/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 { --- a/C4/Auth_with_shibboleth.pm +++ a/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 ); --- a/C4/Circulation.pm +++ a/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; } --- a/C4/Letters.pm +++ a/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( { --- a/C4/Message.pm +++ a/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) = @_; --- a/C4/Reserves.pm +++ a/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 --- a/Koha/Patron.pm +++ a/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"; } } --- a/Koha/Patron/Password/Recovery.pm +++ a/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); } --- a/Koha/Patrons/Import.pm +++ a/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) } ); } --- a/Koha/REST/V1/TwoFactorAuth.pm +++ a/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 => { --- a/Koha/Ticket.pm +++ a/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 } ); + } } } } --- a/basket/sendbasket.pl +++ a/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'); --- a/circ/pendingreserves.pl +++ a/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' }; --- a/installer/data/mysql/atomicupdate/bug-12802.pl +++ a/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'"); + }, +}; --- a/installer/data/mysql/mandatory/sysprefs.sql +++ a/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'), --- a/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc +++ a/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 %] --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref +++ a/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 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt +++ a/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(); --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/transferstoreceive.tt +++ a/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 ) %] --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/clubs/clubs-tab.tt +++ a/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 ) %] --- a/members/memberentry.pl +++ a/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 => $@ ); --- a/members/notices.pl +++ a/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 --- a/members/two_factor_auth.pl +++ a/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 => { --- a/misc/cronjobs/notice_unprocessed_suggestions.pl +++ a/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', --- a/misc/cronjobs/overdue_notices.pl +++ a/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' : '', --- a/misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl +++ a/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', --- a/opac/opac-memberentry.pl +++ a/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 }); + } }; } } --- a/opac/opac-registration-verify.pl +++ a/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 }); + } }; } } --- a/opac/opac-sendbasket.pl +++ a/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'); --- a/opac/opac-sendshelf.pl +++ a/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; --- a/opac/opac-shareshelf.pl +++ a/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, } ); --- a/opac/opac-shelves.pl +++ a/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 ); --- a/pos/pay.pl +++ a/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( --- a/suggestion/suggestion.pl +++ a/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', --- a/t/db_dependent/Koha/Patron.t +++ a/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; --- a/t/db_dependent/Koha/Patrons.t +++ a/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; --- a/t/db_dependent/Koha/Recall.t +++ a/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' }); --- a/t/db_dependent/Letters.t +++ a/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' ); +}; --- a/t/db_dependent/Message.t +++ a/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'"); --- a/virtualshelves/sendshelf.pl +++ a/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; --