Bugzilla – Attachment 125007 Details for
Bug 27947
Add default cancellation reasons to article requests
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 27947: (QA follow-up) Refactor routes
Bug-27947-QA-follow-up-Refactor-routes.patch (text/plain), 17.82 KB, created by
Martin Renvoize (ashimema)
on 2021-09-17 16:16:35 UTC
(
hide
)
Description:
Bug 27947: (QA follow-up) Refactor routes
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2021-09-17 16:16:35 UTC
Size:
17.82 KB
patch
obsolete
>From 3dede5850c0f0fa9d9b88214cd770e6f7862ae1f Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >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 <tomascohen@theke.io> >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > 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
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=125007">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 27947
:
124063
|
124064
|
124065
|
124849
|
124850
|
124851
|
124952
|
124953
|
124954
|
124955
|
124956
|
124993
|
124994
|
124995
|
124996
|
124997
|
124998
|
124999
|
125002
|
125003
|
125004
|
125005
|
125006
|
125007
|
125008
|
125009
|
125010
|
125016
|
125038
|
125039
|
125040
|
125041
|
125042
|
125043
|
125044
|
125045
|
125046
|
125047
|
125165
|
125166
|
125167
|
125168
|
125169
|
125170
|
125171
|
125172
|
125173
|
125174
|
125739
|
125740