From 60a253c983df1cf50193ee60df730e762ce8f504 Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
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 <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 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.39.5