From 04cf146ff41d23aa9a53c4306956da15dd615f61 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Wed, 8 Jun 2022 13:40:00 +0000 Subject: [PATCH] Bug 11889: (follow-up) Tests and new get_shared_shelves Content-Type: text/plain; charset=utf-8 This is a manual reconstruction of an older patch, that git did not want to apply anymore (sha trouble etc.) --- Koha/Patron.pm | 11 ++++-- Koha/Virtualshelves.pm | 13 +++++++ t/db_dependent/Koha/Patrons.t | 68 +++++++++++++++++++++++++++++---- t/db_dependent/Virtualshelves.t | 16 ++++++-- 4 files changed, 94 insertions(+), 14 deletions(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index a642b0dfaf..f88ce98dc3 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -388,18 +388,23 @@ sub delete { $hold->cancel; } + # FIXME Could be $patron->get_lists # If ListOwnershipUponPatronDeletion = transfer, change ownership of all # public and shared lists to the user who deleted them. if ( C4::Context->preference('ListOwnershipUponPatronDeletion') eq 'transfer' ) { my $userenv = C4::Context->userenv(); my $usernumber = (ref($userenv) eq 'HASH') ? $userenv->{'number'} : 0; my @publiclists = Koha::Virtualshelves->get_public_shelves; - my @sharedlists = Koha::Virtualshelves->search({ 'me.owner' => $self->borrowernumber, 'me.shelfnumber' => { -ident => 'virtualshelfshares.shelfnumber' } }, { prefetch => 'virtualshelfshares' }); + my @sharedlists = Koha::Virtualshelves->get_shared_shelves({ borrowernumber => $self->borrowernumber }); foreach my $plist ( @publiclists ) { - $plist->set({ owner => $usernumber })->store; + if ( $plist->owner == $self->borrowernumber ) { + my $unique_name = $plist->shelfname . '_' . $self->borrowernumber; + $plist->set({ owner => $usernumber, shelfname => $unique_name })->store; + } } foreach my $slist ( @sharedlists ) { - $slist->set({ owner => $usernumber })->store; + my $unique_name = $slist->shelfname . '_' . $self->borrowernumber; + $slist->set({ owner => $usernumber, shelfname => $unique_name })->store; # if staff member had a share, remove it $slist->remove_share( $usernumber ); } diff --git a/Koha/Virtualshelves.pm b/Koha/Virtualshelves.pm index 981e8e9506..57b085e193 100644 --- a/Koha/Virtualshelves.pm +++ b/Koha/Virtualshelves.pm @@ -180,6 +180,19 @@ sub get_shelves_containing_record { ); } +sub get_shared_shelves { + my ( $self, $params ) = @_; + my $borrowernumber = $params->{borrowernumber} || 0; + + $self->search( + { + 'me.owner' => $borrowernumber, + 'me.shelfnumber' => { -ident => 'virtualshelfshares.shelfnumber' } + }, + { prefetch => 'virtualshelfshares' } + ); +} + sub _type { return 'Virtualshelve'; } diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index c8b4d4f3d6..a5fcad2430 100755 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -42,6 +42,7 @@ use Koha::Patron::Categories; use Koha::Patron::Relationship; use Koha::Database; use Koha::DateUtils qw( dt_from_string output_pref ); +use Koha::Virtualshelf; use Koha::Virtualshelves; use Koha::Notice::Messages; @@ -459,22 +460,40 @@ subtest "move_to_deleted" => sub { }; subtest "delete" => sub { - plan tests => 7; + plan tests => 11; t::lib::Mocks::mock_preference( 'BorrowersLog', 1 ); + t::lib::Mocks::mock_preference( 'ListOwnershipUponPatronDeletion', 'transfer' ); + Koha::Virtualshelves->search({})->delete; + my $userenv = C4::Context->userenv(); my $patron = $builder->build( { source => 'Borrower' } ); + my $patron_for_sharing = (ref($userenv) eq 'HASH' ) ? $userenv->{'number'} : 0; my $retrieved_patron = Koha::Patrons->find( $patron->{borrowernumber} ); my $hold = $builder->build( { source => 'Reserve', - value => { borrowernumber => $patron->{borrowernumber} } - } - ); - my $list = $builder->build( - { source => 'Virtualshelve', - value => { owner => $patron->{borrowernumber} } + value => { borrowernumber => $patron->{borrowernumber} } } ); my $modification = $builder->build_object({ class => 'Koha::Patron::Modifications', value => { borrowernumber => $patron->{borrowernumber} } }); + my $private_list = Koha::Virtualshelf->new({ + shelfname => "private", + owner => $patron->{borrowernumber}, + category => 1 + } + )->store; + my $public_list = Koha::Virtualshelf->new({ + shelfname => "public", + owner => $patron->{borrowernumber}, + category => 2 + } + )->store; + my $list_to_share = Koha::Virtualshelf->new({ + shelfname => "shared", + owner => $patron->{borrowernumber}, + category => 1 + } + )->store; + my $shared_shelf = eval { $list_to_share->share("valid key")->accept("valid key", $patron_for_sharing) }; my $deleted = $retrieved_patron->delete; is( ref($deleted), 'Koha::Patron', 'Koha::Patron->delete should return the deleted patron object if the patron has been correctly deleted' ); @@ -484,12 +503,45 @@ subtest "delete" => sub { is( Koha::Holds->search( { borrowernumber => $patron->{borrowernumber} } )->count, 0, q|Koha::Patron->delete should have cancelled patron's holds 2| ); - is( Koha::Virtualshelves->search( { owner => $patron->{borrowernumber} } )->count, 0, q|Koha::Patron->delete should have deleted patron's lists| ); + my $transferred_lists = Koha::Virtualshelves->search({ owner => $patron_for_sharing })->count; + is( $transferred_lists, 2, 'Public and shared lists should stay in database under a different owner with a unique name, while private lists delete, with ListOwnershipPatronDeletion set to Transfer'); + is( Koha::Virtualshelfshares->search({ borrowernumber => $patron_for_sharing })->count, 0, "New owner of list should have shares removed" ); + is( Koha::Virtualshelves->search({ owner => $patron->{borrowernumber} })->count, 0, q|Koha::Patron->delete should have deleted patron's lists/removed their ownership| ); is( Koha::Patron::Modifications->search( { borrowernumber => $patron->{borrowernumber} } )->count, 0, q|Koha::Patron->delete should have deleted patron's modifications| ); my $number_of_logs = $schema->resultset('ActionLog')->search( { module => 'MEMBERS', action => 'DELETE', object => $retrieved_patron->borrowernumber } )->count; is( $number_of_logs, 1, 'With BorrowerLogs, Koha::Patron->delete should have logged' ); + + t::lib::Mocks::mock_preference( 'ListOwnershipUponPatronDeletion', 'delete' ); + Koha::Virtualshelves->search({})->delete; + my $patron2 = $builder->build( { source => 'Borrower' } ); + my $retrieved_patron2 = Koha::Patrons->find( $patron2->{borrowernumber} ); + my $private_list2 = Koha::Virtualshelf->new({ + shelfname => "private", + owner => $patron2->{borrowernumber}, + category => 1 + } + )->store; + my $public_list2 = Koha::Virtualshelf->new({ + shelfname => "public", + owner => $patron2->{borrowernumber}, + category => 2 + } + )->store; + my $list_to_share2 = Koha::Virtualshelf->new({ + shelfname => "shared", + owner => $patron2->{borrowernumber}, + category => 1 + } + )->store; + + my $shared_shelf2 = eval { $list_to_share2->share("valid key") }; + my $deleted2 = $retrieved_patron2->delete; + + is( Koha::Virtualshelves->search( { owner => $patron2->{borrowernumber} } )->count, 0, q|Koha::Patron->delete should have deleted patron's lists| ); + $transferred_lists = Koha::Virtualshelves->search({})->count; + is( $transferred_lists, 0, 'All lists should be deleted with ListOwnershipUponPatronDeletion set to Delete'); }; subtest 'Koha::Patrons->delete' => sub { diff --git a/t/db_dependent/Virtualshelves.t b/t/db_dependent/Virtualshelves.t index e9444f06cd..6c71456ad4 100755 --- a/t/db_dependent/Virtualshelves.t +++ b/t/db_dependent/Virtualshelves.t @@ -439,7 +439,7 @@ subtest 'Shelf permissions' => sub { }; subtest 'Get shelves' => sub { - plan tests => 4; + plan tests => 5; my $patron1 = $builder->build({ source => 'Borrower', }); @@ -477,19 +477,29 @@ subtest 'Get shelves' => sub { public => 1, } )->store; + my $shelf_to_share = Koha::Virtualshelf->new({ + shelfname => "shared shelf", + owner => $patron1->{borrowernumber}, + category => 1, + } + )->store; my $private_shelves = Koha::Virtualshelves->get_private_shelves; is( $private_shelves->count, 0, 'Without borrowernumber given, get_private_shelves should not return any shelf' ); $private_shelves = Koha::Virtualshelves->get_private_shelves({ borrowernumber => $patron1->{borrowernumber} }); - is( $private_shelves->count, 2, 'get_private_shelves should return all shelves for a given patron' ); + is( $private_shelves->count, 3, 'get_private_shelves should return all shelves for a given patron' ); $private_shelf2_1->share('a key')->accept('a key', $patron1->{borrowernumber}); $private_shelves = Koha::Virtualshelves->get_private_shelves({ borrowernumber => $patron1->{borrowernumber} }); - is( $private_shelves->count, 3, 'get_private_shelves should return all shelves for a given patron, even the shared ones' ); + is( $private_shelves->count, 4, 'get_private_shelves should return all shelves for a given patron, even the shared ones' ); my $public_shelves = Koha::Virtualshelves->get_public_shelves; is( $public_shelves->count, 2, 'get_public_shelves should return all public shelves, no matter who is the owner' ); + my $shared_shelf = eval { $shelf_to_share->share("valid key") }; + my $shared_shelves = Koha::Virtualshelves->get_shared_shelves({ borrowernumber => $patron1->{borrowernumber} }); + is( $shared_shelves->count, 1, 'get_shared_shelves should return shared shelves' ); + teardown(); }; -- 2.20.1