From cde22ddd9e41b32cba0904ecaa44a83ba26346bc Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Wed, 11 Sep 2024 14:04:17 +0000 Subject: [PATCH] Bug 37892: Fix guarantor restriction, add tests Content-Type: text/plain; charset=utf-8 [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 Signed-off-by: Brendan Lawlor Signed-off-by: Baptiste Wojtkowski Bug 37892: Fix patron updates Signed-off-by: Olivier V Signed-off-by: Brendan Lawlor Signed-off-by: Baptiste Wojtkowski Bug 37892: Fix patron creation Signed-off-by: Olivier V Signed-off-by: Brendan Lawlor Signed-off-by: Baptiste Wojtkowski Bug 37892: Fix memberentry.pl Signed-off-by: Olivier V Signed-off-by: Brendan Lawlor Signed-off-by: Baptiste Wojtkowski 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 Signed-off-by: Brendan Lawlor 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 Bug 37892: (follow-up) Tidyness Signed-off-by: Marcel de Rooy [EDIT] Renamed a subtest to patron creation tests in Patron.t. --- Koha/Patron.pm | 28 +++++++++++++++++++-- members/memberentry.pl | 2 +- t/db_dependent/Koha/Patron.t | 47 +++++++++++++++++++++++++++++++++++- 3 files changed, 73 insertions(+), 4 deletions(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index fcbd6cd71e..241465fcb6 100644 --- a/Koha/Patron.pm +++ b/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 diff --git a/members/memberentry.pl b/members/memberentry.pl index 2227df425f..0d09b16bbd 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -285,7 +285,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'; } } diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t index 970235ecc5..b267d20375 100755 --- a/t/db_dependent/Koha/Patron.t +++ b/t/db_dependent/Koha/Patron.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 35; +use Test::More tests => 36; 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; -- 2.30.2