Bug 39014: (follow-up) Refactoring and renaming to can_be_guaranteed_by

To test:
1. prove t/db_dependent/Koha/Patron.t

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Lari Taskula 2025-10-17 00:44:38 +03:00 committed by Lucas Gass
parent 382ba17a8c
commit d214d441a8
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
5 changed files with 116 additions and 44 deletions

View File

@ -32,7 +32,10 @@ use Exception::Class (
'Koha::Exceptions::Patron::Relationship::InvalidRelationship' => {
isa => 'Koha::Exceptions::Patron::Relationship',
description => 'The specified relationship is invalid',
fields => [ 'relationship', 'no_relationship', 'invalid_guarantor', 'child_guarantor' ]
fields => [
'relationship', 'no_relationship', 'invalid_guarantor', 'child_guarantor',
'guarantor_cant_have_guarantors', 'guarantor'
],
},
'Koha::Exceptions::Patron::Relationship::NoGuarantor' => {
isa => 'Koha::Exceptions::Patron::Relationship',

View File

@ -2802,34 +2802,59 @@ sub _anonymize_column {
$self->$col($val);
}
=head3 validate_guarantor
=head3 can_be_guaranteed_by
Koha::Patron->validate_guarantor(\@guarantors, $contactname, $category );
$patron->can_be_guaranteed_by( [
$guarantor1,
$guarantor2,
...
] );
Validates guarantor patron.
Given an array of guarantors, checks if they can be guarantors for this patron.
Throws Koha::Exceptions::Patron::Relationship::InvalidRelationship if a proposed
guarantor cannot be a guarantor for this patron.
$patron->can_be_guaranteed_by();
Given no guarantors, checks if this patron must be guaranteed by someone. Read
"can be guaranteed by nobody".
Throws Koha::Exceptions::Patron::Relationship::NoGuarantor if this patron must
have a guarantor.
=cut
sub validate_guarantor {
my ( $self, $guarantors, $contactname, $category ) = @_;
sub can_be_guaranteed_by {
my ( $self, $guarantors ) = @_;
my $valid_guarantor = @$guarantors ? @$guarantors : $contactname;
my $category = $self->category;
if ( C4::Context->preference('ChildNeedsGuarantor')
and ( $category->category_type eq 'C' or $category->can_be_guarantee )
and !$valid_guarantor )
and scalar @$guarantors == 0 )
{
Koha::Exceptions::Patron::Relationship::NoGuarantor->throw();
}
if ( $self->is_guarantor && scalar @$guarantors > 0 ) {
Koha::Exceptions::Patron::Relationship::InvalidRelationship->throw( guarantor_cant_have_guarantors => 1 );
}
foreach my $guarantor (@$guarantors) {
next if ref($guarantor) ne 'Koha::Patron';
if ( $guarantor->is_guarantee ) {
Koha::Exceptions::Patron::Relationship::InvalidRelationship->throw( invalid_guarantor => 1 );
Koha::Exceptions::Patron::Relationship::InvalidRelationship->throw(
invalid_guarantor => 1,
guarantor => $guarantor,
);
} elsif ( $guarantor->is_child ) {
Koha::Exceptions::Patron::Relationship::InvalidRelationship->throw( child_guarantor => 1 );
}
}
return 1;
}
=head3 add_guarantor

View File

@ -195,10 +195,16 @@
<li id="ERROR_child_is_guarantor">Child patron cannot be a guarantor.</li>
[% END %]
[% IF ( ERROR_guarantor_is_guarantee ) %]
<li id="ERROR_guarantor_is_guarantee">A guarantee cannot be a guarantor.</li>
<li id="ERROR_guarantor_is_guarantee">
Cannot add a guarantor. The proposed guarantor
<a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% guarantor_is_guarantee.borrowernumber | uri %]" target="blank">
[% guarantor_is_guarantee.firstname | html %] [% guarantor_is_guarantee.surname | html %] ([% guarantor_is_guarantee.cardnumber | html %])
</a>
is a guarantee of someone else, and guarantees cannot be guarantors.</li
>
[% END %]
[% IF ( ERROR_guarantee_is_guarantor ) %]
<li id="ERROR_guarantee_is_guarantor">A guarantor cannot be a guarantee.</li>
[% IF ( ERROR_guarantor_cant_have_guarantors ) %]
<li id="ERROR_guarantor_cant_have_guarantors">Cannot add a guarantor. This patron is already a guarantor, and guarantors cannot have guarantors.</li>
[% END %]
[% IF ( ERROR_cannot_delete_guarantor ) %]
<li id="ERROR_cannot_delete_guarantor">Cannot delete guarantor(s). A child patron needs a guarantor.</li>

View File

@ -23,7 +23,8 @@ use Modern::Perl;
use Try::Tiny;
# external modules
use CGI qw ( -utf8 );
use CGI qw ( -utf8 );
use Scalar::Util qw( blessed );
# internal modules
use C4::Auth qw( get_template_and_user haspermission );
@ -52,7 +53,8 @@ use Koha::SMS::Providers;
my $input = CGI->new;
my %data;
my $dbh = C4::Context->dbh;
my $dbh = C4::Context->dbh;
my $logger = Koha::Logger->get( { interface => 'intranet ' } );
my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
{
@ -273,33 +275,61 @@ if (@delete_guarantor) {
if ( C4::Context->preference('ChildNeedsGuarantor') && $will_remove_last ) {
push @errors, 'ERROR_cannot_delete_guarantor';
} else {
my @deleted_guarantors;
foreach my $id (@delete_guarantor) {
my $r = Koha::Patron::Relationships->find($id);
$r->delete() if $r;
if ($r) {
$r->delete();
push @deleted_guarantors, $id;
}
}
foreach my $deleted_guarantor_id (@deleted_guarantors) {
@guarantors = grep { ref($_) eq 'Koha::Patron' && $_->id ne $deleted_guarantor_id } @guarantors;
}
}
}
#Check if guarantor requirements are met
if ( ( $op eq 'cud-save' || $op eq 'cud-insert' ) ) {
unless ( $patron && $patron->is_guarantor ) {
try {
Koha::Patron->validate_guarantor( \@guarantors, $newdata{'contactname'}, $category );
} catch {
if ( ref($_) eq "Koha::Exceptions::Patron::Relationship::NoGuarantor" ) {
push @errors, "ERROR_child_no_guarantor";
} elsif ( ref($_) eq "Koha::Exceptions::Patron::Relationship::InvalidRelationship" ) {
if ( $_->full_message eq "Guarantee patron cannot be a guarantor." ) {
push @errors, "ERROR_guarantor_is_guarantee";
} elsif ( $_->full_message eq "Child patron cannot be a guarantor." ) {
push @errors, "ERROR_child_is_guarantor";
}
my $guarantors_to_be_validated =
@guarantors ? \@guarantors : $newdata{'contactname'} ? [ $newdata{'contactname'} ] : [];
if ( ( $op eq 'cud-save' || $op eq 'cud-insert' ) && $guarantors_to_be_validated ) {
try {
if ($patron) {
if ( $patron->categorycode eq $categorycode ) {
$patron->can_be_guaranteed_by($guarantors_to_be_validated);
} else {
# If $patron's categorycode is about to change, check guarantor requirements against
# the new category but keep the original $patron object intact
# (this clone()s the $patron)
( bless {%$patron}, ref $patron )->categorycode($categorycode)
->can_be_guaranteed_by($guarantors_to_be_validated);
}
return;
};
} else {
push @errors, "ERROR_guarantee_is_guarantor";
}
} else {
Koha::Patron->new( { categorycode => $categorycode } )->can_be_guaranteed_by($guarantors_to_be_validated);
}
} catch {
unless ( blessed($_) ) {
$logger->error($_);
} else {
if ( $_->isa("Koha::Exceptions::Patron::Relationship::NoGuarantor") ) {
push @errors, "ERROR_child_no_guarantor";
} elsif ( $_->isa("Koha::Exceptions::Patron::Relationship::InvalidRelationship") ) {
if ( $_->invalid_guarantor ) {
push @errors, "ERROR_guarantor_is_guarantee";
$template->param( guarantor_is_guarantee => $_->guarantor );
} elsif ( $_->child_guarantor ) {
push @errors, "ERROR_child_is_guarantor";
} elsif ( $_->guarantor_cant_have_guarantors ) {
push @errors, "ERROR_guarantor_cant_have_guarantors";
}
} else {
$logger->error( $_->error );
$_->rethrow;
}
}
return;
};
}
my @valid_relationships = split( /\|/, C4::Context->preference('borrowerRelationship'), -1 );

View File

@ -2624,7 +2624,7 @@ subtest 'get_lists_with_patron() tests' => sub {
$schema->storage->txn_rollback;
};
subtest 'validate_guarantor() tests' => sub {
subtest 'can_be_guaranteed_by() tests' => sub {
plan tests => 5;
@ -2659,6 +2659,12 @@ subtest 'validate_guarantor() tests' => sub {
value => { categorycode => $guarantor_category->{categorycode} }
}
);
my $patron2 = $builder->build_object(
{
class => 'Koha::Patrons',
value => { categorycode => $guarantor_category->{categorycode} }
}
);
my @guarantors;
my $contactname = "";
@ -2666,28 +2672,30 @@ subtest 'validate_guarantor() tests' => sub {
t::lib::Mocks::mock_preference( 'ChildNeedsGuarantor', 0 );
lives_ok { $child->validate_guarantor( \@guarantors, $contactname, $category ); }
lives_ok { $child->can_be_guaranteed_by( \@guarantors ); }
'Validation passes when ChildNeedsGuarantor syspref is disabled';
t::lib::Mocks::mock_preference( 'ChildNeedsGuarantor', 1 );
throws_ok { $child->validate_guarantor( \@guarantors, $contactname, $category ); }
throws_ok { $child->can_be_guaranteed_by( \@guarantors ); }
'Koha::Exceptions::Patron::Relationship::NoGuarantor',
'Exception thrown when guarantor is required but not provided.';
@guarantors = ( $patron, $child2 );
throws_ok { $child->validate_guarantor( \@guarantors, $contactname, $category ); }
throws_ok { $child->can_be_guaranteed_by( \@guarantors ); }
'Koha::Exceptions::Patron::Relationship::InvalidRelationship',
'Exception thrown when child patron is added as guarantor.';
@guarantors = ($patron);
lives_ok { $child->validate_guarantor( \@guarantors, undef, $category ); }
'Validation passes when valid guarantors are passed in array';
t::lib::Mocks::mock_preference( 'borrowerRelationship', 'parent' );
$child2->add_guarantor( { guarantor_id => $patron->id, relationship => 'parent' } );
@guarantors = ($patron2);
throws_ok { $patron->can_be_guaranteed_by( \@guarantors ); }
'Koha::Exceptions::Patron::Relationship::InvalidRelationship',
'Exception thrown when trying to add a guarantor to a guarantor.';
@guarantors = ();
$contactname = "Guarantor";
lives_ok { $child->validate_guarantor( \@guarantors, $contactname, $category ); }
'Validation passes when guarantor contanct name is passed';
@guarantors = ($patron);
lives_ok { $child->can_be_guaranteed_by( \@guarantors ); }
'Validation passes when valid guarantors are passed in array';
$schema->storage->txn_rollback;
};