Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 12133: Add requirements for guarantor and guarantee

Browse source 
Add two requirements when registering a new patron:

- A child patron must have a guarantor. This is controlled by
  a new syspref ChildNeedsGuarantor.
- A guarantor cannot be a guarantee.

Test plan:
1. Add a child patron without guarantor or child patron with guarantee
as guarantor succesfully.
2. Apply this patch.
3. Add a child patron as a guarantor.
=> Error is raised.
4. Turn syspref "ChildNeedsGuarantor" ON.
5. Add a child patron without a guarantor and error "Child needs a
guarantor" is raised.
6. Add guarantor. Guarantor can either be existing patron or added with
"Contact" section.
=> Save without errors.

Also prove t/db_dependent/Koha/Patron.t

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This commit is contained in:
Emmi Takkinen 2023-02-17 13:15:17 +02:00 committed by Tomas Cohen Arazi
parent 2120e3d4c8
commit d9a989eaf0
Signed by: tomascohen
GPG key ID: 0A272EA1B2F3C15F
8 changed files with 210 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
Koha/Exceptions/Patron/Relationship.pm
View file

@ -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,6 +50,9 @@ sub full_message {
if ( $self->no_relationship ) {
$msg = sprintf( "No relationship passed." );
}
elsif ( $self->invalid_guarantor ) {
$msg = sprintf("Child patron cannot be a guarantor.");
}
else {
$msg = sprintf("Invalid relationship passed, '%s' is not defined.", $self->relationship );
}

38
Koha/Patron.pm
View file

@ -191,7 +191,10 @@ to db
=cut
sub store {
my ($self) = @_;
my $self = shift;
my $params = @_ ? shift : {};
my $guarantors = $params->{guarantors} // [];
$self->_result->result_source->schema->txn_do(
sub {
@ -288,6 +291,21 @@ 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 or $guarantor->category->can_be_guarantee) {
Koha::Exceptions::Patron::Relationship::InvalidRelationship
->throw( invalid_guarantor => 1 );
}
}
$self = $self->SUPER::store;
$self->add_enrolment_fee_if_needed(0);
@ -333,6 +351,24 @@ 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 $self->contactname eq ""
and !@$guarantors )
{
Koha::Exceptions::Patron::Relationship::NoGuarantor->throw();
}
foreach my $guarantor (@$guarantors) {
if ( $guarantor->is_child or $guarantor->category->can_be_guarantee) {
Koha::Exceptions::Patron::Relationship::InvalidRelationship
->throw( invalid_guarantor => 1 );
}
}
# Actionlogs
if ( C4::Context->preference("BorrowersLog") ) {
my $info;

17
installer/data/mysql/atomicupdate/bug_12133.pl Executable file
View file

@ -0,0 +1,17 @@
use Modern::Perl;
return {
bug_number => "12133",
description => "Add system preference ChildNeedsGuarantor",
up => sub {
my ($args) = @_;
my ($dbh, $out) = @$args{qw(dbh out)};
$dbh->do( q{
INSERT IGNORE INTO systempreferences (variable,value,explanation,options,type)
VALUES('ChildNeedsGuarantor', 0, 'If ON, a child patron must have a guarantor when adding the patron.', '', 'YesNo');
} );
say $out "Added new system preference 'ChildNeedsGuarantor')\n";
},
};

1
installer/data/mysql/mandatory/sysprefs.sql
View file

@ -146,6 +146,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
('ChargeFinesOnClosedDays','0',NULL,'Charge fines on days the library is closed.','YesNo'),
('CheckPrevCheckout','hardno','hardyes|softyes|softno|hardno','By default, for every item checked out, should we warn if the patron has borrowed that item in the past?','Choice'),
('CheckPrevCheckoutDelay','0', NULL,'Maximum number of days that will trigger a warning if the patron has borrowed that item in the past when CheckPrevCheckout is enabled.','free'),
('ChildNeedsGuarantor', 0, 'If ON, a child patron must have a guarantor when adding the patron.', '', 'YesNo'),
('CircAutoPrintQuickSlip','qslip',NULL,'Choose what should happen when an empty barcode field is submitted in circulation: Display a print quick slip window, Display a print slip window or Clear the screen.','Choice'),
('CircConfirmItemParts', '0', NULL, 'Require staff to confirm that all parts of an item are present at checkin/checkout.', 'Yes/No'),
('CircControl','ItemHomeLibrary','PickupLibrary|PatronLibrary|ItemHomeLibrary','Specify the agency that controls the circulation and fines policy','Choice'),

7
koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
View file

@ -381,6 +381,13 @@ Patrons:
1: Allow
0: "Don't allow"
- staff to set the ability for a patron's charges to be viewed by linked patrons in the OPAC.
-
- "A child patron"
- pref: "ChildNeedsGuarantor"
choices:
1: "must have"
0: "doesn't need"
- a guarantor when adding the patron.
Privacy:
-

9
koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
View file

@ -183,6 +183,15 @@ legend:hover {
[% IF ERROR_bad_email_alternative %]
<li id="ERROR_bad_email_alternative">The alternative email is invalid.</li>
[% END %]
[% IF ( ERROR_child_no_guarantor ) %]
<li id="ERROR_child_no_guarantor">A child patron needs a guarantor.</li>
[% END %]
[% IF ( ERROR_guarantor_is_guarantee ) %]
<li id="ERROR_guarantor_is_guarantee">A guarantor cannot be a guarantee.</li>
[% END %]
[% IF ( ERROR_cannot_delete_guarantor ) %]
<li id="ERROR_cannot_delete_guarantor">Cannot delete guarantor(s). A child patron needs a guarantor.</li>
[% END %]
</ul>
</div>
[% END %]

48
members/memberentry.pl
View file

@ -111,12 +111,18 @@ my $guarantor = undef;
$guarantor = Koha::Patrons->find( $guarantor_id ) if $guarantor_id;
$template->param( guarantor => $guarantor );
my @delete_guarantor = $input->multi_param('delete_guarantor');
foreach my $id ( @delete_guarantor ) {
my $r = Koha::Patron::Relationships->find( $id );
$r->delete() if $r;
#Search existing guarantor id(s) and new ones from params
my @guarantors;
my @new_guarantor_ids = grep { $_ ne '' } $input->multi_param('new_guarantor_id');
foreach my $new_guarantor_id (@new_guarantor_ids) {
my $new_guarantor = Koha::Patrons->find( { borrowernumber => $new_guarantor_id } );
push @guarantors, $new_guarantor;
}
my @existing_guarantors = $patron->guarantor_relationships()->guarantors->as_list unless !$patron;
push @guarantors, @existing_guarantors;
## Deal with debarments
$template->param(
restriction_types => scalar Koha::Patron::Restriction::Types->search()
@ -253,6 +259,36 @@ if ( ( $op eq 'insert' ) and !$nodouble ) {
}
}
#Attempt to delete guarantors
my @delete_guarantor = $input->multi_param('delete_guarantor');
if(@delete_guarantor){
if ( C4::Context->preference('ChildNeedsGuarantor')
&& scalar @guarantors - scalar @delete_guarantor == 0 ) {
push @errors, 'ERROR_cannot_delete_guarantor';
} else {
foreach my $id ( @delete_guarantor ) {
my $r = Koha::Patron::Relationships->find( $id );
$r->delete() if $r;
}
}
}
#Check if guarantor requirements are met
my $valid_guarantor = @guarantors ? @guarantors : $newdata{'contactname'};
if ( ( $op eq 'save' || $op eq '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 'save' || $op eq 'insert' ) && $guarantor->is_child || $guarantor->category->can_be_guarantee ) {
push @errors, 'ERROR_guarantor_is_guarantee';
}
}
###############test to take the right zipcode, country and city name ##############
# set only if parameter was passed from the form
$newdata{'city'} = $input->param('city') if defined($input->param('city'));
@ -391,7 +427,7 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){
delete $newdata{password2};
$success = 1;
$patron = try {
Koha::Patron->new(\%newdata)->store;
Koha::Patron->new(\%newdata)->store({ guarantors => \@guarantors });
} catch {
$success = 0;
$nok = 1;
@ -485,7 +521,7 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){
delete $newdata{password2};
try {
$patron->set(\%newdata)->store if scalar(keys %newdata) > 1;
$patron->set(\%newdata)->store({ guarantors => \@guarantors }) 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;
} catch {

88
t/db_dependent/Koha/Patron.t
View file

@ -2145,3 +2145,91 @@ subtest 'get_lists_with_patron() tests' => sub {
$schema->storage->txn_rollback;
};
subtest 'guarantor requirements tests' => sub {
plan tests => 6;
$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 $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 @guarantors = ( $patron, $child3 );
throws_ok { $child2->store( { guarantors => \@guarantors } ); }
'Koha::Exceptions::Patron::Relationship::InvalidRelationship',
'Exception thrown when child patron is added as guarantor.';
#test ModMember
@guarantors = ( $patron );
$child2->store( { guarantors => \@guarantors } )->discard_changes();
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.';
};