From b1d295bdb4cfa903ea7a852c9094902752fef9eb Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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 <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
---
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.30.2