From 6cc8896bd1de0f059003626542c9ee7141d55868 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 17 Sep 2021 11:42:34 -0300 Subject: [PATCH] Bug 27947: (QA follow-up) Refactor routes This patch refactors the route specs a bit, and also reorganizes code for easier tracking. Unused exceptions that were added earlier are removed for now. A follow-up patch will add tests to this routes. Signed-off-by: Tomas Cohen Arazi Signed-off-by: Martin Renvoize --- Koha/Exceptions/ArticleRequests.pm | 17 --- Koha/REST/V1/ArticleRequests.pm | 54 +++++++-- Koha/REST/V1/Patrons.pm | 49 -------- api/v1/swagger/parameters.json | 9 -- .../swagger/parameters/article_request.json | 23 ---- api/v1/swagger/paths.json | 10 +- api/v1/swagger/paths/article_requests.json | 70 ----------- api/v1/swagger/paths/article_requests.yaml | 111 ++++++++++++++++++ api/v1/swagger/paths/public_patrons.json | 70 ----------- api/v1/swagger/swagger.yaml | 4 + 10 files changed, 165 insertions(+), 252 deletions(-) delete mode 100644 Koha/Exceptions/ArticleRequests.pm delete mode 100644 api/v1/swagger/parameters/article_request.json delete mode 100644 api/v1/swagger/paths/article_requests.json create mode 100644 api/v1/swagger/paths/article_requests.yaml diff --git a/Koha/Exceptions/ArticleRequests.pm b/Koha/Exceptions/ArticleRequests.pm deleted file mode 100644 index 669529ca47..0000000000 --- a/Koha/Exceptions/ArticleRequests.pm +++ /dev/null @@ -1,17 +0,0 @@ -package Koha::Exceptions::ArticleRequests; - -use Modern::Perl; - -use Exception::Class ( - - 'Koha::Exceptions::ArticleRequests' => { - description => 'Something went wrong!', - }, - 'Koha::Exceptions::ArticleRequests::FailedCancel' => { - isa => 'Koha::Exceptions::ArticleRequests', - description => 'Failed to cancel article request' - } - -); - -1; \ No newline at end of file diff --git a/Koha/REST/V1/ArticleRequests.pm b/Koha/REST/V1/ArticleRequests.pm index a026b8887c..77c69a62d6 100644 --- a/Koha/REST/V1/ArticleRequests.pm +++ b/Koha/REST/V1/ArticleRequests.pm @@ -42,9 +42,9 @@ Controller function that handles cancelling a Koha::ArticleRequest object sub cancel { my $c = shift->openapi->valid_input or return; - my $ar = Koha::ArticleRequests->find( $c->validation->param('ar_id') ); + my $article_request = Koha::ArticleRequests->find( $c->validation->param('article_request_id') ); - unless ( $ar ) { + unless ( $article_request ) { return $c->render( status => 404, openapi => { error => "Article request not found" } @@ -56,19 +56,55 @@ sub cancel { return try { - $ar->cancel($reason, $notes); + $article_request->cancel($reason, $notes); return $c->render( status => 204, openapi => q{} ); } catch { - if ( blessed $_ && $_->isa('Koha::Exceptions::ArticleRequests::FailedCancel') ) { - return $c->render( - status => 403, - openapi => { error => "Article request cannot be canceled" } - ); - } + $c->unhandled_exception($_); + }; +} + +=head3 patron_cancel + +Controller function that handles cancelling a patron's Koha::ArticleRequest object + +=cut + +sub patron_cancel { + my $c = shift->openapi->valid_input or return; + + my $patron = Koha::Patrons->find( $c->validation->param('patron_id') ); + + unless ( $patron ) { + return $c->render( + status => 404, + openapi => { error => "Patron not found" } + ); + } + + my $article_request = $patron->article_requests->find( $c->validation->param('article_request_id') ); + unless ( $article_request ) { + return $c->render( + status => 404, + openapi => { error => "Article request not found" } + ); + } + + my $reason = $c->validation->param('cancellation_reason'); + my $notes = $c->validation->param('notes'); + + return try { + + $article_request->cancel( $reason, $notes ); + return $c->render( + status => 204, + openapi => q{} + ); + } + catch { $c->unhandled_exception($_); }; } diff --git a/Koha/REST/V1/Patrons.pm b/Koha/REST/V1/Patrons.pm index 6790e9f6b8..80b64ee963 100644 --- a/Koha/REST/V1/Patrons.pm +++ b/Koha/REST/V1/Patrons.pm @@ -419,53 +419,4 @@ sub guarantors_can_see_checkouts { }; } -=head3 cancel_article_request - -Controller function that handles cancelling a patron's Koha::ArticleRequest object - -=cut - -sub cancel_article_request { - my $c = shift->openapi->valid_input or return; - - my $patron = Koha::Patrons->find( $c->validation->param('patron_id') ); - - unless ( $patron ) { - return $c->render( - status => 404, - openapi => { error => "Patron not found" } - ); - } - - my $ar = $patron->article_requests->find( $c->validation->param('ar_id') ); - - unless ( $ar ) { - return $c->render( - status => 404, - openapi => { error => "Article request not found" } - ); - } - - my $reason = $c->validation->param('cancellation_reason'); - my $notes = $c->validation->param('notes'); - - return try { - - $ar->cancel($reason, $notes); - return $c->render( - status => 204, - openapi => q{} - ); - } catch { - if ( blessed $_ && $_->isa('Koha::Exceptions::ArticleRequests::FailedCancel') ) { - return $c->render( - status => 403, - openapi => { error => "Article request cannot be canceled" } - ); - } - - $c->unhandled_exception($_); - }; -} - 1; diff --git a/api/v1/swagger/parameters.json b/api/v1/swagger/parameters.json index 120b8371f1..95464e1461 100644 --- a/api/v1/swagger/parameters.json +++ b/api/v1/swagger/parameters.json @@ -56,15 +56,6 @@ "cashup_id_pp": { "$ref": "parameters/cashup.json#/cashup_id_pp" }, - "ar_id_pp": { - "$ref": "parameters/article_request.json#/ar_id_pp" - }, - "ar_reason_qp": { - "$ref": "parameters/article_request.json#/ar_reason_qp" - }, - "ar_notes_qp": { - "$ref": "parameters/article_request.json#/ar_notes_qp" - }, "match": { "name": "_match", "in": "query", diff --git a/api/v1/swagger/parameters/article_request.json b/api/v1/swagger/parameters/article_request.json deleted file mode 100644 index 5644936af5..0000000000 --- a/api/v1/swagger/parameters/article_request.json +++ /dev/null @@ -1,23 +0,0 @@ -{ - "ar_id_pp": { - "name": "ar_id", - "in": "path", - "description": "Article request identifier", - "required": true, - "type": "integer" - }, - "ar_reason_qp": { - "name": "cancellation_reason", - "in": "query", - "description": "Article request cancellation reason", - "required": false, - "type": "string" - }, - "ar_notes_qp": { - "name": "notes", - "in": "query", - "description": "Article request custom cancellation reason", - "required": false, - "type": "string" - } -} diff --git a/api/v1/swagger/paths.json b/api/v1/swagger/paths.json index 91949ac46b..b668b5c98e 100644 --- a/api/v1/swagger/paths.json +++ b/api/v1/swagger/paths.json @@ -17,8 +17,8 @@ "/acquisitions/funds": { "$ref": "paths/acquisitions_funds.json#/~1acquisitions~1funds" }, - "/article_requests/{ar_id}": { - "$ref": "paths/article_requests.json#/~1article_requests~1{ar_id}" + "/article_requests/{article_request_id}": { + "$ref": "paths/article_requests.yaml#/~1article_requests~1{article_request_id}" }, "/biblios/{biblio_id}": { "$ref": "paths/biblios.json#/~1biblios~1{biblio_id}" @@ -164,6 +164,9 @@ "/public/biblios/{biblio_id}": { "$ref": "paths/biblios.json#/~1public~1biblios~1{biblio_id}" }, + "/public/patrons/{patron_id}/article_requests/{article_request_id}": { + "$ref": "paths/article_requests.yaml#/~1public~1patrons~1{patron_id}~1article_requests~1{article_request_id}" + }, "/public/patrons/{patron_id}/password": { "$ref": "paths/public_patrons.json#/~1public~1patrons~1{patron_id}~1password" }, @@ -173,9 +176,6 @@ "/public/patrons/{patron_id}/guarantors/can_see_checkouts": { "$ref": "paths/public_patrons.json#/~1public~1patrons~1{patron_id}~1guarantors~1can_see_checkouts" }, - "/public/patrons/{patron_id}/article_requests/{ar_id}": { - "$ref": "paths/public_patrons.json#/~1public~1patrons~1{patron_id}~1article_requests~1{ar_id}" - }, "/quotes": { "$ref": "paths/quotes.json#/~1quotes" }, diff --git a/api/v1/swagger/paths/article_requests.json b/api/v1/swagger/paths/article_requests.json deleted file mode 100644 index 22ac503d63..0000000000 --- a/api/v1/swagger/paths/article_requests.json +++ /dev/null @@ -1,70 +0,0 @@ -{ - "/article_requests/{ar_id}": { - "delete": { - "x-mojo-to": "ArticleRequests#cancel", - "operationId": "cancelArticleRequest", - "tags": [ - "article_requests" - ], - "summary": "Cancel article requests", - "parameters": [ - { - "$ref": "../parameters.json#/ar_id_pp" - }, - { - "$ref": "../parameters.json#/ar_reason_qp" - }, - { - "$ref": "../parameters.json#/ar_notes_qp" - } - ], - "produces": ["application/json"], - "responses": { - "204": { - "description": "Article request canceled" - }, - "400": { - "description": "Bad request", - "schema": { - "$ref": "../definitions.json#/error" - } - }, - "401": { - "description": "Authentication required", - "schema": { - "$ref": "../definitions.json#/error" - } - }, - "403": { - "description": "Access forbidden", - "schema": { - "$ref": "../definitions.json#/error" - } - }, - "404": { - "description": "Patron 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": { - "permissions": { - "reserveforothers": "1" - } - } - } - } -} \ No newline at end of file diff --git a/api/v1/swagger/paths/article_requests.yaml b/api/v1/swagger/paths/article_requests.yaml new file mode 100644 index 0000000000..fbdfe07a43 --- /dev/null +++ b/api/v1/swagger/paths/article_requests.yaml @@ -0,0 +1,111 @@ +--- +"/article_requests/{article_request_id}": + delete: + x-mojo-to: ArticleRequests#cancel + operationId: cancelArticleRequest + tags: + - article_requests + summary: Cancel article requests + parameters: + - name: article_request_id + in: path + description: Article request identifier + required: true + type: integer + - name: cancellation_reason + in: query + description: Article request cancellation reason + required: false + type: string + - name: notes + in: query + description: Article request custom cancellation reason + required: false + type: string + produces: + - application/json + responses: + "204": + description: Article request canceled + "400": + description: Bad request + schema: + $ref: ../definitions.json#/error + "401": + description: Authentication required + schema: + $ref: ../definitions.json#/error + "403": + description: Access forbidden + schema: + $ref: ../definitions.json#/error + "404": + description: Patron 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: + permissions: + reserveforothers: "1" +"/public/patrons/{patron_id}/article_requests/{article_request_id}": + delete: + x-mojo-to: ArticleRequests#patron_cancel + operationId: publicCancelPatronArticleRequest + tags: + - article_requests + summary: Cancel patron's article requests + parameters: + - $ref: ../parameters.json#/patron_id_pp + - name: article_request_id + in: path + description: Article request identifier + required: true + type: integer + - name: cancellation_reason + in: query + description: Article request cancellation reason + required: false + type: string + - name: notes + in: query + description: Article request custom cancellation reason + required: false + type: string + produces: + - application/json + responses: + "204": + description: Patron's article request canceled + "400": + description: Bad request + schema: + $ref: ../definitions.json#/error + "401": + description: Authentication required + schema: + $ref: ../definitions.json#/error + "403": + description: Access forbidden + schema: + $ref: ../definitions.json#/error + "404": + description: Patron 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: + allow-owner: true diff --git a/api/v1/swagger/paths/public_patrons.json b/api/v1/swagger/paths/public_patrons.json index af06a0c7c7..90d6cfe4c4 100644 --- a/api/v1/swagger/paths/public_patrons.json +++ b/api/v1/swagger/paths/public_patrons.json @@ -242,75 +242,5 @@ "allow-owner": true } } - }, - "/public/patrons/{patron_id}/article_requests/{ar_id}": { - "delete": { - "x-mojo-to": "Patrons#cancel_article_request", - "operationId": "cancelPatronArticleRequest", - "tags": [ - "patrons", - "article_requests" - ], - "summary": "Cancel patron's article requests", - "parameters": [ - { - "$ref": "../parameters.json#/patron_id_pp" - }, - { - "$ref": "../parameters.json#/ar_id_pp" - }, - { - "$ref": "../parameters.json#/ar_reason_qp" - }, - { - "$ref": "../parameters.json#/ar_notes_qp" - } - ], - "produces": ["application/json"], - "responses": { - "204": { - "description": "Patron's article request canceled" - }, - "400": { - "description": "Bad request", - "schema": { - "$ref": "../definitions.json#/error" - } - }, - "401": { - "description": "Authentication required", - "schema": { - "$ref": "../definitions.json#/error" - } - }, - "403": { - "description": "Access forbidden", - "schema": { - "$ref": "../definitions.json#/error" - } - }, - "404": { - "description": "Patron 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": { - "allow-owner": true - } - } } } diff --git a/api/v1/swagger/swagger.yaml b/api/v1/swagger/swagger.yaml index 2e4e9bc09f..29b04cef64 100644 --- a/api/v1/swagger/swagger.yaml +++ b/api/v1/swagger/swagger.yaml @@ -31,6 +31,10 @@ info: context; If it is not included in the request, then the request context will default to using your api comsumer's assigned home library. tags: + - name: "article_requests" + x-displayName: Article requests + description: | + Manage article requests - name: "biblios" x-displayName: Biblios description: | -- 2.20.1