From 574aaf2a71584e136264c650ad22c26744327792 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Mon, 20 Feb 2017 20:07:26 +0200 Subject: [PATCH] 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 Signed-off-by: Josef Moravec Signed-off-by: Tomas Cohen Arazi --- Koha/REST/V1/Hold.pm | 59 ++++++++++++++------------- api/v1/swagger/paths/holds.json | 90 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 121 insertions(+), 28 deletions(-) diff --git a/Koha/REST/V1/Hold.pm b/Koha/REST/V1/Hold.pm index a31a9e6e8f..377b05633a 100644 --- a/Koha/REST/V1/Hold.pm +++ b/Koha/REST/V1/Hold.pm @@ -28,7 +28,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; @@ -37,11 +37,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; @@ -52,18 +52,18 @@ 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" } ); } my $biblio; @@ -71,9 +71,11 @@ sub add { my $item = Koha::Items->find( $itemnumber ); $biblio = $item->biblio; if ($biblionumber and $biblionumber != $biblio->biblionumber) { - return $c->$cb({ - error => "Item $itemnumber doesn't belong to biblio $biblionumber" - }, 400); + return $c->render( + status => 400, + openapi => { + error => "Item $itemnumber doesn't belong to biblio $biblionumber" + }); } $biblionumber ||= $biblio->biblionumber; } else { @@ -86,9 +88,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); @@ -104,24 +106,26 @@ 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 $hold = Koha::Holds->find( $reserve_id ); - return $c->$cb({error => "Reserve not found"}, 404) - unless $hold; + unless ($hold) { + return $c->render( status => 404, + openapi => {error => "Reserve not found"} ); + } my $body = $c->req->json; @@ -143,21 +147,22 @@ sub edit { C4::Reserves::ModReserve($params); $hold = Koha::Holds->find($reserve_id); - return $c->$cb($hold, 200); + return $c->render( status => 200, openapi => $hold ); } 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 $hold = Koha::Holds->find( $reserve_id ); - return $c->$cb({error => "Reserve not found"}, 404) - unless $hold; + unless ($hold) { + 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 7d679756ea..b2f542ac40 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.14.1