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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 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 100755
--- 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