Attachment 135813 Details for Bug 11889 – Bug 11889: (follow-up) Tests and new get_shared_shelves

[patch] Bug 11889: (follow-up) Tests and new get_shared_shelves

Bug-11889-follow-up-Tests-and-new-getsharedshelves.patch (text/plain), 10.37 KB, created by Marcel de Rooy on 2022-06-08 13:49:42 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2022-06-08 13:49:42 UTC

Size: 10.37 KB

patch

obsolete

>From 04cf146ff41d23aa9a53c4306956da15dd615f61 Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>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

Actions: View | Diff | Splinter Review

Attachments on bug 11889: 25972 | 60224 | 60306 | 60307 | 60308 | 60481 | 61319 | 61320 | 61321 | 61322 | 61451 | 61452 | 61453 | 61454 | 108244 | 108245 | 108246 | 108247 | 135808 | 135809 | 135810 | 135811 | 135812 | 135813 | 135814 | 135815 | 135816 | 135817 | 135818 | 135819 | 135820 | 135821 | 135857 | 135858 | 135859 | 135860 | 135861 | 135862 | 135863 | 135864 | 135865 | 135866 | 135867 | 135868 | 135869 | 135870 | 135871 | 135872 | 135873 | 135874 | 135875 | 135876 | 135891 | 135892 | 135893 | 135894 | 135895 | 135896 | 135897 | 135898 | 135899 | 135900 | 135901