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>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Koha/Exceptions/ArticleRequests.pm

@@ -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;

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($_);
     };
 }

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;

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",

api/v1/swagger/parameters/article_request.json

@@ -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"
-  }
-}

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"
   },

api/v1/swagger/paths/article_requests.json

@@ -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"
-                  }
-            }
-        }
-    }
-}

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

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
-            }
-        }
     }
 }

api/v1/swagger/swagger.yaml

@@ -56,6 +56,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: |