Attachment 64037 Details for Bug 17499 – [SIGNED-OFF] Bug 17499: Add basic validation for messaging preference

[patch] [SIGNED-OFF] Bug 17499: Add basic validation for messaging preference

SIGNED-OFF-Bug-17499-Add-basic-validation-for-mess.patch (text/plain), 8.56 KB, created by Josef Moravec on 2017-06-06 11:05:43 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Josef Moravec

Created: 2017-06-06 11:05:43 UTC

Size: 8.56 KB

patch

obsolete

>From 204475ae3d4f20e2b1ae5dc615cc9ed784aee302 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] [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 <veron@veron.ch>
>
>Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
>---
> 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

Actions: View | Diff | Splinter Review

Attachments on bug 17499: 63427 | 63428 | 63429 | 63430 | 63431 | 63432 | 63433 | 63434 | 63786 | 63787 | 63788 | 63789 | 63790 | 63791 | 63792 | 63793 | 63801 | 63802 | 63803 | 63804 | 63805 | 63806 | 63807 | 63808 | 63809 | 63810 | 63811 | 63812 | 63878 | 63895 | 64035 | 64036 | 64037 | 64038 | 64039 | 64040 | 64041 | 64042 | 64043 | 64044 | 64795 | 66686 | 69434 | 69435 | 69458 | 91864 | 93863 | 93865 | 93872 | 94748 | 99479 | 99634 | 99668 | 99669 | 99670 | 103441 | 103442 | 103443 | 106653 | 106654 | 106655 | 106656 | 106657 | 107612 | 107613 | 107614 | 107615 | 107616 | 113010 | 113011 | 113127 | 113129 | 113686 | 113687 | 113765 | 113766 | 113767 | 113768 | 118452 | 118453 | 119470 | 119471 | 119472 | 119473 | 119474 | 119475 | 119476 | 119477 | 119478 | 119479 | 119480 | 119481 | 119482 | 119483 | 119484 | 119485 | 119486 | 119487 | 119488 | 119489 | 119490 | 119491 | 141408 | 141409 | 141410 | 141411 | 141412 | 141413 | 141414 | 141415 | 141416 | 141417 | 141418 | 146529 | 146530 | 146531 | 146532 | 146533 | 146534 | 146535 | 146536 | 146537 | 146538 | 146539 | 146540 | 151900 | 151901 | 151902 | 151903 | 151904 | 151905 | 151906 | 151907 | 151908 | 151909 | 151910 | 151911 | 151937 | 151938 | 151939 | 151940 | 151941 | 151942 | 151943 | 151944 | 151945 | 151946 | 151947 | 151948 | 155365 | 155366 | 155367 | 155368 | 155369 | 155370 | 155371 | 155372 | 155373 | 155374 | 155375 | 155376