From d5ba2b1c4f48f16bf37974deb4cd3e81c70d4d29 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Mon, 20 Jun 2022 10:03:59 +0000 Subject: [PATCH] Bug 30933: Change for patron->delete Test plan: Run t/db_dependent/Koha/Patrons.t Set pref ListOwnershipUponPatronDeletion to transfer. Set pref ListOwnerDesignated to some valid borrowernumber. Pick a user with public or shared list, delete from interface. Pick a user with public or shared list, delete by script. (*) Verify in both cases that new list owner is the designated one. (*) Tip: Create another branch. Move the patron to be deleted to that branch. And delete by script with: misc/cronjobs/delete_patons.pl -c -v -library YOUR_CODE Signed-off-by: Marcel de Rooy Signed-off-by: David Nind Signed-off-by: Martin Renvoize Signed-off-by: Tomas Cohen Arazi --- Koha/Patron.pm | 22 ++++++++++++++-------- t/db_dependent/Koha/Patrons.t | 17 ++++++++++++++++- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 6e1520f628..4dface4de1 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -388,16 +388,13 @@ sub delete { } # Handle lists (virtualshelves) - my $userenv = C4::Context->userenv; - my $usernumber = ref($userenv) eq 'HASH' ? $userenv->{'number'} : 0; + my $new_owner = _new_owner(); my $lists = $self->virtualshelves; while( my $list = $lists->next ) { - # if staff member had a share, remove it - $list->remove_share( $usernumber ) if $usernumber && $list->is_private; - if( C4::Context->preference('ListOwnershipUponPatronDeletion') eq 'transfer' && - $usernumber && ( $list->is_public || $list->is_shared )) - { - $list->set({ owner => $usernumber })->store; # transfer ownership of public/shared list + if( $new_owner && ( $list->is_public || $list->is_shared )) { + # if new_owner had a share, remove it + $list->remove_share( $new_owner ) if $list->is_private; + $list->set({ owner => $new_owner })->store; # transfer ownership of public/shared list } else { # delete $list->delete; } @@ -415,6 +412,15 @@ sub delete { return $self; } +sub _new_owner { + if( C4::Context->preference('ListOwnershipUponPatronDeletion') eq 'transfer' ) { + # designated owner overrides userenv + my $designated_owner = C4::Context->preference('ListOwnerDesignated'); + return $designated_owner if Koha::Patrons->find($designated_owner); + my $userenv = C4::Context->userenv; + return $userenv->{'number'} if $userenv; + } +} =head3 category diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index 60ea9e6a53..c8f8ce32af 100755 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -460,9 +460,10 @@ subtest "move_to_deleted" => sub { }; subtest "delete" => sub { - plan tests => 13; + plan tests => 16; t::lib::Mocks::mock_preference( 'BorrowersLog', 1 ); t::lib::Mocks::mock_preference( 'ListOwnershipUponPatronDeletion', 'transfer' ); + t::lib::Mocks::mock_preference( 'ListOwnerDesignated', undef ); Koha::Virtualshelves->delete; my $patron = $builder->build_object({ class => 'Koha::Patrons' }); @@ -511,6 +512,20 @@ subtest "delete" => sub { my $number_of_logs = $schema->resultset('ActionLog')->search( { module => 'MEMBERS', action => 'DELETE', object => $patron->borrowernumber } )->count; is( $number_of_logs, 1, 'With BorrowerLogs, Koha::Patron->delete should have logged' ); + # Test deletion with designated fallback owner + my $designated_owner = $builder->build_object({ class => 'Koha::Patrons' }); + t::lib::Mocks::mock_preference( 'ListOwnerDesignated', $designated_owner->id ); + $patron = $builder->build_object({ class => 'Koha::Patrons' }); + $private_list = Koha::Virtualshelf->new({ shelfname => "PR1", owner => $patron->id })->store; + $public_list = Koha::Virtualshelf->new({ shelfname => "PU1", public => 1, owner => $patron->id })->store; + $list_to_share = Koha::Virtualshelf->new({ shelfname => "SH1", owner => $patron->id })->store; + $list_to_share->share("valid key")->accept( "valid key", $patron_for_sharing->id ); + $patron->delete; + is( Koha::Virtualshelves->find( $private_list->id ), undef, 'Private list gone' ); + is( $public_list->discard_changes->get_column('owner'), $designated_owner->id, 'Public list transferred' ); + is( $list_to_share->discard_changes->get_column('owner'), $designated_owner->id, 'Shared list transferred' ); + + # Finally test deleting lists t::lib::Mocks::mock_preference( 'ListOwnershipUponPatronDeletion', 'delete' ); Koha::Virtualshelves->delete; my $patron2 = $builder->build_object({ class => 'Koha::Patrons' }); -- 2.39.5