Attachment 128784 Details for Bug 29741 – Bug 29741: (follow-up) Make DELETE /patrons use the new validation method

[patch] Bug 29741: (follow-up) Make DELETE /patrons use the new validation method

Bug-29741-follow-up-Make-DELETE-patrons-use-the-ne.patch (text/plain), 4.48 KB, created by Tomás Cohen Arazi (tcohen) on 2021-12-20 18:15:01 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2021-12-20 18:15:01 UTC

Size: 4.48 KB

patch

obsolete

>From f98a802aae6c4e521c57aa489ba93d41508e79df Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>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 <tomascohen@theke.io>
>---
> 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

Actions: View | Diff | Splinter Review

Attachments on bug 29741: 128778 | 128779 | 128784 | 128814 | 128815 | 128816 | 128994 | 128995 | 128996