Attachment 119475 Details for Bug 17499 – Bug 17499: (follow-up) More explicit exceptions

[patch] Bug 17499: (follow-up) More explicit exceptions

Bug-17499-follow-up-More-explicit-exceptions.patch (text/plain), 26.48 KB, created by Lari Taskula on 2021-04-12 19:37:18 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Lari Taskula

Created: 2021-04-12 19:37:18 UTC

Size: 26.48 KB

patch

obsolete

>From e6e6598b4a22971eee54fb8f93698c0489c90be7 Mon Sep 17 00:00:00 2001
>From: Lari Taskula <lari.taskula@hypernova.fi>
>Date: Wed, 4 Nov 2020 15:19:22 +0000
>Subject: [PATCH] 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
>---
> Koha/Exceptions/Patron.pm                     |   6 +-
> Koha/Exceptions/Patron/Category.pm            |  30 +++++
> Koha/Exceptions/Patron/Message/Preference.pm  |  67 ++++++++++
> Koha/Exceptions/Patron/Message/Transport.pm   |  32 +++++
> Koha/Patron/Message/Preference.pm             |  76 +++++------
> .../Koha/Patron/Message/Preferences.t         | 118 +++++++++---------
> 6 files changed, 231 insertions(+), 98 deletions(-)
> create mode 100644 Koha/Exceptions/Patron/Category.pm
> create mode 100644 Koha/Exceptions/Patron/Message/Preference.pm
> create mode 100644 Koha/Exceptions/Patron/Message/Transport.pm
>
>diff --git a/Koha/Exceptions/Patron.pm b/Koha/Exceptions/Patron.pm
>index 1bd51eb55e..dacd97167d 100644
>--- a/Koha/Exceptions/Patron.pm
>+++ b/Koha/Exceptions/Patron.pm
>@@ -18,7 +18,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 {
>diff --git a/Koha/Exceptions/Patron/Category.pm b/Koha/Exceptions/Patron/Category.pm
>new file mode 100644
>index 0000000000..3eac4d2ba2
>--- /dev/null
>+++ b/Koha/Exceptions/Patron/Category.pm
>@@ -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;
>diff --git a/Koha/Exceptions/Patron/Message/Preference.pm b/Koha/Exceptions/Patron/Message/Preference.pm
>new file mode 100644
>index 0000000000..23f78029f5
>--- /dev/null
>+++ b/Koha/Exceptions/Patron/Message/Preference.pm
>@@ -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;
>diff --git a/Koha/Exceptions/Patron/Message/Transport.pm b/Koha/Exceptions/Patron/Message/Transport.pm
>new file mode 100644
>index 0000000000..6a7e00a96e
>--- /dev/null
>+++ b/Koha/Exceptions/Patron/Message/Transport.pm
>@@ -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;
>diff --git a/Koha/Patron/Message/Preference.pm b/Koha/Patron/Message/Preference.pm
>index 9f8900eaec..69d8420256 100644
>--- a/Koha/Patron/Message/Preference.pm
>+++ b/Koha/Patron/Message/Preference.pm
>@@ -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;
>@@ -285,16 +288,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
> 
>@@ -314,15 +308,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);
>     }
> 
>@@ -344,25 +336,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,
>             );
>         }
>     }
>@@ -371,12 +365,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;
>@@ -398,10 +399,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) {
>@@ -409,19 +411,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,
>                         );
>                     }
>                 }
>@@ -446,9 +450,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,
>                 );
>             }
>         }
>diff --git a/t/db_dependent/Koha/Patron/Message/Preferences.t b/t/db_dependent/Koha/Patron/Message/Preferences.t
>index 89273ea234..7bc08103a6 100644
>--- a/t/db_dependent/Koha/Patron/Message/Preferences.t
>+++ b/t/db_dependent/Koha/Patron/Message/Preferences.t
>@@ -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;
>     };
>-- 
>2.25.1

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