From 29f4d54e1bd4d473cf666d8baf190e309eb823ca Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Thu, 2 Apr 2020 18:40:49 -0300
Subject: [PATCH] Bug 25048: Make successful resource deletion return 204

This patch adapts the spec and the controllers so existing routes return
204 and an empty response body when a successful deletion happens.

Right now we have a coding guideline but haven't adapted the existing
routes.

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/*.t
=> FAIL: Some tests fail
3. Apply this patch
4. Repeat 2.
=> SUCCESS: Tests pass!
5. Sign off :-D

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
---
 Koha/REST/V1/Acquisitions/Vendors.pm           | 2 +-
 Koha/REST/V1/Cities.pm                         | 5 ++++-
 Koha/REST/V1/Holds.pm                          | 5 ++++-
 Koha/REST/V1/Patrons.pm                        | 5 ++++-
 api/v1/swagger/paths/acquisitions_orders.json  | 5 +----
 api/v1/swagger/paths/acquisitions_vendors.json | 7 ++-----
 api/v1/swagger/paths/cities.json               | 7 ++-----
 api/v1/swagger/paths/holds.json                | 7 ++-----
 api/v1/swagger/paths/patrons.json              | 7 ++-----
 9 files changed, 22 insertions(+), 28 deletions(-)

diff --git a/Koha/REST/V1/Acquisitions/Vendors.pm b/Koha/REST/V1/Acquisitions/Vendors.pm
index 8fcf715cfb..8720a42629 100644
--- a/Koha/REST/V1/Acquisitions/Vendors.pm
+++ b/Koha/REST/V1/Acquisitions/Vendors.pm
@@ -159,7 +159,7 @@ sub delete {
         $vendor->delete;
 
         return $c->render(
-            status  => 200,
+            status  => 204,
             openapi => q{}
         );
     }
diff --git a/Koha/REST/V1/Cities.pm b/Koha/REST/V1/Cities.pm
index 6b76daae10..cfa8d53422 100644
--- a/Koha/REST/V1/Cities.pm
+++ b/Koha/REST/V1/Cities.pm
@@ -126,7 +126,10 @@ sub delete {
 
     return try {
         $city->delete;
-        return $c->render( status => 200, openapi => "" );
+        return $c->render(
+            status  => 204,
+            openapi => q{}
+        );
     }
     catch {
         $c->unhandled_exception($_);
diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm
index 2a8c055d3f..87d7c05336 100644
--- a/Koha/REST/V1/Holds.pm
+++ b/Koha/REST/V1/Holds.pm
@@ -262,7 +262,10 @@ sub delete {
     return try {
         $hold->cancel;
 
-        return $c->render( status => 200, openapi => {} );
+        return $c->render(
+            status  => 204,
+            openapi => q{}
+        );
     }
     catch {
         $c->unhandled_exception($_);
diff --git a/Koha/REST/V1/Patrons.pm b/Koha/REST/V1/Patrons.pm
index 8ff645c46e..331ced1464 100644
--- a/Koha/REST/V1/Patrons.pm
+++ b/Koha/REST/V1/Patrons.pm
@@ -279,7 +279,10 @@ sub delete {
 
         # check if loans, reservations, debarrment, etc. before deletion!
         $patron->delete;
-        return $c->render( status => 200, openapi => {} );
+        return $c->render(
+            status  => 204,
+            openapi => q{}
+        );
     }
     catch {
         unless ($patron) {
diff --git a/api/v1/swagger/paths/acquisitions_orders.json b/api/v1/swagger/paths/acquisitions_orders.json
index a0232d84b3..b9e00200a0 100644
--- a/api/v1/swagger/paths/acquisitions_orders.json
+++ b/api/v1/swagger/paths/acquisitions_orders.json
@@ -332,10 +332,7 @@
             ],
             "responses": {
                 "204": {
-                    "description": "Order deleted",
-                    "schema": {
-                        "type": "string"
-                    }
+                    "description": "Order deleted"
                 },
                 "401": {
                     "description": "Authentication required",
diff --git a/api/v1/swagger/paths/acquisitions_vendors.json b/api/v1/swagger/paths/acquisitions_vendors.json
index 02952318e1..2e1eb88107 100644
--- a/api/v1/swagger/paths/acquisitions_vendors.json
+++ b/api/v1/swagger/paths/acquisitions_vendors.json
@@ -264,11 +264,8 @@
         "application/json"
       ],
       "responses": {
-        "200": {
-          "description": "Vendor deleted",
-          "schema": {
-            "type": "string"
-          }
+        "204": {
+          "description": "Vendor deleted"
         },
         "401": {
           "description": "Authentication required",
diff --git a/api/v1/swagger/paths/cities.json b/api/v1/swagger/paths/cities.json
index 77ee394af4..01c7bed402 100644
--- a/api/v1/swagger/paths/cities.json
+++ b/api/v1/swagger/paths/cities.json
@@ -238,11 +238,8 @@
         "application/json"
       ],
       "responses": {
-        "200": {
-          "description": "City deleted",
-          "schema": {
-            "type": "string"
-          }
+        "204": {
+          "description": "City deleted"
         },
         "401": {
           "description": "Authentication required",
diff --git a/api/v1/swagger/paths/holds.json b/api/v1/swagger/paths/holds.json
index 2bb0ed0179..e4e925d8ef 100644
--- a/api/v1/swagger/paths/holds.json
+++ b/api/v1/swagger/paths/holds.json
@@ -355,11 +355,8 @@
       ],
       "produces": ["application/json"],
       "responses": {
-        "200": {
-          "description": "Successful deletion",
-          "schema": {
-            "type": "object"
-          }
+        "204": {
+          "description": "Hold deleted"
         },
         "401": {
           "description": "Authentication required",
diff --git a/api/v1/swagger/paths/patrons.json b/api/v1/swagger/paths/patrons.json
index c8798f28d5..3e4e7739a1 100644
--- a/api/v1/swagger/paths/patrons.json
+++ b/api/v1/swagger/paths/patrons.json
@@ -669,11 +669,8 @@
       }],
       "produces": ["application/json"],
       "responses": {
-        "200": {
-          "description": "Patron deleted successfully",
-          "schema": {
-            "type": "object"
-          }
+        "204": {
+          "description": "Patron deleted"
         },
         "400": {
           "description": "Patron deletion failed",
-- 
2.24.2 (Apple Git-127)