gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 17499: (follow-up) More explicit exceptions 

Adds and throws more detailed exceptions.

This is useful for APIs and generating translatable errors in GUI.

In short, replaces Koha::Exceptions::BadParameter with:

Koha::Exceptions::Patron::NotFound
Koha::Exceptions::Patron::Category
Koha::Exceptions::Patron::Category::NotFound
Koha::Exceptions::Patron::Message::Preference
Koha::Exceptions::Patron::Message::Preference::AttributeNotFound
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceOutOfRange
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestRequired
Koha::Exceptions::Patron::Message::Preference::EmailAddressRequired
Koha::Exceptions::Patron::Message::Preference::NoTransportType
Koha::Exceptions::Patron::Message::Preference::SMSNumberRequired

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-04 15:19:22 +00:00 committed by Tomas Cohen Arazi
parent 33aa562f79
commit a0e5f4e0d9
No known key found for this signature in database
GPG Key ID: 0A272EA1B2F3C15F
6 changed files with 231 additions and 98 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
Koha/Exceptions/Patron.pm
View File

@ -32,7 +32,11 @@ use Exception::Class (
isa => 'Koha::Exceptions::Patron',
description => "Mandatory extended attribute missing",
fields => ['type']
}
},
'Koha::Exceptions::Patron::NotFound' => {
isa => 'Koha::Exceptions::Patron',
description => "Patron not found"
},
);
sub full_message {

30
Koha/Exceptions/Patron/Category.pm Normal file
View File

@ -0,0 +1,30 @@
package Koha::Exceptions::Patron::Category;
# This file is part of Koha.
#
# Koha is free software; you can redistribute it and/or modify it
# under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# Koha is distributed in the hope that it will be useful, but
# WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with Koha; if not, see <http://www.gnu.org/licenses>.
use Modern::Perl;
use Exception::Class (
'Koha::Exceptions::Patron::Category' => {
description => "Something went wrong!"
},
'Koha::Exceptions::Patron::Category::NotFound' => {
isa => 'Koha::Exceptions::Patron::Category',
description => "Patron category not found"
},
);
1;

67
Koha/Exceptions/Patron/Message/Preference.pm Normal file
View File

@ -0,0 +1,67 @@
package Koha::Exceptions::Patron::Message::Preference;
# This file is part of Koha.
#
# Koha is free software; you can redistribute it and/or modify it
# under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# Koha is distributed in the hope that it will be useful, but
# WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with Koha; if not, see <http://www.gnu.org/licenses>.
use Modern::Perl;
use Exception::Class (
'Koha::Exceptions::Patron::Message::Preference' => {
description => 'Something went wrong'
},
'Koha::Exceptions::Patron::Message::Preference::AttributeNotFound' => {
isa => 'Koha::Exceptions::Patron::Message::Preference',
description => "Message attribute not found",
fields => ['message_attribute_id']
},
'Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceOutOfRange' => {
isa => 'Koha::Exceptions::Patron::Message::Preference',
description => "Days in advance is out of range",
fields => ['min','max', 'message_name']
},
'Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceNotAvailable' => {
isa => 'Koha::Exceptions::Patron::Message::Preference',
description => "Days in advance cannot be selected for this preference",
fields => ['message_name']
},
'Koha::Exceptions::Patron::Message::Preference::DigestNotAvailable' => {
isa => 'Koha::Exceptions::Patron::Message::Preference',
description => "Digest is not available for this message type",
fields => ['message_name']
},
'Koha::Exceptions::Patron::Message::Preference::DigestRequired' => {
isa => 'Koha::Exceptions::Patron::Message::Preference',
description => "Digest must be selected for this message type",
fields => ['message_name']
},
'Koha::Exceptions::Patron::Message::Preference::EmailAddressRequired' => {
isa => 'Koha::Exceptions::Patron::Message::Preference',
description => "Patron has no email address, cannot use email transport type.",
fields => ['message_name', 'borrowernumber' ]
},
'Koha::Exceptions::Patron::Message::Preference::NoTransportType' => {
isa => 'Koha::Exceptions::Patron::Message::Preference',
description => "Transport type not available for this message.",
fields => ['message_name', 'transport_type']
},
'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' ]
},
);
1;

32
Koha/Exceptions/Patron/Message/Transport.pm Normal file
View File

@ -0,0 +1,32 @@
package Koha::Exceptions::Patron::Message::Transport;
# This file is part of Koha.
#
# Koha is free software; you can redistribute it and/or modify it
# under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# Koha is distributed in the hope that it will be useful, but
# WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with Koha; if not, see <http://www.gnu.org/licenses>.
use Modern::Perl;
use Exception::Class (
'Koha::Exceptions::Patron::Message::Transport' => {
description => 'Something went wrong'
},
'Koha::Exceptions::Patron::Message::Transport::TypeNotFound' => {
isa => 'Koha::Exceptions::Patron::Message::Transport',
description => "Transport type does not exist.",
fields => ['transport_type']
},
);
1;

76
Koha/Patron/Message/Preference.pm
View File

@ -21,6 +21,9 @@ use Modern::Perl;
use Koha::Database;
use Koha::Exceptions;
use Koha::Exceptions::Patron::Message::Preference;
use Koha::Exceptions::Patron::Message::Transport;
use Koha::Exceptions::Patron::Category;
use Koha::Patron::Categories;
use Koha::Patron::Message::Attributes;
use Koha::Patron::Message::Preferences;
@ -284,16 +287,7 @@ sub store {
Makes a basic validation for object.
Throws following exceptions regarding parameters.
- Koha::Exceptions::MissingParameter
- Koha::Exceptions::TooManyParameters
- Koha::Exceptions::BadParameter
See $_->parameter to identify the parameter causing the exception.
Throws Koha::Exceptions::DuplicateObject if this preference already exists.
Returns Koha::Patron::Message::Preference object.
Returns Koha::Patron::Message::Preference object, or throws and exception.
=cut
@ -311,15 +305,13 @@ sub validate {
);
}
if ($self->borrowernumber) {
Koha::Exceptions::BadParameter->throw(
Koha::Exceptions::Patron::NotFound->throw(
error => 'Patron not found.',
parameter => 'borrowernumber',
) unless Koha::Patrons->find($self->borrowernumber);
}
if ($self->categorycode) {
Koha::Exceptions::BadParameter->throw(
Koha::Exceptions::Patron::Category::NotFound->throw(
error => 'Category not found.',
parameter => 'categorycode',
) unless Koha::Patron::Categories->find($self->categorycode);
}
@ -341,25 +333,27 @@ sub validate {
$self->message_attribute_id
);
unless ($attr) {
Koha::Exceptions::BadParameter->throw(
Koha::Exceptions::Patron::Message::Preference::AttributeNotFound->throw(
error => 'Message attribute with id '.$self->message_attribute_id
.' not found',
parameter => 'message_attribute_id'
message_attribute_id => $self->message_attribute_id,
);
}
if (defined $self->days_in_advance) {
if ($attr && $attr->takes_days == 0) {
Koha::Exceptions::BadParameter->throw(
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceNotAvailable->throw(
error => 'days_in_advance cannot be defined for '.
$attr->message_name . '.',
parameter => 'days_in_advance',
message_name => $attr->message_name,
);
}
elsif ($self->days_in_advance < 0 || $self->days_in_advance > 30) {
Koha::Exceptions::BadParameter->throw(
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceOutOfRange->throw(
error => 'days_in_advance has to be a value between 0-30 for '.
$attr->message_name . '.',
parameter => 'days_in_advance',
message_name => $attr->message_name,
min => 0,
max => 30,
);
}
}
@ -368,12 +362,19 @@ sub validate {
message_attribute_id => $self->message_attribute_id,
is_digest => $self->wants_digest ? 1 : 0,
});
Koha::Exceptions::BadParameter->throw(
error => (!$self->wants_digest ? 'Digest must be selected'
: 'Digest cannot be selected')
. ' for '.$attr->message_name.'.',
parameter => 'wants_digest',
) if $transports->count == 0;
unless ($transports->count) {
if (!$self->wants_digest) {
Koha::Exceptions::Patron::Message::Preference::DigestRequired->throw(
error => 'Digest must be selected for '.$attr->message_name.'.',
message_name => $attr->message_name
);
} else {
Koha::Exceptions::Patron::Message::Preference::DigestNotAvailable->throw(
error => 'Digest cannot be selected for '.$attr->message_name.'.',
message_name => $attr->message_name
);
}
}
}
return $self;
@ -395,10 +396,11 @@ sub _set_message_transport_types {
message_transport_type => $type
});
unless ($transport) {
Koha::Exceptions::BadParameter->throw(
Koha::Exceptions::Patron::Message::Preference::NoTransportType->throw(
error => 'No transport configured for '.$self->message_name.
" transport type $type.",
parameter => 'message_transport_types'
message_name => $self->message_name,
transport_type => $type,
);
}
if (defined $self->borrowernumber) {
@ -406,19 +408,21 @@ sub _set_message_transport_types {
if ($type eq 'email') {
if ( !$patron->email )
{
Koha::Exceptions::BadParameter->throw(
Koha::Exceptions::Patron::Message::Preference::EmailAddressRequired->throw(
error => 'Patron has not set email address, '.
'cannot use email as message transport',
parameter => 'message_transport_types'
message_name => $self->message_name,
borrowernumber => $self->borrowernumber,
);
}
}
elsif ($type eq 'sms') {
if ( !$patron->smsalertnumber ){
Koha::Exceptions::BadParameter->throw(
error => 'Patron has not set sms number, '.
'cannot set sms as message transport',
parameter => 'message_transport_types'
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,
);
}
}
@ -443,9 +447,9 @@ sub _validate_message_transport_types {
unless (Koha::Patron::Message::Transport::Types->find({
message_transport_type => $type
})) {
Koha::Exceptions::BadParameter->throw(
Koha::Exceptions::Patron::Message::Transport::TypeNotFound->throw(
error => "Message transport type '$type' does not exist",
parameter => 'message_transport_types',
transport_type => $type,
);
}
}

118
t/db_dependent/Koha/Patron/Message/Preferences.t
View File

@ -360,23 +360,23 @@ HERE
eval {
$preference->message_transport_types('email')->store;
};
is (ref $@, 'Koha::Exceptions::BadParameter',
is (ref $@, 'Koha::Exceptions::Patron::Message::Preference::EmailAddressRequired',
'Adding a message preference with invalid message_transport_type'
.' => Koha::Exceptions::BadParameter');
is ($@->parameter, 'message_transport_types', 'The previous exception '
.' tells us it was the message_transport_types');
like ($@->error, qr/^Patron has not set email address/, 'Exception '
.' is because of patron has not set email address.');
.' => Koha::Exceptions::Patron::Message::Preference::EmailAddressRequired');
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;
};
is (ref $@, 'Koha::Exceptions::BadParameter',
is (ref $@, 'Koha::Exceptions::Patron::Message::Preference::SMSNumberRequired',
'Adding a message preference with invalid message_transport_type'
.' => Koha::Exceptions::BadParameter');
is ($@->parameter, 'message_transport_types', 'The previous exception '
.' tells us it was the message_transport_types');
like ($@->error, qr/^Patron has not set sms number/, 'Exception '
.' is because of patron has not set sms number.');
.' => Koha::Exceptions::Patron::Message::Preference::SMSNumberRequired');
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.');
$schema->storage->txn_rollback;
};
@ -466,27 +466,23 @@ subtest 'Test adding a new preference with invalid parameters' => sub {
};
subtest 'Bad parameter' => sub {
plan tests => 22;
plan tests => 18;
$schema->storage->txn_begin;
eval { Koha::Patron::Message::Preference->new({
borrowernumber => -999,
})->store };
is(ref $@, 'Koha::Exceptions::BadParameter',
is(ref $@, 'Koha::Exceptions::Patron::NotFound',
'Adding a message preference with invalid borrowernumber'
.' => Koha::Exceptions::BadParameter');
is ($@->parameter, 'borrowernumber', 'The previous exception tells us it'
.' was the borrowernumber.');
.' => Koha::Exceptions::Patron::NotFound');
eval { Koha::Patron::Message::Preference->new({
categorycode => 'nonexistent',
})->store };
is(ref $@, 'Koha::Exceptions::BadParameter',
is(ref $@, 'Koha::Exceptions::Patron::Category::NotFound',
'Adding a message preference with invalid categorycode'
.' => Koha::Exceptions::BadParameter');
is($@->parameter, 'categorycode', 'The previous exception tells us it'
.' was the categorycode.');
.' => Koha::Exceptions::Patron::Category::NotFound');
my $attribute = build_a_test_attribute({ takes_days => 0 });
my $patron = $builder->build_object({ class => 'Koha::Patrons' });
@ -495,33 +491,40 @@ subtest 'Test adding a new preference with invalid parameters' => sub {
message_attribute_id => $attribute->message_attribute_id,
days_in_advance => 10,
})->store };
is(ref $@, 'Koha::Exceptions::BadParameter',
is(ref $@, 'Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceNotAvailable',
'Adding a message preference with days in advance option when not'
.' available => Koha::Exceptions::BadParameter');
is($@->parameter, 'days_in_advance', 'The previous exception tells us it'
.' was the days_in_advance.');
.' available => Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceNotAvailable');
$attribute->set({ takes_days => 1 })->store;
eval { Koha::Patron::Message::Preference->new({
borrowernumber => $patron->borrowernumber,
message_attribute_id => $attribute->message_attribute_id,
days_in_advance => -1,
})->store };
is(ref $@, 'Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceOutOfRange',
'Adding a message preference with days in advance option is out of range'
.' => Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceOutOfRange');
is ($@->min, 0, 'The previous exception min value is 0.');
eval { Koha::Patron::Message::Preference->new({
borrowernumber => $patron->borrowernumber,
message_attribute_id => $attribute->message_attribute_id,
days_in_advance => 31,
})->store };
is(ref $@, 'Koha::Exceptions::BadParameter',
'Adding a message preference with days in advance option too large'
.' => Koha::Exceptions::BadParameter');
is($@->parameter, 'days_in_advance', 'The previous exception tells us it'
.' was the days_in_advance.');
is(ref $@, 'Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceOutOfRange',
'Adding a message preference with days in advance option is out of range'
.' => Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceOutOfRange');
is ($@->max, 30, 'The previous exception max value is 30.');
eval { Koha::Patron::Message::Preference->new({
borrowernumber => $patron->borrowernumber,
message_transport_types => ['nonexistent']
})->store };
is (ref $@, 'Koha::Exceptions::BadParameter',
is (ref $@, 'Koha::Exceptions::Patron::Message::Transport::TypeNotFound',
'Adding a message preference with invalid message_transport_type'
.' => Koha::Exceptions::BadParameter');
is ($@->parameter, 'message_transport_types', 'The previous exception '
.'tells us it was the message_transport_types.');
.' => Koha::Exceptions::Patron::Message::Transport::TypeNotFound');
is ($@->transport_type, 'nonexistent', 'The previous exception '
.'tells us it which transport type it was.');
my $mtt_new = $builder->build_object({ class => 'Koha::Patron::Message::Transport::Types' });
eval {
@ -531,13 +534,13 @@ subtest 'Test adding a new preference with invalid parameters' => sub {
message_transport_types => [$mtt_new->message_transport_type],
wants_digest => 1,
})->store };
is (ref $@, 'Koha::Exceptions::BadParameter',
is (ref $@, 'Koha::Exceptions::Patron::Message::Preference::NoTransportType',
'Adding a message preference with invalid message_transport_type'
.' => Koha::Exceptions::BadParameter');
is ($@->parameter, 'message_transport_types', 'The previous exception '
.'tells us it was the message_transport_types.');
like ($@->error, qr/^No transport configured/, 'Exception is because of '
.'given message_transport_type is not a valid option.');
.' => Koha::Exceptions::Patron::Message::Preference::NoTransportType');
is ($@->message_name, $attribute->message_name, 'The previous exception '
.'tells us which message it was.');
is ($@->transport_type, $mtt_new->message_transport_type, 'The previous exception '
.'tells us which message transport type it was.');
eval {
Koha::Patron::Message::Preference->new({
@ -546,13 +549,11 @@ subtest 'Test adding a new preference with invalid parameters' => sub {
message_transport_types => [],
wants_digest => 1,
})->store };
is (ref $@, 'Koha::Exceptions::BadParameter',
'Adding a message preference with invalid message_transport_type'
.' => Koha::Exceptions::BadParameter');
is ($@->parameter, 'wants_digest', 'The previous exception tells us it'
.' was the wants_digest');
like ($@->error, qr/^Digest cannot be selected/, 'Exception s because of'
.' given digest is not available for this transport.');
is (ref $@, 'Koha::Exceptions::Patron::Message::Preference::DigestNotAvailable',
'Adding a message preference that has no digest setting'
.' => Koha::Exceptions::Patron::Message::Preference::DigestNotAvailable');
is ($@->message_name, $attribute->message_name, 'The previous exception tells us'
.' which message it was');
eval {
Koha::Patron::Message::Preference->new({
@ -561,27 +562,22 @@ subtest 'Test adding a new preference with invalid parameters' => sub {
message_transport_types => [],
wants_digest => 0,
})->store };
is (ref $@, 'Koha::Exceptions::BadParameter',
'Adding a message preference with invalid message_transport_type'
.' => Koha::Exceptions::BadParameter');
is ($@->parameter, 'wants_digest', 'The previous exception tells us it'
.' was the wants_digest');
like ($@->error, qr/^Digest must be selected/, 'Exception s because of'
.' digest has to be on for this transport.');
is (ref $@, 'Koha::Exceptions::Patron::Message::Preference::DigestRequired',
'Adding a message preference, that requires digest, with no digest'
.' => Koha::Exceptions::Patron::Message::Preference::DigestRequired');
is ($@->message_name, $attribute->message_name, 'The previous exception tells us'
.' which message it was');
eval {
Koha::Patron::Message::Preference->new({
borrowernumber => $patron->borrowernumber,
message_attribute_id => -1,
message_transport_types => [],
})->store };
is (ref $@, 'Koha::Exceptions::BadParameter',
is (ref $@, 'Koha::Exceptions::Patron::Message::Preference::AttributeNotFound',
'Adding a message preference with invalid message_transport_type'
.' => Koha::Exceptions::BadParameter');
is ($@->parameter, 'message_attribute_id', 'The previous exception tells'
.' us it was the message_attribute_id');
like ($@->error, qr/^Message attribute with id -1 not found/, 'Exception '
.' is because of given message attribute id is not found.');
.' => KKoha::Exceptions::Patron::Message::Preference::AttributeNotFound');
is ($@->message_attribute_id, -1, 'The previous exception tells'
.' us which message attribute id it was.');
$schema->storage->txn_rollback;
};