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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

diff --git a/Koha/REST/V1/Acquisitions/Vendors.pm b/Koha/REST/V1/Acquisitions/Vendors.pm
index 8fcf715..8720a42 100644 (file)
--- 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 6b76daa..cfa8d53 100644 (file)
--- 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 2a8c055..87d7c05 100644 (file)
--- 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 8ff645c..331ced1 100644 (file)
--- 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 a0232d8..b9e0020 100644 (file)
--- 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 0295231..2e1eb88 100644 (file)
--- 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 77ee394..01c7bed 100644 (file)
--- 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 2bb0ed0..e4e925d 100644 (file)
--- 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 c8798f2..3e4e773 100644 (file)
--- 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",