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 <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This commit is contained in:
Lari Taskula 2020-11-18 02:15:23 +00:00 committed by Tomas Cohen Arazi
parent ea9dbf6a81
commit f139509818
No known key found for this signature in database
GPG Key ID: 0A272EA1B2F3C15F
2 changed files with 133 additions and 44 deletions

View File

@ -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}

View File

@ -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;