Attachment 61176 Details for Bug 16825 – Bug 16825: Add API route for getting an item

[patch] Bug 16825: Add API route for getting an item

Bug-16825-Add-API-route-for-getting-an-item.patch (text/plain), 19.16 KB, created by Lari Taskula on 2017-03-16 16:38:52 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Lari Taskula

Created: 2017-03-16 16:38:52 UTC

Size: 19.16 KB

patch

obsolete

>From f3fb1786aed53a936ef5538c5f3d705c626554f9 Mon Sep 17 00:00:00 2001
>From: Lari Taskula <larit@student.uef.fi>
>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

Actions: View | Diff | Splinter Review

Attachments on bug 16825: 52943 | 52947 | 53035 | 53108 | 53111 | 53222 | 53837 | 53921 | 53923 | 54052 | 54686 | 54687 | 56122 | 56123 | 56124 | 56125 | 56131 | 56133 | 56290 | 56292 | 56293 | 61176 | 64275 | 65007 | 67865 | 91051 | 91061 | 91124 | 91399 | 91401 | 92639