Bug 16825 - Add API route for getting an item
Summary: Add API route for getting an item
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: REST api (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Johanna Räisä
QA Contact: Kyle M Hall
URL: https://wiki.koha-community.org/wiki/...
Keywords:
Depends on: 18137
Blocks: 16652 16826
  Show dependency treegraph
 
Reported: 2016-06-28 14:17 UTC by Lari Taskula
Modified: 2019-09-13 13:14 UTC (History)
12 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.11.00,19.05.04


Attachments
Bug 16825: Add API route for getting an item (14.27 KB, patch)
2016-06-29 09:14 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 16825: Add API route for getting an item (13.55 KB, patch)
2016-06-29 15:39 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 16825: Add API route for getting an item (13.50 KB, patch)
2016-07-01 15:58 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 16825: Add API route for getting an item (13.55 KB, patch)
2016-07-05 18:36 UTC, Jiri Kozlovsky
Details | Diff | Splinter Review
Bug 16825: Add API route for getting an item (13.54 KB, patch)
2016-07-05 19:05 UTC, Jiri Kozlovsky
Details | Diff | Splinter Review
Bug 16825: Add API route for getting an item (13.63 KB, patch)
2016-07-08 11:57 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 16825: (followup) Add API route for getting an item (2.00 KB, patch)
2016-08-01 13:42 UTC, Jiri Kozlovsky
Details | Diff | Splinter Review
Bug 16826: [QA Follow-up] Add 'new_status' to Swagger definition (2.15 KB, patch)
2016-08-02 16:26 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 16825: [QA Follow-up] Add 'new_status' to Swagger definition (2.14 KB, patch)
2016-08-02 19:04 UTC, Jiri Kozlovsky
Details | Diff | Splinter Review
Bug 16825: [QA Follow-up] Add 'new_status' to Swagger definition (868 bytes, patch)
2016-08-05 14:59 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 16825: (followup) Add API route for getting an item (2.05 KB, patch)
2016-08-22 12:44 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 16825: [QA Follow-up] Add 'new_status' to Swagger definition (868 bytes, patch)
2016-08-22 12:44 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 16825: Add API route for getting an item (14.83 KB, patch)
2016-10-11 07:13 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 16825: Add API route for getting an item (14.97 KB, patch)
2016-10-11 07:16 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 16825: (followup) Add API route for getting an item (2.05 KB, patch)
2016-10-11 07:17 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 16825: [QA Follow-up] Add 'new_status' to Swagger definition (1.19 KB, patch)
2016-10-11 07:22 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 16825: [QA Follow-up] Add 'new_status' to Swagger definition (1.20 KB, patch)
2016-10-11 08:02 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 16825: [QA Follow-up] Add 'new_status' to Swagger definition (1.20 KB, patch)
2016-10-11 08:04 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 16825: Add API route for getting an item (14.84 KB, patch)
2016-10-12 14:29 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 16825: (followup) Add API route for getting an item (2.05 KB, patch)
2016-10-12 14:30 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 16825: [QA Follow-up] Add 'new_status' to Swagger definition (1.09 KB, patch)
2016-10-12 14:30 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 16825: Add API route for getting an item (19.16 KB, patch)
2017-03-16 16:38 UTC, Lari Taskula
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16825: Add API route for getting an item (19.23 KB, patch)
2017-06-14 08:36 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 16825: (squashable) Fix test cleanup issues (1.61 KB, patch)
2017-07-12 12:10 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 16825: Add API route for getting an item (19.30 KB, patch)
2017-10-10 11:21 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 16825: Add API route for getting an item (23.75 KB, patch)
2019-06-27 13:00 UTC, Johanna Räisä
Details | Diff | Splinter Review
Bug 16825: Add API route for getting an item (20.03 KB, patch)
2019-06-28 08:57 UTC, Johanna Räisä
Details | Diff | Splinter Review
Bug 16825: Add API route for getting an item (20.09 KB, patch)
2019-06-28 19:47 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 16825: Add API route for getting an item (20.10 KB, patch)
2019-07-08 05:27 UTC, Johanna Räisä
Details | Diff | Splinter Review
Bug 16825: Add API route for getting an item (20.12 KB, patch)
2019-07-08 07:35 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 16825: Add API route for getting an item (20.17 KB, patch)
2019-09-06 12:06 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lari Taskula 2016-06-28 14:17:49 UTC
Add API route for getting an item

GET /api/v1/items/{itemnumber} Gets one Item
Comment 1 Lari Taskula 2016-06-29 09:14:29 UTC Comment hidden (obsolete)
Comment 2 Lari Taskula 2016-06-29 15:39:25 UTC Comment hidden (obsolete)
Comment 3 Lari Taskula 2016-07-01 15:58:18 UTC Comment hidden (obsolete)
Comment 4 Jiri Kozlovsky 2016-07-05 18:36:00 UTC Comment hidden (obsolete)
Comment 5 Jiri Kozlovsky 2016-07-05 19:05:23 UTC Comment hidden (obsolete)
Comment 6 Benjamin Rokseth 2016-07-08 11:57:14 UTC
Created attachment 53222 [details] [review]
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.

To test:
1. Open a browser tab on Koha staff and log in (to create CGISESSID
   cookie).
2. Send GET request to http://yourlibrary/api/v1/items/YYY
   where YYY is an existing itemnumber.
3. Make sure the returned data is correct.
4. Run unit tests in t/db_dependent/api/v1/items.t

Signed-off-by: jirislav <mail@jkozlovsky.cz>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 7 Jiri Kozlovsky 2016-08-01 13:42:16 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2016-08-02 11:50:54 UTC
items.new_status is missing in the swagger file.
Please have a look at bug 15126 comment 53.
We need a way to keep this file up-to-date.
Comment 9 Lari Taskula 2016-08-02 16:26:19 UTC Comment hidden (obsolete)
Comment 10 Lari Taskula 2016-08-02 16:34:43 UTC
I created Bug 17032 to add similar tests for what's already in master.
Comment 11 Jiri Kozlovsky 2016-08-02 19:04:58 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2016-08-03 08:03:35 UTC
(In reply to Lari Taskula from comment #9)
> Also, to avoid forgotten columns in definitions, adds a test to
> compare item-definition to database's items-table.

Lari, thanks for that!
Do you think you could provide a global check?
It would look at the api/v1/definitions/*.json files and compare the columns with Koha::Objects->columns (added by bug 16889).
Comment 13 Lari Taskula 2016-08-03 15:33:53 UTC
(In reply to Jiri Kozlovsky from comment #11)
> Lari: You have probably changed in your comment bug ID for 16826
> https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16825#c9

Indeed. Sorry about that!

(In reply to Jonathan Druart from comment #12)
> (In reply to Lari Taskula from comment #9)
> > Also, to avoid forgotten columns in definitions, adds a test to
> > compare item-definition to database's items-table.
> 
> Lari, thanks for that!
> Do you think you could provide a global check?
> It would look at the api/v1/definitions/*.json files and compare the columns
> with Koha::Objects->columns (added by bug 16889).

As discussed on IRC, I have provided a test that does this, in Bug 17032.
Comment 14 Lari Taskula 2016-08-05 14:59:19 UTC Comment hidden (obsolete)
Comment 15 Lari Taskula 2016-08-22 12:44:05 UTC
Created attachment 54686 [details] [review]
Bug 16825: (followup) Add API route for getting an item

Fixed tab-character.

Signed-off-by: Jiri Kozlovsky <mail@jkozlovsky.cz>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Lari Taskula <larit@student.uef.fi>
Comment 16 Lari Taskula 2016-08-22 12:44:27 UTC
Created attachment 54687 [details] [review]
Bug 16825: [QA Follow-up] Add 'new_status' to Swagger definition

Adds missing "new_status" to item's Swagger definition.

Signed-off-by: Jiri Kozlovsky <mail@jkozlovsky.cz>
Comment 17 Lari Taskula 2016-10-11 07:13:02 UTC
Created attachment 56122 [details] [review]
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.

To test:
1. Open a browser tab on Koha staff and log in (to create CGISESSID
   cookie).
2. Send GET request to http://yourlibrary/api/v1/items/YYY
   where YYY is an existing itemnumber.
3. Make sure the returned data is correct.
4. Run unit tests in t/db_dependent/api/v1/items.t

Signed-off-by: jirislav <mail@jkozlovsky.cz>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 18 Lari Taskula 2016-10-11 07:16:40 UTC
Created attachment 56123 [details] [review]
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.

To test:
1. Apply patch and run minifySwagger.pl
   perl misc/devel/minifySwagger.pl -s api/v1/swagger/swagger.json -d api/v1/swagger/swagger.min.json
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

Signed-off-by: jirislav <mail@jkozlovsky.cz>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 19 Lari Taskula 2016-10-11 07:17:17 UTC
Created attachment 56124 [details] [review]
Bug 16825: (followup) Add API route for getting an item

Signed-off-by: Jiri Kozlovsky <mail@jkozlovsky.cz>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Lari Taskula <larit@student.uef.fi>
Comment 20 Lari Taskula 2016-10-11 07:22:16 UTC
Created attachment 56125 [details] [review]
Bug 16825: [QA Follow-up] Add 'new_status' to Swagger definition

Adds missing "new_status" to item's Swagger definition.

To test:
1. Run t/db_dependent/api/v1/swagger/definitions.t
2. Observe that test fails because of missing new_status.
3. Apply patch and test again
4. Observe that test no longer complains about new_status.

(minify swagger)
perl misc/devel/minifySwagger.pl -s api/v1/swagger/swagger.json -d api/v1/swagger

Signed-off-by: Jiri Kozlovsky <mail@jkozlovsky.cz>
Comment 21 Lari Taskula 2016-10-11 07:24:31 UTC
Rebased on top of master and added Swagger-minifying into test plan.
Comment 22 Lari Taskula 2016-10-11 08:02:38 UTC
Created attachment 56131 [details] [review]
Bug 16825: [QA Follow-up] Add 'new_status' to Swagger definition

Signed-off-by: Jiri Kozlovsky <mail@jkozlovsky.cz>
Comment 23 Lari Taskula 2016-10-11 08:04:16 UTC
Created attachment 56133 [details] [review]
Bug 16825: [QA Follow-up] Add 'new_status' to Swagger definition

Adds missing "new_status" to item's Swagger definition.

To test:
1. Run t/db_dependent/api/v1/swagger/definitions.t
2. Observe that test fails because of missing new_status.
3. Apply patch and test again
4. Observe that test no longer complains about new_status.

(minify swagger)
perl misc/devel/minifySwagger.pl -s api/v1/swagger/swagger.json -d api/v1/swagger/swagger.min.json

Signed-off-by: Jiri Kozlovsky <mail@jkozlovsky.cz>

Fixed a typo in minifying command.
Comment 24 Lari Taskula 2016-10-12 14:29:31 UTC
Created attachment 56290 [details] [review]
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.

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

Signed-off-by: jirislav <mail@jkozlovsky.cz>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 25 Lari Taskula 2016-10-12 14:30:32 UTC
Created attachment 56292 [details] [review]
Bug 16825: (followup) Add API route for getting an item

Signed-off-by: Jiri Kozlovsky <mail@jkozlovsky.cz>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Lari Taskula <larit@student.uef.fi>
Comment 26 Lari Taskula 2016-10-12 14:30:48 UTC
Created attachment 56293 [details] [review]
Bug 16825: [QA Follow-up] Add 'new_status' to Swagger definition

Signed-off-by: Jiri Kozlovsky <mail@jkozlovsky.cz>
Comment 27 Lari Taskula 2016-10-12 14:31:07 UTC
Removed minifying step from test plan after Bug 17432.
Comment 28 Lari Taskula 2017-03-16 16:38:52 UTC
Created attachment 61176 [details] [review]
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
Comment 29 Lari Taskula 2017-03-16 16:41:12 UTC
Rewritten, now Mojolicious::Plugin::OpenAPI compatible.
Comment 30 Lari Taskula 2017-03-16 17:03:22 UTC
(In reply to Lari Taskula from comment #29)
> Rewritten, now Mojolicious::Plugin::OpenAPI compatible.

Forgot to include this in the previous comment - Additional note for testers: requires JSON::Validator version 0.96, otherwise tests fail for the following reason: 

GET /api/v1/items/340856 [{"message":"Does not match date-time format.","path":"\/timestamp"},{"message":"anyOf failed: Does not match date-time format.","path":"\/itemlost_on"},{"message":"anyOf failed: Does not match date-time format.","path":"\/withdrawn_on"}].
Comment 31 Josef Moravec 2017-06-14 08:36:28 UTC
Created attachment 64275 [details] [review]
[SIGNED-OFF] 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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 32 Katrin Fischer 2017-06-15 20:31:12 UTC
Excluding itemnotes_nonpublic is good thinking, but what about other fields that are set to not display in the OPAC in the frameworks?

I know we got some libraries using more_subfields_xml to store additional information that is not for thought to be seen by normal users.
I think those should only show if you have catalogue permission.
Comment 33 Lari Taskula 2017-07-12 12:10:57 UTC
Created attachment 65007 [details] [review]
Bug 16825: (squashable) Fix test cleanup issues

Roll back failed at the end of the test. Apply this patch to fix the issue.

To test:
1. prove t/db_dependent/api/v1/items.t
2. Check your database to make sure nothing persists after the test
Comment 34 Tomás Cohen Arazi 2017-10-05 16:12:46 UTC
Lari, the second patch doens't apply. Please rebase/fix.
Comment 35 Lari Taskula 2017-10-10 11:21:00 UTC
Created attachment 67865 [details] [review]
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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 36 Marcel de Rooy 2018-03-09 07:30:43 UTC
Running the test fails with me.

    # Subtest: Confirm effectiveness of OpacHiddenItems
        1..7
        ok 1 - GET /api/v1/items/21
        not ok 2 - 200 OK
        #   Failed test '200 OK'
        #   at t/db_dependent/api/v1/items.t line 85.
        #          got: '500'
        #     expected: '200'
        not ok 3 - exact match for JSON Pointer "/itemnumber"
        #   Failed test 'exact match for JSON Pointer "/itemnumber"'
        #   at t/db_dependent/api/v1/items.t line 85.
        #          got: undef
        #     expected: '21'
        not ok 4 - exact match for JSON Pointer "/biblionumber"
        #   Failed test 'exact match for JSON Pointer "/biblionumber"'
        #   at t/db_dependent/api/v1/items.t line 85.
        #          got: undef
        #     expected: '32'
        ok 5 - exact match for JSON Pointer "/itemnotes_nonpublic"
        ok 6 - GET /api/v1/items/21
        ok 7 - 404 Not Found
        # Looks like you failed 3 tests of 7.
    not ok 3 - Confirm effectiveness of OpacHiddenItems
    #   Failed test 'Confirm effectiveness of OpacHiddenItems'
    #   at t/db_dependent/api/v1/items.t line 96.
    ok 4 - GET /api/v1/items/21
    not ok 5 - 200 OK
    #   Failed test '200 OK'
    #   at t/db_dependent/api/v1/items.t line 100.
    #          got: '500'
    #     expected: '200'
    not ok 6 - exact match for JSON Pointer "/itemnumber"
    #   Failed test 'exact match for JSON Pointer "/itemnumber"'
    #   at t/db_dependent/api/v1/items.t line 100.
    #          got: undef
    #     expected: '21'
    not ok 7 - exact match for JSON Pointer "/biblionumber"
    #   Failed test 'exact match for JSON Pointer "/biblionumber"'
    #   at t/db_dependent/api/v1/items.t line 100.
    #          got: undef
    #     expected: '32'
    not ok 8 - exact match for JSON Pointer "/itemnotes_nonpublic"
    #   Failed test 'exact match for JSON Pointer "/itemnotes_nonpublic"'
    #   at t/db_dependent/api/v1/items.t line 100.
    #          got: undef
    #     expected: 'Wb_gNO'
    # Looks like you failed 5 tests of 8.
not ok 2 - get() tests
#   Failed test 'get() tests'
#   at t/db_dependent/api/v1/items.t line 107.
Comment 37 Johanna Räisä 2019-06-27 13:00:03 UTC
Created attachment 91051 [details] [review]
Bug 16825: Add API route for getting an item

I modified swagger and functions to be similar to other REST work. Also fixed the tests to pass.
Comment 38 Johanna Räisä 2019-06-27 13:02:49 UTC
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

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
Comment 39 Tomás Cohen Arazi 2019-06-27 13:31:52 UTC
Hi, Johanna.

This work has been initiated before a few agreements were reached about how to write endpoints for the API.  I've got a few remarks on this regard:

- There's a problem with the attribute mappings. There's an RFC for this endpoint's spec [1]. The attribute mappings should respect the RFC (I'm not sure its been voted, but you should follow it, given the people involved on writing and discussing it).
- We don't mix privileged users/non-privileged users use cases on the same endpoint anymore. An endpoint that requires admin privileges belongs to /items. An endpoint that doesn't require admin privileges goes to /public/items. This comment related to the check for OpacHiddenItems and the patron status. Non privileged users should get a 403 on this endpoint. If you need general unprivileged access, then add an endpoint to /public/items/{item_id}
- Don't add to x-primitives, that went too far in my opinion. Just put the definition in the path spec.
- For the tests, take t/db_dependent/api/v1/libraries.t as an example. I like it that you added stub tests for unimplemented routes, but try to clean it up by setting Basic authentication as libraries.t

I can help on any doubts you might have or even coding. Just ping me (tcohen on IRC)

[1] https://wiki.koha-community.org/wiki/Items_endpoint_RFC
Comment 40 Johanna Räisä 2019-06-28 05:45:39 UTC
Hi, Tomas.

Thanks for the points! I looked that this bug was stuck on test passing. Just took Lari's work and mainly fixed those problems.

I will look into those points and let's see again. :)
Comment 41 Johanna Räisä 2019-06-28 08:57:27 UTC
Created attachment 91061 [details] [review]
Bug 16825: Add API route for getting an item

GET /api/v1/items/{item_id} Gets one Item

This patch adds route to get one item from koha.items table.

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

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
Comment 42 Tomás Cohen Arazi 2019-06-28 15:18:33 UTC
(In reply to Johanna Räisä from comment #41)
> Created attachment 91061 [details] [review] [review]
> Bug 16825: Add API route for getting an item

Very efficient Johanna!
Comment 43 Michal Denar 2019-06-28 19:47:01 UTC
Created attachment 91124 [details] [review]
Bug 16825: Add API route for getting an item

GET /api/v1/items/{item_id} Gets one Item

This patch adds route to get one item from koha.items table.

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

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>
Comment 44 Bouzid Fergani 2019-07-05 19:19:57 UTC
Hi,
Thanks to rebase this patch.
Bug 16825 - Add API route for getting an item

91124 - Bug 16825: Add API route for getting an item

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 16825: Add API route for getting an item
Using index info to reconstruct a base tree...
M	api/v1/swagger/definitions.json
M	api/v1/swagger/paths.json
Falling back to patching base and 3-way merge...
Auto-merging api/v1/swagger/paths.json
CONFLICT (content): Merge conflict in api/v1/swagger/paths.json
Auto-merging api/v1/swagger/definitions.json
error: Failed to merge in the changes.
Patch failed at 0001 Bug 16825: Add API route for getting an item
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-16825-Add-API-route-for-getting-an-item-uUF0c9.patch

Bouzid.
Comment 45 Michal Denar 2019-07-05 19:52:31 UTC
Johanna,
can You rebase it on master?

Thank you.
Comment 46 Johanna Räisä 2019-07-08 05:27:48 UTC
Created attachment 91399 [details] [review]
Bug 16825: Add API route for getting an item

Fixed the merge conflict. Patch should apply now to master.
Comment 47 Michal Denar 2019-07-08 07:35:24 UTC
Created attachment 91401 [details] [review]
Bug 16825: Add API route for getting an item

GET /api/v1/items/{item_id} Gets one Item

This patch adds route to get one item from koha.items table.

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

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>
Comment 48 Kyle M Hall 2019-09-06 12:06:44 UTC
Created attachment 92639 [details] [review]
Bug 16825: Add API route for getting an item

GET /api/v1/items/{item_id} Gets one Item

This patch adds route to get one item from koha.items table.

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

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 49 Katrin Fischer 2019-09-06 12:41:55 UTC
Not unhappy to see this moving at all! But it looks like it hasn't been voted on:
https://wiki.koha-community.org/wiki/REST_api_RFCs
https://wiki.koha-community.org/wiki/Items_endpoint_RFC

Could we do a quick terminology check befor pushing/releasing and update the wiki? On first glance it looks quite good to me!
Comment 50 Magnus Enger 2019-09-10 06:40:17 UTC
Added to the agenda of the 11 September 2019 de meeting:
https://wiki.koha-community.org/wiki/Development_IRC_meeting_11_September_2019
Comment 51 Martin Renvoize 2019-09-12 15:40:41 UTC
Agreed the terms seem sane.. not enough people present at meeting to hold a vote so I've taken the approach of asking key parties involved in the API design directly for comments.. all have agreed so I'm going to push.
Comment 52 Martin Renvoize 2019-09-12 15:41:43 UTC
Nice work!

Pushed to master for 19.11.00
Comment 53 Fridolin SOMERS 2019-09-13 13:14:51 UTC
I choose to backport this enhancement on API.

Pushed to 19.05.x for 19.05.04