From ddd441418ead4261bbd627e907bd011af63e98e6 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 13 Dec 2019 16:01:46 +0100 Subject: [PATCH] Bug 21684: Adjust some tests --- Koha/Patron.pm | 5 ++--- Koha/REST/V1/Patrons.pm | 2 +- t/db_dependent/Koha/Patrons.t | 24 ++++++++---------------- t/db_dependent/Virtualshelves.t | 4 ++-- 4 files changed, 13 insertions(+), 22 deletions(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 2de2965e7f..27e552a48c 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -357,7 +357,6 @@ other lists are kept. sub delete { my ($self) = @_; - my $deleted; $self->_result->result_source->schema->txn_do( sub { # Cancel Patron's holds @@ -382,12 +381,12 @@ sub delete { # FIXME Could be $patron->get_lists $_->delete for Koha::Virtualshelves->search( { owner => $self->borrowernumber } ); - $deleted = $self->SUPER::delete; + $self->SUPER::delete; logaction( "MEMBERS", "DELETE", $self->borrowernumber, "" ) if C4::Context->preference("BorrowersLog"); } ); - return $deleted; + return $self; } diff --git a/Koha/REST/V1/Patrons.pm b/Koha/REST/V1/Patrons.pm index fb9e51f67a..dfd5cb3486 100644 --- a/Koha/REST/V1/Patrons.pm +++ b/Koha/REST/V1/Patrons.pm @@ -276,7 +276,7 @@ sub delete { $patron = Koha::Patrons->find( $c->validation->param('patron_id') ); # check if loans, reservations, debarrment, etc. before deletion! - my $res = $patron->delete; + $patron->delete; return $c->render( status => 200, openapi => {} ); } catch { diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index 4d7063c497..66af494ff8 100644 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -36,6 +36,7 @@ use Koha::ActionLogs; use Koha::Holds; use Koha::Old::Holds; use Koha::Patrons; +use Koha::Old::Patrons; use Koha::Patron::Categories; use Koha::Patron::Relationship; use Koha::Database; @@ -479,7 +480,7 @@ subtest "delete" => sub { ); my $deleted = $retrieved_patron->delete; - is( $deleted, 1, 'Koha::Patron->delete should return 1 if the patron has been correctly deleted' ); + is( ref($deleted), 'Koha::Patron', 'Koha::Patron->delete should return the deleted patron object if the patron has been correctly deleted' ); is( Koha::Patrons->find( $patron->{borrowernumber} ), undef, 'Koha::Patron->delete should have deleted the patron' ); @@ -494,27 +495,18 @@ subtest "delete" => sub { }; subtest 'Koha::Patrons->delete' => sub { - plan tests => 4; - - my $mod_patron = Test::MockModule->new( 'Koha::Patron' ); - my $moved_to_deleted = 0; - $mod_patron->mock( 'move_to_deleted', sub { $moved_to_deleted++; } ); + plan tests => 3; my $patron1 = $builder->build_object({ class => 'Koha::Patrons' }); my $patron2 = $builder->build_object({ class => 'Koha::Patrons' }); my $id1 = $patron1->borrowernumber; - my $set = Koha::Patrons->search({ borrowernumber => { '>=' => $id1 }}); + my $set = Koha::Patrons->search({ borrowernumber => { -in => [$patron1->borrowernumber, $patron2->borrowernumber]}}); is( $set->count, 2, 'Two patrons found as expected' ); is( $set->delete({ move => 1 }), 2, 'Two patrons deleted' ); - is( $moved_to_deleted, 2, 'Patrons moved to deletedborrowers' ); - - # Add again, test if we can raise an exception - $mod_patron->mock( 'delete', sub { return -1; } ); - $patron1 = $builder->build_object({ class => 'Koha::Patrons' }); - $id1 = $patron1->borrowernumber; - $set = Koha::Patrons->search({ borrowernumber => { '>=' => $id1 }}); - throws_ok { $set->delete } 'Koha::Exceptions::Patron::FailedDelete', - 'Exception raised for deleting patron'; + my $deleted_patrons = Koha::Old::Patrons->search({ borrowernumber => { -in => [$patron1->borrowernumber, $patron2->borrowernumber]}}); + is( $deleted_patrons->count, 2, 'Patrons moved to deletedborrowers' ); + + # See other tests in t/db_dependent/Koha/Objects.t }; subtest 'add_enrolment_fee_if_needed' => sub { diff --git a/t/db_dependent/Virtualshelves.t b/t/db_dependent/Virtualshelves.t index 9f830fb45b..fb14573e37 100644 --- a/t/db_dependent/Virtualshelves.t +++ b/t/db_dependent/Virtualshelves.t @@ -84,7 +84,7 @@ subtest 'CRUD' => sub { is( $number_of_shelves, 2, 'Another patron should be able to create a shelf with an existing shelfname'); my $is_deleted = Koha::Virtualshelves->find( $shelf->shelfnumber )->delete; - is( $is_deleted, 1, 'The shelf has been deleted correctly' ); + ok( $is_deleted, 'The shelf has been deleted correctly' ); $number_of_shelves = Koha::Virtualshelves->search->count; is( $number_of_shelves, 1, 'To be sure the shelf has been deleted' ); @@ -162,7 +162,7 @@ subtest 'Sharing' => sub { is( Koha::Virtualshelfshares->search->count, 2, 'Check that number of shares went down again' ); # Remove the first accept - is( $shelf_to_share->remove_share( $share_with_me->{borrowernumber} ), 1, '1 share should have been removed if the shelf was shared with this patron' ); + ok( $shelf_to_share->remove_share( $share_with_me->{borrowernumber} ), '1 share should have been removed if the shelf was shared with this patron' ); $number_of_shelves_shared = Koha::Virtualshelfshares->search->count; is( $number_of_shelves_shared, 1, 'To be sure the share has been removed' ); -- 2.11.0