From c0d37fd2ddb8f85ae27e6d239798bdf9f01c53c2 Mon Sep 17 00:00:00 2001 From: Emmi Takkinen Date: Wed, 4 Nov 2020 10:08:03 +0200 Subject: [PATCH 2/2] 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 ca719cbe42..93f37f58f4 100644 --- a/Koha/Exceptions/Patron/Relationship.pm +++ b/Koha/Exceptions/Patron/Relationship.pm @@ -30,8 +30,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 { @@ -44,9 +48,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 8f3d8e6990..ae9079238f 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -172,7 +172,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 { @@ -201,6 +204,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 c2099c283e..a3c0411d5b 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -474,7 +474,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 @@ -560,7 +560,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 1b0d24552a..09b03e8a30 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 => 6; +use Test::More tests => 7; use Test::Exception; use Koha::Database; @@ -365,3 +365,38 @@ subtest 'is_superlibrarian() 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.17.1