From beed2feb3d481b68f12a58fcfd03a76704324a23 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Thu, 5 Nov 2020 11:53:43 -0300 Subject: [PATCH] Bug 14708: (QA follow-up) Add tests This patch adds tests for the change. It also simplifies the delete() method structure a bit. It fixes the error 500 the tests were raising. To test: 1. Run: $ kshell k$ prove t/db_dependent/api/v1/patrons.t => FAIL: Tests fail! 2. Apply this patch 3. Repeat 1 => SUCCESS: Tests pass! The new behaviour (code 403) is tested! 4. Sign off :-D Signed-off-by: Tomas Cohen Arazi Signed-off-by: Jonathan Druart (cherry picked from commit b092bdd20d94a516db0b7c4533149ac52f96aac5) Signed-off-by: Andrew Fuerste-Henry --- Koha/REST/V1/Patrons.pm | 47 +++++++++++++-------------------- t/db_dependent/api/v1/patrons.t | 8 +++++- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/Koha/REST/V1/Patrons.pm b/Koha/REST/V1/Patrons.pm index c3916d6414..5eba4bdee3 100644 --- a/Koha/REST/V1/Patrons.pm +++ b/Koha/REST/V1/Patrons.pm @@ -304,40 +304,31 @@ Controller function that handles deleting a Koha::Patron object sub delete { my $c = shift->openapi->valid_input or return; - my $patron; + my $patron = Koha::Patrons->find( $c->validation->param('patron_id') ); + + unless ( $patron ) { + return $c->render( + status => 404, + openapi => { error => "Patron not found" } + ); + } return try { - $patron = Koha::Patrons->find( $c->validation->param('patron_id') ); - # check if loans, reservations, debarrment, etc. before deletion! - try { - $patron->delete; - return $c->render( - status => 204, - openapi => q{} - ); - } catch { - if ( $_->isa('Koha::Exceptions::Patron::FailedDeleteAnonymousPatron') ) { - return $c->render( - status => 403, - openapi => { error => "Anonymous patron cannot be deleted" } - ); - } - else { - $c->unhandled_exception($_); - } - }; - } - catch { - unless ($patron) { + $patron->delete; + return $c->render( + status => 204, + openapi => q{} + ); + } catch { + if ( blessed $_ && $_->isa('Koha::Exceptions::Patron::FailedDeleteAnonymousPatron') ) { return $c->render( - status => 404, - openapi => { error => "Patron not found" } + status => 403, + openapi => { error => "Anonymous patron cannot be deleted" } ); } - else { - $c->unhandled_exception($_); - } + + $c->unhandled_exception($_); }; } diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t index 8d99f5ea81..40993191a7 100644 --- a/t/db_dependent/api/v1/patrons.t +++ b/t/db_dependent/api/v1/patrons.t @@ -405,7 +405,7 @@ subtest 'delete() tests' => sub { $schema->storage->txn_rollback; subtest 'librarian access test' => sub { - plan tests => 5; + plan tests => 8; $schema->storage->txn_begin; @@ -425,6 +425,12 @@ subtest 'delete() tests' => sub { my $patron = $builder->build_object({ class => 'Koha::Patrons' }); + t::lib::Mocks::mock_preference('AnonymousPatron', $patron->borrowernumber); + $t->delete_ok("//$userid:$password@/api/v1/patrons/" . $patron->borrowernumber) + ->status_is(403, 'Anonymous patron cannot be deleted') + ->json_is( { error => 'Anonymous patron cannot be deleted' } ); + + t::lib::Mocks::mock_preference('AnonymousPatron', 0); # back to default $t->delete_ok("//$userid:$password@/api/v1/patrons/" . $patron->borrowernumber) ->status_is(204, 'SWAGGER3.2.4') ->content_is('', 'SWAGGER3.3.4'); -- 2.39.5