Bugzilla – Attachment 61454 Details for
Bug 11889
Option to keep public or shared lists when deleting patron
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 11889: [FOLLOW-UP] Tests and new get_shared_shelves subroutine
Bug-11889-FOLLOW-UP-Tests-and-new-getsharedshelves.patch (text/plain), 11.02 KB, created by
Biblibre Sandboxes
on 2017-03-22 14:05:38 UTC
(
hide
)
Description:
Bug 11889: [FOLLOW-UP] Tests and new get_shared_shelves subroutine
Filename:
MIME Type:
Creator:
Biblibre Sandboxes
Created:
2017-03-22 14:05:38 UTC
Size:
11.02 KB
patch
obsolete
>From 9937ed3aa7de32457dd72c88205bc36296820349 Mon Sep 17 00:00:00 2001 >From: Aleisha Amohia <aleishaamohia@hotmail.com> >Date: Mon, 20 Feb 2017 21:51:39 +0000 >Subject: [PATCH] Bug 11889: [FOLLOW-UP] Tests and new get_shared_shelves > subroutine >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >To test: >1) prove t/db_dependent/Virtualshelves.t >2) prove t/db_dependent/Koha/Patrons.t > >Sponsored-by: Catalyst IT > >Signed-off-by: Cédric Vita <cedric.vita@dracenie.com> >Signed-off-by: Cédric Vita <cedric.vita@dracenie.com> >--- > Koha/Patron.pm | 11 ++++-- > Koha/Virtualshelves.pm | 13 ++++++++ > t/db_dependent/Koha/Patrons.t | 74 +++++++++++++++++++++++++++++++++++------ > t/db_dependent/Virtualshelves.t | 16 +++++++-- > 4 files changed, 97 insertions(+), 17 deletions(-) > >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index 6311566..dfafd34 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -68,18 +68,23 @@ sub delete { > # Delete Patron's holds > $self->holds->delete; > >+ # 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 a8d1b20..20ba341 100644 >--- a/Koha/Virtualshelves.pm >+++ b/Koha/Virtualshelves.pm >@@ -158,6 +158,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 24a4a3d..43f95db 100644 >--- a/t/db_dependent/Koha/Patrons.t >+++ b/t/db_dependent/Koha/Patrons.t >@@ -27,12 +27,14 @@ use C4::Biblio; > use C4::Circulation; > use C4::Members; > use C4::Circulation; >+use C4::Context; > > use Koha::Holds; > use Koha::Patron; > use Koha::Patrons; > use Koha::Database; > use Koha::DateUtils; >+use Koha::Virtualshelf; > use Koha::Virtualshelves; > > use t::lib::TestBuilder; >@@ -65,7 +67,7 @@ my $new_patron_2 = Koha::Patron->new( > )->store; > > C4::Context->_new_userenv('xxx'); >-C4::Context->set_userenv(0,0,0,'firstname','surname', $library->{branchcode}, 'Midway Public Library', '', '', ''); >+C4::Context->set_userenv(1,'userid','usercnum','firstname','surname', $library->{branchcode}, 'Midway Public Library', '', '', ''); > > is( Koha::Patrons->search->count, $nb_of_patrons + 2, 'The 2 patrons should have been added' ); > >@@ -337,32 +339,82 @@ subtest "move_to_deleted" => sub { > }; > > subtest "delete" => sub { >- plan tests => 5; >+ plan tests => 9; > 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} } >+ value => { borrowernumber => $patron->{borrowernumber} } > } > ); >- my $list = $builder->build( >- { source => 'Virtualshelve', >- value => { owner => $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( $deleted, 1, 'Koha::Patron->delete should return 1 if the patron has been correctly deleted' ); > >+ is( $deleted, 1, 'Koha::Patron->delete should return 1 if the patron has been correctly deleted' ); > is( Koha::Patrons->find( $patron->{borrowernumber} ), undef, 'Koha::Patron->delete should have deleted the patron' ); >- > is( Koha::Holds->search( { borrowernumber => $patron->{borrowernumber} } )->count, 0, q|Koha::Patron->delete should have deleted patron's holds| ); > >- 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| ); > >- my $number_of_logs = $schema->resultset('ActionLog')->search( { module => 'MEMBERS', action => 'DELETE', object => $retrieved_patron->borrowernumber } )->count; >+ 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' ); >+ >+ 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 'add_enrolment_fee_if_needed' => sub { >diff --git a/t/db_dependent/Virtualshelves.t b/t/db_dependent/Virtualshelves.t >index c867056..7352dac 100644 >--- a/t/db_dependent/Virtualshelves.t >+++ b/t/db_dependent/Virtualshelves.t >@@ -350,7 +350,7 @@ subtest 'Shelf permissions' => sub { > }; > > subtest 'Get shelves' => sub { >- plan tests => 4; >+ plan tests => 5; > my $patron1 = $builder->build({ > source => 'Borrower', > }); >@@ -388,19 +388,29 @@ subtest 'Get shelves' => sub { > category => 2, > } > )->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.7.4
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=61454">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
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