From d99d3c428370b6caf1c886c4b1073866b6efef04 Mon Sep 17 00:00:00 2001
From: Lari Taskula <lari.taskula@jns.fi>
Date: Mon, 20 Feb 2017 20:07:26 +0200
Subject: [PATCH] [SIGNED-OFF] Bug 18137: Make /holds
 Mojolicious::Plugin::OpenAPI compatible

Also
- adding some missing and new response definitions into Swagger spec.
- fixing failing tests due to Bug 17932's change of boolean values

To test:
1. prove t/db_dependent/api/v1/holds.t

Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
---
 Koha/REST/V1/Hold.pm            | 49 +++++++++++-----------
 api/v1/swagger/paths/holds.json | 90 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 114 insertions(+), 25 deletions(-)

diff --git a/Koha/REST/V1/Hold.pm b/Koha/REST/V1/Hold.pm
index d72e5df..ee9e482 100644
--- a/Koha/REST/V1/Hold.pm
+++ b/Koha/REST/V1/Hold.pm
@@ -27,7 +27,7 @@ use Koha::Holds;
 use Koha::DateUtils;
 
 sub list {
-    my ($c, $args, $cb) = @_;
+    my $c = shift->openapi->valid_input or return;
 
     my $params = $c->req->query_params->to_hash;
     my @valid_params = Koha::Holds->_resultset->result_source->columns;
@@ -36,11 +36,11 @@ sub list {
     }
     my $holds = Koha::Holds->search($params);
 
-    return $c->$cb($holds, 200);
+    return $c->render(status => 200, openapi => $holds);
 }
 
 sub add {
-    my ($c, $args, $cb) = @_;
+    my $c = shift->openapi->valid_input or return;
 
     my $body = $c->req->json;
 
@@ -51,26 +51,26 @@ sub add {
     my $expirationdate = $body->{expirationdate};
     my $borrower = Koha::Patrons->find($borrowernumber);
     unless ($borrower) {
-        return $c->$cb({error => "Borrower not found"}, 404);
+        return $c->render( status  => 404,
+                           openapi => {error => "Borrower not found"} );
     }
 
     unless ($biblionumber or $itemnumber) {
-        return $c->$cb({
+        return $c->render( status => 400, openapi => {
             error => "At least one of biblionumber, itemnumber should be given"
-        }, 400);
+        } );
     }
     unless ($branchcode) {
-        return $c->$cb({
-            error => "Branchcode is required"
-        }, 400);
+        return $c->render( status  => 400,
+                           openapi => { error => "Branchcode is required" } );
     }
 
     if ($itemnumber) {
         my $item_biblionumber = C4::Biblio::GetBiblionumberFromItemnumber($itemnumber);
         if ($biblionumber and $biblionumber != $item_biblionumber) {
-            return $c->$cb({
+            return $c->render( status => 400, openapi => {
                 error => "Item $itemnumber doesn't belong to biblio $biblionumber"
-            }, 400);
+            } );
         }
         $biblionumber ||= $item_biblionumber;
     }
@@ -83,9 +83,9 @@ sub add {
       : CanBookBeReserved( $borrowernumber, $biblionumber );
 
     unless ($can_reserve eq 'OK') {
-        return $c->$cb({
+        return $c->render( status => 403, openapi => {
             error => "Reserve cannot be placed. Reason: $can_reserve"
-        }, 403);
+        } );
     }
 
     my $priority = C4::Reserves::CalculatePriority($biblionumber);
@@ -101,24 +101,25 @@ sub add {
         $biblio->{title}, $itemnumber);
 
     unless ($reserve_id) {
-        return $c->$cb({
+        return $c->render( status => 500, openapi => {
             error => "Error while placing reserve. See Koha logs for details."
-        }, 500);
+        } );
     }
 
     my $reserve = Koha::Holds->find($reserve_id);
 
-    return $c->$cb($reserve, 201);
+    return $c->render( status => 201, openapi => $reserve );
 }
 
 sub edit {
-    my ($c, $args, $cb) = @_;
+    my $c = shift->openapi->valid_input or return;
 
-    my $reserve_id = $args->{reserve_id};
+    my $reserve_id = $c->validation->param('reserve_id');
     my $reserve = C4::Reserves::GetReserve($reserve_id);
 
     unless ($reserve) {
-        return $c->$cb({error => "Reserve not found"}, 404);
+        return $c->render( status  => 404,
+                           openapi => {error => "Reserve not found"} );
     }
 
     my $body = $c->req->json;
@@ -141,22 +142,22 @@ sub edit {
     C4::Reserves::ModReserve($params);
     $reserve = Koha::Holds->find($reserve_id);
 
-    return $c->$cb($reserve, 200);
+    return $c->render( status => 200, openapi => $reserve );
 }
 
 sub delete {
-    my ($c, $args, $cb) = @_;
+    my $c = shift->openapi->valid_input or return;
 
-    my $reserve_id = $args->{reserve_id};
+    my $reserve_id = $c->validation->param('reserve_id');
     my $reserve = C4::Reserves::GetReserve($reserve_id);
 
     unless ($reserve) {
-        return $c->$cb({error => "Reserve not found"}, 404);
+        return $c->render( status => 404, openapi => {error => "Reserve not found"} );
     }
 
     C4::Reserves::CancelReserve({ reserve_id => $reserve_id });
 
-    return $c->$cb({}, 200);
+    return $c->render( status => 200, openapi => {} );
 }
 
 1;
diff --git a/api/v1/swagger/paths/holds.json b/api/v1/swagger/paths/holds.json
index 7d67975..b2f542a 100644
--- a/api/v1/swagger/paths/holds.json
+++ b/api/v1/swagger/paths/holds.json
@@ -1,6 +1,7 @@
 {
   "/holds": {
     "get": {
+      "x-mojo-to": "Hold#list",
       "operationId": "listHolds",
       "tags": ["patrons", "holds"],
       "parameters": [
@@ -118,11 +119,35 @@
             "$ref": "../definitions.json#/holds"
           }
         },
+        "401": {
+          "description": "Authentication required",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "403": {
+          "description": "Hold not allowed",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
         "404": {
           "description": "Borrower not found",
           "schema": {
             "$ref": "../definitions.json#/error"
           }
+        },
+        "500": {
+          "description": "Internal server error",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "503": {
+          "description": "Under maintenance",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
         }
       },
       "x-koha-authorization": {
@@ -134,6 +159,7 @@
       }
     },
     "post": {
+      "x-mojo-to": "Hold#add",
       "operationId": "addHold",
       "tags": ["patrons", "holds"],
       "parameters": [{
@@ -184,6 +210,12 @@
             "$ref": "../definitions.json#/error"
           }
         },
+        "401": {
+          "description": "Authentication required",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
         "403": {
           "description": "Hold not allowed",
           "schema": {
@@ -197,7 +229,13 @@
           }
         },
         "500": {
-          "description": "Internal error",
+          "description": "Internal server error",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "503": {
+          "description": "Under maintenance",
           "schema": {
             "$ref": "../definitions.json#/error"
           }
@@ -213,6 +251,7 @@
   },
   "/holds/{reserve_id}": {
     "put": {
+      "x-mojo-to": "Hold#edit",
       "operationId": "editHold",
       "tags": ["holds"],
       "parameters": [{
@@ -258,11 +297,35 @@
             "$ref": "../definitions.json#/error"
           }
         },
+        "401": {
+          "description": "Authentication required",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "403": {
+          "description": "Hold not allowed",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
         "404": {
           "description": "Hold not found",
           "schema": {
             "$ref": "../definitions.json#/error"
           }
+        },
+        "500": {
+          "description": "Internal server error",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "503": {
+          "description": "Under maintenance",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
         }
       },
       "x-koha-authorization": {
@@ -274,6 +337,7 @@
       }
     },
     "delete": {
+      "x-mojo-to": "Hold#delete",
       "operationId": "deleteHold",
       "tags": ["holds"],
       "parameters": [{
@@ -288,11 +352,35 @@
             "type": "object"
           }
         },
+        "401": {
+          "description": "Authentication required",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "403": {
+          "description": "Hold not allowed",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
         "404": {
           "description": "Hold not found",
           "schema": {
             "$ref": "../definitions.json#/error"
           }
+        },
+        "500": {
+          "description": "Internal server error",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "503": {
+          "description": "Under maintenance",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
         }
       },
       "x-koha-authorization": {
-- 
2.1.4