From dbd69d66506fbde8a6b8a2b2120d962561bd32b6 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>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
---
 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.39.5