From 204475ae3d4f20e2b1ae5dc615cc9ed784aee302 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Wed, 26 Oct 2016 14:54:40 +0300 Subject: [PATCH] [SIGNED-OFF] Bug 17499: Add basic validation for messaging preference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch adds simple validation for messaging preferences. The validation includes - check that only either borrowernumber or categorycode is given, but not both - throw exception if patron for the given borrowernumber is not found - throw exception if category for the given categorycode is not found - throw exception if days in advance cannot be configured - throw exception if days in advance configuration is invalid (value between 0-30) - throw exception if digest is not available To test: 1. Run t/db_dependent/Koha/Patron/Message/Preferences.t Signed-off-by: Marc Véron Signed-off-by: Josef Moravec --- Koha/Patron/Message/Preference.pm | 97 ++++++++++++++++++++++++ t/db_dependent/Koha/Patron/Message/Preferences.t | 83 +++++++++++++++++++- 2 files changed, 179 insertions(+), 1 deletion(-) diff --git a/Koha/Patron/Message/Preference.pm b/Koha/Patron/Message/Preference.pm index daeae5c..ce055d7 100644 --- a/Koha/Patron/Message/Preference.pm +++ b/Koha/Patron/Message/Preference.pm @@ -20,6 +20,11 @@ package Koha::Patron::Message::Preference; use Modern::Perl; use Koha::Database; +use Koha::Exceptions; +use Koha::Patron::Categories; +use Koha::Patron::Message::Attributes; +use Koha::Patron::Message::Transports; +use Koha::Patrons; use base qw(Koha::Object); @@ -33,6 +38,98 @@ Koha::Patron::Message::Preference - Koha Patron Message Preference object class =cut +=head3 store + +Makes a validation before actual Koha::Object->store so that proper exceptions +can be thrown. See C for documentation about exceptions. + +=cut + +sub store { + my $self = shift; + + return $self->validate->SUPER::store(@_); +} + +=head3 validate + +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. + +Returns Koha::Patron::Message::Preference object. + +=cut + +sub validate { + my ($self) = @_; + + if ($self->borrowernumber && $self->categorycode) { + Koha::Exceptions::TooManyParameters->throw( + error => 'Both borrowernumber and category given, only one accepted', + parameter => ['borrowernumber', 'categorycode'], + ); + } + if (!$self->borrowernumber && !$self->categorycode) { + Koha::Exceptions::MissingParameter->throw( + error => 'borrowernumber or category required, none given', + parameter => ['borrowernumber', 'categorycode'], + ); + } + if ($self->borrowernumber) { + Koha::Exceptions::BadParameter->throw( + error => 'Patron not found.', + parameter => 'borrowernumber', + ) unless Koha::Patrons->find($self->borrowernumber); + } + if ($self->categorycode) { + Koha::Exceptions::BadParameter->throw( + error => 'Category not found.', + parameter => 'categorycode', + ) unless Koha::Patron::Categories->find($self->categorycode); + } + + my $attr; + if ($self->days_in_advance || $self->wants_digest) { + $attr = Koha::Patron::Message::Attributes->find( + $self->message_attribute_id + ); + } + if ($self->days_in_advance) { + if ($attr && $attr->takes_days == 0) { + Koha::Exceptions::BadParameter->throw( + error => 'days_in_advance cannot be defined for '. + $attr->message_name . ' .', + parameter => 'days_in_advance', + ); + } + elsif ($self->days_in_advance < 0 || $self->days_in_advance > 30) { + Koha::Exceptions::BadParameter->throw( + error => 'days_in_advance has to be a value between 0-30 for '. + $attr->message_name . ' .', + parameter => 'days_in_advance', + ); + } + } + if ($self->wants_digest) { + my $transports = Koha::Patron::Message::Transports->search({ + message_attribute_id => $self->message_attribute_id, + is_digest => 1, + }); + Koha::Exceptions::BadParameter->throw( + error => 'Digest not available for '.$attr->message_name.' .', + parameter => 'wants_digest', + ) if $transports->count == 0; + } + + return $self; +} + =head3 type =cut diff --git a/t/db_dependent/Koha/Patron/Message/Preferences.t b/t/db_dependent/Koha/Patron/Message/Preferences.t index b9cef3a..b608981 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 => 2; +use Test::More tests => 3; use t::lib::Mocks; use t::lib::TestBuilder; @@ -99,6 +99,87 @@ subtest 'Test Koha::Patron::Message::Preferences' => sub { $schema->storage->txn_rollback; }; +subtest 'Test adding a new preference with invalid parameters' => sub { + plan tests => 3; + + subtest 'Missing parameters' => sub { + plan tests => 1; + + eval { Koha::Patron::Message::Preference->new->store }; + is(ref $@, 'Koha::Exceptions::MissingParameter', + 'Adding a message preference without parameters' + .' => Koha::Exceptions::MissingParameter'); + }; + + subtest 'Too many parameters' => sub { + plan tests => 1; + + $schema->storage->txn_begin; + + my $patron = build_a_test_patron(); + eval { Koha::Patron::Message::Preference->new({ + borrowernumber => $patron->borrowernumber, + categorycode => $patron->categorycode, + })->store }; + is(ref $@, 'Koha::Exceptions::TooManyParameters', + 'Adding a message preference for both borrowernumber and categorycode' + .' => Koha::Exceptions::TooManyParameters'); + + $schema->storage->txn_rollback; + }; + + subtest 'Bad parameter' => sub { + plan tests => 8; + + $schema->storage->txn_begin; + + eval { Koha::Patron::Message::Preference->new({ + borrowernumber => -999, + })->store }; + is(ref $@, 'Koha::Exceptions::BadParameter', + 'Adding a message preference with invalid borrowernumber' + .' => Koha::Exceptions::BadParameter'); + is ($@->parameter, 'borrowernumber', 'The previous exception tells us it' + .' was the borrowernumber.'); + + eval { Koha::Patron::Message::Preference->new({ + categorycode => 'nonexistent', + })->store }; + is(ref $@, 'Koha::Exceptions::BadParameter', + 'Adding a message preference with invalid categorycode' + .' => Koha::Exceptions::BadParameter'); + is($@->parameter, 'categorycode', 'The previous exception tells us it' + .' was the categorycode.'); + + my $attribute = build_a_test_attribute({ takes_days => 0 }); + my $patron = build_a_test_patron(); + eval { Koha::Patron::Message::Preference->new({ + borrowernumber => $patron->borrowernumber, + message_attribute_id => $attribute->message_attribute_id, + days_in_advance => 10, + })->store }; + is(ref $@, 'Koha::Exceptions::BadParameter', + '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.'); + + $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 => 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.'); + + $schema->storage->txn_rollback; + }; +}; + sub build_a_test_attribute { my ($params) = @_; -- 2.1.4