From 481db96b559e9995f3f3edb1b1595f468f4bfbcd Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Tue, 18 May 2021 09:06:45 -0300 Subject: [PATCH] Bug 28369: Set additionalProperties: false and fix things This patch performs a cleanup on the holds routes: - It adds additionalProperties: false to all missing routes - It syncs the spec with the guidelines - additionalProperties: false highlighted some attributes read in the controlled weren't declared on the spec, adding them - PUT /holds/:hold_id is moved to PATCH, as it makes more sense and gathered opinions lean towards that To test: 1. Apply the tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => FAIL: Lots of things are wrong with the API 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi Signed-off-by: David Nind Signed-off-by: Martin Renvoize --- Koha/REST/V1/Holds.pm | 4 +-- api/v1/swagger/paths/holds.json | 43 ++++++++++++++++++++++++--------- 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm index b6545b1997..08bf1a1fe3 100644 --- a/Koha/REST/V1/Holds.pm +++ b/Koha/REST/V1/Holds.pm @@ -278,8 +278,8 @@ sub edit { ); } - $pickup_library_id //= $hold->branchcode; - my $priority = $body->{priority} // $hold->priority; + $pickup_library_id //= $hold->branchcode; + my $priority = $body->{priority} // $hold->priority; # suspended_until can also be set to undef my $suspended_until = exists $body->{suspended_until} ? $body->{suspended_until} : $hold->suspend_until; diff --git a/api/v1/swagger/paths/holds.json b/api/v1/swagger/paths/holds.json index 366cf35838..2a863eefdf 100644 --- a/api/v1/swagger/paths/holds.json +++ b/api/v1/swagger/paths/holds.json @@ -8,7 +8,7 @@ { "name": "hold_id", "in": "query", - "description": "Internal reserve identifier", + "description": "Internal hold identifier", "type": "integer" }, { @@ -120,6 +120,15 @@ }, { "$ref": "../parameters.json#/per_page" + }, + { + "$ref": "../parameters.json#/q_param" + }, + { + "$ref": "../parameters.json#/q_body" + }, + { + "$ref": "../parameters.json#/q_header" } ], "produces": ["application/json"], @@ -188,6 +197,11 @@ "description": "Internal biblio identifier", "type": [ "integer", "null" ] }, + "hold_date": { + "description": "The date the hold was placed", + "type": [ "string", "null" ], + "format": "date" + }, "item_id": { "description": "Internal item identifier", "type": [ "integer", "null" ] @@ -214,7 +228,8 @@ "type": [ "boolean", "null" ] } }, - "required": [ "patron_id", "pickup_library_id" ] + "required": [ "patron_id", "pickup_library_id" ], + "additionalProperties": false } }, { @@ -279,7 +294,7 @@ } }, "/holds/{hold_id}": { - "put": { + "patch": { "x-mojo-to": "Holds#edit", "operationId": "editHold", "tags": ["holds"], @@ -298,16 +313,17 @@ "type": "integer", "minimum": 1 }, - "branchcode": { - "description": "Pickup location", + "pickup_library_id": { + "description": "Internal library identifier for the pickup library", "type": "string" }, - "suspend_until": { - "description": "Suspend until", + "suspended_until": { + "description": "Date until which the hold has been suspended", "type": "string", - "format": "date" + "format": "date-time" } - } + }, + "additionalProperties": false } } ], @@ -510,7 +526,8 @@ "type": "string", "format": "date" } - } + }, + "additionalProperties": false } } ], @@ -736,7 +753,8 @@ "type": "string", "description": "Internal identifier for the pickup library" } - } + }, + "additionalProperties": false } } ], @@ -753,7 +771,8 @@ "type": "string", "description": "Internal identifier for the pickup library" } - } + }, + "additionalProperties": false } }, "400": { -- 2.20.1