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