From 859223f2af6bb96f82227588cc52780301f386d2 Mon Sep 17 00:00:00 2001 From: Alex Buckley Date: Wed, 31 Oct 2018 13:52:29 +0000 Subject: [PATCH] Bug 21241: (follow-up) Syspref to control fallback to SMS when no email is defined This patch adds a new system preference (MessageTransportTypeFallbackToSMSIfNoPatronEmail) which if enabled Koha will send suggestion notices by SMS if a borrower has a defined SMSalertnumber and no email. The use of the syspref prevents automatic fallback to sending suggestion notices as SMS when there's no defined email. Test plan: 1. Chose a patron who has no email address set, but does have a smsalertnumber set (this value is set in the Patron messaging preferences section after the SMSSendDriver syspref is set) 2. Log into the OPAC with that user and submit a suggestion 3. In the staff client go to Acquisitions->Suggestions and tick the suggestion and set its status to 'Accepted' 4. In the database query the message_queue and notice the message_transport_type of the message is set to 'email' even though the patron has no email address set. 5. Apply patches, restart memcached and plack 6. Check the 'MessageTransportTypeFallbackToSMSIfNoPatronEmail' syspref is disabled 7. Repeat steps 2,3 and observe in the message_queue table the message_transport_type = 'email' i.e. If the syspref is disabled then the message is still sent by email to borrowers with defined smsalertnumber and no email address 8. Enable the 'MessageTransportTypeFallbackToSMSIfNoPatronEmail' syspref and repeat steps 2,3 and notice the message_transport_type = 'sms' i.e. If the syspref is enabled then the message is sent by sms to borrowers with defined smsalertnumber and no email address 9. Repeat steps 2,3 with a patron with an email address and no smsalertnumber trying with the 'MessageTransportTypeFallbackToSMSIfNoPatronEmail' syspref enabled and disabled and notice in both cases the message_transport_type = email. i.e. If a borrower has an email address defined the suggestion notice will always be sent via email 10. Repeat steps 2,3 with a patron with no email or smsalertnumber trying with the 'MessageTransportTypeFallbackToSMSIfNoPatronEmail' syspref enabled and disabled and notice in both cases the message_transport_type = email i.e. If the borrower has no smsalertnumber and no email defined then the suggestion notice will be sent by 'email' 11. Run t/db_dependent/Suggestions.t Sponsored-By: Brimbank Libraries, Australia --- C4/Suggestions.pm | 14 ++-- ...control_fallback_to_sms_if_no_email_defined.sql | 2 + .../prog/en/modules/admin/preferences/patrons.pref | 6 ++ t/db_dependent/Suggestions.t | 74 ++++++++++++++++++---- 4 files changed, 77 insertions(+), 19 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_21241-add_syspref_to_control_fallback_to_sms_if_no_email_defined.sql diff --git a/C4/Suggestions.pm b/C4/Suggestions.pm index e3a7ffd..27ee447 100644 --- a/C4/Suggestions.pm +++ b/C4/Suggestions.pm @@ -504,14 +504,16 @@ sub ModSuggestion { my $full_suggestion = GetSuggestion( $suggestion->{suggestionid} ); my $patron = Koha::Patrons->find( $full_suggestion->{suggestedby} ); my $transport; - - #Set message_transport_type of suggestion notice to email by default, unless the patron has a smsalertnumber set and no email address set - if ($patron->smsalertnumber && (!$patron->email)) { - $transport="sms"; + #If MessageTransportTypeFallbackToSMSIfNoPatronEmail syspref is enabled and the patron has a smsalertnumber but no email address then set message_transport_type of suggestion notice to sms + if (C4::Context->preference("MessageTransportTypeFallbackToSMSIfNoPatronEmail")) { + if (($patron->smsalertnumber) && (!$patron->email)) { + $transport="sms"; + } else { + $transport="email"; + } } else { - $transport="email"; + $transport = "email"; } - if ( my $letter = C4::Letters::GetPreparedLetter( module => 'suggestions', diff --git a/installer/data/mysql/atomicupdate/bug_21241-add_syspref_to_control_fallback_to_sms_if_no_email_defined.sql b/installer/data/mysql/atomicupdate/bug_21241-add_syspref_to_control_fallback_to_sms_if_no_email_defined.sql new file mode 100644 index 0000000..f1cdba8 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_21241-add_syspref_to_control_fallback_to_sms_if_no_email_defined.sql @@ -0,0 +1,2 @@ +INSERT INTO systempreferences (variable, value, options, explanation, type) VALUES ('MessageTransportTypeFallbackToSMSIfNoPatronEmail', 0, 'Enable|Disable', 'Send messages by SMS if no patron email is defined', 'YesNo'); + 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 2496069..857b8b5 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 @@ -7,6 +7,12 @@ Patrons: no: "Don't send" - an email to newly created patrons with their account details. - + - pref: MessageTransportTypeFallbackToSMSIfNoPatronEmail + choices: + yes: Enable + no: Disable + - Send messages by SMS if no patron email is defined + - - pref: UseEmailReceipts choices: yes: Send diff --git a/t/db_dependent/Suggestions.t b/t/db_dependent/Suggestions.t index 2d2bfb5..4996051 100644 --- a/t/db_dependent/Suggestions.t +++ b/t/db_dependent/Suggestions.t @@ -18,7 +18,7 @@ use Modern::Perl; use DateTime::Duration; -use Test::More tests => 103; +use Test::More tests => 106; use Test::Warn; use t::lib::Mocks; @@ -74,8 +74,19 @@ my $member = { surname => 'my surname', categorycode => $patron_category->{categorycode}, branchcode => 'CPL', + smsalertnumber => 12345, }; + +my $member2 = { + firstname => 'my firstname', + surname => 'my surname', + categorycode => $patron_category->{categorycode}, + branchcode => 'CPL', + email => 'to@example.com', +}; + my $borrowernumber = Koha::Patron->new($member)->store->borrowernumber; +my $borrowernumber2 = Koha::Patron->new($member2)->store->borrowernumber; my $biblionumber1 = 1; my $my_suggestion = { @@ -116,7 +127,18 @@ my $my_suggestion_with_budget = { note => 'my note', budgetid => $budget_id, }; - +my $my_suggestion_with_budget2 = { + title => 'my title 3', + author => 'my author 3', + publishercode => 'my publishercode 3', + suggestedby => $borrowernumber2, + biblionumber => $biblionumber1, + managedby => '', + manageddate => '', + accepteddate => dt_from_string, + note => 'my note', + budgetid => $budget_id, +}; is( CountSuggestion(), 0, 'CountSuggestion without the status returns 0' ); is( CountSuggestion('ASKED'), 0, 'CountSuggestion returns the correct number of suggestions' ); @@ -184,8 +206,12 @@ my $mod_suggestion3 = { STATUS => 'CHECKED', suggestionid => $my_suggestionid, }; -$status = ModSuggestion($mod_suggestion3); +#Test the message_transport_type of suggestion notices + +#Check the message_transport_type when the 'MessageTransportTypeFallbackToSMSIfNoPatronEmail' syspref is disabled +t::lib::Mocks::mock_preference( 'MessageTransportTypeFallbackToSMSIfNoPatronEmail', 0 ); +$status = ModSuggestion($mod_suggestion3); is( $status, 1, 'ModSuggestion modifies one entry' ); $suggestion = GetSuggestion($my_suggestionid); is( $suggestion->{STATUS}, $mod_suggestion3->{STATUS}, 'ModSuggestion modifies the status correctly' ); @@ -193,9 +219,31 @@ $messages = C4::Letters::GetQueuedMessages({ borrowernumber => $borrowernumber, }); is( @$messages, 1, 'ModSuggestion sends an email if the status is updated' ); - +is ($messages->[0]->{message_transport_type}, 'email', 'When MessageTransportTypeFallbackToSMSIfNoPatronEmail syspref is disabled the suggestion message_transport_type is always email'); is( CountSuggestion('CHECKED'), 1, 'CountSuggestion returns the correct number of suggestions' ); +#Check the message_transport_type when the 'MessageTransportTypeFallbackToSMSIfNoPatronEmail' syspref is enabled and the borrower has a smsalertnumber and no email +t::lib::Mocks::mock_preference( 'MessageTransportTypeFallbackToSMSIfNoPatronEmail', 1 ); +ModSuggestion($mod_suggestion3); +$messages = C4::Letters::GetQueuedMessages({ + borrowernumber => $borrowernumber, +}); +is ($messages->[1]->{message_transport_type}, 'sms', 'When MessageTransportTypeFallbackToSMSIfNoPatronEmail syspref is enabled the suggestion message_transport_type is sms if the borrower has no email'); + +#Make a new suggestion for a borrower with defined email and no smsalertnumber +my $my_suggestion_2_id = NewSuggestion($my_suggestion_with_budget2); + +#Check the message_transport_type when the 'MessageTransportTypeFallbackToSMSIfNoPatronEmail' syspref is enabled and the borrower has a defined email and no smsalertnumber +t::lib::Mocks::mock_preference( 'MessageTransportTypeFallbackToSMSIfNoPatronEmail', 1 ); +my $mod_suggestion4 = { + STATUS => 'CHECKED', + suggestionid => $my_suggestion_2_id, +}; +ModSuggestion($mod_suggestion4); +$messages = C4::Letters::GetQueuedMessages({ + borrowernumber => $borrowernumber2, +}); +is ($messages->[0]->{message_transport_type}, 'email', 'When MessageTransportTypeFallbackToSMSIfNoPatronEmail syspref is enabled the suggestion message_transport_type is email if the borrower has an email'); is( GetSuggestionInfo(), undef, 'GetSuggestionInfo without the suggestion id returns undef' ); $suggestion = GetSuggestionInfo($my_suggestionid); @@ -234,7 +282,7 @@ is( $suggestion->{borrnumsuggestedby}, $my_suggestion->{suggestedby}, 'GetSugges my $suggestions = GetSuggestionByStatus(); is( @$suggestions, 0, 'GetSuggestionByStatus without the status returns an empty array' ); $suggestions = GetSuggestionByStatus('CHECKED'); -is( @$suggestions, 1, 'GetSuggestionByStatus returns the correct number of suggestions' ); +is( @$suggestions, 2, 'GetSuggestionByStatus returns the correct number of suggestions' ); is( $suggestions->[0]->{suggestionid}, $my_suggestionid, 'GetSuggestionByStatus returns the suggestion id correctly' ); is( $suggestions->[0]->{title}, $mod_suggestion1->{title}, 'GetSuggestionByStatus returns the title correctly' ); is( $suggestions->[0]->{author}, $mod_suggestion1->{author}, 'GetSuggestionByStatus returns the author correctly' ); @@ -257,7 +305,7 @@ $suggestion = GetSuggestion($my_suggestionid); is( $suggestion->{biblionumber}, $biblionumber2, 'ConnectSuggestionAndBiblio updates the biblio number correctly' ); my $search_suggestion = SearchSuggestion(); -is( @$search_suggestion, 2, 'SearchSuggestion without arguments returns all suggestions' ); +is( @$search_suggestion, 3, 'SearchSuggestion without arguments returns all suggestions' ); $search_suggestion = SearchSuggestion({ title => $mod_suggestion1->{title}, @@ -289,7 +337,7 @@ is( @$search_suggestion, 0, 'SearchSuggestion returns the correct number of sugg $search_suggestion = SearchSuggestion({ STATUS => $mod_suggestion3->{STATUS}, }); -is( @$search_suggestion, 1, 'SearchSuggestion returns the correct number of suggestions' ); +is( @$search_suggestion, 2, 'SearchSuggestion returns the correct number of suggestions' ); $search_suggestion = SearchSuggestion({ STATUS => q|| @@ -303,11 +351,11 @@ is( @$search_suggestion, 0, 'SearchSuggestion returns the correct number of sugg $search_suggestion = SearchSuggestion({ budgetid => '', }); -is( @$search_suggestion, 2, 'SearchSuggestion (budgetid = "") returns the correct number of suggestions' ); +is( @$search_suggestion, 3, 'SearchSuggestion (budgetid = "") returns the correct number of suggestions' ); $search_suggestion = SearchSuggestion({ budgetid => $budget_id, }); -is( @$search_suggestion, 1, 'SearchSuggestion (budgetid = $budgetid) returns the correct number of suggestions' ); +is( @$search_suggestion, 2, 'SearchSuggestion (budgetid = $budgetid) returns the correct number of suggestions' ); $search_suggestion = SearchSuggestion({ budgetid => '__NONE__', }); @@ -315,7 +363,7 @@ is( @$search_suggestion, 1, 'SearchSuggestion (budgetid = "__NONE__") returns th $search_suggestion = SearchSuggestion({ budgetid => '__ANY__', }); -is( @$search_suggestion, 2, 'SearchSuggestion (budgetid = "__ANY__") returns the correct number of suggestions' ); +is( @$search_suggestion, 3, 'SearchSuggestion (budgetid = "__ANY__") returns the correct number of suggestions' ); my $del_suggestion = { title => 'my deleted title', @@ -324,7 +372,7 @@ my $del_suggestion = { }; my $del_suggestionid = NewSuggestion($del_suggestion); -is( CountSuggestion('CHECKED'), 2, 'CountSuggestion returns the correct number of suggestions' ); +is( CountSuggestion('CHECKED'), 3, 'CountSuggestion returns the correct number of suggestions' ); is( DelSuggestion(), '0E0', 'DelSuggestion without arguments returns 0E0' ); is( DelSuggestion($borrowernumber), '', 'DelSuggestion without the suggestion id returns an empty string' ); @@ -333,8 +381,8 @@ $suggestion = DelSuggestion($borrowernumber, $my_suggestionid); is( $suggestion, 1, 'DelSuggestion deletes one suggestion' ); $suggestions = GetSuggestionByStatus('CHECKED'); -is( @$suggestions, 1, 'DelSuggestion deletes one suggestion' ); -is( $suggestions->[0]->{title}, $del_suggestion->{title}, 'DelSuggestion deletes the correct suggestion' ); +is( @$suggestions, 2, 'DelSuggestion deletes one suggestion' ); +is( $suggestions->[1]->{title}, $del_suggestion->{title}, 'DelSuggestion deletes the correct suggestion' ); # Test budgetid fk $my_suggestion->{budgetid} = ''; # If budgetid == '', NULL should be set in DB -- 2.1.4