From f98a802aae6c4e521c57aa489ba93d41508e79df Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 20 Dec 2021 15:11:02 -0300 Subject: [PATCH] Bug 29741: (follow-up) Make DELETE /patrons use the new validation method This patch adapts the route so it uses the newly introduced Koha::Patron->safe_to_delete method. To test: 1. Run: $ kshell k$ prove t/db_dependent/api/v1/patrons.t => SUCCESS: Tests pass 2. Apply this patch 3. Repeat 1 => SUCCESS: Tests still pass! 4. Sign off :-D Note: There's a trivial behavior change, in which the 'anonymous patron' use case is caugh eariler than the ->delete call. I left the exception catch block just in case, who knows :-D Signed-off-by: Tomas Cohen Arazi --- Koha/REST/V1/Patrons.pm | 47 ++++++++++++++++++++------------- t/db_dependent/api/v1/patrons.t | 2 +- 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/Koha/REST/V1/Patrons.pm b/Koha/REST/V1/Patrons.pm index 5b82447c8e..122b99a2ce 100644 --- a/Koha/REST/V1/Patrons.pm +++ b/Koha/REST/V1/Patrons.pm @@ -20,6 +20,7 @@ use Modern::Perl; use Mojo::Base 'Mojolicious::Controller'; use Koha::Database; +use Koha::Exceptions; use Koha::Patrons; use Scalar::Util qw( blessed ); @@ -330,40 +331,48 @@ sub delete { return try { - if ( $patron->checkouts->count > 0 ) { + my $safe_to_delete = $patron->safe_to_delete; + + if ( $safe_to_delete eq 'ok' ) { + $patron->_result->result_source->schema->txn_do( + sub { + $patron->move_to_deleted; + $patron->delete; + + return $c->render( + status => 204, + openapi => q{} + ); + } + ); + } + elsif ( $safe_to_delete eq 'has_checkouts' ) { return $c->render( status => 409, openapi => { error => 'Pending checkouts prevent deletion' } ); } - - my $account = $patron->account; - - if ( $account->outstanding_debits->total_outstanding > 0 ) { + elsif ( $safe_to_delete eq 'has_debt' ) { return $c->render( status => 409, openapi => { error => 'Pending debts prevent deletion' } ); } - - if ( $patron->guarantee_relationships->count > 0 ) { + elsif ( $safe_to_delete eq 'has_guarantees' ) { return $c->render( status => 409, openapi => { error => 'Patron is a guarantor and it prevents deletion' } ); } - - $patron->_result->result_source->schema->txn_do( - sub { - $patron->move_to_deleted; - $patron->delete; - - return $c->render( - status => 204, - openapi => q{} - ); - } - ); + elsif ( $safe_to_delete eq 'is_anonymous_patron' ) { + return $c->render( + status => 403, + openapi => { error => 'Anonymous patron cannot be deleted' } + ); + } + else { + Koha::Exceptions::Exception->throw( "Koha::Patron->safe_to_delete returned an unexpected value: $safe_to_delete" ); + } } catch { if ( blessed $_ && $_->isa('Koha::Exceptions::Patron::FailedDeleteAnonymousPatron') ) { return $c->render( diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t index 1081f0e086..96a98522a9 100755 --- a/t/db_dependent/api/v1/patrons.t +++ b/t/db_dependent/api/v1/patrons.t @@ -665,6 +665,7 @@ subtest 'delete() tests' => sub { $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::lib::Mocks::mock_preference( 'borrowerRelationship', 'parent' ); @@ -700,7 +701,6 @@ subtest 'delete() tests' => sub { # Remove guarantee $patron->guarantee_relationships->delete; - 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.32.0