From 7d298d346387df1f0fdad84ea94458cb7998bd82 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Wed, 18 Nov 2020 02:15:23 +0000 Subject: [PATCH] Bug 17499: (follow-up) Contact information vs. mtt validation This patch adds a subroutine that handles message transport type validation by first checking related patron contact information. As an example, if there is no email address, we shouldn't let email be selected as a messaging transport type. The reason to isolate logic into a separate subroutine is that C4/Reserves.pm _koha_notify_reserve() also uses the same logic and we should not duplicate it. C4::Reserves::_koha_notify_reserve() will be adjusted to use this new sub in Bug 18595. To test: 1. prove t/db_dependent/Koha/Patron/Message/Preferences.t Sponsored-by: The National Library of Finland Signed-off-by: Michal Denar --- Koha/Patron/Message/Preference.pm | 95 +++++++++++-------- .../Koha/Patron/Message/Preferences.t | 82 +++++++++++++++- 2 files changed, 133 insertions(+), 44 deletions(-) diff --git a/Koha/Patron/Message/Preference.pm b/Koha/Patron/Message/Preference.pm index f50df687c9..4de5bc5725 100644 --- a/Koha/Patron/Message/Preference.pm +++ b/Koha/Patron/Message/Preference.pm @@ -228,6 +228,34 @@ sub message_transport_types { } } +=head3 mtt_deliverable + +$preference->mtt_deliverable('sms'[, $borrowernumer]); + +Returns true if given message transport type can be used to deliver message to +patron. + +By default, uses the borrowernumber bound to C<$preference>, but this may be +overridden by providing optional C<$borrowernumber> parameter. + +=cut + +sub mtt_deliverable { + my ( $self, $mtt, $borrowernumber ) = @_; + + $borrowernumber //= $self->borrowernumber; + + return 0 unless ($borrowernumber); + + my $patron = Koha::Patrons->find($self->borrowernumber); + + return (( $mtt eq 'email' and $patron->notice_email_address ) # No email address + or ( $mtt eq 'sms' and $patron->smsalertnumber ) # No SMS number + or ( $mtt eq 'itiva' and C4::Context->preference('TalkingTechItivaPhoneNotification') ) # Notice is handled by TalkingTech_itiva_outbound.pl + or ( $mtt eq 'phone' and $patron->phone )) # No phone number to call + ? 1 : 0; +} + =head3 set $preference->set({ @@ -404,48 +432,31 @@ sub _set_message_transport_types { ); } if (defined $self->borrowernumber) { - my $patron = Koha::Patrons->find($self->borrowernumber); - if ($type eq 'email') { - if ( !$patron->email ) - { - Koha::Exceptions::Patron::Message::Preference::EmailAddressRequired->throw( - error => 'Patron has not set email address, '. - 'cannot use email as message transport', - message_name => $self->message_name, - borrowernumber => $self->borrowernumber, - ); - } - } - 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( - error => 'Patron has not set SMS number'. - 'cannot use sms as message transport', - message_name => $self->message_name, - borrowernumber => $self->borrowernumber, - ); - } + if ( ! $self->mtt_deliverable($type) ) { + Koha::Exceptions::Patron::Message::Preference::EmailAddressRequired->throw( + error => 'Patron has not set email address, '. + 'cannot use email as message transport', + message_name => $self->message_name, + borrowernumber => $self->borrowernumber, + ) if $type eq 'email'; + 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, + ) if $type eq 'itiva'; + 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, + ) if $type eq 'phone'; + Koha::Exceptions::Patron::Message::Preference::SMSNumberRequired->throw( + error => 'Patron has not set SMS number'. + 'cannot use sms as message transport', + message_name => $self->message_name, + borrowernumber => $self->borrowernumber, + ) if $type eq 'sms'; } } $self->{'_message_transport_types'}->{$type} diff --git a/t/db_dependent/Koha/Patron/Message/Preferences.t b/t/db_dependent/Koha/Patron/Message/Preferences.t index 75cdb600e5..a402ae9c3c 100644 --- a/t/db_dependent/Koha/Patron/Message/Preferences.t +++ b/t/db_dependent/Koha/Patron/Message/Preferences.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 7; +use Test::More tests => 8; use t::lib::Mocks; use t::lib::TestBuilder; @@ -373,7 +373,7 @@ HERE is_digest => 1 })->store; t::lib::Mocks::mock_preference('TalkingTechItivaPhoneNotification', 0); - $patron->set({ email => '', phone => '', smsalertnumber => '' })->store; + $patron->set({ email => '', emailpro => '', B_email => '', phone => '', smsalertnumber => '' })->store; eval { $preference->message_transport_types('email')->store; }; @@ -473,6 +473,84 @@ subtest 'Test Koha::Patron::Message::Preference->message_name' => sub { $schema->storage->txn_rollback; }; +subtest 'Test Koha::Patron::Message::Preference->mtt_deliverable' => sub { + plan tests => 10; + + $schema->storage->txn_begin; + + my $patron = $builder->build_object({ class => 'Koha::Patrons' }); + my ($preference, $mtt1, $mtt2) = build_a_test_complete_preference({ + patron => $patron + }); + + # Test email and smsalertnumber validation + eval { Koha::Patron::Message::Transport::Types->new({ + message_transport_type => 'email' + })->store }; + eval { Koha::Patron::Message::Transport::Types->new({ + message_transport_type => 'sms' + })->store }; + eval { Koha::Patron::Message::Transport::Types->new({ + message_transport_type => 'phone' + })->store }; + eval { Koha::Patron::Message::Transport::Types->new({ + message_transport_type => 'itiva' + })->store }; + Koha::Patron::Message::Transport->new({ + message_attribute_id => $preference->message_attribute_id, + message_transport_type => 'email', + is_digest => 1 + })->store; + Koha::Patron::Message::Transport->new({ + message_attribute_id => $preference->message_attribute_id, + message_transport_type => 'sms', + 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 => 'itiva', + is_digest => 1 + })->store; + $patron->set({ + email => 'nobody@koha-community.org', + emailpro => 'nobody@koha-community.org', + B_email => 'nobody@koha-community.org', + smsalertnumber => '123', + phone => '123', + })->store; + t::lib::Mocks::mock_preference('TalkingTechItivaPhoneNotification', 1); + + is($preference->mtt_deliverable('email'), 1, 'mtt_deliverable - email'); + is($preference->mtt_deliverable('itiva'), 1, 'mtt_deliverable - itiva'); + is($preference->mtt_deliverable('phone'), 1, 'mtt_deliverable - phone'); + is($preference->mtt_deliverable('sms'), 1, 'mtt_deliverable - sms'); + + $patron->set({email => '',})->store; + is($preference->mtt_deliverable('email'), 1, 'mtt_deliverable - emailpro'); + + $patron->set({emailpro => '',})->store; + is($preference->mtt_deliverable('email'), 1, 'mtt_deliverable - B_email'); + + $patron->set({B_email => '',})->store; + is($preference->mtt_deliverable('email'), 0, 'mtt_deliverable - email false'); + + t::lib::Mocks::mock_preference('TalkingTechItivaPhoneNotification', 0); + is($preference->mtt_deliverable('itiva'), 0, 'mtt_deliverable - itiva false'); + + $patron->set({phone => '',})->store; + is($preference->mtt_deliverable('phone'), 0, 'mtt_deliverable - phone false'); + + $patron->set({smsalertnumber => '',})->store; + is($preference->mtt_deliverable('sms'), 0, 'mtt_deliverable - smsalertnumber false'); + + $schema->storage->txn_rollback; +}; + subtest 'Test adding a new preference with invalid parameters' => sub { plan tests => 4; -- 2.30.2