Bug 11889: (follow-up) Get rid of FIXME in Koha::Patron
Instead of get_lists, I added ->virtualshelves. I removed the addition of the suffix to the shelf name here; if we really want that (doubts), we should do it pref based. Removing a staff member with public lists would result in weird names with shelf numbers on the OPAC. (Should be very easy to restore.) Used this opportunity to refactor the patron->delete subtest a bit: switching to build_object, removing the 'retrieved patrons', differentiating between staff patron and patron for sharing, adding a few tests. Test plan: Enable transfer ownership. Delete a patron with lists in interface. Check what happened. Run t/db_dependent/Koha/Patrons.t. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This commit is contained in:
parent
1d1432cfb5
commit
aff32dd6dd
2 changed files with 73 additions and 82 deletions
|
@ -387,32 +387,22 @@ 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->as_list;
|
||||
my @sharedlists = Koha::Virtualshelves->get_shared_shelves({ borrowernumber => $self->borrowernumber })->as_list;
|
||||
foreach my $plist ( @publiclists ) {
|
||||
if ( $plist->owner == $self->borrowernumber ) {
|
||||
my $unique_name = $plist->shelfname . '_' . $self->borrowernumber;
|
||||
$plist->set({ owner => $usernumber, shelfname => $unique_name })->store;
|
||||
}
|
||||
}
|
||||
foreach my $slist ( @sharedlists ) {
|
||||
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 );
|
||||
# Handle lists (virtualshelves)
|
||||
my $userenv = C4::Context->userenv;
|
||||
my $usernumber = ref($userenv) eq 'HASH' ? $userenv->{'number'} : 0;
|
||||
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
|
||||
} else { # delete
|
||||
$list->delete;
|
||||
}
|
||||
}
|
||||
|
||||
# Delete any remaining lists that this user is an owner of (always private lists,
|
||||
# only public and shared lists if ListOwnershipUponPatronDeletion = delete)
|
||||
$_->delete for Koha::Virtualshelves->search({ owner => $self->borrowernumber })->as_list;
|
||||
|
||||
# We cannot have a FK on borrower_modifications.borrowernumber, the table is also used
|
||||
# for patron selfreg
|
||||
$_->delete for Koha::Patron::Modifications->search( { borrowernumber => $self->borrowernumber } )->as_list;
|
||||
|
@ -2194,6 +2184,17 @@ sub decoded_secret {
|
|||
return $self->secret;
|
||||
}
|
||||
|
||||
=head3 virtualshelves
|
||||
|
||||
my $shelves = $patron->virtualshelves;
|
||||
|
||||
=cut
|
||||
|
||||
sub virtualshelves {
|
||||
my $self = shift;
|
||||
return Koha::Virtualshelves->_new_from_dbic( scalar $self->_result->virtualshelves );
|
||||
}
|
||||
|
||||
=head2 Internal methods
|
||||
|
||||
=head3 _type
|
||||
|
|
|
@ -460,91 +460,81 @@ subtest "move_to_deleted" => sub {
|
|||
};
|
||||
|
||||
subtest "delete" => sub {
|
||||
plan tests => 11;
|
||||
plan tests => 13;
|
||||
t::lib::Mocks::mock_preference( 'BorrowersLog', 1 );
|
||||
t::lib::Mocks::mock_preference( 'ListOwnershipUponPatronDeletion', 'transfer' );
|
||||
Koha::Virtualshelves->delete;
|
||||
|
||||
my $patron = $builder->build_object({ class => 'Koha::Patrons' });
|
||||
my $patron_for_sharing = $builder->build_object({ class => 'Koha::Patrons' });
|
||||
my $staff_patron = $builder->build_object({ class => 'Koha::Patrons' });
|
||||
t::lib::Mocks::mock_userenv({ patron => $staff_patron });
|
||||
|
||||
my $patron = $builder->build( { source => 'Borrower' } );
|
||||
my $patron_for_sharing = $staff_patron->borrowernumber;
|
||||
my $retrieved_patron = Koha::Patrons->find( $patron->{borrowernumber} );
|
||||
my $hold = $builder->build(
|
||||
{ source => 'Reserve',
|
||||
value => { borrowernumber => $patron->{borrowernumber} }
|
||||
}
|
||||
);
|
||||
my $modification = $builder->build_object({ class => 'Koha::Patron::Modifications', value => { borrowernumber => $patron->{borrowernumber} } });
|
||||
my $hold = $builder->build_object({ class => 'Koha::Holds', 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},
|
||||
public => 0,
|
||||
}
|
||||
)->store;
|
||||
shelfname => "private",
|
||||
owner => $patron->borrowernumber,
|
||||
public => 0,
|
||||
})->store;
|
||||
my $public_list = Koha::Virtualshelf->new({
|
||||
shelfname => "public",
|
||||
owner => $patron->{borrowernumber},
|
||||
public => 1,
|
||||
}
|
||||
)->store;
|
||||
shelfname => "public",
|
||||
owner => $patron->borrowernumber,
|
||||
public => 1,
|
||||
})->store;
|
||||
my $list_to_share = Koha::Virtualshelf->new({
|
||||
shelfname => "shared",
|
||||
owner => $patron->{borrowernumber},
|
||||
public => 0,
|
||||
}
|
||||
)->store;
|
||||
shelfname => "shared",
|
||||
owner => $patron->borrowernumber,
|
||||
public => 0,
|
||||
})->store;
|
||||
|
||||
my $shared_shelf = eval { $list_to_share->share("valid key")->accept("valid key", $patron_for_sharing) };
|
||||
my $deleted = $retrieved_patron->delete;
|
||||
$list_to_share->share("valid key")->accept( "valid key", $patron_for_sharing->borrowernumber );
|
||||
$list_to_share->share("valid key")->accept( "valid key", $staff_patron->borrowernumber ); # this share should be removed at deletion too
|
||||
my $deleted = $patron->delete;
|
||||
is( ref($deleted), 'Koha::Patron', 'Koha::Patron->delete should return the deleted patron object if the patron has been correctly deleted' );
|
||||
ok( $patron->borrowernumber, 'Still have the deleted borrowernumber' );
|
||||
|
||||
is( Koha::Patrons->find( $patron->{borrowernumber} ), undef, 'Koha::Patron->delete should have deleted the patron' );
|
||||
is( Koha::Patrons->find( $patron->borrowernumber ), undef, 'Koha::Patron->delete should have deleted the patron' );
|
||||
|
||||
is (Koha::Old::Holds->search( { reserve_id => $hold->{ reserve_id } } )->count, 1, q|Koha::Patron->delete should have cancelled patron's holds| );
|
||||
is (Koha::Old::Holds->search({ reserve_id => $hold->reserve_id })->count, 1, q|Koha::Patron->delete should have cancelled patron's holds| );
|
||||
|
||||
is( Koha::Holds->search( { borrowernumber => $patron->{borrowernumber} } )->count, 0, q|Koha::Patron->delete should have cancelled patron's holds 2| );
|
||||
is( Koha::Holds->search( { borrowernumber => $patron->borrowernumber } )->count, 0, q|Koha::Patron->delete should have cancelled patron's holds 2| );
|
||||
|
||||
my $transferred_lists = Koha::Virtualshelves->search({ owner => $patron_for_sharing })->count;
|
||||
my $transferred_lists = Koha::Virtualshelves->search({ owner => $staff_patron->borrowernumber })->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::Virtualshelfshares->search({ borrowernumber => $staff_patron->borrowernumber })->count, 0, "New owner of list should have shares removed" );
|
||||
is( Koha::Virtualshelfshares->search({ borrowernumber => $patron_for_sharing->borrowernumber })->count, 1, "But the other share is still there" );
|
||||
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| );
|
||||
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;
|
||||
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} );
|
||||
Koha::Virtualshelves->delete;
|
||||
my $patron2 = $builder->build_object({ class => 'Koha::Patrons' });
|
||||
my $private_list2 = Koha::Virtualshelf->new({
|
||||
shelfname => "private",
|
||||
owner => $patron2->{borrowernumber},
|
||||
public => 0,
|
||||
}
|
||||
)->store;
|
||||
shelfname => "private",
|
||||
owner => $patron2->borrowernumber,
|
||||
public => 0,
|
||||
})->store;
|
||||
my $public_list2 = Koha::Virtualshelf->new({
|
||||
shelfname => "public",
|
||||
owner => $patron2->{borrowernumber},
|
||||
public => 1,
|
||||
}
|
||||
)->store;
|
||||
shelfname => "public",
|
||||
owner => $patron2->borrowernumber,
|
||||
public => 1,
|
||||
})->store;
|
||||
my $list_to_share2 = Koha::Virtualshelf->new({
|
||||
shelfname => "shared",
|
||||
owner => $patron2->{borrowernumber},
|
||||
public => 0,
|
||||
}
|
||||
)->store;
|
||||
shelfname => "shared",
|
||||
owner => $patron2->borrowernumber,
|
||||
public => 0,
|
||||
})->store;
|
||||
$list_to_share2->share("valid key")->accept( "valid key", $patron_for_sharing->borrowernumber );
|
||||
|
||||
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');
|
||||
# Delete patron2, check if shelves and shares are now empty
|
||||
$patron2->delete;
|
||||
is( Koha::Virtualshelves->count, 0, 'All lists should be gone now' );
|
||||
is( Koha::Virtualshelfshares->count, 0, 'All shares should be gone too' );
|
||||
};
|
||||
|
||||
subtest 'Koha::Patrons->delete' => sub {
|
||||
|
|
Loading…
Reference in a new issue