From 14e2185f9408f89d14ee5b8fabf7cc2abdd632f8 Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Wed, 20 Dec 2017 16:19:18 -0300
Subject: [PATCH] Bug 19841: Make AddMember raise an exception if categorycode
 is invalid

This patch makes C4::Members::AddMember raise a Koha::Exceptions::BadParameter
exception. The current behaviour is to just explode. Code calling
AddReturn should work as expected, unless they are looking for a
specific failure message.

It is just explodes in a way we know what happened, in the case the
categorycode is invalid.

To test:
- Apply the Unit tests patch
- Run:
  $ kshell
 k$ prove t/db_dependent/Members.t
=> FAIL: Tests fail because AddReturn doesn't raise an exception
- Apply this patch
- Run:
  $ kshell
 k$ prove t/db_dependent/Members.t
=> SUCCESS: Tests pass!
- Sign off :-D

Signed-off-by: Dominic Pichette <dominic.pichette@inlibro.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 C4/Members.pm | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/C4/Members.pm b/C4/Members.pm
index d8985bc06b..2d942819a6 100644
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -387,6 +387,14 @@ sub AddMember {
     my $dbh = C4::Context->dbh;
     my $schema = Koha::Database->new()->schema;
 
+    my $category = Koha::Patron::Categories->find( $data{categorycode} );
+    unless ($category) {
+        Koha::Exceptions::BadParameter->throw(
+            error => 'Invalid parameter passed',
+            parameter => 'categorycode'
+        );
+    }
+
     # trim whitespace from data which has some non-whitespace in it.
     foreach my $field_name (keys(%data)) {
         if ( defined $data{$field_name} && $data{$field_name} =~ /\S/ ) {
@@ -399,7 +407,7 @@ sub AddMember {
       if ( $data{'userid'} eq '' || !Check_Userid( $data{'userid'} ) );
 
     # add expiration date if it isn't already there
-    $data{dateexpiry} ||= Koha::Patron::Categories->find( $data{categorycode} )->get_expiry_date;
+    $data{dateexpiry} ||= $category->get_expiry_date;
 
     # add enrollment date if it isn't already there
     unless ( $data{'dateenrolled'} ) {
@@ -412,12 +420,11 @@ sub AddMember {
         }
     }
 
-    my $patron_category = $schema->resultset('Category')->find( $data{'categorycode'} );
     $data{'privacy'} =
-        $patron_category->default_privacy() eq 'default' ? 1
-      : $patron_category->default_privacy() eq 'never'   ? 2
-      : $patron_category->default_privacy() eq 'forever' ? 0
-      :                                                    undef;
+        $category->default_privacy() eq 'default' ? 1
+      : $category->default_privacy() eq 'never'   ? 2
+      : $category->default_privacy() eq 'forever' ? 0
+      :                                             undef;
 
     $data{'privacy_guarantor_checkouts'} = 0 unless defined( $data{'privacy_guarantor_checkouts'} );
 
-- 
2.39.5