From f3fb1786aed53a936ef5538c5f3d705c626554f9 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Mon, 27 Jun 2016 16:03:49 +0300 Subject: [PATCH] Bug 16825: Add API route for getting an item GET /api/v1/items/{itemnumber} Gets one Item This patch adds route to get one item from koha.items table. This table has a column "itemnotes_nonpublic", which should not be returned if the user has no catalogue permission. The OpacHiddenItems syspref restriction is considered for users who are not staff. They see "Item Not Found" when trying to view item hidden from OPAC. Only staff can see those items. To test: 1. Apply patch 2. Open a browser tab on Koha staff and log in (to create CGISESSID cookie). 3. Send GET request to http://yourlibrary/api/v1/items/YYY where YYY is an existing itemnumber. 4. Make sure the returned data is correct. 5. Run unit tests in t/db_dependent/api/v1/items.t --- Koha/REST/V1/Item.pm | 72 ++++++++++++++ api/v1/swagger/definitions.json | 3 + api/v1/swagger/definitions/hold.json | 3 +- api/v1/swagger/definitions/item.json | 188 +++++++++++++++++++++++++++++++++++ api/v1/swagger/parameters.json | 3 + api/v1/swagger/parameters/item.json | 9 ++ api/v1/swagger/paths.json | 3 + api/v1/swagger/paths/items.json | 47 +++++++++ api/v1/swagger/x-primitives.json | 6 +- t/db_dependent/api/v1/items.t | 151 ++++++++++++++++++++++++++++ 10 files changed, 483 insertions(+), 2 deletions(-) create mode 100644 Koha/REST/V1/Item.pm create mode 100644 api/v1/swagger/definitions/item.json create mode 100644 api/v1/swagger/parameters/item.json create mode 100644 api/v1/swagger/paths/items.json create mode 100644 t/db_dependent/api/v1/items.t diff --git a/Koha/REST/V1/Item.pm b/Koha/REST/V1/Item.pm new file mode 100644 index 0000000..7e00a9e --- /dev/null +++ b/Koha/REST/V1/Item.pm @@ -0,0 +1,72 @@ +package Koha::REST::V1::Item; + +# 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 Mojo::Base 'Mojolicious::Controller'; +use Mojo::JSON; + +use C4::Auth qw( haspermission ); +use C4::Items qw( GetHiddenItemnumbers ); + +use Koha::Items; + +use Try::Tiny; + +sub get { + my $c = shift->openapi->valid_input or return; + + my $item; + try { + $item = Koha::Items->find($c->validation->param('itemnumber')); + + # Hide non-public itemnotes if user has no staff access + my $user = $c->stash('koha.user'); + unless ($user && haspermission($user->userid, {catalogue => 1})) { + + my @hiddenitems = C4::Items::GetHiddenItemnumbers( ({ + itemnumber => $item->itemnumber}) ); + my %hiddenitems = map { $_ => 1 } @hiddenitems; + + # Pretend it was not found as it's hidden from OPAC to regular users + if ($hiddenitems{$item->itemnumber}) { + return $c->render( status => 404, + openapi => { error => 'Item not found'} ) ; + } + + $item->set({ itemnotes_nonpublic => undef }); + } + + return $c->render( status => 200, openapi => $item ); + } + catch { + unless ( defined $item ) { + return $c->render( status => 404, + openapi => { error => 'Item not found'} ); + } + if ( $_->isa('DBIx::Class::Exception') ) { + return $c->render( status => 500, + openapi => { error => $_->{msg} } ); + } + else { + return $c->render( status => 500, + openapi => { error => "Something went wrong, check the logs."} ); + } + }; +} + +1; diff --git a/api/v1/swagger/definitions.json b/api/v1/swagger/definitions.json index 939baa8..7563a06 100644 --- a/api/v1/swagger/definitions.json +++ b/api/v1/swagger/definitions.json @@ -11,6 +11,9 @@ "holds": { "$ref": "definitions/holds.json" }, + "item": { + "$ref": "definitions/item.json" + }, "patron": { "$ref": "definitions/patron.json" } diff --git a/api/v1/swagger/definitions/hold.json b/api/v1/swagger/definitions/hold.json index 5f41318..9da485e 100644 --- a/api/v1/swagger/definitions/hold.json +++ b/api/v1/swagger/definitions/hold.json @@ -47,7 +47,8 @@ "description": "date and time the hold was last updated" }, "itemnumber": { - "$ref": "../x-primitives.json#/itemnumber" + "type": "integer", + "description": "internally assigned item identifier" }, "waitingdate": { "type": ["string", "null"], diff --git a/api/v1/swagger/definitions/item.json b/api/v1/swagger/definitions/item.json new file mode 100644 index 0000000..f7f1d71 --- /dev/null +++ b/api/v1/swagger/definitions/item.json @@ -0,0 +1,188 @@ +{ + "type": "object", + "properties": { + "itemnumber": { + "$ref": "../x-primitives.json#/itemnumber" + }, + "biblionumber": { + "$ref": "../x-primitives.json#/biblionumber" + }, + "biblioitemnumber": { + "$ref": "../x-primitives.json#/biblioitemnumber" + }, + "barcode": { + "type": ["string", "null"], + "description": "item barcode" + }, + "dateaccessioned": { + "type": ["string", "null"], + "format": "date", + "description": "date the item was acquired or added to Koha" + }, + "booksellerid": { + "type": ["string", "null"], + "description": "where the item was purchased" + }, + "homebranch": { + "type": ["string", "null"], + "description": "library that owns this item" + }, + "price": { + "type": ["number", "null"], + "description": "purchase price" + }, + "replacementprice": { + "type": ["number", "null"], + "description": "cost the library charges to replace the item if it has been marked lost" + }, + "replacementpricedate": { + "type": ["string", "null"], + "format": "date", + "description": "the date the price is effective from" + }, + "datelastborrowed": { + "type": ["string", "null"], + "format": "date", + "description": "the date the item was last checked out/issued" + }, + "datelastseen": { + "type": ["string", "null"], + "format": "date", + "description": "the date the item was last see (usually the last time the barcode was scanned or inventory was done)" + }, + "stack": { + "type": ["integer", "null"], + "description": "?" + }, + "notforloan": { + "type": "integer", + "description": "authorized value defining why this item is not for loan" + }, + "damaged": { + "type": "integer", + "description": "authorized value defining this item as damaged" + }, + "itemlost": { + "type": "integer", + "description": "authorized value defining this item as lost" + }, + "itemlost_on": { + "type": ["string", "null"], + "format": "date-time", + "description": "the date and time an item was last marked as lost, NULL if not lost" + }, + "withdrawn": { + "type": "integer", + "description": "authorized value defining this item as withdrawn" + }, + "withdrawn_on": { + "type": ["string", "null"], + "format": "date-time", + "description": "the date and time an item was last marked as withdrawn, NULL if not withdrawn" + }, + "itemcallnumber": { + "type": ["string", "null"], + "description": "call number for this item" + }, + "coded_location_qualifier": { + "type": ["string", "null"], + "description": "coded location qualifier" + }, + "issues": { + "type": ["integer", "null"], + "description": "number of times this item has been checked out/issued" + }, + "renewals": { + "type": ["integer", "null"], + "description": "number of times this item has been renewed" + }, + "reserves": { + "type": ["integer", "null"], + "description": "number of times this item has been placed on hold/reserved" + }, + "restricted": { + "type": ["integer", "null"], + "description": "authorized value defining use restrictions for this item" + }, + "itemnotes": { + "type": ["string", "null"], + "description": "public notes on this item" + }, + "itemnotes_nonpublic": { + "type": ["string", "null"], + "description": "non-public notes on this item" + }, + "holdingbranch": { + "type": ["string", "null"], + "description": "library that is currently in possession item" + }, + "paidfor": { + "type": ["string", "null"], + "description": "?" + }, + "timestamp": { + "type": "string", + "format": "date-time", + "description": "date and time this item was last altered" + }, + "location": { + "type": ["string", "null"], + "description": "authorized value for the shelving location for this item" + }, + "permanent_location": { + "type": ["string", "null"], + "description": "linked to the CART and PROC temporary locations feature, stores the permanent shelving location" + }, + "onloan": { + "type": ["string", "null"], + "format": "date", + "description": "defines if item is checked out (NULL for not checked out, and checkout date for checked out)" + }, + "cn_source": { + "type": ["string", "null"], + "description": "classification source used on this item" + }, + "cn_sort": { + "type": ["string", "null"], + "description": "?" + }, + "ccode": { + "type": ["string", "null"], + "description": "authorized value for the collection code associated with this item" + }, + "materials": { + "type": ["string", "null"], + "description": "materials specified" + }, + "uri": { + "type": ["string", "null"], + "description": "URL for the item" + }, + "itype": { + "type": ["string", "null"], + "description": "itemtype defining the type for this item" + }, + "more_subfields_xml": { + "type": ["string", "null"], + "description": "additional 952 subfields in XML format" + }, + "enumchron": { + "type": ["string", "null"], + "description": "serial enumeration/chronology for the item" + }, + "copynumber": { + "type": ["string", "null"], + "description": "copy number" + }, + "stocknumber": { + "type": ["string", "null"], + "description": "inventory number" + }, + "new_status": { + "type": ["string", "null"], + "description": "'new' value, whatever free-text information." + } + }, + "additionalProperties": false, + "required": ["itemnumber", "biblionumber", "biblioitemnumber", "notforloan", "damaged", "itemlost", "withdrawn"] +} diff --git a/api/v1/swagger/parameters.json b/api/v1/swagger/parameters.json index eeb156a..f11031f 100644 --- a/api/v1/swagger/parameters.json +++ b/api/v1/swagger/parameters.json @@ -10,5 +10,8 @@ }, "holdIdPathParam": { "$ref": "parameters/hold.json#/holdIdPathParam" + }, + "itemnumberPathParam": { + "$ref": "parameters/item.json#/itemnumberPathParam" } } diff --git a/api/v1/swagger/parameters/item.json b/api/v1/swagger/parameters/item.json new file mode 100644 index 0000000..c215adb --- /dev/null +++ b/api/v1/swagger/parameters/item.json @@ -0,0 +1,9 @@ +{ + "itemnumberPathParam": { + "name": "itemnumber", + "in": "path", + "description": "Internal item identifier", + "required": true, + "type": "integer" + } +} diff --git a/api/v1/swagger/paths.json b/api/v1/swagger/paths.json index b1fcf40..73729c1 100644 --- a/api/v1/swagger/paths.json +++ b/api/v1/swagger/paths.json @@ -11,6 +11,9 @@ "/holds/{reserve_id}": { "$ref": "paths/holds.json#/~1holds~1{reserve_id}" }, + "/items/{itemnumber}": { + "$ref": "paths/items.json#/~1items~1{itemnumber}" + }, "/patrons": { "$ref": "paths/patrons.json#/~1patrons" }, diff --git a/api/v1/swagger/paths/items.json b/api/v1/swagger/paths/items.json new file mode 100644 index 0000000..47007dc --- /dev/null +++ b/api/v1/swagger/paths/items.json @@ -0,0 +1,47 @@ +{ + "/items/{itemnumber}": { + "get": { + "x-mojo-to": "Item#get", + "operationId": "getItem", + "tags": ["items"], + "parameters": [{ + "$ref": "../parameters.json#/itemnumberPathParam" + } + ], + "consumes": ["application/json"], + "produces": ["application/json"], + "responses": { + "200": { + "description": "An item", + "schema": { + "$ref": "../definitions.json#/item" + } + }, + "400": { + "description": "Missing or wrong parameters", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "404": { + "description": "Item 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" + } + } + } + } + } +} diff --git a/api/v1/swagger/x-primitives.json b/api/v1/swagger/x-primitives.json index ae4e739..b6c6a09 100644 --- a/api/v1/swagger/x-primitives.json +++ b/api/v1/swagger/x-primitives.json @@ -1,4 +1,8 @@ { + "biblioitemnumber": { + "type": "integer", + "description": "internally assigned biblioitem identifier" + }, "biblionumber": { "type": "integer", "description": "internally assigned biblio identifier" @@ -25,7 +29,7 @@ "description": "patron's first name" }, "itemnumber": { - "type": ["integer", "null"], + "type": "integer", "description": "internally assigned item identifier" }, "phone": { diff --git a/t/db_dependent/api/v1/items.t b/t/db_dependent/api/v1/items.t new file mode 100644 index 0000000..c2bd025 --- /dev/null +++ b/t/db_dependent/api/v1/items.t @@ -0,0 +1,151 @@ +#!/usr/bin/env perl + +# Copyright 2016 Koha-Suomi +# +# 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 => 4; +use Test::Mojo; +use Test::Warn; + +use t::lib::TestBuilder; +use t::lib::Mocks; + +use C4::Auth; +use Koha::Items; +use Koha::Database; + +my $schema = Koha::Database->new->schema; +my $builder = t::lib::TestBuilder->new; + +# FIXME: sessionStorage defaults to mysql, but it seems to break transaction handling +# this affects the other REST api tests +t::lib::Mocks::mock_preference( 'SessionStorage', 'tmp' ); + +my $remote_address = '127.0.0.1'; +my $t = Test::Mojo->new('Koha::REST::V1'); + +subtest 'list() tests' => sub { + plan tests => 2; + + # Not yet implemented + my $tx = $t->ua->build_tx(GET => "/api/v1/items"); + $t->request_ok($tx) + ->status_is(404); +}; + +subtest 'get() tests' => sub { + plan tests => 8; + + $schema->storage->txn_begin; + + my $biblio = $builder->build({ + source => 'Biblio' + }); + my $biblionumber = $biblio->{biblionumber}; + my $item = $builder->build({ + source => 'Item', + value => { + biblionumber => $biblionumber, + withdrawn => 1, # tested with OpacHiddenItems + } + }); + my $itemnumber = $item->{itemnumber}; + + my ($patron, $sessionid) = create_user_and_session({ authorized=>0 }); + my ($librarian, $lib_sessionid) = create_user_and_session({ authorized=>4 }); + + my $nonExistentItemnumber = -14362719; + my $tx = $t->ua->build_tx(GET => "/api/v1/items/$nonExistentItemnumber"); + $tx->req->cookies({name => 'CGISESSID', value => $sessionid}); + $t->request_ok($tx) + ->status_is(404); + + subtest 'Confirm effectiveness of OpacHiddenItems' => sub { + plan tests => 7; + + t::lib::Mocks::mock_preference('OpacHiddenItems', ''); + $tx = $t->ua->build_tx(GET => "/api/v1/items/$itemnumber"); + $tx->req->cookies({name => 'CGISESSID', value => $sessionid}); + $t->request_ok($tx) + ->status_is(200) + ->json_is('/itemnumber' => $itemnumber) + ->json_is('/biblionumber' => $biblionumber) + ->json_is('/itemnotes_nonpublic' => undef); + + t::lib::Mocks::mock_preference('OpacHiddenItems', 'withdrawn: [1]'); + $tx = $t->ua->build_tx(GET => "/api/v1/items/$itemnumber"); + $tx->req->cookies({name => 'CGISESSID', value => $sessionid}); + $t->request_ok($tx) + ->status_is(404); + }; + + $tx = $t->ua->build_tx(GET => "/api/v1/items/$itemnumber"); + $tx->req->cookies({name => 'CGISESSID', value => $lib_sessionid}); + $t->request_ok($tx) + ->status_is(200) + ->json_is('/itemnumber' => $itemnumber) + ->json_is('/biblionumber' => $biblionumber) + ->json_is('/itemnotes_nonpublic' => $item->{itemnotes_nonpublic}); + + $schema->storage->txn_rollback; +}; + +subtest 'update() tests' => sub { + plan tests => 2; + + # Not yet implemented + my $tx = $t->ua->build_tx(PUT => "/api/v1/items/1"); + $t->request_ok($tx) + ->status_is(404); +}; + +subtest 'delete() tests' => sub { + plan tests => 2; + + # Not yet implemented + my $tx = $t->ua->build_tx(DELETE => "/api/v1/items/1"); + $t->request_ok($tx) + ->status_is(404); +}; + +sub create_user_and_session { + + my $args = shift; + my $flags = ( $args->{authorized} ) ? $args->{authorized} : 0; + my $dbh = C4::Context->dbh; + + my $user = $builder->build( + { + source => 'Borrower', + value => { + flags => $flags + } + } + ); + + # Create a session for the authorized user + my $session = C4::Auth::get_session(''); + $session->param( 'number', $user->{borrowernumber} ); + $session->param( 'id', $user->{userid} ); + $session->param( 'ip', '127.0.0.1' ); + $session->param( 'lasttime', time() ); + $session->flush; + + return ( $user->{borrowernumber}, $session->id ); +} -- 1.9.1