Bug 37892: Fix guarantor restriction, add tests

[SQUASHED IN QA]

These patches will alter the checks for a patron that prevent a category with
'can_be_guarantee' from being a guarantor.  Two patrons in the same category should be
allowed to have a guarantee/guarantor relationship

The tests below assume you are using the KTD sample data. Update borrowernumbers if not.

To test:
 0 - Apply tests patch
 1 - Set the 'Patron' category as 'Can be a guarantee'
 2 - Add a relationship between two patrons of the same category
     This is restricted from the staff interface
     perl -e 'use Koha::Patrons; my $p = Koha::Patrons->find(5)->add_guarantor({ guarantor_id => 23, relationship => 'father'});'
 3 - Note there is no warning or exception. This should be allowed.
 4 - Checkout an item to Edna (borrowernumber 5)
 5 - Set 'TrackLastPatronActivityTriggers' to 'Checking in an item'
 6 - Try to check the item in, KABOOM
 7 - Set 'TrackLastPatronActivityTriggers' to 'Checking out an item'
 8 - Try to issue an item to Enda, KABOOM
 9 - prove -v t/db_dependent/Koha/Patron.t, fail
10 - Apply second patch
11 - prove -v t/db_dependent/Koha/Patron.t, one more test passes, but then fail
12 - Apply third patch
13 - prove -v t/db_dependent/Koha/Patron.t, pass!
14 - restart_all
15 - Checkout to Enda, OK!
16 - Checkin from Edna, OK!
17 - Find two more patrons in the category and attempt to link them
18 - 'Guarantor cannot be a guarantee'
19 - Apply fourth patch
20 - You can add a guarantor from the same category in interface
21 - Try to add a guarantor to the guarantor assigned in 20
22 - Confirm you cannot add a guarantor - "Guarantor cannot be a guarantee"

Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>

Bug 37892: Fix patron updates

Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>

Bug 37892: Fix patron creation

Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>

Bug 37892: Fix memberentry.pl

Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>

Bug 37892: (follow-up) Fix patron creation

This patch fixes the 22. of the test plan (22 - Confirm you cannot add a guarantor - "Guarantor cannot be a guarantee"
        )

Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>

Bug 37892: (follow-up) Fix patron creation

TEST PLAN:
1 - Do the 22 parts of the test plan
2 - Add a guarantor to one patron not selected before (let's say A is
  the guarantee, B the guarantor)
3 - Try and add a guarantor to B -> you will success
4 - Remove B's guarantor
5 - Apply this patch
6 - Repeat 3 -> you will not be able to

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>

Bug 37892: (follow-up) Tidyness

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
[EDIT]
Renamed a subtest to patron creation tests in Patron.t.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 391e150e07)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Koha/Patron.pm

@@ -315,7 +315,7 @@ sub store {
                 }
 
                 foreach my $guarantor (@$guarantors) {
-                    if ( $guarantor->is_child or $guarantor->category->can_be_guarantee ) {
+                    if ( $guarantor->is_child ) {
                         Koha::Exceptions::Patron::Relationship::InvalidRelationship->throw( invalid_guarantor => 1 );
                     }
                 }
@@ -372,7 +372,7 @@ sub store {
                 }
 
                 foreach my $guarantor (@$guarantors) {
-                    if ( $guarantor->is_child or $guarantor->category->can_be_guarantee ) {
+                    if ( $guarantor->is_child ) {
                         Koha::Exceptions::Patron::Relationship::InvalidRelationship->throw( invalid_guarantor => 1 );
                     }
                 }
@@ -533,6 +533,18 @@ sub guarantor_relationships {
     return Koha::Patron::Relationships->search( { guarantee_id => $self->id } );
 }
 
+=head3 is_guarantee
+
+Returns true if the patron has a guarantor.
+
+=cut
+
+sub is_guarantee {
+    my ($self) = @_;
+    return $self->guarantor_relationships()->count();
+}
+
+
 =head3 guarantee_relationships
 
 Returns Koha::Patron::Relationships object for this patron's guarantors
@@ -557,6 +569,18 @@ sub guarantee_relationships {
     );
 }
 
+=head3 is_guarantor
+
+Returns true if the patron is a guarantor.
+
+=cut
+
+sub is_guarantor {
+    my ($self) = @_;
+    return $self->guarantee_relationships()->count();
+}
+
+
 =head3 relationships_debt
 
 Returns the amount owed by the patron's guarantors *and* the other guarantees of those guarantors

members/memberentry.pl

@@ -280,7 +280,7 @@ if (   ( $op eq 'cud-save' || $op eq 'cud-insert' )
 }
 
 foreach my $guarantor (@guarantors) {
-    if ( ( $op eq 'cud-save' || $op eq 'cud-insert' ) && $guarantor->is_child || $guarantor->category->can_be_guarantee ) {
+    if ( ( $op eq 'cud-save' || $op eq 'cud-insert' ) && ($guarantor->is_child ) || $guarantor->is_guarantee || $patron->is_guarantor)  {
         push @errors, 'ERROR_guarantor_is_guarantee';
     }
 }

t/db_dependent/Koha/Patron.t

@@ -19,7 +19,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 33;
+use Test::More tests => 34;
 use Test::Exception;
 use Test::Warn;
 use Time::Fake;
@@ -410,6 +410,51 @@ subtest 'add_guarantor() tests' => sub {
     $schema->storage->txn_rollback;
 };
 
+subtest 'guarantor checks on patron creation / update tests' => sub {
+
+    plan tests => 2;
+
+    $schema->storage->txn_begin;
+
+    t::lib::Mocks::mock_preference( 'borrowerRelationship', 'guarantor' );
+    my $category = $builder->build_object(
+        { class => 'Koha::Patron::Categories', value => { can_be_guarantee => 1, category_type => 'A' } } );
+
+    my $guarantor =
+        $builder->build_object( { class => 'Koha::Patrons', value => { categorycode => $category->categorycode } } );
+    my $guarantee =
+        $builder->build_object( { class => 'Koha::Patrons', value => { categorycode => $category->categorycode } } );
+
+    subtest 'patron update tests' => sub {
+        plan tests => 4;
+        ok(
+            $guarantee->add_guarantor( { guarantor_id => $guarantor->borrowernumber, relationship => 'guarantor' } ),
+            "Relationship is added, no problem"
+        );
+        is( $guarantor->guarantee_relationships->count, 1, 'Relationship added' );
+        ok( $guarantor->surname("Duck")->store(), "Updating guarantor is okay" );
+        ok( $guarantee->surname("Duck")->store(), "Updating guarantee is okay" );
+    };
+
+    subtest 'patron creation tests' => sub {
+        plan tests => 1;
+
+        my $new_guarantee = $builder->build_object(
+            { class => 'Koha::Patrons', value => { categorycode => $category->categorycode } } );
+        my $new_guarantee_hash = $new_guarantee->unblessed;
+        $new_guarantee->delete();
+
+        delete $new_guarantee_hash->{borrowernumber};
+
+        ok(
+            Koha::Patron->new($new_guarantee_hash)->store( { guarantors => [$guarantor] } ),
+            "We can add the new patron and indicate guarantor"
+        );
+    };
+
+    $schema->storage->txn_rollback;
+};
+
 subtest 'relationships_debt() tests' => sub {
 
     plan tests => 168;