Attachment 87446 Details for Bug 17003 – Bug 17003: (follow-up) Update definitions according to voted RFC

[patch] Bug 17003: (follow-up) Update definitions according to voted RFC

Bug-17003-follow-up-Update-definitions-according-t.patch (text/plain), 7.75 KB, created by Michal Denar on 2019-04-05 11:45:22 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Michal Denar

Created: 2019-04-05 11:45:22 UTC

Size: 7.75 KB

patch

obsolete

>From 6e59ee74687b3542ffa9fcf092d2fc563a1d2c19 Mon Sep 17 00:00:00 2001
>From: Josef Moravec <josef.moravec@gmail.com>
>Date: Fri, 29 Mar 2019 07:53:27 +0000
>Subject: [PATCH] Bug 17003: (follow-up) Update definitions according to voted
> RFC
>
>Test plan:
>1) Have some patrons with checkouts, some renewable and some not
>renewable
>2) Use your favorite API tester and access GET
>http://koha.url/api/v1/checkouts/{checkout_id}/allows_renewal
>3) Check the response is OK according to voted RFC:
>https://wiki.koha-community.org/wiki/Checkouts_endpoint_RFC#Checkout_renewability_2
>4) prove t/db_dependent/api/v1/checkouts.t
>
>Signed-off-by: Michal Denar <black23@gmail.com>
>
>Signed-off-by: Michal Denar <black23@gmail.com>
>---
> Koha/REST/V1/Checkouts.pm                      | 24 ++++++++++++++++++++++--
> api/v1/swagger/definitions.json                |  4 ++--
> api/v1/swagger/definitions/allows_renewal.json | 21 +++++++++++++++++++++
> api/v1/swagger/definitions/renewability.json   | 13 -------------
> api/v1/swagger/paths.json                      |  4 ++--
> api/v1/swagger/paths/checkouts.json            | 12 ++++++------
> t/db_dependent/api/v1/checkouts.t              | 18 ++++++++++++++----
> 7 files changed, 67 insertions(+), 29 deletions(-)
> create mode 100644 api/v1/swagger/definitions/allows_renewal.json
> delete mode 100644 api/v1/swagger/definitions/renewability.json
>
>diff --git a/Koha/REST/V1/Checkouts.pm b/Koha/REST/V1/Checkouts.pm
>index b1bc8632e6..bb349a78ec 100644
>--- a/Koha/REST/V1/Checkouts.pm
>+++ b/Koha/REST/V1/Checkouts.pm
>@@ -24,6 +24,7 @@ use C4::Auth qw( haspermission );
> use C4::Context;
> use C4::Circulation;
> use Koha::Checkouts;
>+use Koha::IssuingRules;
> 
> use Try::Tiny;
> 
>@@ -128,7 +129,13 @@ sub renew {
>     );
> }
> 
>-sub renewability {
>+=head3 allows_renewal
>+
>+Checks if the checkout could be renewed and return the related information.
>+
>+=cut
>+
>+sub allows_renewal {
>     my $c = shift->openapi->valid_input or return;
> 
>     my $checkout_id = $c->validation->param('checkout_id');
>@@ -146,9 +153,22 @@ sub renewability {
> 
>     my $renewable = Mojo::JSON->false;
>     $renewable = Mojo::JSON->true if $can_renew;
>+
>+    my $rule = Koha::IssuingRules->get_effective_issuing_rule(
>+        {
>+            categorycode => $checkout->patron->categorycode,
>+            itemtype     => $checkout->item->effective_itemtype,
>+            branchcode   => $checkout->branchcode,
>+        }
>+    );
>     return $c->render(
>         status => 200,
>-        openapi => { renewable => $renewable, error => $error }
>+        openapi => {
>+            allows_renewal => $renewable,
>+            max_renewals => $rule->renewalsallowed,
>+            current_renewals => $checkout->renewals,
>+            error => $error
>+        }
>     );
> }
> 
>diff --git a/api/v1/swagger/definitions.json b/api/v1/swagger/definitions.json
>index 8fe086da7e..e3001b5e60 100644
>--- a/api/v1/swagger/definitions.json
>+++ b/api/v1/swagger/definitions.json
>@@ -32,8 +32,8 @@
>   "patron_balance": {
>     "$ref": "definitions/patron_balance.json"
>   },
>-  "renewability": {
>-    "$ref": "definitions/renewability.json"
>+  "allows_renewal": {
>+    "$ref": "definitions/allows_renewal.json"
>   },
>   "vendor": {
>     "$ref": "definitions/vendor.json"
>diff --git a/api/v1/swagger/definitions/allows_renewal.json b/api/v1/swagger/definitions/allows_renewal.json
>new file mode 100644
>index 0000000000..6db074b3fe
>--- /dev/null
>+++ b/api/v1/swagger/definitions/allows_renewal.json
>@@ -0,0 +1,21 @@
>+{
>+  "type": "object",
>+  "properties": {
>+    "allows_renewal": {
>+      "type": "boolean",
>+      "description": "Renewability status; true = renewable, false = not renewable"
>+    },
>+    "max_renewals": {
>+      "type": "integer",
>+      "description": "Maximum number of possible renewals"
>+    },
>+     "current_renewals": {
>+      "type": "integer",
>+      "description": "Current used renewals"
>+    },
>+    "error": {
>+      "type": ["string", "null"],
>+      "description": "Description on false allows_renewal."
>+    }
>+  }
>+}
>diff --git a/api/v1/swagger/definitions/renewability.json b/api/v1/swagger/definitions/renewability.json
>deleted file mode 100644
>index 654691e8b4..0000000000
>--- a/api/v1/swagger/definitions/renewability.json
>+++ /dev/null
>@@ -1,13 +0,0 @@
>-{
>-  "type": "object",
>-  "properties": {
>-    "renewable": {
>-      "type": "boolean",
>-      "description": "Renewability status; true = renewable, false = not renewable"
>-    },
>-    "error": {
>-      "type": ["string", "null"],
>-      "description": "Description on false renewability."
>-    }
>-  }
>-}
>diff --git a/api/v1/swagger/paths.json b/api/v1/swagger/paths.json
>index 421071154e..605430996b 100644
>--- a/api/v1/swagger/paths.json
>+++ b/api/v1/swagger/paths.json
>@@ -41,8 +41,8 @@
>   "/libraries/{library_id}": {
>     "$ref": "paths/libraries.json#/~1libraries~1{library_id}"
>   },
>-  "/checkouts/{checkout_id}/renewability": {
>-    "$ref": "paths/checkouts.json#/~1checkouts~1{checkout_id}~1renewability"
>+  "/checkouts/{checkout_id}/allows_renewal": {
>+    "$ref": "paths/checkouts.json#/~1checkouts~1{checkout_id}~1allows_renewal"
>   },
>   "/patrons": {
>     "$ref": "paths/patrons.json#/~1patrons"
>diff --git a/api/v1/swagger/paths/checkouts.json b/api/v1/swagger/paths/checkouts.json
>index 4aa4f12c9d..70791fd28c 100644
>--- a/api/v1/swagger/paths/checkouts.json
>+++ b/api/v1/swagger/paths/checkouts.json
>@@ -93,19 +93,19 @@
>       }
>     }
>   },
>-  "/checkouts/{checkout_id}/renewability": {
>+  "/checkouts/{checkout_id}/allows_renewal": {
>     "get": {
>-      "x-mojo-to": "Checkout#renewability",
>-      "operationId": "renewabilityCheckout",
>+      "x-mojo-to": "Checkouts#allows_renewal",
>+      "operationId": "allows_renewalCheckout",
>       "tags": ["patrons", "checkouts"],
>       "parameters": [{
>-        "$ref": "../parameters.json#/checkoutIdPathParam"
>+        "$ref": "../parameters.json#/checkout_id_pp"
>       }],
>       "produces": ["application/json"],
>       "responses": {
>         "200": {
>-          "description": "Checkout renewability",
>-          "schema": { "$ref": "../definitions.json#/renewability" }
>+          "description": "Checkout renewability information",
>+          "schema": { "$ref": "../definitions.json#/allows_renewal" }
>         },
>         "403": {
>           "description": "Forbidden",
>diff --git a/t/db_dependent/api/v1/checkouts.t b/t/db_dependent/api/v1/checkouts.t
>index ea513d5061..9e36ff804b 100644
>--- a/t/db_dependent/api/v1/checkouts.t
>+++ b/t/db_dependent/api/v1/checkouts.t
>@@ -144,9 +144,14 @@ $t->post_ok( "//$unauth_userid:$unauth_password@/api/v1/checkouts/" . $issue3->i
>               required_permissions => { circulate => "circulate_remaining_permissions" }
>             });
> 
>-$t->get_ok( "//$userid:$password@/api/v1/checkouts/" . $issue2->issue_id . "/renewability")
>+$t->get_ok( "//$userid:$password@/api/v1/checkouts/" . $issue2->issue_id . "/allows_renewal")
>   ->status_is(200)
>-  ->json_is({ renewable => Mojo::JSON->true, error => undef });
>+  ->json_is({
>+        allows_renewal   => Mojo::JSON->true,
>+        max_renewals     => 1,
>+        current_renewals => 0,
>+        error            => undef
>+    });
> 
> $t->post_ok( "//$userid:$password@/api/v1/checkouts/" . $issue2->issue_id . "/renewal" )
>   ->status_is(201)
>@@ -158,6 +163,11 @@ $t->post_ok( "//$userid:$password@/api/v1/checkouts/" . $issue1->issue_id . "/re
>   ->status_is(403)
>   ->json_is({ error => 'Renewal not authorized (too_many)' });
> 
>-$t->get_ok( "//$userid:$password@/api/v1/checkouts/" . $issue2->issue_id . "/renewability")
>+$t->get_ok( "//$userid:$password@/api/v1/checkouts/" . $issue2->issue_id . "/allows_renewal")
>   ->status_is(200)
>-  ->json_is({ renewable => Mojo::JSON->false, error => 'too_many' });
>+  ->json_is({
>+        allows_renewal   => Mojo::JSON->false,
>+        max_renewals     => 1,
>+        current_renewals => 1,
>+        error            => 'too_many'
>+    });
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 17003: 54709 | 54710 | 56507 | 57489 | 57493 | 58099 | 85475 | 85476 | 87155 | 87156 | 87157 | 87445 | 87446 | 88260 | 88261 | 88262 | 88589 | 88590 | 88591 | 91102 | 91103 | 91104 | 91105