Attachment 60495 Details for Bug 18137 – Bug 18137: Make /holds Mojolicious::Plugin::OpenAPI compatible

[patch] Bug 18137: Make /holds Mojolicious::Plugin::OpenAPI compatible

Bug-18137-Make-holds-MojoliciousPluginOpenAPI-comp.patch (text/plain), 9.86 KB, created by Lari Taskula on 2017-02-21 13:11:26 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Lari Taskula

Created: 2017-02-21 13:11:26 UTC

Size: 9.86 KB

patch

obsolete

>From 4a2098ac4bac06100894985dc60383d54c6aeb6c Mon Sep 17 00:00:00 2001
>From: Lari Taskula <lari.taskula@jns.fi>
>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
>---
> Koha/REST/V1/Hold.pm            | 55 ++++++++++++-------------
> Koha/Schema/Result/Reserve.pm   |  5 +++
> api/v1/swagger/paths/holds.json | 90 ++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 122 insertions(+), 28 deletions(-)
>
>diff --git a/Koha/REST/V1/Hold.pm b/Koha/REST/V1/Hold.pm
>index 7f0b1ff..c3712b5 100644
>--- a/Koha/REST/V1/Hold.pm
>+++ b/Koha/REST/V1/Hold.pm
>@@ -27,20 +27,20 @@ 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;
>     foreach my $key (keys %$params) {
>         delete $params->{$key} unless grep { $key eq $_ } @valid_params;
>     }
>-    my $holds = Koha::Holds->search($params)->unblessed;
>+    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 = C4::Reserves::GetReserve($reserve_id);
>+    my $hold = Koha::Holds->find($reserve_id);
> 
>-    return $c->$cb($reserve, 201);
>+    return $c->render( status => 201, openapi => $hold );
> }
> 
> 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;
>@@ -138,24 +139,24 @@ sub edit {
>         suspend_until => $suspend_until,
>     };
>     C4::Reserves::ModReserve($params);
>-    $reserve = C4::Reserves::GetReserve($reserve_id);
>+    $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/Koha/Schema/Result/Reserve.pm b/Koha/Schema/Result/Reserve.pm
>index 4d56006..eb5d07f 100644
>--- a/Koha/Schema/Result/Reserve.pm
>+++ b/Koha/Schema/Result/Reserve.pm
>@@ -334,4 +334,9 @@ __PACKAGE__->belongs_to(
>   },
> );
> 
>+__PACKAGE__->add_columns(
>+    '+lowestPriority' => { is_boolean => 1 },
>+    '+suspend' => { is_boolean => 1 }
>+);
>+
> 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": {
>-- 
>1.9.1

Actions: View | Diff | Splinter Review

Attachments on bug 18137: 60490 | 60491 | 60492 | 60493 | 60494 | 60495 | 60533 | 60837 | 60838 | 60839 | 60840 | 60841 | 60842 | 60843 | 60866 | 60867 | 61163 | 61166 | 62780 | 62781 | 62782 | 62783 | 62784 | 62785 | 62786 | 62787 | 62788 | 62789 | 62816 | 63025 | 63026 | 63027 | 63028 | 63029 | 63030 | 63031 | 63032 | 63033 | 63034 | 63177 | 63178 | 63179 | 63180 | 63181 | 63182 | 63183 | 63184 | 63185 | 63186 | 63187 | 63188 | 63189 | 63190 | 63191 | 63192 | 63193 | 63194 | 63195 | 63196 | 63283 | 65106 | 65107 | 65108 | 65109 | 65110 | 65111 | 65112 | 65113 | 65114 | 65115 | 65404 | 65713 | 65714 | 66802 | 66803 | 66804 | 66805 | 66806 | 66807 | 66808 | 66809 | 66810 | 66811 | 66812 | 66813 | 66814 | 66815 | 66816 | 66817 | 66818 | 66819