diff --git a/Koha/Exceptions/Patron/Relationship.pm b/Koha/Exceptions/Patron/Relationship.pm
index abc5baa2c0..768d5a1b0f 100644
--- a/Koha/Exceptions/Patron/Relationship.pm
+++ b/Koha/Exceptions/Patron/Relationship.pm
@@ -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',
diff --git a/Koha/Patron.pm b/Koha/Patron.pm
index 3740132295..7b8b69315b 100644
--- a/Koha/Patron.pm
+++ b/Koha/Patron.pm
@@ -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
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
index 312fe4b1bd..9e8bf04c0b 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
@@ -195,10 +195,16 @@
Child patron cannot be a guarantor.
[% END %]
[% IF ( ERROR_guarantor_is_guarantee ) %]
- A guarantee cannot be a guarantor.
+
+ Cannot add a guarantor. The proposed guarantor
+
+ [% guarantor_is_guarantee.firstname | html %] [% guarantor_is_guarantee.surname | html %] ([% guarantor_is_guarantee.cardnumber | html %])
+
+ is a guarantee of someone else, and guarantees cannot be guarantors.
[% END %]
- [% IF ( ERROR_guarantee_is_guarantor ) %]
- A guarantor cannot be a guarantee.
+ [% IF ( ERROR_guarantor_cant_have_guarantors ) %]
+ Cannot add a guarantor. This patron is already a guarantor, and guarantors cannot have guarantors.
[% END %]
[% IF ( ERROR_cannot_delete_guarantor ) %]
Cannot delete guarantor(s). A child patron needs a guarantor.
diff --git a/members/memberentry.pl b/members/memberentry.pl
index 2ee52ea66a..ec02bec7d1 100755
--- a/members/memberentry.pl
+++ b/members/memberentry.pl
@@ -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 );
diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t
index 35e1b4d8ff..44802ce4ba 100755
--- a/t/db_dependent/Koha/Patron.t
+++ b/t/db_dependent/Koha/Patron.t
@@ -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;
};