Attachment #148070 for bug #12133

View | Details | Raw Unified | Return to bug 12133
Collapse All | Expand All

Lines 32-39 use Exception::Class ( Link Here

(-)a/Koha/Exceptions/Patron/Relationship.pm (-2 / +9 lines)
32	'Koha::Exceptions::Patron::Relationship::InvalidRelationship' => {	32	'Koha::Exceptions::Patron::Relationship::InvalidRelationship' => {
33	isa => 'Koha::Exceptions::Patron::Relationship',	33	isa => 'Koha::Exceptions::Patron::Relationship',
34	description => 'The specified relationship is invalid',	34	description => 'The specified relationship is invalid',
35	fields => ['relationship','no_relationship']	35	fields => [ 'relationship', 'no_relationship', 'invalid_guarantor' ]
36	}	36	},
		37	'Koha::Exceptions::Patron::Relationship::NoGuarantor' => {
		38	isa => 'Koha::Exceptions::Patron::Relationship',
		39	description => 'Child patron needs a guarantor',
		40	},
37	);	41	);
38		42
39	sub full_message {	43	sub full_message {
Lines 46-51 sub full_message { Link Here
46	if ( $self->no_relationship ) {	50	if ( $self->no_relationship ) {
47	$msg = sprintf( "No relationship passed." );	51	$msg = sprintf( "No relationship passed." );
48	}	52	}
		53	elsif ( $self->invalid_guarantor ) {
		54	$msg = sprintf("Child patron cannot be a guarantor.");
		55	}
49	else {	56	else {
50	$msg = sprintf("Invalid relationship passed, '%s' is not defined.", $self->relationship );	57	$msg = sprintf("Invalid relationship passed, '%s' is not defined.", $self->relationship );
51	}	58	}

Lines 185-191 to db Link Here

(-)a/Koha/Patron.pm (-1 / +37 lines)
185	=cut	185	=cut
186		186
187	sub store {	187	sub store {
188	my ($self) = @_;	188	my $self = shift;
		189	my $params = @_ ? shift : {};
		190
		191	my $guarantors = $params->{guarantors} // [];
189		192
190	$self->_result->result_source->schema->txn_do(	193	$self->_result->result_source->schema->txn_do(
191	sub {	194	sub {
Lines 281-286 sub store { Link Here
281		284
282	$self->borrowernumber(undef);	285	$self->borrowernumber(undef);
283		286
		287	if ( C4::Context->preference('ChildNeedsGuarantor')
		288	and $self->is_child
		289	and $self->contactname eq ""
		290	and !@$guarantors )
		291	{
		292	Koha::Exceptions::Patron::Relationship::NoGuarantor->throw();
		293	}
		294
		295	foreach my $guarantor (@$guarantors) {
		296	if ( $guarantor->is_child ) {
		297	Koha::Exceptions::Patron::Relationship::InvalidRelationship
		298	->throw( invalid_guarantor => 1 );
		299	}
		300	}
		301
284	$self = $self->SUPER::store;	302	$self = $self->SUPER::store;
285		303
286	$self->add_enrolment_fee_if_needed(0);	304	$self->add_enrolment_fee_if_needed(0);
Lines 314-319 sub store { Link Here
314		332
315	}	333	}
316		334
		335	my @existing_guarantors = $self->guarantor_relationships()->guarantors->as_list;
		336	push @$guarantors, @existing_guarantors;
		337
		338	if ( C4::Context->preference('ChildNeedsGuarantor')
		339	and $self->is_child
		340	and $self->contactname eq ""
		341	and !@$guarantors )
		342	{
		343	Koha::Exceptions::Patron::Relationship::NoGuarantor->throw();
		344	}
		345
		346	foreach my $guarantor (@$guarantors) {
		347	if ( $guarantor->is_child ) {
		348	Koha::Exceptions::Patron::Relationship::InvalidRelationship
		349	->throw( invalid_guarantor => 1 );
		350	}
		351	}
		352
317	# Actionlogs	353	# Actionlogs
318	if ( C4::Context->preference("BorrowersLog") ) {	354	if ( C4::Context->preference("BorrowersLog") ) {
319	my $info;	355	my $info;




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";
    },
};

Lines 138-143 INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` Link Here

(-)a/installer/data/mysql/mandatory/sysprefs.sql (+1 lines)
138	('ChargeFinesOnClosedDays','0',NULL,'Charge fines on days the library is closed.','YesNo'),	138	('ChargeFinesOnClosedDays','0',NULL,'Charge fines on days the library is closed.','YesNo'),
139	('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'),	139	('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'),
140	('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'),	140	('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'),
		141	('ChildNeedsGuarantor', 0, 'If ON, a child patron must have a guarantor when adding the patron.', '', 'YesNo'),
141	('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'),	142	('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'),
142	('CircConfirmItemParts', '0', NULL, 'Require staff to confirm that all parts of an item are present at checkin/checkout.', 'Yes/No'),	143	('CircConfirmItemParts', '0', NULL, 'Require staff to confirm that all parts of an item are present at checkin/checkout.', 'Yes/No'),
143	('CircControl','ItemHomeLibrary','PickupLibrary\|PatronLibrary\|ItemHomeLibrary','Specify the agency that controls the circulation and fines policy','Choice'),	144	('CircControl','ItemHomeLibrary','PickupLibrary\|PatronLibrary\|ItemHomeLibrary','Specify the agency that controls the circulation and fines policy','Choice'),

Lines 349-354 Patrons: Link Here

(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref (+7 lines)
349	1: Allow	349	1: Allow
350	0: "Don't allow"	350	0: "Don't allow"
351	- staff to set the ability for a patron's fines to be viewed by linked patrons in the OPAC.	351	- staff to set the ability for a patron's fines to be viewed by linked patrons in the OPAC.
		352	-
		353	- "A child patron"
		354	- pref: "ChildNeedsGuarantor"
		355	choices:
		356	1: "must have"
		357	0: "doesn't need"
		358	- a guarantor when adding the patron.
352		359
353	Privacy:	360	Privacy:
354	-	361	-

Lines 195-200 legend:hover { Link Here

(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt (+6 lines)
195	[% IF ERROR_bad_email_alternative %]	195	[% IF ERROR_bad_email_alternative %]
196	<li id="ERROR_bad_email_alternative">The alternative email is invalid.</li>	196	<li id="ERROR_bad_email_alternative">The alternative email is invalid.</li>
197	[% END %]	197	[% END %]
		198	[% IF ( ERROR_child_no_guarantor ) %]
		199	<li id="ERROR_child_no_guarantor">A child patron needs a guarantor.</li>
		200	[% END %]
		201	[% IF ( ERROR_guarantor_is_guarantee ) %]
		202	<li id="ERROR_guarantor_is_guarantee">A guarantor cannot be a guarantee.</li>
		203	[% END %]
198	</ul>	204	</ul>
199	</div>	205	</div>
200	[% END %]	206	[% END %]

Lines 116-121 foreach my $id ( @delete_guarantor ) { Link Here

(-)a/members/memberentry.pl (-2 / +30 lines)
116	$r->delete() if $r;	116	$r->delete() if $r;
117	}	117	}
118		118
		119	#Search existing guarantor id(s) and new ones from params
		120	my @guarantors;
		121	my @new_guarantor_ids = grep { $_ ne '' } $input->multi_param('new_guarantor_id');
		122
		123	foreach my $new_guarantor_id (@new_guarantor_ids) {
		124	my $new_guarantor = Koha::Patrons->find( { borrowernumber => $new_guarantor_id } );
		125	push @guarantors, $new_guarantor;
		126	}
		127
		128	my @existing_guarantors = $patron->guarantor_relationships()->guarantors->as_list unless !$patron;
		129	push @guarantors, @existing_guarantors;
		130
119	## Deal with debarments	131	## Deal with debarments
120	$template->param(	132	$template->param(
121	restriction_types => scalar Koha::Patron::Restriction::Types->search()	133	restriction_types => scalar Koha::Patron::Restriction::Types->search()
Lines 265-270 if ( ( $op eq 'insert' ) and !$nodouble ) { Link Here
265	}	277	}
266	}	278	}
267		279
		280	#Check if guarantor requirements are met
		281	my $valid_guarantor = @guarantors ? @guarantors : $newdata{'contactname'};
		282	if ( C4::Context->preference('ChildNeedsGuarantor')
		283	&& $category->category_type eq 'C'
		284	&& !$valid_guarantor
		285	&& ( $op eq 'save' \|\| $op eq 'insert' ) )
		286	{
		287	push @errors, 'ERROR_child_no_guarantor';
		288	}
		289
		290	foreach my $guarantor (@guarantors) {
		291	if ( $guarantor->is_child && ( $op eq 'save' \|\| $op eq 'insert' ) ) {
		292	push @errors, 'ERROR_guarantor_is_guarantee';
		293	}
		294	}
		295
268	###############test to take the right zipcode, country and city name ##############	296	###############test to take the right zipcode, country and city name ##############
269	# set only if parameter was passed from the form	297	# set only if parameter was passed from the form
270	$newdata{'city'} = $input->param('city') if defined($input->param('city'));	298	$newdata{'city'} = $input->param('city') if defined($input->param('city'));
Lines 428-434 if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ Link Here
428	if ($op eq 'insert'){	456	if ($op eq 'insert'){
429	# we know it's not a duplicate borrowernumber or there would already be an error	457	# we know it's not a duplicate borrowernumber or there would already be an error
430	delete $newdata{password2};	458	delete $newdata{password2};
431	$patron = eval { Koha::Patron->new(\%newdata)->store };	459	$patron = eval { Koha::Patron->new(\%newdata)->store({ guarantors => \@guarantors }) };
432	if ( $@ ) {	460	if ( $@ ) {
433	# FIXME Urgent error handling here, we cannot fail without relevant feedback	461	# FIXME Urgent error handling here, we cannot fail without relevant feedback
434	# Lot of code will need to be removed from this script to handle exceptions raised by Koha::Patron->store	462	# Lot of code will need to be removed from this script to handle exceptions raised by Koha::Patron->store
Lines 514-520 if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ Link Here
514	delete $newdata{password2};	542	delete $newdata{password2};
515		543
516	eval {	544	eval {
517	$patron->set(\%newdata)->store if scalar(keys %newdata) > 1; # bug 4508 - avoid crash if we're not	545	$patron->set(\%newdata)->store({ guarantors => \@guarantors }) if scalar(keys %newdata) > 1; # bug 4508 - avoid crash if we're not
518	# updating any columns in the borrowers table,	546	# updating any columns in the borrowers table,
519	# which can happen if we're only editing the	547	# which can happen if we're only editing the
520	# patron attributes or messaging preferences sections	548	# patron attributes or messaging preferences sections





use Modern::Perl;

use Test::More tests => 21;
use Test::Exception;
use Test::Warn;



    $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' } } )
      ->{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 @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 } );

    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.';
};

Return to bug 12133

Lines 19-25 Link Here

(-)a/t/db_dependent/Koha/Patron.t (-2 / +89 lines)
19		19
20	use Modern::Perl;	20	use Modern::Perl;
21		21
22	use Test::More tests => 20;	22	use Test::More tests => 21;
23	use Test::Exception;	23	use Test::Exception;
24	use Test::Warn;	24	use Test::Warn;
25		25
Lines 1398-1400 subtest 'get_savings tests' => sub { Link Here
1398		1398
1399	$schema->storage->txn_rollback;	1399	$schema->storage->txn_rollback;
1400	};	1400	};
1401	-	1401
		1402	subtest 'guarantor requirements tests' => sub {
		1403
		1404	plan tests => 6;
		1405
		1406	$schema->storage->txn_begin;
		1407
		1408	my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode};
		1409	my $child_category = $builder->build(
		1410	{ source => 'Category', value => { category_type => 'C' } } )
		1411	->{categorycode};
		1412	my $patron_category = $builder->build(
		1413	{ source => 'Category', value => { category_type => 'A' } } )
		1414	->{categorycode};
		1415
		1416	t::lib::Mocks::mock_preference( 'ChildNeedsGuarantor', 0 );
		1417
		1418	my $child = Koha::Patron->new(
		1419	{
		1420	branchcode => $branchcode,
		1421	categorycode => $child_category,
		1422	contactname => ''
		1423	}
		1424	);
		1425	$child->store();
		1426
		1427	ok(
		1428	Koha::Patrons->find( $child->id ),
		1429	'Child patron can be stored without guarantor when ChildNeedsGuarantor is off.'
		1430	);
		1431
		1432	t::lib::Mocks::mock_preference( 'ChildNeedsGuarantor', 1 );
		1433
		1434	my $child2 = Koha::Patron->new(
		1435	{
		1436	branchcode => $branchcode,
		1437	categorycode => $child_category,
		1438	contactname => ''
		1439	}
		1440	);
		1441	my $child3 = $builder->build_object(
		1442	{
		1443	class => 'Koha::Patrons',
		1444	value => { categorycode => $child_category }
		1445	}
		1446	);
		1447	my $patron = $builder->build_object(
		1448	{
		1449	class => 'Koha::Patrons',
		1450	value => { categorycode => $patron_category }
		1451	}
		1452	);
		1453
		1454	throws_ok { $child2->store(); }
		1455	'Koha::Exceptions::Patron::Relationship::NoGuarantor',
		1456	'Exception thrown when guarantor is required but not provided.';
		1457
		1458	my @guarantors = ( $patron, $child3 );
		1459	throws_ok { $child2->store( { guarantors => \@guarantors } ); }
		1460	'Koha::Exceptions::Patron::Relationship::InvalidRelationship',
		1461	'Exception thrown when child patron is added as guarantor.';
		1462
		1463	#test ModMember
		1464	@guarantors = ( $patron );
		1465	$child2->store( { guarantors => \@guarantors } );
1466
1467	t::lib::Mocks::mock_preference( 'borrowerRelationship', '' );
1468
1469	my $relationship = Koha::Patron::Relationship->new(
1470	{ guarantor_id => $patron->borrowernumber,
1471	guarantee_id => $child2->borrowernumber,
1472	relationship => ''
1473	}
1474	);
1475	$relationship->store();
1476
1477	ok( $child2->store(), 'Child patron can be modified and stored when guarantor is stored');
1478
1479	@guarantors = ( $child3 );
1480	throws_ok { $child2->store( { guarantors => \@guarantors } ); }
1481	'Koha::Exceptions::Patron::Relationship::InvalidRelationship',
1482	'Exception thrown when child patron is modified and child patron is added as guarantor.';
1483
1484	$relationship->delete;
1485	throws_ok { $child2->store(); }
1486	'Koha::Exceptions::Patron::Relationship::NoGuarantor',
1487	'Exception thrown when guarantor is deleted.';
1488	};

Line 0 Link Here

(-)a/installer/data/mysql/atomicupdate/bug_12133.pl (+17 lines)
	1	use Modern::Perl;
	2
	3	return {
	4	bug_number => "12133",
	5	description => "Add system preference ChildNeedsGuarantor",
	6	up => sub {
	7	my ($args) = @_;
	8	my ($dbh, $out) = @$args{qw(dbh out)};
	9
	10	$dbh->do( q{
	11	INSERT IGNORE INTO systempreferences (variable,value,explanation,options,type)
	12	VALUES('ChildNeedsGuarantor', 0, 'If ON, a child patron must have a guarantor when adding the patron.', '', 'YesNo');
	13	} );
	14
	15	say $out "Added new system preference 'ChildNeedsGuarantor')\n";
	16	},
	17	};