diff --git a/Koha/Exceptions/Patron/Relationship.pm b/Koha/Exceptions/Patron/Relationship.pm index f02264e6f0..abc5baa2c0 100644 --- a/Koha/Exceptions/Patron/Relationship.pm +++ b/Koha/Exceptions/Patron/Relationship.pm @@ -32,7 +32,7 @@ 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' ] + fields => [ 'relationship', 'no_relationship', 'invalid_guarantor', 'child_guarantor' ] }, 'Koha::Exceptions::Patron::Relationship::NoGuarantor' => { isa => 'Koha::Exceptions::Patron::Relationship', @@ -50,6 +50,8 @@ sub full_message { if ( $self->no_relationship ) { $msg = sprintf("No relationship passed."); } elsif ( $self->invalid_guarantor ) { + $msg = sprintf("Guarantee patron cannot be a guarantor."); + } elsif ( $self->child_guarantor ) { $msg = sprintf("Child patron cannot be a guarantor."); } else { $msg = sprintf( "Invalid relationship passed, '%s' is not defined.", $self->relationship ); diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 8da13cecfc..3740132295 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -207,8 +207,6 @@ sub store { my $self = shift; my $params = @_ ? shift : {}; - my $guarantors = $params->{guarantors} // []; - $self->_result->result_source->schema->txn_do( sub { if ( @@ -329,20 +327,6 @@ sub store { $self->borrowernumber(undef); - if ( C4::Context->preference('ChildNeedsGuarantor') - and ( $self->is_child or $self->category->can_be_guarantee ) - and $self->contactname eq "" - and !@$guarantors ) - { - Koha::Exceptions::Patron::Relationship::NoGuarantor->throw(); - } - - foreach my $guarantor (@$guarantors) { - if ( $guarantor->is_child ) { - Koha::Exceptions::Patron::Relationship::InvalidRelationship->throw( invalid_guarantor => 1 ); - } - } - $self = $self->SUPER::store; $self->add_enrolment_fee_if_needed(0); @@ -386,23 +370,6 @@ sub store { } - my @existing_guarantors = $self->guarantor_relationships()->guarantors->as_list; - push @$guarantors, @existing_guarantors; - - if ( C4::Context->preference('ChildNeedsGuarantor') - and ( $self->is_child or $self->category->can_be_guarantee ) - and ( !defined $self->contactname || $self->contactname eq "" ) - and !@$guarantors ) - { - Koha::Exceptions::Patron::Relationship::NoGuarantor->throw(); - } - - foreach my $guarantor (@$guarantors) { - if ( $guarantor->is_child ) { - Koha::Exceptions::Patron::Relationship::InvalidRelationship->throw( invalid_guarantor => 1 ); - } - } - # Actionlogs if ( C4::Context->preference("BorrowersLog") || C4::Context->preference("CardnumberLog") ) { my $info; @@ -2835,6 +2802,36 @@ sub _anonymize_column { $self->$col($val); } +=head3 validate_guarantor + + Koha::Patron->validate_guarantor(\@guarantors, $contactname, $category ); + + Validates guarantor patron. + +=cut + +sub validate_guarantor { + my ( $self, $guarantors, $contactname, $category ) = @_; + + my $valid_guarantor = @$guarantors ? @$guarantors : $contactname; + + if ( C4::Context->preference('ChildNeedsGuarantor') + and ( $category->category_type eq 'C' or $category->can_be_guarantee ) + and !$valid_guarantor ) + { + Koha::Exceptions::Patron::Relationship::NoGuarantor->throw(); + } + + foreach my $guarantor (@$guarantors) { + if ( $guarantor->is_guarantee ) { + Koha::Exceptions::Patron::Relationship::InvalidRelationship->throw( invalid_guarantor => 1 ); + } elsif ( $guarantor->is_child ) { + Koha::Exceptions::Patron::Relationship::InvalidRelationship->throw( child_guarantor => 1 ); + } + } + +} + =head3 add_guarantor my $relationship = $patron->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 ee80593a9f..312fe4b1bd 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt @@ -195,7 +195,10 @@
  • Child patron cannot be a guarantor.
    • [% END %] [% IF ( ERROR_guarantor_is_guarantee ) %] -
  • A guarantor cannot be a guarantee.
    • +
  • A guarantee cannot be a guarantor.
    • + [% END %] + [% IF ( ERROR_guarantee_is_guarantor ) %] +
  • A guarantor cannot be a guarantee.
    • [% 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 abaff5c6d3..2ee52ea66a 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -281,20 +281,24 @@ if (@delete_guarantor) { } #Check if guarantor requirements are met -my $valid_guarantor = @guarantors ? @guarantors : $newdata{'contactname'}; -if ( ( $op eq 'cud-save' || $op eq 'cud-insert' ) - && C4::Context->preference('ChildNeedsGuarantor') - && ( $category->category_type eq 'C' || $category->can_be_guarantee ) - && !$valid_guarantor ) -{ - push @errors, 'ERROR_child_no_guarantor'; -} - -foreach my $guarantor (@guarantors) { - if ( ( $op eq 'cud-save' || $op eq 'cud-insert' ) - && ( $guarantor->is_child || $guarantor->is_guarantee || ( $patron && $patron->is_guarantor ) ) ) - { - push @errors, 'ERROR_guarantor_is_guarantee'; +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"; + } + } + return; + }; + } else { + push @errors, "ERROR_guarantee_is_guarantor"; } } @@ -447,7 +451,7 @@ if ( ( !$nok ) and $nodouble and ( $op eq 'cud-insert' or $op eq 'cud-save' ) ) delete $newdata{password2}; $success = 1; $patron = try { - Koha::Patron->new( \%newdata )->store( { guarantors => \@guarantors } ); + Koha::Patron->new( \%newdata )->store(); } catch { $success = 0; $nok = 1; @@ -561,7 +565,7 @@ if ( ( !$nok ) and $nodouble and ( $op eq 'cud-insert' or $op eq 'cud-save' ) ) delete $newdata{guarantor_relationship}; try { - $patron->set( \%newdata )->store( { guarantors => \@guarantors } ) if scalar( keys %newdata ) > 1; + $patron->set( \%newdata )->store() if scalar( keys %newdata ) > 1; # bug 4508 - avoid crash if we're not updating any columns in the borrowers table (editing patron attrs or msg prefs) $success = 1; diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t index 9f166cc52c..35e1b4d8ff 100755 --- a/t/db_dependent/Koha/Patron.t +++ b/t/db_dependent/Koha/Patron.t @@ -2624,93 +2624,70 @@ subtest 'get_lists_with_patron() tests' => sub { $schema->storage->txn_rollback; }; -subtest 'guarantor requirements tests' => sub { +subtest 'validate_guarantor() tests' => sub { - plan tests => 6; + plan tests => 5; $schema->storage->txn_begin; my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; - my $child_category = - $builder->build( { source => 'Category', value => { category_type => 'C', can_be_guarantee => 1 } } ) - ->{categorycode}; - my $patron_category = - $builder->build( { source => 'Category', value => { category_type => 'A', can_be_guarantee => 0 } } ) - ->{categorycode}; - - t::lib::Mocks::mock_preference( 'ChildNeedsGuarantor', 0 ); + my $guarantee_category = + $builder->build( + { source => 'Category', value => { categorycode => "GUARANTEE", category_type => 'C', can_be_guarantee => 1 } } + ); + my $guarantor_category = + $builder->build( + { source => 'Category', value => { categorycode => "GUARANTOR", category_type => 'A', can_be_guarantee => 0 } } + ); my $child = Koha::Patron->new( { branchcode => $branchcode, - categorycode => $child_category, + categorycode => $guarantee_category->{categorycode}, contactname => '' } ); - $child->store(); - - ok( - Koha::Patrons->find( $child->id ), - 'Child patron can be stored without guarantor when ChildNeedsGuarantor is off.' - ); - - t::lib::Mocks::mock_preference( 'ChildNeedsGuarantor', 1 ); - - my $child2 = Koha::Patron->new( - { - branchcode => $branchcode, - categorycode => $child_category, - contactname => '' - } - ); - my $child3 = $builder->build_object( + my $child2 = $builder->build_object( { class => 'Koha::Patrons', - value => { categorycode => $child_category } + value => { categorycode => $guarantee_category->{categorycode} } } ); my $patron = $builder->build_object( { class => 'Koha::Patrons', - value => { categorycode => $patron_category } + value => { categorycode => $guarantor_category->{categorycode} } } ); - throws_ok { $child2->store(); } + my @guarantors; + my $contactname = ""; + my $category = Koha::Patron::Categories->find( $guarantee_category->{categorycode} ); + + t::lib::Mocks::mock_preference( 'ChildNeedsGuarantor', 0 ); + + lives_ok { $child->validate_guarantor( \@guarantors, $contactname, $category ); } + 'Validation passes when ChildNeedsGuarantor syspref is disabled'; + + t::lib::Mocks::mock_preference( 'ChildNeedsGuarantor', 1 ); + + throws_ok { $child->validate_guarantor( \@guarantors, $contactname, $category ); } 'Koha::Exceptions::Patron::Relationship::NoGuarantor', 'Exception thrown when guarantor is required but not provided.'; - my @guarantors = ( $patron, $child3 ); - throws_ok { $child2->store( { guarantors => \@guarantors } ); } + @guarantors = ( $patron, $child2 ); + throws_ok { $child->validate_guarantor( \@guarantors, $contactname, $category ); } 'Koha::Exceptions::Patron::Relationship::InvalidRelationship', 'Exception thrown when child patron is added as guarantor.'; - #test ModMember @guarantors = ($patron); - $child2->store( { guarantors => \@guarantors } )->discard_changes(); + lives_ok { $child->validate_guarantor( \@guarantors, undef, $category ); } + 'Validation passes when valid guarantors are passed in array'; - t::lib::Mocks::mock_preference( 'borrowerRelationship', '' ); - - my $relationship = Koha::Patron::Relationship->new( - { - guarantor_id => $patron->borrowernumber, - guarantee_id => $child2->borrowernumber, - relationship => '' - } - ); - $relationship->store(); - - ok( $child2->store(), 'Child patron can be modified and stored when guarantor is stored' ); - - @guarantors = ($child3); - throws_ok { $child2->store( { guarantors => \@guarantors } ); } - 'Koha::Exceptions::Patron::Relationship::InvalidRelationship', - 'Exception thrown when child patron is modified and child patron is added as guarantor.'; - - $relationship->delete; - throws_ok { $child2->store(); } - 'Koha::Exceptions::Patron::Relationship::NoGuarantor', - 'Exception thrown when guarantor is deleted.'; + @guarantors = (); + $contactname = "Guarantor"; + lives_ok { $child->validate_guarantor( \@guarantors, $contactname, $category ); } + 'Validation passes when guarantor contanct name is passed'; $schema->storage->txn_rollback; };