From 211d56417285349ec9e6a7220bdc010a196c77ee 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 | 41 +++++++++++++++---------------- opac/opac-memberentry.pl | 39 ++++++++++++++--------------- opac/opac-registration-verify.pl | 39 ++++++++++++++--------------- t/db_dependent/Koha/Patrons.t | 21 ++++++++-------- 5 files changed, 95 insertions(+), 87 deletions(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 9282b38307..32f20fb670 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -2144,16 +2144,29 @@ sub to_api_mapping { 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 @@ -2198,12 +2211,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 0193cc3777..9301e2396b 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -450,31 +450,28 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq '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, + my $result = $patron->queue_notice( + { + letter_params => { + module => 'members', + letter_code => 'WELCOME', + branchcode => $patron->branchcode, + lang => $patron->lang || 'default', + tables => { + 'branches' => $patron->branchcode, + 'borrowers' => $patron->borrowernumber, + }, + want_librarian => 1, }, - want_librarian => 1, - ) or return; - - my $message_id = EnqueueLetter( - { - letter => $letter, - borrowernumber => $patron->id, - to_address => $emailaddr, - message_transport_type => 'email' - } - ); + message_transports => ['email', 'sms'], + } + ); + foreach my $message_id (@{ $result->{message_ids} }) { SendQueuedMessages({ message_id => $message_id }); - }; - if ($@) { - $template->param( error_alert => $@ ); } + }; + if ($@) { + $template->param( error_alert => $@ ); } } diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl index 0115d78f98..d79ce7363e 100755 --- a/opac/opac-memberentry.pl +++ b/opac/opac-memberentry.pl @@ -249,29 +249,26 @@ if ( $action eq '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, + my $result = $patron->queue_notice( + { + letter_params => { + module => 'members', + letter_code => 'WELCOME', + branchcode => $patron->branchcode, + lang => $patron->lang || 'default', + tables => { + 'branches' => $patron->branchcode, + 'borrowers' => $patron->borrowernumber, + }, + want_librarian => 1, }, - want_librarian => 1, - ) or return; - - my $message_id = EnqueueLetter( - { - letter => $letter, - borrowernumber => $patron->id, - to_address => $emailaddr, - message_transport_type => 'email' - } - ); + message_transports => ['email', 'sms'], + } + ); + foreach my $message_id (@{ $result->{message_ids} }) { SendQueuedMessages({ message_id => $message_id }); - }; - } + } + }; } # Notify library of new patron registration diff --git a/opac/opac-registration-verify.pl b/opac/opac-registration-verify.pl index 673507ab3e..9a43f09ee7 100755 --- a/opac/opac-registration-verify.pl +++ b/opac/opac-registration-verify.pl @@ -92,29 +92,26 @@ if ( # 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, + my $result = $patron->queue_notice( + { + letter_params => { + module => 'members', + letter_code => 'WELCOME', + branchcode => $patron->branchcode, + lang => $patron->lang || 'default', + tables => { + 'branches' => $patron->branchcode, + 'borrowers' => $patron->borrowernumber, + }, + want_librarian => 1, }, - want_librarian => 1, - ) or return; - - my $message_id = EnqueueLetter( - { - letter => $letter, - borrowernumber => $patron->id, - to_address => $emailaddr, - message_transport_type => 'email' - } - ); + message_transports => ['email', 'sms'], + } + ); + foreach my $message_id (@{ $result->{message_ids} }) { SendQueuedMessages({ message_id => $message_id }); - }; - } + } + }; } # Notify library of new patron registration diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index 8684c8aa15..4bfc0aeaec 100755 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -20,6 +20,7 @@ use Modern::Perl; use Test::More tests => 45; +use Test::Deep qw( cmp_deeply ignore ); use Test::Warn; use Test::Exception; use Test::MockModule; @@ -2275,25 +2276,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"); @@ -2305,9 +2306,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.30.2