From 20742c1bc9937bcb14a90864108bf7abda009a8a Mon Sep 17 00:00:00 2001 From: Jiri Kozlovsky 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 This patch on top of previous changes adds the condition that the old issue included in any response must have itemnumber. Those which does not have itemnumber does not count as such old issue is completely useless. --- Koha/REST/V1/Checkout.pm | 49 +++ api/v1/swagger.json | 516 +++++++++++++++++++++++++++++++ api/v1/swagger/paths.json | 6 + api/v1/swagger/paths/checkouts.json | 54 ++++ t/db_dependent/api/v1/checkoutshistory.t | 160 ++++++++++ 5 files changed, 785 insertions(+) create mode 100644 api/v1/swagger.json 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 ac5dea5..5234223 100644 --- a/Koha/REST/V1/Checkout.pm +++ b/Koha/REST/V1/Checkout.pm @@ -23,6 +23,7 @@ use C4::Auth qw( haspermission ); use C4::Context; use C4::Circulation; use Koha::Issues; +use Koha::OldIssues; sub list { my ($c, $args, $cb) = @_; @@ -86,4 +87,52 @@ 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'); + + my %attributes = ( itemnumber => { "!=", undef } ); + if ($borrowernumber) { + return $c->$cb({ + error => "Patron doesn't exist" + }, 404) unless Koha::Patrons->find($borrowernumber); + + $attributes{borrowernumber} = $borrowernumber; + } + + # Retrieve all the issues in the history, but only the issue_id due to possible perfomance issues + my $checkouts = Koha::OldIssues->search( + \%attributes, + { 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/swagger.json b/api/v1/swagger.json new file mode 100644 index 0000000..0928ee7 --- /dev/null +++ b/api/v1/swagger.json @@ -0,0 +1,516 @@ +{ + "swagger": "2.0", + "info": { + "title": "Koha REST API", + "version": "1", + "license": { + "name": "GPL v3", + "url": "http://www.gnu.org/licenses/gpl.txt" + }, + "contact": { + "name": "Koha Team", + "url": "http://koha-community.org/" + } + }, + "basePath": "/api/v1", + "paths": { + "/patrons": { + "get": { + "operationId": "listPatrons", + "tags": ["patrons"], + "produces": [ + "application/json" + ], + "responses": { + "200": { + "description": "A list of patrons", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/patron" + } + } + }, + "403": { + "description": "Access forbidden", + "schema": { + "$ref": "#/definitions/error" + } + } + } + } + }, + "/patrons/{borrowernumber}": { + "get": { + "operationId": "getPatron", + "tags": ["patrons"], + "parameters": [ + { + "$ref": "#/parameters/borrowernumberPathParam" + } + ], + "produces": [ + "application/json" + ], + "responses": { + "200": { + "description": "A patron", + "schema": { + "$ref": "#/definitions/patron" + } + }, + "403": { + "description": "Access forbidden", + "schema": { + "$ref": "#/definitions/error" + } + }, + "404": { + "description": "Patron not found", + "schema": { + "$ref": "#/definitions/error" + } + } + } + } + }, + "/holds": { + "get": { + "operationId": "listHolds", + "tags": ["borrowers", "holds"], + "parameters": [ + { + "name": "reserve_id", + "in": "query", + "description": "Internal reserve identifier", + "type": "integer" + }, + { + "name": "borrowernumber", + "in": "query", + "description": "Internal borrower identifier", + "type": "integer" + }, + { + "name": "reservedate", + "in": "query", + "description": "Reserve date", + "type": "string" + }, + { + "name": "biblionumber", + "in": "query", + "description": "Internal biblio identifier", + "type": "integer" + }, + { + "name": "branchcode", + "in": "query", + "description": "Branch code", + "type": "string" + }, + { + "name": "notificationdate", + "in": "query", + "description": "Notification date", + "type": "string" + }, + { + "name": "reminderdate", + "in": "query", + "description": "Reminder date", + "type": "string" + }, + { + "name": "cancellationdate", + "in": "query", + "description": "Cancellation date", + "type": "string" + }, + { + "name": "reservenotes", + "in": "query", + "description": "Reserve notes", + "type": "string" + }, + { + "name": "priority", + "in": "query", + "description": "Priority", + "type": "integer" + }, + { + "name": "found", + "in": "query", + "description": "Found status", + "type": "string" + }, + { + "name": "timestamp", + "in": "query", + "description": "Time of latest update", + "type": "string" + }, + { + "name": "itemnumber", + "in": "query", + "description": "Internal item identifier", + "type": "integer" + }, + { + "name": "waitingdate", + "in": "query", + "description": "Date the item was marked as waiting for the patron", + "type": "string" + }, + { + "name": "expirationdate", + "in": "query", + "description": "Date the hold expires", + "type": "string" + }, + { + "name": "lowestPriority", + "in": "query", + "description": "Lowest priority", + "type": "integer" + }, + { + "name": "suspend", + "in": "query", + "description": "Suspended", + "type": "integer" + }, + { + "name": "suspend_until", + "in": "query", + "description": "Suspended until", + "type": "string" + } + ], + "produces": ["application/json"], + "responses": { + "200": { + "description": "A list of holds", + "schema": { "$ref": "#/definitions/holds" } + }, + "404": { + "description": "Borrower not found", + "schema": { "$ref": "#/definitions/error" } + } + } + }, + "post": { + "operationId": "addHold", + "tags": ["borrowers", "holds"], + "parameters": [ + { + "name": "body", + "in": "body", + "description": "A JSON object containing informations about the new hold", + "required": true, + "schema": { + "type": "object", + "properties": { + "borrowernumber": { + "description": "Borrower internal identifier", + "type": "integer" + }, + "biblionumber": { + "description": "Biblio internal identifier", + "type": "integer" + }, + "itemnumber": { + "description": "Item internal identifier", + "type": "integer" + }, + "branchcode": { + "description": "Pickup location", + "type": "string" + }, + "expirationdate": { + "description": "Hold end date", + "type": "string", + "format": "date" + } + } + } + } + ], + "consumes": ["application/json"], + "produces": ["application/json"], + "responses": { + "201": { + "description": "Created hold", + "schema": { "$ref": "#/definitions/hold" } + }, + "400": { + "description": "Missing or wrong parameters", + "schema": { "$ref": "#/definitions/error" } + }, + "403": { + "description": "Hold not allowed", + "schema": { "$ref": "#/definitions/error" } + }, + "404": { + "description": "Borrower not found", + "schema": { "$ref": "#/definitions/error" } + }, + "500": { + "description": "Internal error", + "schema": { "$ref": "#/definitions/error" } + } + } + } + }, + "/holds/{reserve_id}": { + "put": { + "operationId": "editHold", + "tags": ["holds"], + "parameters": [ + { "$ref": "#/parameters/holdIdPathParam" }, + { + "name": "body", + "in": "body", + "description": "A JSON object containing fields to modify", + "required": true, + "schema": { + "type": "object", + "properties": { + "priority": { + "description": "Position in waiting queue", + "type": "integer", + "minimum": 1 + }, + "branchcode": { + "description": "Pickup location", + "type": "string" + }, + "suspend_until": { + "description": "Suspend until", + "type": "string", + "format": "date" + } + } + } + } + ], + "consumes": ["application/json"], + "produces": ["application/json"], + "responses": { + "200": { + "description": "Updated hold", + "schema": { "$ref": "#/definitions/hold" } + }, + "400": { + "description": "Missing or wrong parameters", + "schema": { "$ref": "#/definitions/error" } + }, + "404": { + "description": "Hold not found", + "schema": { "$ref": "#/definitions/error" } + } + } + }, + "delete": { + "operationId": "deleteHold", + "tags": ["holds"], + "parameters": [ + { "$ref": "#/parameters/holdIdPathParam" } + ], + "produces": ["application/json"], + "responses": { + "200": { + "description": "Successful deletion", + "schema": { + "type": "object" + } + }, + "404": { + "description": "Hold not found", + "schema": { "$ref": "#/definitions/error" } + } + } + } + }, + "/checkouts": { + "get": { + "operationId": "listCheckouts", + "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", + "schema": { + "$ref": "#/definitions/checkouts" + } + }, + "403": { + "description": "Access forbidden", + "schema": { "$ref": "#/definitions/error" } + }, + "404": { + "description": "Borrower not found", + "schema": { + "$ref": "#/definitions/error" + } + } + } + } + }, + "/checkouts/{checkout_id}": { + "get": { + "operationId": "getCheckout", + "tags": ["borrowers", "checkouts"], + "parameters": [ + { + "name": "checkout_id", + "in": "path", + "description": "Internal checkout identifier", + "required": true, + "type": "integer" + } + ], + "produces": ["application/json"], + "responses": { + "200": { + "description": "Updated borrower's checkout", + "schema": { "$ref": "#/definitions/checkout" } + }, + "403": { + "description": "Access forbidden", + "schema": { "$ref": "#/definitions/error" } + }, + "404": { + "description": "Checkout not found", + "schema": { "$ref": "#/definitions/error" } + } + } + }, + "put": { + "operationId": "renewCheckout", + "tags": ["borrowers", "checkouts"], + "parameters": [ + { + "name": "checkout_id", + "in": "path", + "description": "Internal checkout identifier", + "required": true, + "type": "integer" + } + ], + "produces": ["application/json"], + "responses": { + "200": { + "description": "Updated borrower's checkout", + "schema": { "$ref": "#/definitions/checkout" } + }, + "403": { + "description": "Cannot renew checkout", + "schema": { "$ref": "#/definitions/error" } + }, + "404": { + "description": "Checkout not found", + "schema": { "$ref": "#/definitions/error" } + } + } + } + }, + "/checkouts/history": { + "get": { + "operationId": "listhistoryCheckouts", + "tags": ["patrons", "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": ["patrons", "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": { + "$ref": "./definitions/index.json" + }, + "parameters": { + "borrowernumberPathParam": { + "name": "borrowernumber", + "in": "path", + "description": "Internal patron identifier", + "required": true, + "type": "integer" + }, + "holdIdPathParam": { + "name": "reserve_id", + "in": "path", + "description": "Internal hold identifier", + "required": true, + "type": "integer" + } + } +} diff --git a/api/v1/swagger/paths.json b/api/v1/swagger/paths.json index 05c29aa..57cdc97 100644 --- a/api/v1/swagger/paths.json +++ b/api/v1/swagger/paths.json @@ -11,6 +11,12 @@ "/checkouts/{checkout_id}": { "$ref": "paths/checkouts.json#/~1checkouts~1{checkout_id}" }, + "/checkouts/history": { + "$ref": "paths/checkouts.json#/~1checkouts~1history" + }, + "/checkouts/history/{checkout_id}": { + "$ref": "paths/checkouts.json#/~1checkouts~1history~1{checkout_id}" + }, "/patrons": { "$ref": "paths/patrons.json#/~1patrons" }, diff --git a/api/v1/swagger/paths/checkouts.json b/api/v1/swagger/paths/checkouts.json index e49a33e..c3cc225 100644 --- a/api/v1/swagger/paths/checkouts.json +++ b/api/v1/swagger/paths/checkouts.json @@ -93,5 +93,59 @@ } } } + }, + "/checkouts/history": { + "get": { + "operationId": "listhistoryCheckouts", + "tags": ["patrons", "checkouts"], + "parameters": [ + { "$ref": "../parameters.json#/borrowernumberQueryParam" } + ], + "produces": [ + "application/json" + ], + "responses": { + "200": { + "description": "A list of checkouts history", + "schema": { + "$ref": "../definitions.json#/checkouts" + } + }, + "403": { + "description": "Access forbidden", + "schema": { "$ref": "../definitions.json#/error" } + }, + "404": { + "description": "Borrower not found", + "schema": { + "$ref": "../definitions.json#/error" + } + } + } + } + }, + "/checkouts/history/{checkout_id}": { + "get": { + "operationId": "gethistoryCheckout", + "tags": ["patrons", "checkouts"], + "parameters": [ + { "$ref": "../parameters.json#/checkoutIdPathParam" } + ], + "produces": ["application/json"], + "responses": { + "200": { + "description": "Got borrower's checkout", + "schema": { "$ref": "../definitions.json#/checkout" } + }, + "403": { + "description": "Access forbidden", + "schema": { "$ref": "../definitions.json#/error" } + }, + "404": { + "description": "Checkout not found", + "schema": { "$ref": "../definitions.json#/error" } + } + } + } } } diff --git a/t/db_dependent/api/v1/checkoutshistory.t b/t/db_dependent/api/v1/checkoutshistory.t new file mode 100644 index 0000000..c9d988a --- /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({}) + int rand(150000); +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; +} -- 1.9.1