Attachment 103944 Details for Bug 25048 – Bug 25048: Make successful resource deletion return 204

[patch] Bug 25048: Make successful resource deletion return 204

Bug-25048-Make-successful-resource-deletion-return.patch (text/plain), 5.95 KB, created by Kyle M Hall (khall) on 2020-04-29 14:00:19 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2020-04-29 14:00:19 UTC

Size: 5.95 KB

patch

obsolete

>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)

Actions: View | Diff | Splinter Review

Attachments on bug 25048: 102332 | 102333 | 102403 | 102404 | 103943 | 103944