From ea9dbf6a81e3f9b36b4b9025fd35c806c78eb915 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Wed, 18 Nov 2020 04:10:48 +0000 Subject: [PATCH] Bug 17499: (follow-up) Validate phone and itiva transport types Message transport types 'phone' and 'itiva' were not validated unlike 'email' and 'sms'. This patch adds equivalent checks to the missing mtts. To test: 1. prove t/db_dependent/Koha/Patron/Message/Preferences.t Sponsored-by: The National Library of Finland Signed-off-by: Michal Denar Signed-off-by: Sam Lau Signed-off-by: Marcel de Rooy Signed-off-by: Tomas Cohen Arazi --- Koha/Exceptions/Patron/Message/Preference.pm | 10 +++++ Koha/Patron/Message/Preference.pm | 21 +++++++++ .../Koha/Patron/Message/Preferences.t | 43 +++++++++++++++++-- 3 files changed, 71 insertions(+), 3 deletions(-) diff --git a/Koha/Exceptions/Patron/Message/Preference.pm b/Koha/Exceptions/Patron/Message/Preference.pm index 23f78029f5..ddde51e181 100644 --- a/Koha/Exceptions/Patron/Message/Preference.pm +++ b/Koha/Exceptions/Patron/Message/Preference.pm @@ -57,11 +57,21 @@ use Exception::Class ( description => "Transport type not available for this message.", fields => ['message_name', 'transport_type'] }, + 'Koha::Exceptions::Patron::Message::Preference::PhoneNumberRequired' => { + isa => 'Koha::Exceptions::Patron::Message::Preference', + description => "Patron has no phone number, cannot use phone transport type.", + fields => ['message_name', 'borrowernumber' ] + }, 'Koha::Exceptions::Patron::Message::Preference::SMSNumberRequired' => { isa => 'Koha::Exceptions::Patron::Message::Preference', description => "Patron has no SMS number, cannot use sms transport type.", fields => ['message_name', 'borrowernumber' ] }, + 'Koha::Exceptions::Patron::Message::Preference::TalkingTechItivaPhoneNotificationRequired' => { + isa => 'Koha::Exceptions::Patron::Message::Preference', + description => "System preference TalkingTechItivaPhoneNotification is disabled, cannot use itiva transport type.", + fields => ['message_name', 'borrowernumber' ] + }, ); 1; diff --git a/Koha/Patron/Message/Preference.pm b/Koha/Patron/Message/Preference.pm index 53b17011a7..f50df687c9 100644 --- a/Koha/Patron/Message/Preference.pm +++ b/Koha/Patron/Message/Preference.pm @@ -416,6 +416,27 @@ sub _set_message_transport_types { ); } } + elsif ($type eq 'itiva') { + if (!C4::Context->preference('TalkingTechItivaPhoneNotification')) { + Koha::Exceptions::Patron::Message::Preference::TalkingTechItivaPhoneNotificationRequired->throw( + error => 'System preference TalkingTechItivaPhoneNotification disabled'. + 'cannot use itiva as message transport', + message_name => $self->message_name, + borrowernumber => $self->borrowernumber, + ); + } + } + elsif ($type eq 'phone') { + if ( !$patron->phone ) { + Koha::Exceptions::Patron::Message::Preference::PhoneNumberRequired->throw( + error => 'Patron has not set phone number'. + 'cannot use phone as message transport', + message_name => $self->message_name, + borrowernumber => $self->borrowernumber, + ); + } + + } elsif ($type eq 'sms') { if ( !$patron->smsalertnumber ){ Koha::Exceptions::Patron::Message::Preference::SMSNumberRequired->throw( diff --git a/t/db_dependent/Koha/Patron/Message/Preferences.t b/t/db_dependent/Koha/Patron/Message/Preferences.t index 7bc08103a6..75cdb600e5 100644 --- a/t/db_dependent/Koha/Patron/Message/Preferences.t +++ b/t/db_dependent/Koha/Patron/Message/Preferences.t @@ -282,7 +282,7 @@ HERE }; subtest 'set message_transport_types' => sub { - plan tests => 12; + plan tests => 18; $schema->storage->txn_begin; @@ -339,10 +339,16 @@ HERE }); is($stored_transports->count, 2, 'Two transports selected'); - # Test email and smsalertnumber validation + # Test contact information validation eval { Koha::Patron::Message::Transport::Types->new({ message_transport_type => 'email' })->store }; + eval { Koha::Patron::Message::Transport::Types->new({ + message_transport_type => 'itiva' + })->store }; + eval { Koha::Patron::Message::Transport::Types->new({ + message_transport_type => 'phone' + })->store }; eval { Koha::Patron::Message::Transport::Types->new({ message_transport_type => 'sms' })->store }; @@ -351,12 +357,23 @@ HERE message_transport_type => 'email', is_digest => 1 })->store; + Koha::Patron::Message::Transport->new({ + message_attribute_id => $preference->message_attribute_id, + message_transport_type => 'itiva', + is_digest => 1 + })->store; + Koha::Patron::Message::Transport->new({ + message_attribute_id => $preference->message_attribute_id, + message_transport_type => 'phone', + is_digest => 1 + })->store; Koha::Patron::Message::Transport->new({ message_attribute_id => $preference->message_attribute_id, message_transport_type => 'sms', is_digest => 1 })->store; - $patron->set({ email => '', smsalertnumber => '' })->store; + t::lib::Mocks::mock_preference('TalkingTechItivaPhoneNotification', 0); + $patron->set({ email => '', phone => '', smsalertnumber => '' })->store; eval { $preference->message_transport_types('email')->store; }; @@ -367,6 +384,26 @@ HERE .' tells us it which message name it was.'); is ($@->borrowernumber, $patron->borrowernumber, 'The previous exception ' .' tells us which patron it was.'); + eval { + $preference->message_transport_types('itiva')->store; + }; + is (ref $@, 'Koha::Exceptions::Patron::Message::Preference::TalkingTechItivaPhoneNotificationRequired', + 'Adding a message preference with invalid message_transport_type' + .' => Koha::Exceptions::Patron::Message::Preference::TalkingTechItivaPhoneNotificationRequired'); + is ($@->message_name, $preference->message_name, 'The previous exception ' + .' tells us it which message name it was.'); + is ($@->borrowernumber, $patron->borrowernumber, 'The previous exception ' + .' tells us which patron it was.'); + eval { + $preference->message_transport_types('phone')->store; + }; + is (ref $@, 'Koha::Exceptions::Patron::Message::Preference::PhoneNumberRequired', + 'Adding a message preference with invalid message_transport_type' + .' => Koha::Exceptions::Patron::Message::Preference::PhoneNumberRequired'); + is ($@->message_name, $preference->message_name, 'The previous exception ' + .' tells us it which message name it was.'); + is ($@->borrowernumber, $patron->borrowernumber, 'The previous exception ' + .' tells us which patron it was.'); eval { $preference->message_transport_types('sms')->store; }; -- 2.39.5