From 29f4d54e1bd4d473cf666d8baf190e309eb823ca Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi 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 Signed-off-by: Kyle M Hall --- 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)