From 8460304b42d8dac5590ca029b1c6bc295bb09d19 Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Thu, 21 Jul 2022 14:21:17 +0200 Subject: [PATCH] Bug 33221: Send WELCOME notices by sms too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This affects patron creation from staff and OPAC interfaces, and also when patrons are imported by misc/import_patrons.pl Since it's not possible at the moment to provide an SMS number when creating an account from OPAC, the changes at OPAC will only be useful once that possibility is added (see bug 20859) Test plan: 1. Enable system preference AutoEmailNewUser 2. Set system preference SMSSendDriver to 'Test' 3. Verify in "About" page that SMS::Send is installed 4. In "Notices and Slips" tool, edit the 'WELCOME' letter and verify that both email and sms parts are filled. Use a different title and body for the sms part 5. Create a new borrower with an email address and an SMS number 6. Verify in the "Notices" tab that two notices are pending, one for email, one for sms 7. Click on "Send welcome email" in the "More" menu 8. Verify that two new notices have been queued (like in step 6) 9. Create a 'borrowers.csv' file with the following content: cardnumber,surname,categorycode,branchcode,email,smsalertnumber foo,Foo,PT,CPL,foo@example.com,+33123456789 10. Import it with the following command misc/import_patrons.pl -v -c -m cardnumber -f borrowers.csv --email-new 11. Verify that the patron has been imported and that two new notices have been queued (like in step 6) 12. Enable system preference PatronSelfRegistration and disable system preference PatronSelfRegistrationVerifyByEmail 13. Go to OPAC and register a new account with an email address 14. Verify that the email notice was queued 15. Enable PatronSelfRegistrationVerifyByEmail 16. Go to OPAC and register a new account with an email address. Go through the verification process 17. Verify that the email notice was queued Sponsored-by: Médiathèque de Montauban --- Koha/Patron.pm | 42 ++++++++++++++++++++++---------- members/memberentry.pl | 34 ++++++++++++-------------- opac/opac-memberentry.pl | 34 ++++++++++++-------------- opac/opac-registration-verify.pl | 34 ++++++++++++-------------- t/db_dependent/Koha/Patrons.t | 22 ++++++++--------- 5 files changed, 88 insertions(+), 78 deletions(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index fcbd6cd71e..556da6f049 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -2538,16 +2538,29 @@ sub strings_map { Koha::Patrons->queue_notice({ letter_params => $letter_params, message_transports => \@message_transports }); Koha::Patrons->queue_notice({ letter_params => $letter_params, message_transports => \@message_transports, test_mode => 1 }); - Queue messages to a patron. Can pass a message that is part of the message_attributes - table or supply the transport to use. +Queue messages to a patron. Can pass a message that is part of the +message_attributes table or supply the transport to use. - If passed a message name we retrieve the patrons preferences for transports - Otherwise we use the supplied transport. In the case of email or sms we fall back to print if - we have no address/number for sending +If passed a message name we retrieve the patrons preferences for transports +Otherwise we use the supplied transport. In the case of email or sms we fall +back to print if we have no address/number for sending - $letter_params is a hashref of the values to be passed to GetPreparedLetter +$letter_params is a hashref of the values to be passed to GetPreparedLetter - test_mode will only report which notices would be sent, but nothing will be queued +test_mode will only report which notices would be sent, but nothing will be +queued + +Returns a hashref with the following keys: + +=over + +=item * C - a list of transport types for which a message has been queued + +=item * C - a list of transport types for which a message has not been queued + +=item * C - a list of ids of message that have been queued (only if test_mode is disabled) + +=back =cut @@ -2592,12 +2605,15 @@ sub queue_notice { next if $mtt eq 'print' && $print_sent; $letter_params->{message_transport_type} = $mtt; my $letter = C4::Letters::GetPreparedLetter( %$letter_params ); - C4::Letters::EnqueueLetter({ - letter => $letter, - borrowernumber => $self->borrowernumber, - from_address => $from_email_address, - message_transport_type => $mtt - }) unless $test_mode; + unless ($test_mode) { + my $message_id = C4::Letters::EnqueueLetter({ + letter => $letter, + borrowernumber => $self->borrowernumber, + from_address => $from_email_address, + message_transport_type => $mtt + }); + push @{$return{message_ids}}, $message_id if $message_id; + } push @{$return{sent}}, $mtt; $print_sent = 1 if $mtt eq 'print'; } diff --git a/members/memberentry.pl b/members/memberentry.pl index 2227df425f..1da4c53367 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -452,27 +452,25 @@ if ((!$nok) and $nodouble and ($op eq 'cud-insert' or $op eq 'cud-save')){ # if we manage to find a valid email address, send notice if ($emailaddr) { eval { - my $letter = GetPreparedLetter( - module => 'members', - letter_code => 'WELCOME', - branchcode => $patron->branchcode,, - lang => $patron->lang || 'default', - tables => { - 'branches' => $patron->branchcode, - 'borrowers' => $patron->borrowernumber, - }, - want_librarian => 1, - ) or return; - - my $message_id = EnqueueLetter( + my $result = $patron->queue_notice( { - letter => $letter, - borrowernumber => $patron->id, - to_address => $emailaddr, - message_transport_type => 'email' + letter_params => { + module => 'members', + letter_code => 'WELCOME', + branchcode => $patron->branchcode, + lang => $patron->lang || 'default', + tables => { + 'branches' => $patron->branchcode, + 'borrowers' => $patron->borrowernumber, + }, + want_librarian => 1, + }, + message_transports => ['email', 'sms'], } ); - SendQueuedMessages( { message_id => $message_id } ) if $message_id; + foreach my $message_id ( @{ $result->{message_ids} } ) { + SendQueuedMessages( { message_id => $message_id } ); + } }; if ($@) { $template->param( error_alert => $@ ); diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl index b671dde7c0..e0a563ddfa 100755 --- a/opac/opac-memberentry.pl +++ b/opac/opac-memberentry.pl @@ -265,27 +265,25 @@ if ( $op eq 'cud-create' ) { # if we manage to find a valid email address, send notice if ($emailaddr) { eval { - my $letter = GetPreparedLetter( - module => 'members', - letter_code => 'WELCOME', - branchcode => $patron->branchcode,, - lang => $patron->lang || 'default', - tables => { - 'branches' => $patron->branchcode, - 'borrowers' => $patron->borrowernumber, - }, - want_librarian => 1, - ) or return; - - my $message_id = EnqueueLetter( + my $result = $patron->queue_notice( { - letter => $letter, - borrowernumber => $patron->id, - to_address => $emailaddr, - message_transport_type => 'email' + letter_params => { + module => 'members', + letter_code => 'WELCOME', + branchcode => $patron->branchcode, + lang => $patron->lang || 'default', + tables => { + 'branches' => $patron->branchcode, + 'borrowers' => $patron->borrowernumber, + }, + want_librarian => 1, + }, + message_transports => ['email', 'sms'], } ); - SendQueuedMessages( { message_id => $message_id } ) if $message_id; + foreach my $message_id ( @{ $result->{message_ids} } ) { + SendQueuedMessages( { message_id => $message_id } ); + } }; } } diff --git a/opac/opac-registration-verify.pl b/opac/opac-registration-verify.pl index d3ae183384..6f5eab1201 100755 --- a/opac/opac-registration-verify.pl +++ b/opac/opac-registration-verify.pl @@ -126,27 +126,25 @@ elsif ( $rego_found # if we manage to find a valid email address, send notice if ($emailaddr) { eval { - my $letter = GetPreparedLetter( - module => 'members', - letter_code => 'WELCOME', - branchcode => $patron->branchcode,, - lang => $patron->lang || 'default', - tables => { - 'branches' => $patron->branchcode, - 'borrowers' => $patron->borrowernumber, - }, - want_librarian => 1, - ) or return; - - my $message_id = EnqueueLetter( + my $result = $patron->queue_notice( { - letter => $letter, - borrowernumber => $patron->id, - to_address => $emailaddr, - message_transport_type => 'email' + letter_params => { + module => 'members', + letter_code => 'WELCOME', + branchcode => $patron->branchcode, + lang => $patron->lang || 'default', + tables => { + 'branches' => $patron->branchcode, + 'borrowers' => $patron->borrowernumber, + }, + want_librarian => 1, + }, + message_transports => ['email', 'sms'], } ); - SendQueuedMessages( { message_id => $message_id } ) if $message_id; + foreach my $message_id ( @{ $result->{message_ids} } ) { + SendQueuedMessages( { message_id => $message_id } ); + } }; } } diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index d3adc77dd7..4cd20cbdfa 100755 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 43; +use Test::More tests => 44; use Test::Warn; use Test::Exception; use Test::MockModule; @@ -2354,25 +2354,25 @@ subtest 'queue_notice' => sub { is( $patron->queue_notice(), undef, "Nothing is done if no params passed"); is( $patron->queue_notice({ letter_params => $letter_params }),undef, "Nothing done if only letter"); - is_deeply( + cmp_deeply( $patron->queue_notice({ letter_params => $letter_params, message_transports => \@mtts }), - {sent => ['email'] }, "Email sent" + {sent => ['email'], message_ids => [ignore()] }, "Email sent" ); $patron->email("")->store; - is_deeply( + cmp_deeply( $patron->queue_notice({ letter_params => $letter_params, message_transports => \@mtts }), - {sent => ['print'],fallback => ['email']}, "Email fallsback to print if no email" + {sent => ['print'],fallback => ['email'], message_ids => [ignore()]}, "Email fallsback to print if no email" ); push @mtts, 'sms'; - is_deeply( + cmp_deeply( $patron->queue_notice({ letter_params => $letter_params, message_transports => \@mtts }), - {sent => ['print','sms'],fallback => ['email']}, "Email fallsback to print if no email, sms sent" + {sent => ['print','sms'],fallback => ['email'], message_ids => [ignore(), ignore()]}, "Email fallsback to print if no email, sms sent" ); $patron->smsalertnumber("")->store; my $counter = Koha::Notice::Messages->search({borrowernumber => $patron->borrowernumber })->count; - is_deeply( + cmp_deeply( $patron->queue_notice({ letter_params => $letter_params, message_transports => \@mtts }), - {sent => ['print'],fallback => ['email','sms']}, "Email fallsback to print if no emai, sms fallsback to print if no sms, only one print sent" + {sent => ['print'],fallback => ['email','sms'], message_ids => [ignore()]}, "Email fallsback to print if no emai, sms fallsback to print if no sms, only one print sent" ); is( Koha::Notice::Messages->search({borrowernumber => $patron->borrowernumber })->count, $counter+1,"Count of queued notices went up by one"); @@ -2384,9 +2384,9 @@ subtest 'queue_notice' => sub { is( $patron->queue_notice({ letter_params => $letter_params, message_transports => \@mtts, message_name => 'Hold_Filled' }),undef, "Nothing done if transports and name sent"); $patron->email(q|awesome@ismymiddle.name|)->store; - is_deeply( + cmp_deeply( $patron->queue_notice({ letter_params => $letter_params, message_name => 'Hold_Filled' }), - {sent => ['email'] }, "Email sent when using borrower preferences" + {sent => ['email'], message_ids => [ignore()] }, "Email sent when using borrower preferences" ); $counter = Koha::Notice::Messages->search({borrowernumber => $patron->borrowernumber })->count; is_deeply( -- 2.34.1