Attachment 53852 Details for Bug 17005 – Bug 17005: REST API: add routes to list checkouts history

[patch] Bug 17005: REST API: add routes to list checkouts history

Bug-17005-REST-API-add-routes-to-list-checkouts-hi.patch (text/plain), 11.71 KB, created by Jiri Kozlovsky on 2016-08-01 16:41:45 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jiri Kozlovsky

Created: 2016-08-01 16:41:45 UTC

Size: 11.71 KB

patch

obsolete

>From facfd9ad1cb204f84f793b1e6e3bb06520ce8eee Mon Sep 17 00:00:00 2001
>From: Jiri Kozlovsky <mail@jkozlovsky.cz>
>Date: Mon, 1 Aug 2016 17:17:17 +0200
>Subject: [PATCH] Bug 17005: REST API: add routes to list checkouts history
>
>GET /checkouts/history                                 (get all old_issues' issue_id)
>GET /checkouts/history?borrowernumber={borrowernumber} (get all borrower's old_issues' issue_id)
>GET /checkouts/history/{checkout_id}                   (get old_issue)
>
>+ unit tests in t/db_dependent/api/v1/checkoutshistory.t
>
>Test plan:
>1. Open a browser tab on Koha staff and log in (to create CGISESSID
>   cookie). You should have permission circulate_remaining_permissions.
>2. Go to http://yourlibrary/api/v1/checkouts/history?borrowernumber=XXX
>   (replace XXX with a borrowernumber that has some checkouts history)
>   and check you receive all the issues from patron's history
>3. Go to http://yourlibrary/api/v1/checkouts/history/YYY (replace YYY
>   with an existing checkout id) and check you receive correct data
>4. Run unit tests in t/db_dependent/api/v1/checkoutshistory.t
>
>Now the endpoint with borrowernumber does not return all the details
>from within the old issues, but only their issue_id. This is made in
>order to enhance API performance. E.g. VuFind can retrieve all the
>issue_ids & use AJAX to query details about those, so user sees how the
>details come up one by one & does not have to wait for all of them at
>once (could be very long with really old users)
>
>In order to get issue details, use the endpoint with the checkout_id.
>---
> Koha/REST/V1/Checkout.pm                 |  47 +++++++++
> api/v1/definitions/checkout.json         |   4 +-
> api/v1/swagger.json                      |  66 +++++++++++++
> t/db_dependent/api/v1/checkoutshistory.t | 160 +++++++++++++++++++++++++++++++
> 4 files changed, 275 insertions(+), 2 deletions(-)
> create mode 100644 t/db_dependent/api/v1/checkoutshistory.t
>
>diff --git a/Koha/REST/V1/Checkout.pm b/Koha/REST/V1/Checkout.pm
>index 68046a6..ce4efd7 100644
>--- a/Koha/REST/V1/Checkout.pm
>+++ b/Koha/REST/V1/Checkout.pm
>@@ -22,6 +22,7 @@ use Mojo::Base 'Mojolicious::Controller';
> use C4::Auth qw( haspermission );
> use C4::Circulation;
> use Koha::Issues;
>+use Koha::OldIssues;
> 
> sub list {
>     my ($c, $args, $cb) = @_;
>@@ -94,4 +95,50 @@ sub renew {
>     return $c->$cb($checkout->unblessed, 200);
> }
> 
>+sub listhistory {
>+    my ($c, $args, $cb) = @_;
>+
>+    my $user = $c->stash('koha.user');
>+    unless ($user && haspermission($user->userid, { circulate => "circulate_remaining_permissions" })) {
>+        return $c->$cb({error => "You don't have the required permission"}, 403);
>+    }
>+
>+    my $borrowernumber = $c->param('borrowernumber');
>+
>+    if ($borrowernumber) {
>+        return $c->$cb({
>+            error => "Patron doesn't exist"
>+        }, 404) unless Koha::Patrons->find($borrowernumber);
>+    }
>+
>+    # Retrieve all the issues in the history, but only the issue_id due to possible perfomance issues
>+    my $checkouts = Koha::OldIssues->search( 
>+      $borrowernumber ?
>+      { borrowernumber => $borrowernumber } : {},
>+      { columns => [qw/issue_id/]}
>+    );
>+
>+    $c->$cb($checkouts->unblessed, 200);
>+}
>+
>+sub gethistory {
>+    my ($c, $args, $cb) = @_;
>+
>+    my $user = $c->stash('koha.user');
>+    unless ($user && haspermission($user->userid, { circulate => "circulate_remaining_permissions" })) {
>+        return $c->$cb({error => "You don't have the required permission"}, 403);
>+    }
>+
>+    my $checkout_id = $args->{checkout_id};
>+    my $checkout = Koha::OldIssues->find($checkout_id);
>+
>+    if (!$checkout) {
>+        return $c->$cb({
>+            error => "Checkout doesn't exist"
>+        }, 404);
>+    }
>+
>+    return $c->$cb($checkout->unblessed, 200);
>+}
>+
> 1;
>diff --git a/api/v1/definitions/checkout.json b/api/v1/definitions/checkout.json
>index c56c6df..24bfec3 100644
>--- a/api/v1/definitions/checkout.json
>+++ b/api/v1/definitions/checkout.json
>@@ -6,11 +6,11 @@
>           "description": "internally assigned checkout identifier"
>         },
>         "borrowernumber": {
>-            "type": "string",
>+            "type": ["string", "null"],
>             "description": "internally assigned user identifier"
>         },
>         "itemnumber": {
>-            "type": "string",
>+            "type": ["string", "null"],
>             "description": "internally assigned item identifier"
>         },
>         "date_due": {
>diff --git a/api/v1/swagger.json b/api/v1/swagger.json
>index 68eeea6..c590613 100644
>--- a/api/v1/swagger.json
>+++ b/api/v1/swagger.json
>@@ -426,6 +426,72 @@
>           }
>         }
>       }
>+    },
>+    "/checkouts/history": {
>+      "get": {
>+        "operationId": "listhistoryCheckouts",
>+        "tags": ["borrowers", "checkouts"],
>+        "parameters": [
>+          {
>+            "name": "borrowernumber",
>+            "in": "query",
>+            "description": "Internal patron identifier",
>+            "required": false,
>+            "type": "integer"
>+          }
>+        ],
>+        "produces": [
>+          "application/json"
>+        ],
>+        "responses": {
>+          "200": {
>+            "description": "A list of checkouts history",
>+            "schema": {
>+              "$ref": "#/definitions/checkouts"
>+            }
>+          },
>+          "403": {
>+            "description": "Access forbidden",
>+            "schema": { "$ref": "#/definitions/error" }
>+          },
>+          "404": {
>+            "description": "Borrower not found",
>+            "schema": {
>+              "$ref": "#/definitions/error"
>+            }
>+          }
>+        }
>+      }
>+    },
>+    "/checkouts/history/{checkout_id}": {
>+      "get": {
>+        "operationId": "gethistoryCheckout",
>+        "tags": ["borrowers", "checkouts"],
>+        "parameters": [
>+          {
>+            "name": "checkout_id",
>+            "in": "path",
>+            "description": "Internal checkout identifier",
>+            "required": true,
>+            "type": "integer"
>+          }
>+        ],
>+        "produces": ["application/json"],
>+        "responses": {
>+          "200": {
>+            "description": "Got borrower's checkout",
>+            "schema": { "$ref": "#/definitions/checkout" }
>+          },
>+          "403": {
>+            "description": "Access forbidden",
>+            "schema": { "$ref": "#/definitions/error" }
>+          },
>+          "404": {
>+            "description": "Checkout not found",
>+            "schema": { "$ref": "#/definitions/error" }
>+          }
>+        }
>+      }
>     }
>   },
>   "definitions": {
>diff --git a/t/db_dependent/api/v1/checkoutshistory.t b/t/db_dependent/api/v1/checkoutshistory.t
>new file mode 100644
>index 0000000..6d57f03
>--- /dev/null
>+++ b/t/db_dependent/api/v1/checkoutshistory.t
>@@ -0,0 +1,160 @@
>+#!/usr/bin/env perl
>+
>+# This file is part of Koha.
>+#
>+# Koha is free software; you can redistribute it and/or modify it under the
>+# terms of the GNU General Public License as published by the Free Software
>+# Foundation; either version 3 of the License, or (at your option) any later
>+# version.
>+#
>+# Koha is distributed in the hope that it will be useful, but WITHOUT ANY
>+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
>+# A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
>+#
>+# You should have received a copy of the GNU General Public License along
>+# with Koha; if not, write to the Free Software Foundation, Inc.,
>+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>+
>+use Modern::Perl;
>+
>+use Test::More tests => 27;
>+use Test::MockModule;
>+use Test::Mojo;
>+use t::lib::TestBuilder;
>+
>+use DateTime;
>+use MARC::Record;
>+
>+use C4::Context;
>+use C4::Biblio;
>+use C4::Circulation;
>+use C4::Items;
>+
>+use Koha::Database;
>+use Koha::Patron;
>+use Koha::OldIssue;
>+use Koha::OldIssues;
>+
>+my $schema = Koha::Database->schema;
>+$schema->storage->txn_begin;
>+my $dbh = C4::Context->dbh;
>+my $builder = t::lib::TestBuilder->new;
>+$dbh->{RaiseError} = 1;
>+
>+$ENV{REMOTE_ADDR} = '127.0.0.1';
>+my $t = Test::Mojo->new('Koha::REST::V1');
>+
>+$dbh->do('DELETE FROM issues');
>+$dbh->do('DELETE FROM items');
>+$dbh->do('DELETE FROM issuingrules');
>+my $loggedinuser = $builder->build({ source => 'Borrower' });
>+
>+$dbh->do(q{
>+    INSERT INTO user_permissions (borrowernumber, module_bit, code)
>+    VALUES (?, 1, 'circulate_remaining_permissions')
>+}, undef, $loggedinuser->{borrowernumber});
>+
>+my $session = C4::Auth::get_session('');
>+$session->param('number', $loggedinuser->{ borrowernumber });
>+$session->param('id', $loggedinuser->{ userid });
>+$session->param('ip', '127.0.0.1');
>+$session->param('lasttime', time());
>+$session->flush;
>+
>+my $borrower = $builder->build({ source => 'Borrower' });
>+my $borrowernumber = $borrower->{borrowernumber};
>+
>+my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode };
>+my $module = new Test::MockModule('C4::Context');
>+$module->mock('userenv', sub { { branch => $branchcode } });
>+
>+my $tx = $t->ua->build_tx(GET => "/api/v1/checkouts/history?borrowernumber=$borrowernumber");
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(200)
>+  ->json_is([]);
>+
>+my $notexisting_borrowernumber = $borrowernumber + 1;
>+$tx = $t->ua->build_tx(GET => "/api/v1/checkouts/history?borrowernumber=$notexisting_borrowernumber");
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(404)
>+  ->json_has('/error');
>+
>+my $biblionumber = create_biblio('RESTful Web APIs');
>+my $itemnumber1 = create_item($biblionumber, 'TEST000001');
>+my $itemnumber2 = create_item($biblionumber, 'TEST000002');
>+
>+my $date_due = DateTime->now->add(weeks => 2);
>+
>+my $issueId = Koha::OldIssues->count({}) + 1500;
>+my $issue1;
>+$issue1 = Koha::OldIssue->new({ issue_id => $issueId, borrowernumber => $borrowernumber, itemnumber => $itemnumber1, date_due => $date_due});
>+$issue1->store();
>+
>+my $date_due1 = Koha::DateUtils::dt_from_string( $issue1->date_due );
>+
>+$tx = $t->ua->build_tx(GET => "/api/v1/checkouts/history?borrowernumber=$borrowernumber");
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(200)
>+  ->json_is('/0/issue_id' => $issueId)
>+  ->json_hasnt('/1')
>+  ->json_hasnt('/error');
>+
>+$tx = $t->ua->build_tx(GET => "/api/v1/checkouts/history/" . $issue1->issue_id);
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(200)
>+  ->json_is('/borrowernumber' => $borrowernumber)
>+  ->json_is('/itemnumber' => $itemnumber1)
>+  ->json_is('/date_due' => $date_due1->ymd . ' ' . $date_due1->hms)
>+  ->json_hasnt('/error');
>+
>+$issue1->delete();
>+
>+$tx = $t->ua->build_tx(GET => "/api/v1/checkouts/history/" . $issue1->issue_id);
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(404)
>+  ->json_has('/error');
>+
>+$tx = $t->ua->build_tx(GET => "/api/v1/checkouts/history?borrowernumber=$borrowernumber");
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(200)
>+  ->json_hasnt('/0')
>+  ->json_hasnt('/error');
>+
>+Koha::Patrons->find($borrowernumber)->delete();
>+
>+$tx = $t->ua->build_tx(GET => "/api/v1/checkouts/history?borrowernumber=$borrowernumber");
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(404)
>+  ->json_has('/error');
>+
>+sub create_biblio {
>+    my ($title) = @_;
>+
>+    my $record = new MARC::Record;
>+    $record->append_fields(
>+        new MARC::Field('200', ' ', ' ', a => $title),
>+    );
>+
>+    my ($biblionumber) = C4::Biblio::AddBiblio($record, '');
>+
>+    return $biblionumber;
>+}
>+
>+sub create_item {
>+    my ($biblionumber, $barcode) = @_;
>+
>+    my $item = {
>+        barcode => $barcode,
>+    };
>+
>+    my $itemnumber = C4::Items::AddItem($item, $biblionumber);
>+
>+    return $itemnumber;
>+}
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 17005: 53843 | 53851 | 53852 | 53858 | 54198 | 56698 | 56699 | 61197 | 87254 | 87255 | 87383 | 87384 | 87385 | 87405 | 87406 | 87407 | 87442 | 87443 | 87444 | 87447 | 87448 | 87449 | 88263 | 88264 | 88265 | 88329 | 88330 | 88331 | 91166 | 91167 | 91168 | 91235 | 91236 | 91237 | 91238 | 93757 | 93758 | 93759 | 93760