Attachment #157310 for bug #26170

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

Lines 337-342 sub GetBorrowersToExpunge { Link Here

(-)a/C4/Members.pm (+1 lines)
337	$query .= q\| WHERE category_type <> 'S'	337	$query .= q\| WHERE category_type <> 'S'
338	AND ( borrowers.flags IS NULL OR borrowers.flags = 0 )	338	AND ( borrowers.flags IS NULL OR borrowers.flags = 0 )
339	AND tmp.guarantor_id IS NULL	339	AND tmp.guarantor_id IS NULL
		340	AND borrowers.protected = 0
340	\|;	341	\|;
341	my @query_params;	342	my @query_params;
342	if ( $filterbranch && $filterbranch ne "" ) {	343	if ( $filterbranch && $filterbranch ne "" ) {

Lines 23-28 use Exception::Class ( Link Here

(-)a/Koha/Exceptions/Patron.pm (+4 lines)
23	isa => 'Koha::Exceptions::Patron',	23	isa => 'Koha::Exceptions::Patron',
24	description => "Deleting patron failed, AnonymousPatron is not deleteable"	24	description => "Deleting patron failed, AnonymousPatron is not deleteable"
25	},	25	},
		26	'Koha::Exceptions::Patron::FailedDeleteProtectedPatron' => {
		27	isa => 'Koha::Exceptions::Patron',
		28	description => "Deleting patron failed, patron is protected"
		29	},
26	'Koha::Exceptions::Patron::InvalidUserid' => {	30	'Koha::Exceptions::Patron::InvalidUserid' => {
27	isa => 'Koha::Exceptions::Patron',	31	isa => 'Koha::Exceptions::Patron',
28	description => 'Field userid is not valid (probably not unique)',	32	description => 'Field userid is not valid (probably not unique)',

Lines 398-403 sub delete { Link Here

(-)a/Koha/Patron.pm (+14 lines)
398	my $anonymous_patron = C4::Context->preference("AnonymousPatron");	398	my $anonymous_patron = C4::Context->preference("AnonymousPatron");
399	Koha::Exceptions::Patron::FailedDeleteAnonymousPatron->throw() if $anonymous_patron && $self->id eq $anonymous_patron;	399	Koha::Exceptions::Patron::FailedDeleteAnonymousPatron->throw() if $anonymous_patron && $self->id eq $anonymous_patron;
400		400
		401	# Check if patron is protected
		402	Koha::Exceptions::Patron::FailedDeleteProtectedPatron->throw() if $self->protected == 1;
		403
401	$self->_result->result_source->schema->txn_do(	404	$self->_result->result_source->schema->txn_do(
402	sub {	405	sub {
403	# Cancel Patron's holds	406	# Cancel Patron's holds
Lines 627-632 sub merge_with { Link Here
627	my $anonymous_patron = C4::Context->preference("AnonymousPatron");	630	my $anonymous_patron = C4::Context->preference("AnonymousPatron");
628	return if $anonymous_patron && $self->id eq $anonymous_patron;	631	return if $anonymous_patron && $self->id eq $anonymous_patron;
629		632
		633	# Do not merge other patrons into a protected patron
		634	return if $self->protected;
		635
630	my @patron_ids = @{ $patron_ids };	636	my @patron_ids = @{ $patron_ids };
631		637
632	# Ensure the keeper isn't in the list of patrons to merge	638	# Ensure the keeper isn't in the list of patrons to merge
Lines 645-650 sub merge_with { Link Here
645		651
646	next unless $patron;	652	next unless $patron;
647		653
		654	# Do not merge protected patrons into other patrons
		655	next if $patron->protected;
		656
648	# Unbless for safety, the patron will end up being deleted	657	# Unbless for safety, the patron will end up being deleted
649	$results->{merged}->{$patron_id}->{patron} = $patron->unblessed;	658	$results->{merged}->{$patron_id}->{patron} = $patron->unblessed;
650		659
Lines 2466-2471 This method tells if the Koha:Patron object can be deleted. Possible return valu Link Here
2466		2475
2467	=item 'is_anonymous_patron'	2476	=item 'is_anonymous_patron'
2468		2477
		2478	=item 'is_protected'
		2479
2469	=back	2480	=back
2470		2481
2471	=cut	2482	=cut
Lines 2489-2494 sub safe_to_delete { Link Here
2489	elsif ( $self->guarantee_relationships->count ) {	2500	elsif ( $self->guarantee_relationships->count ) {
2490	$error = 'has_guarantees';	2501	$error = 'has_guarantees';
2491	}	2502	}
		2503	elsif ( $self->protected ) {
		2504	$error = 'is_protected';
		2505	}
2492		2506
2493	if ( $error ) {	2507	if ( $error ) {
2494	return Koha::Result::Boolean->new(0)->add_message({ message => $error });	2508	return Koha::Result::Boolean->new(0)->add_message({ message => $error });

Lines 402-407 sub delete { Link Here

(-)a/Koha/REST/V1/Patrons.pm (+1 lines)
402	has_debt => 'Pending debts prevent deletion',	402	has_debt => 'Pending debts prevent deletion',
403	has_guarantees => 'Patron is a guarantor and it prevents deletion',	403	has_guarantees => 'Patron is a guarantor and it prevents deletion',
404	is_anonymous_patron => 'Anonymous patron cannot be deleted',	404	is_anonymous_patron => 'Anonymous patron cannot be deleted',
		405	is_protected => 'Protected patrons cannot be deleted',
405	};	406	};
406		407
407	if ( any { $error->message eq $_ } keys %{$error_descriptions} ) {	408	if ( any { $error->message eq $_ } keys %{$error_descriptions} ) {

Lines 374-379 properties: Link Here

(-)a/api/v1/swagger/definitions/patron.yaml (+4 lines)
374	- object	374	- object
375	- "null"	375	- "null"
376	description: Library of the patron	376	description: Library of the patron
		377	protected:
		378	type:
		379	- boolean
		380	description: Protected status of the patron
377	additionalProperties: false	381	additionalProperties: false
378	required:	382	required:
379	- surname	383	- surname

Lines 339-344 Link Here

(-)a/api/v1/swagger/paths/patrons.yaml (+6 lines)
339	description: Search on login_attempts	339	description: Search on login_attempts
340	required: false	340	required: false
341	type: string	341	type: string
		342	- name: protected
		343	in: query
		344	description: Search on protected status
		345	required: false
		346	type: boolean
342	- $ref: "../swagger.yaml#/parameters/match"	347	- $ref: "../swagger.yaml#/parameters/match"
343	- $ref: "../swagger.yaml#/parameters/order_by"	348	- $ref: "../swagger.yaml#/parameters/order_by"
344	- $ref: "../swagger.yaml#/parameters/page"	349	- $ref: "../swagger.yaml#/parameters/page"
Lines 603-608 Link Here
603	* `has_debt`: The patron has pending debts	608	* `has_debt`: The patron has pending debts
604	* `has_guarantees`: The patron has guarantees	609	* `has_guarantees`: The patron has guarantees
605	* `is_anonymous_patron`: The system-wide anonymous patron cannot be deleted	610	* `is_anonymous_patron`: The system-wide anonymous patron cannot be deleted
		611	* `is_protected`: Protected patrons cannot be deleted
606	schema:	612	schema:
607	$ref: "../swagger.yaml#/definitions/error"	613	$ref: "../swagger.yaml#/definitions/error"
608	"500":	614	"500":

Lines 84-90 Link Here

(-)a/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc (-1 / +5 lines)
84	[% END %]	84	[% END %]
85		85
86	[% IF CAN_user_borrowers_delete_borrowers %]	86	[% IF CAN_user_borrowers_delete_borrowers %]
87	<li><a id="deletepatron" href="#">Delete</a></li>	87	[% IF ( patron.protected == 1 ) %]
		88	<li class="disabled"><a data-toggle="tooltip" data-placement="left" title="Patron is protected" id="deletepatron" href="#">Delete</a></li>
		89	[% ELSE %]
		90	<li><a id="deletepatron" href="#">Delete</a></li>
		91	[% END %]
88	[% ELSE %]	92	[% ELSE %]
89	<li class="disabled"><a data-toggle="tooltip" data-placement="left" title="You are not authorized to delete patrons" id="deletepatron" href="#">Delete</a></li>	93	<li class="disabled"><a data-toggle="tooltip" data-placement="left" title="You are not authorized to delete patrons" id="deletepatron" href="#">Delete</a></li>
90	[% END %]	94	[% END %]




                                            </li>
                                        [% END %]

                                        <li class="radio">
                                            <label for="protected">Protected:</label>
                                            [% IF ( patron.protected == 1 ) %]
                                                <label for="yes-protected">
                                                    Yes
                                                    <input type="radio" id="yes-protected" name="protected" value="1" checked="checked" />
                                                </label>
                                                <label for="no-protected">
                                                    No
                                                    <input type="radio" id="no-protected" name="protected" value="0" />
                                                </label>
                                            [% ELSE %]
                                                <label for="yes-protected">
                                                    Yes
                                                    <input type="radio" id="yes-protected" name="protected" value="1" />
                                                </label>
                                                <label for="no-protected">
                                                    No
                                                    <input type="radio" id="no-protected" name="protected" value="0" checked="checked" />
                                                </label>
                                            [% END %]
                                        </li>

                                        [% IF ( Koha.Preference('CheckPrevCheckout') == 'softyes' || Koha.Preference('CheckPrevCheckout') == 'softno' ) %]
                                            <li>
                                                <label for="checkprevcheckout">Check for previous checkouts: </label>

Lines 471-476 Link Here

(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt (+8 lines)
471	[% END %]	471	[% END %]
472	</li>	472	</li>
473	[% END %]	473	[% END %]
		474	<li id="patron-protected">
		475	<span class="label">Protected: </span>
		476	[% IF ( patron.protected == 1 ) %]
		477	Yes
		478	[% ELSE %]
		479	No
		480	[% END %]
		481	</li>
474		482
475	[% IF ( patron.borrowernotes ) %]	483	[% IF ( patron.borrowernotes ) %]
476	<li id="patron-borrowernotes">	484	<li id="patron-borrowernotes">





subtest 'safe_to_delete() tests' => sub {

    plan tests => 17;

    $schema->storage->txn_begin;


    t::lib::Mocks::mock_userenv( { borrowernumber => $manager->id } );
    $patron->account->pay({ amount => 10, debits => [ $debit ] });

    ## Make it protected
    $patron->protected( 1 );
    ok( !$patron->safe_to_delete, 'Cannot delete, is protected' );
    $message = $patron->safe_to_delete->messages->[0];
    is( $message->type, 'error', 'Type is error' );
    is( $message->message, 'is_protected', 'Cannot delete, is protected' );
    $patron->protected( 0 );

    ## Happy case :-D
    ok( $patron->safe_to_delete, 'Can delete, all conditions met' );
    my $messages = $patron->safe_to_delete->messages;

Lines 1627-1633 subtest 'BorrowersLog tests' => sub { Link Here

(-)a/t/db_dependent/Koha/Patrons.t (-2 / +14 lines)
1627	$schema->storage->txn_rollback;	1627	$schema->storage->txn_rollback;
1628		1628
1629	subtest 'Test Koha::Patrons::merge' => sub {	1629	subtest 'Test Koha::Patrons::merge' => sub {
1630	plan tests => 110;	1630	plan tests => 113;
1631		1631
1632	my $schema = Koha::Database->new()->schema();	1632	my $schema = Koha::Database->new()->schema();
1633		1633
Lines 1639-1644 subtest 'Test Koha::Patrons::merge' => sub { Link Here
1639	my $loser_1 = $builder->build({ source => 'Borrower' })->{borrowernumber};	1639	my $loser_1 = $builder->build({ source => 'Borrower' })->{borrowernumber};
1640	my $loser_2 = $builder->build({ source => 'Borrower' })->{borrowernumber};	1640	my $loser_2 = $builder->build({ source => 'Borrower' })->{borrowernumber};
1641		1641
		1642	my $keeper_protected = $builder->build_object({ class => 'Koha::Patrons' });
		1643	$keeper_protected->protected( 1 )->store;
		1644
		1645	my $loser_protected_obj = $builder->build_object({ class => 'Koha::Patrons' });
		1646	$loser_protected_obj->protected( 1 )->store;
		1647	my $loser_protected = $loser_protected_obj->borrowernumber;
		1648
1642	my $anonymous_patron_orig = C4::Context->preference('AnonymousPatron');	1649	my $anonymous_patron_orig = C4::Context->preference('AnonymousPatron');
1643	my $anonymous_patron = $builder->build({ source => 'Borrower' })->{borrowernumber};	1650	my $anonymous_patron = $builder->build({ source => 'Borrower' })->{borrowernumber};
1644	t::lib::Mocks::mock_preference( 'AnonymousPatron', $anonymous_patron );	1651	t::lib::Mocks::mock_preference( 'AnonymousPatron', $anonymous_patron );
Lines 1661-1667 subtest 'Test Koha::Patrons::merge' => sub { Link Here
1661	is( $loser_2_rs->count(), 1, "Found 1 $r rows for loser_2" );	1668	is( $loser_2_rs->count(), 1, "Found 1 $r rows for loser_2" );
1662	}	1669	}
1663		1670
1664	my $results = $keeper->merge_with([ $loser_1, $loser_2 ]);	1671	my $results_protected = $keeper_protected->merge_with( [ $loser_1 ] );
		1672	is( $results_protected, undef, "Protected patrons cannot have other patrons merged into them" );
		1673	is( Koha::Patrons->search( { borrowernumber => $loser_1 } )->count, 1, "Patron from attempted merge with protected patron still exists" );
		1674
		1675	my $results = $keeper->merge_with([ $loser_1, $loser_2, $loser_protected ]);
1665		1676
1666	while (my ($r, $field) = each(%$resultsets)) {	1677	while (my ($r, $field) = each(%$resultsets)) {
1667	my $keeper_rs =	1678	my $keeper_rs =
Lines 1672-1677 subtest 'Test Koha::Patrons::merge' => sub { Link Here
1672	is( Koha::Patrons->find($loser_1), undef, 'Loser 1 has been deleted' );	1683	is( Koha::Patrons->find($loser_1), undef, 'Loser 1 has been deleted' );
1673	is( Koha::Patrons->find($loser_2), undef, 'Loser 2 has been deleted' );	1684	is( Koha::Patrons->find($loser_2), undef, 'Loser 2 has been deleted' );
1674	is( ref Koha::Patrons->find($anonymous_patron), 'Koha::Patron', 'Anonymous Patron was not deleted' );	1685	is( ref Koha::Patrons->find($anonymous_patron), 'Koha::Patron', 'Anonymous Patron was not deleted' );
		1686	is( ref Koha::Patrons->find($loser_protected), 'Koha::Patron', 'Protected patron was not deleted' );
1675		1687
1676	$anonymous_patron = Koha::Patrons->find($anonymous_patron);	1688	$anonymous_patron = Koha::Patrons->find($anonymous_patron);
1677	$results = $anonymous_patron->merge_with( [ $keeper->id ] );	1689	$results = $anonymous_patron->merge_with( [ $keeper->id ] );





use Modern::Perl;

use Test::More tests => 52;
use Test::MockModule;
use Test::Exception;


$patstodel = GetBorrowersToExpunge( {category_code => 'SMALLSTAFF' } );
is( scalar @$patstodel, 0, 'Regular patron with flags>0 can not be deleted' );

# Test GetBorrowersToExpunge and patrons with "protected" status (borrowers.protected = 1)
$builder->build({
        source => 'Category',
        value => {
            categorycode         => 'PROTECTED',
            description          => 'Protected',
            category_type        => 'A',
        },
});
$borrowernumber = Koha::Patron->new({
    categorycode => 'PROTECTED',
    branchcode => $library2->{branchcode},
})->store->borrowernumber;
$patron = Koha::Patrons->find( $borrowernumber );
$patstodel = GetBorrowersToExpunge( {category_code => 'PROTECTED' } );
is( scalar @$patstodel, 1, 'Patron with default protected status can be deleted' );
$patron->set({ protected => 1 })->store;
$patstodel = GetBorrowersToExpunge( {category_code => 'PROTECTED' } );
is( scalar @$patstodel, 0, 'Patron with protected status set can not be deleted' );

# Regression tests for BZ13502
## Remove all entries with userid='' (should be only 1 max)
$dbh->do(q|DELETE FROM borrowers WHERE userid = ''|);

Lines 579-587 sub _gen_default_values { Link Here

(-)a/t/lib/TestBuilder.pm (-2 / +1 lines)
579	debarred => undef,	579	debarred => undef,
580	borrowernotes => '',	580	borrowernotes => '',
581	secret => undef,	581	secret => undef,
582
583	password_expiration_date => undef,	582	password_expiration_date => undef,
584	anonymized => 0,	583	anonymized => 0,
		584	protected => 0,
585	},	585	},
586	Item => {	586	Item => {
587	notforloan => 0,	587	notforloan => 0,
588	-

Return to bug 26170

Lines 1027-1032 legend:hover { Link Here

(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt (+23 lines)
1027	</li>	1027	</li>
1028	[% END %]	1028	[% END %]
1029		1029
		1030	<li class="radio">
		1031	<label for="protected">Protected:</label>
		1032	[% IF ( patron.protected == 1 ) %]
		1033	<label for="yes-protected">
		1034	Yes
		1035	<input type="radio" id="yes-protected" name="protected" value="1" checked="checked" />
		1036	</label>
		1037	<label for="no-protected">
		1038	No
		1039	<input type="radio" id="no-protected" name="protected" value="0" />
		1040	</label>
		1041	[% ELSE %]
		1042	<label for="yes-protected">
		1043	Yes
		1044	<input type="radio" id="yes-protected" name="protected" value="1" />
		1045	</label>
		1046	<label for="no-protected">
		1047	No
		1048	<input type="radio" id="no-protected" name="protected" value="0" checked="checked" />
		1049	</label>
		1050	[% END %]
		1051	</li>
		1052
1030	[% IF ( Koha.Preference('CheckPrevCheckout') == 'softyes' \|\| Koha.Preference('CheckPrevCheckout') == 'softno' ) %]	1053	[% IF ( Koha.Preference('CheckPrevCheckout') == 'softyes' \|\| Koha.Preference('CheckPrevCheckout') == 'softno' ) %]
1031	<li>	1054	<li>
1032	<label for="checkprevcheckout">Check for previous checkouts: </label>	1055	<label for="checkprevcheckout">Check for previous checkouts: </label>

Lines 1483-1489 subtest 'password expiration tests' => sub { Link Here

(-)a/t/db_dependent/Koha/Patron.t (-1 / +9 lines)
1483		1483
1484	subtest 'safe_to_delete() tests' => sub {	1484	subtest 'safe_to_delete() tests' => sub {
1485		1485
1486	plan tests => 14;	1486	plan tests => 17;
1487		1487
1488	$schema->storage->txn_begin;	1488	$schema->storage->txn_begin;
1489		1489
Lines 1539-1544 subtest 'safe_to_delete() tests' => sub { Link Here
1539	t::lib::Mocks::mock_userenv( { borrowernumber => $manager->id } );	1539	t::lib::Mocks::mock_userenv( { borrowernumber => $manager->id } );
1540	$patron->account->pay({ amount => 10, debits => [ $debit ] });	1540	$patron->account->pay({ amount => 10, debits => [ $debit ] });
1541		1541
		1542	## Make it protected
		1543	$patron->protected( 1 );
		1544	ok( !$patron->safe_to_delete, 'Cannot delete, is protected' );
		1545	$message = $patron->safe_to_delete->messages->[0];
		1546	is( $message->type, 'error', 'Type is error' );
		1547	is( $message->message, 'is_protected', 'Cannot delete, is protected' );
		1548	$patron->protected( 0 );
		1549
1542	## Happy case :-D	1550	## Happy case :-D
1543	ok( $patron->safe_to_delete, 'Can delete, all conditions met' );	1551	ok( $patron->safe_to_delete, 'Can delete, all conditions met' );
1544	my $messages = $patron->safe_to_delete->messages;	1552	my $messages = $patron->safe_to_delete->messages;

Lines 17-23 Link Here

(-)a/t/db_dependent/Members.t (-1 / +21 lines)
17		17
18	use Modern::Perl;	18	use Modern::Perl;
19		19
20	use Test::More tests => 50;	20	use Test::More tests => 52;
21	use Test::MockModule;	21	use Test::MockModule;
22	use Test::Exception;	22	use Test::Exception;
23		23
Lines 349-354 $patron->set({ flags => 4 })->store; Link Here
349	$patstodel = GetBorrowersToExpunge( {category_code => 'SMALLSTAFF' } );	349	$patstodel = GetBorrowersToExpunge( {category_code => 'SMALLSTAFF' } );
350	is( scalar @$patstodel, 0, 'Regular patron with flags>0 can not be deleted' );	350	is( scalar @$patstodel, 0, 'Regular patron with flags>0 can not be deleted' );
351		351
		352	# Test GetBorrowersToExpunge and patrons with "protected" status (borrowers.protected = 1)
		353	$builder->build({
		354	source => 'Category',
		355	value => {
		356	categorycode => 'PROTECTED',
		357	description => 'Protected',
		358	category_type => 'A',
		359	},
		360	});
		361	$borrowernumber = Koha::Patron->new({
		362	categorycode => 'PROTECTED',
		363	branchcode => $library2->{branchcode},
		364	})->store->borrowernumber;
		365	$patron = Koha::Patrons->find( $borrowernumber );
		366	$patstodel = GetBorrowersToExpunge( {category_code => 'PROTECTED' } );
		367	is( scalar @$patstodel, 1, 'Patron with default protected status can be deleted' );
		368	$patron->set({ protected => 1 })->store;
		369	$patstodel = GetBorrowersToExpunge( {category_code => 'PROTECTED' } );
		370	is( scalar @$patstodel, 0, 'Patron with protected status set can not be deleted' );
		371
352	# Regression tests for BZ13502	372	# Regression tests for BZ13502
353	## Remove all entries with userid='' (should be only 1 max)	373	## Remove all entries with userid='' (should be only 1 max)
354	$dbh->do(q\|DELETE FROM borrowers WHERE userid = ''\|);	374	$dbh->do(q\|DELETE FROM borrowers WHERE userid = ''\|);