From d78dbcc00a53cdb6af6c094745af6384547cf67f Mon Sep 17 00:00:00 2001
From: Lari Taskula <lari.taskula@jns.fi>
Date: Wed, 26 Oct 2016 14:54:40 +0300
Subject: [PATCH] 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 <veron@veron.ch>
---
 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<validate()> 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