From 9d9233b35b33b7137c14f3f1d4e975e345c27775 Mon Sep 17 00:00:00 2001 From: Emmi Takkinen Date: Wed, 4 Nov 2020 10:08:03 +0200 Subject: [PATCH 3/3] Bug 12133: Enforce guarantor restrictions on Koha::Patron->store() This patch adds checks for new guarantor requirements to Koha::Patron->store(). If requirements aren't met exception is raised. To test prove t/db_dependent/Koha/Patron.t Sponsored-by: Koha-Suomi Oy --- Koha/Exceptions/Patron/Relationship.pm | 13 ++++++--- Koha/Patron.pm | 19 ++++++++++++- members/memberentry.pl | 4 +-- t/db_dependent/Koha/Patron.t | 37 +++++++++++++++++++++++++- 4 files changed, 66 insertions(+), 7 deletions(-) diff --git a/Koha/Exceptions/Patron/Relationship.pm b/Koha/Exceptions/Patron/Relationship.pm index 616ad830c7..62f6662129 100644 --- a/Koha/Exceptions/Patron/Relationship.pm +++ b/Koha/Exceptions/Patron/Relationship.pm @@ -32,8 +32,12 @@ use Exception::Class ( 'Koha::Exceptions::Patron::Relationship::InvalidRelationship' => { isa => 'Koha::Exceptions::Patron::Relationship', description => 'The specified relationship is invalid', - fields => ['relationship','no_relationship'] - } + fields => ['relationship','no_relationship','invalid_guarantor'] + }, + 'Koha::Exceptions::Patron::Relationship::NoGuarantor' => { + isa => 'Koha::Exceptions::Patron::Relationship', + description => 'Child patron needs a guarantor', + }, ); sub full_message { @@ -46,9 +50,12 @@ sub full_message { if ( $self->no_relationship ) { $msg = sprintf( "No relationship passed." ); } - else { + elsif ( $self->relationship ) { $msg = sprintf("Invalid relationship passed, '%s' is not defined.", $self->relationship ); } + elsif ( $self->invalid_guarantor ) { + $msg = sprintf("Child patron cannot be a guarantor."); + } } elsif ( $self->isa('Koha::Exceptions::Patron::Relationship::DuplicateRelationship') ) { $msg diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 266e60846c..7b402f8859 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -181,7 +181,10 @@ to db =cut sub store { - my ($self) = @_; + my $self = shift; + my $params = @_ ? shift : {}; + + my $guarantor_ids = $params->{guarantor_ids} // []; $self->_result->result_source->schema->txn_do( sub { @@ -214,6 +217,20 @@ sub store { $self->surname( uc($self->surname) ) if C4::Context->preference("uppercasesurnames"); + if ( C4::Context->preference('ChildNeedsGuarantor') and $self->is_child + and $self->contactname eq "" and !@$guarantor_ids ) { + Koha::Exceptions::Patron::Relationship::NoGuarantor->throw(); + } + + foreach my $guarantor_id (@$guarantor_ids){ + my $guarantor = Koha::Patrons->find({ borrowernumber => $guarantor_id }); + if($guarantor->is_child){ + Koha::Exceptions::Patron::Relationship::InvalidRelationship->throw( + invalid_guarantor => 1 + ); + } + } + $self->relationship(undef) # We do not want to store an empty string in this field if defined $self->relationship and $self->relationship eq ""; diff --git a/members/memberentry.pl b/members/memberentry.pl index 2c833d1aca..eeee56109f 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -468,7 +468,7 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ if ($op eq 'insert'){ # we know it's not a duplicate borrowernumber or there would already be an error delete $newdata{password2}; - $patron = eval { Koha::Patron->new(\%newdata)->store }; + $patron = eval { Koha::Patron->new(\%newdata)->store({ guarantor_ids => \@guarantor_ids }) }; if ( $@ ) { # FIXME Urgent error handling here, we cannot fail without relevant feedback # Lot of code will need to be removed from this script to handle exceptions raised by Koha::Patron->store @@ -554,7 +554,7 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ delete $newdata{password2}; eval { - $patron->set(\%newdata)->store if scalar(keys %newdata) > 1; # bug 4508 - avoid crash if we're not + $patron->set(\%newdata)->store({ guarantor_ids => \@guarantor_ids }) if scalar(keys %newdata) > 1; # bug 4508 - avoid crash if we're not # updating any columns in the borrowers table, # which can happen if we're only editing the # patron attributes or messaging preferences sections diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t index 6418d6066f..33a3cca9c4 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 => 16; +use Test::More tests => 17; use Test::Exception; use Test::Warn; @@ -1137,3 +1137,38 @@ subtest 'recalls() tests' => sub { $schema->storage->txn_rollback; }; + +subtest 'guarantor requirements tests' => sub { + + plan tests => 3; + + $schema->storage->txn_begin; + + my $branchcode = $builder->build({ source => 'Branch' })->{branchcode}; + my $child_category = $builder->build({ source => 'Category', value => { category_type => 'C' }})->{categorycode}; + my $patron_category = $builder->build({ source => 'Category', value => { category_type => 'A' }})->{categorycode}; + + t::lib::Mocks::mock_preference( 'ChildNeedsGuarantor', 0 ); + + my $child = Koha::Patron->new({ branchcode => $branchcode, categorycode => $child_category, 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({ class => 'Koha::Patrons', value => { categorycode => $child_category }}); + my $patron = $builder->build_object({ class => 'Koha::Patrons', value => { categorycode => $patron_category }}); + + throws_ok { $child2->store(); } + 'Koha::Exceptions::Patron::Relationship::NoGuarantor', + 'Exception thrown when guarantor is required but not provided.'; + + my @guarantor_ids = ( $patron->id, $child3->id ); + throws_ok { $child2->store({ guarantor_ids => \@guarantor_ids }); } + 'Koha::Exceptions::Patron::Relationship::InvalidRelationship', + 'Exception thrown when child patron is added as guarantor.'; + + $schema->storage->txn_rollback; +}; \ No newline at end of file -- 2.25.1