From fdf8af3c72b2fb3b694c51f7a315eba5cc874719 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 14 Dec 2020 12:13:38 -0300 Subject: [PATCH] Bug 18729: (follow-up) Adjust API to new spec The previous patch introduces some behavioural changes to the API, as well as the data types that need to be passed; all happens in the tests. This patch adapts the route so it complies with those changes: - JSON object containing pickup_library_id attribute is now passed back and forth. - The controller should take care of checking the pickup location is valid, using the available tools. To test: 1. Apply the patches, up to the tests 2. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => ERROR: Tests fail because the controller doesn't implement the desired behavior 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 | 23 ++++++++++++++++++++--- api/v1/swagger/paths/holds.json | 22 ++++++++++++++++++++-- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm index a431b55c5b..81d0023915 100644 --- a/Koha/REST/V1/Holds.pm +++ b/Koha/REST/V1/Holds.pm @@ -518,6 +518,9 @@ sub update_pickup_location { my $c = shift->openapi->valid_input or return; my $hold_id = $c->validation->param('hold_id'); + my $body = $c->validation->param('body'); + my $pickup_library_id = $body->{pickup_library_id}; + my $hold = Koha::Holds->find($hold_id); unless ($hold) { @@ -528,13 +531,27 @@ sub update_pickup_location { } return try { - my $pickup_location = $c->req->json; - $hold->branchcode($pickup_location)->store; + $hold->set_pickup_location({ library_id => $pickup_library_id }); - return $c->render( status => 200, openapi => $pickup_location ); + return $c->render( + status => 200, + openapi => { + pickup_library_id => $pickup_library_id + } + ); } catch { + + if ( blessed $_ and $_->isa('Koha::Exceptions::Hold::InvalidPickupLocation') ) { + return $c->render( + status => 400, + openapi => { + error => "$_" + } + ); + } + $c->unhandled_exception($_); }; } diff --git a/api/v1/swagger/paths/holds.json b/api/v1/swagger/paths/holds.json index ebaed331f2..dd26dd2746 100644 --- a/api/v1/swagger/paths/holds.json +++ b/api/v1/swagger/paths/holds.json @@ -718,7 +718,13 @@ "description": "Pickup location", "required": true, "schema": { - "type": "string" + "type": "object", + "properties": { + "pickup_library_id": { + "type": "string", + "description": "Internal identifier for the pickup library" + } + } } } ], @@ -729,7 +735,19 @@ "200": { "description": "The new pickup location value for the hold", "schema": { - "type": "string" + "type": "object", + "properties": { + "pickup_library_id": { + "type": "string", + "description": "Internal identifier for the pickup library" + } + } + } + }, + "400": { + "description": "Missing or wrong parameters", + "schema": { + "$ref": "../definitions.json#/error" } }, "401": { -- 2.20.1