Add API route for getting an item GET /api/v1/items/{itemnumber} Gets one Item
Created attachment 52943 [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
Created attachment 52947 [details] [review] Bug 16825: Add API route for getting an item
Created attachment 53035 [details] [review] Bug 16825: Add API route for getting an item
Created attachment 53108 [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
Created attachment 53111 [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>
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>
Created attachment 53837 [details] [review] Bug 16825: (followup) Add API route for getting an item Added on top of last patch the OpacHiddenItems syspref restriction for users who are not staff. They see "Item Not Found" when trying to view item hidden from OPAC. Only staff can see those 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: Jiri Kozlovsky <mail@jkozlovsky.cz> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
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.
Created attachment 53921 [details] [review] Bug 16826: [QA Follow-up] Add 'new_status' to Swagger definition Adds missing "new_status" to item's Swagger definition. Also, to avoid forgotten columns in definitions, adds a test to compare item-definition to database's items-table. To test: 1. Run t/db_dependent/api/v1/items.t
I created Bug 17032 to add similar tests for what's already in master.
Created attachment 53923 [details] [review] Bug 16825: [QA Follow-up] Add 'new_status' to Swagger definition Adds missing "new_status" to item's Swagger definition. Also, to avoid forgotten columns in definitions, adds a test to compare item-definition to database's items-table. To test: 1. Run t/db_dependent/api/v1/items.t Signed-off-by: Jiri Kozlovsky <mail@jkozlovsky.cz> Lari: You have probably changed in your comment bug ID for 16826 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16825#c9
(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).
(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.
Created attachment 54052 [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> Removed test for missing properties. It should be done in a separate context. See Bug 17032.
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>
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>
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>
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>
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>
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>
Rebased on top of master and added Swagger-minifying into test plan.
Created attachment 56131 [details] [review] Bug 16825: [QA Follow-up] Add 'new_status' to Swagger definition Signed-off-by: Jiri Kozlovsky <mail@jkozlovsky.cz>
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.
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>
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>
Created attachment 56293 [details] [review] Bug 16825: [QA Follow-up] Add 'new_status' to Swagger definition Signed-off-by: Jiri Kozlovsky <mail@jkozlovsky.cz>
Removed minifying step from test plan after Bug 17432.
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
Rewritten, now Mojolicious::Plugin::OpenAPI compatible.
(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"}].
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>
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.
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
Lari, the second patch doens't apply. Please rebase/fix.
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>
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.
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.
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>
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
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. :)
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>
(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!
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>
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.
Johanna, can You rebase it on master? Thank you.
Created attachment 91399 [details] [review] Bug 16825: Add API route for getting an item Fixed the merge conflict. Patch should apply now to master.
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>
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>
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!
Added to the agenda of the 11 September 2019 de meeting: https://wiki.koha-community.org/wiki/Development_IRC_meeting_11_September_2019
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.
Nice work! Pushed to master for 19.11.00
I choose to backport this enhancement on API. Pushed to 19.05.x for 19.05.04
not backporting enhancement to 18.11.x