Bugzilla – Attachment 81975 Details for
Bug 21241
Set suggestion notices message_transport_type to sms if syspref is enabled and patron has an smsalertnumber but no email address
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 21241: (follow-up) Syspref to control fallback to SMS when no email is defined
Bug-21241-follow-up-Syspref-to-control-fallback-to.patch (text/plain), 13.87 KB, created by
Alex Buckley
on 2018-11-06 06:08:25 UTC
(
hide
)
Description:
Bug 21241: (follow-up) Syspref to control fallback to SMS when no email is defined
Filename:
MIME Type:
Creator:
Alex Buckley
Created:
2018-11-06 06:08:25 UTC
Size:
13.87 KB
patch
obsolete
>From 859223f2af6bb96f82227588cc52780301f386d2 Mon Sep 17 00:00:00 2001 >From: Alex Buckley <alexbuckley@catalyst.net.nz> >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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 21241
:
77960
|
79189
|
81625
|
81975
|
82014
|
82018
|
82019
|
82020
|
83517