From a6f41e41c26f624ddfc73c3402d4e6c2e3407cb2 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 | 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.7.4