Small enhancement to REST API to add the possibility to list libraries. GET /api/v1/libraries List all libraries and branchcodes GET /api/v1/libraries/{branchcode} Get library description and branchcode
Created attachment 51454 [details] [review] Bug 16497 - REST API: add routes to list libraries Simple routes to list all or get one Library GET /api/v1/libraries - List all libraries GET /api/v1/libraries/{branchcode} - Get one Library Test plan: - apply patch - run tests: t/db_dependent/api/v1/libraries.t - test API with some API tool or simple curl e.g.: curl http://host:port/api/v1/libraries curl http://host:port/api/v1/libraries/cpl
Created attachment 52905 [details] [review] Bug 16497 - REST API: add routes to list libraries Simple routes to list all or get one Library GET /api/v1/libraries - List all libraries GET /api/v1/libraries/{branchcode} - Get one Library Test plan: - apply patch - run tests: t/db_dependent/api/v1/libraries.t - test API with some API tool or simple curl e.g.: curl http://host:port/api/v1/libraries curl http://host:port/api/v1/libraries/cpl Signed-off-by: Lari Taskula <larit@student.uef.fi>
Created attachment 53818 [details] [review] Bug 14793: New cataloguing plugin unimarc_field_225a_bis It is similar to unimarc_field_210c_bis, but searches in biblioitems.collectiontitle instead of biblioitems.publishercode Signed-off-by: Laurence Rault <laurence.rault@biblibre.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 53819 [details] [review] Bug 16497 - REST API: add routes to list libraries Simple routes to list all or get one Library GET /api/v1/libraries - List all libraries GET /api/v1/libraries/{branchcode} - Get one Library Test plan: - apply patch - run tests: t/db_dependent/api/v1/libraries.t - test API with some API tool or simple curl e.g.: curl http://host:port/api/v1/libraries curl http://host:port/api/v1/libraries/cpl Signed-off-by: Lari Taskula <larit@student.uef.fi> Signed-off-by: Jiří Kozlovský <mail@jkozlovsky.cz> Works as expected. Nice work! :)
Comment on attachment 53818 [details] [review] Bug 14793: New cataloguing plugin unimarc_field_225a_bis Sorry about this patch, my git bz did not function correctly.
We should figure out what to do for properties 'issuing' and 'branchprinter'. They are missing from api/v1/definitions/library.json but I would guess the reason is that Benjamin noticed kohastructure mentioning that they are not used in Koha. Perhaps they should be completely removed everywhere? I noticed this issue by running the test introduced in Bug 17032 which won't pass this definition because it sees these two "missing" properties.
Created attachment 54943 [details] [review] Bug 16497 - REST API: add routes to list libraries Simple routes to list all or get one Library GET /api/v1/libraries - List all libraries GET /api/v1/libraries/{branchcode} - Get one Library Test plan: - apply patch - run tests: t/db_dependent/api/v1/libraries.t - test API with some API tool or simple curl e.g.: curl http://host:port/api/v1/libraries curl http://host:port/api/v1/libraries/cpl Signed-off-by: Lari Taskula <larit@student.uef.fi> Signed-off-by: Jiří Kozlovský <mail@jkozlovsky.cz>
Rebased against 16699
I think this patch must be rewritten following what has been done in bug 17428 for Cities.
Yup. Perhaps even CRUD? or is that overkill?
(In reply to Benjamin Rokseth from comment #10) > Yup. > > Perhaps even CRUD? or is that overkill? That seems overkill to me, but not so hard to implement. It may make sense to be consistent and supply CRUD for such object.
Created attachment 60801 [details] [review] Bug 16497: [REST] Add /api/v1/libraries CRUD for libraries via REST API. GET /api/v1/libraries - List all libraries GET /api/v1/libraries/{branchcode} - Get one Library POST /api/v1/libraries - Add new Library DELETE /api/v1/libraries/{branchcode} - Delete Library Test plan: - apply patch - run tests: t/db_dependent/api/v1/libraries.t - test API with some API tool or simple curl e.g.: curl http://host:port/api/v1/libraries curl http://host:port/api/v1/libraries/cpl
Created attachment 60802 [details] [review] Bug 16497: Sort definitions.json Before this file grows, we should sort it alphabetically.
Here is CRUD. I switched this bug report to Needs Signoff without thinking first; we should do the migration from Swagger2 to Mojolicious::Plugin::OpenAPI before pushing adding more REST API stuff. See Bug 18137. So I am going to revert the status of this bug report back to In Discussion until we proceed with the migration.
Created attachment 61164 [details] [review] Bug 16497: [REST] Add /api/v1/libraries CRUD for libraries via REST API. GET /api/v1/libraries - List all libraries GET /api/v1/libraries/{branchcode} - Get one Library POST /api/v1/libraries - Add new Library DELETE /api/v1/libraries/{branchcode} - Delete Library Test plan: - apply patch - run tests: t/db_dependent/api/v1/libraries.t - test API with some API tool or simple curl e.g.: curl http://host:port/api/v1/libraries curl http://host:port/api/v1/libraries/cpl
Created attachment 61165 [details] [review] Bug 16497: Sort definitions.json Before this file grows, we should sort it alphabetically.
Created attachment 63160 [details] [review] [SIGNED-OFF] Bug 16497: [REST] Add /api/v1/libraries CRUD for libraries via REST API. GET /api/v1/libraries - List all libraries GET /api/v1/libraries/{branchcode} - Get one Library POST /api/v1/libraries - Add new Library DELETE /api/v1/libraries/{branchcode} - Delete Library Test plan: - apply patch - run tests: t/db_dependent/api/v1/libraries.t - test API with some API tool or simple curl e.g.: curl http://host:port/api/v1/libraries curl http://host:port/api/v1/libraries/cpl Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 63161 [details] [review] [SIGNED-OFF] Bug 16497: Add missing field in definitions due to bug 18066 Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 66837 [details] [review] Bug 16497: [REST] Add /api/v1/libraries CRUD for libraries via REST API. GET /api/v1/libraries - List all libraries GET /api/v1/libraries/{branchcode} - Get one Library POST /api/v1/libraries - Add new Library DELETE /api/v1/libraries/{branchcode} - Delete Library Test plan: - apply patch - run tests: t/db_dependent/api/v1/libraries.t - test API with some API tool or simple curl e.g.: curl http://host:port/api/v1/libraries curl http://host:port/api/v1/libraries/cpl Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Created attachment 66838 [details] [review] Bug 16497: Add missing field in definitions due to bug 18066 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Massive unit test failures. Is the test plan incomplete, or perhaps the automerge from git isn't going right?
Created attachment 68746 [details] [review] Bug 16497: [REST] Add /api/v1/libraries CRUD for libraries via REST API. GET /api/v1/libraries - List all libraries GET /api/v1/libraries/{branchcode} - Get one Library POST /api/v1/libraries - Add new Library DELETE /api/v1/libraries/{branchcode} - Delete Library Test plan: - apply patch - run tests: t/db_dependent/api/v1/libraries.t - test API with some API tool or simple curl e.g.: curl http://host:port/api/v1/libraries curl http://host:port/api/v1/libraries/cpl Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 68747 [details] [review] Bug 16497: Add missing field in definitions due to bug 18066 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 68748 [details] [review] Bug 16497: (followup) New column marcorgcode added Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 68838 [details] [review] Bug 16497: [REST] Add /api/v1/libraries CRUD for libraries via REST API. GET /api/v1/libraries - List all libraries GET /api/v1/libraries/{branchcode} - Get one Library POST /api/v1/libraries - Add new Library DELETE /api/v1/libraries/{branchcode} - Delete Library Test plan: - apply patch - run tests: t/db_dependent/api/v1/libraries.t - test API with some API tool or simple curl e.g.: curl http://host:port/api/v1/libraries curl http://host:port/api/v1/libraries/cpl Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 68839 [details] [review] Bug 16497: [REST] Add /api/v1/libraries CRUD for libraries via REST API. GET /api/v1/libraries - List all libraries GET /api/v1/libraries/{branchcode} - Get one Library POST /api/v1/libraries - Add new Library DELETE /api/v1/libraries/{branchcode} - Delete Library Test plan: - apply patch - run tests: t/db_dependent/api/v1/libraries.t - test API with some API tool or simple curl e.g.: curl http://host:port/api/v1/libraries curl http://host:port/api/v1/libraries/cpl Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 68840 [details] [review] Bug 16497: Add missing field in definitions due to bug 18066 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 68841 [details] [review] Bug 16497: (followup) New column marcorgcode added Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Please check POD coverage.
Created attachment 69508 [details] [review] Bug 16497 - Add POD in Koha/REST/V1/Library.pm
Please: - Implement using objects.search instead of manually crafting the DBIC query - Please review the RFC I've just posted. If you add to it, add new column for your proposals. I plan to have it voted in Dec 27th's dev meeting so we can move on. https://wiki.koha-community.org/wiki/Libraries_endpoint_RFC
Created attachment 83913 [details] [review] Bug 16497: Add /api/v1/libraries CRUD for libraries via REST API. GET /api/v1/libraries - List all libraries GET /api/v1/libraries/{branchcode} - Get one Library POST /api/v1/libraries - Add new Library DELETE /api/v1/libraries/{branchcode} - Delete Library Test plan: - apply patch - run tests: t/db_dependent/api/v1/libraries.t - test API with some API tool or simple curl e.g.: curl http://host:port/api/v1/libraries curl http://host:port/api/v1/libraries/cpl Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 83914 [details] [review] Bug 16497: Add missing field in definitions due to bug 18066 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 83915 [details] [review] Bug 16497: (follow-up) New column marcorgcode added Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 83916 [details] [review] Bug 16497: Add POD in Koha/REST/V1/Library.pm Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 83917 [details] [review] Bug 16497: (follow-up) Adapt to existing guidelines and RFC This patch makes the original implementation match what is specified on the RFC [1]. The controller is updated, and so the tests. To test: - Apply this patches: - Run: $ kshell k$ prove t/db_dependent/api/v1/libraries.t => SUCCESS: Tests pass! [1] https://wiki.koha-community.org/wiki/Libraries_endpoint_RFC Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
I've just implemented what I mentioned on comment 31. The attributes got adapted to the voted RFC and the implementation now is more in line with the current practices we adopted. I'm setting it NSO.
Created attachment 83943 [details] [review] Bug 22125: branches.pickup_location should be flagged as boolean This trivial patch just marks the mentioned column as a boolean. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment on attachment 83943 [details] [review] Bug 22125: branches.pickup_location should be flagged as boolean Sorry, wrong bug number
Created attachment 83945 [details] [review] Bug 16497: Add /api/v1/libraries CRUD for libraries via REST API. GET /api/v1/libraries - List all libraries GET /api/v1/libraries/{branchcode} - Get one Library POST /api/v1/libraries - Add new Library DELETE /api/v1/libraries/{branchcode} - Delete Library Test plan: - apply patch - run tests: t/db_dependent/api/v1/libraries.t - test API with some API tool or simple curl e.g.: curl http://host:port/api/v1/libraries curl http://host:port/api/v1/libraries/cpl Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 83946 [details] [review] Bug 16497: Add missing field in definitions due to bug 18066 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 83947 [details] [review] Bug 16497: (follow-up) New column marcorgcode added Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 83948 [details] [review] Bug 16497: Add POD in Koha/REST/V1/Library.pm Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 83949 [details] [review] Bug 16497: (follow-up) Adapt to existing guidelines and RFC This patch makes the original implementation match what is specified on the RFC [1]. The controller is updated, and so the tests. To test: - Apply this patches: - Run: $ kshell k$ prove t/db_dependent/api/v1/libraries.t => SUCCESS: Tests pass! [1] https://wiki.koha-community.org/wiki/Libraries_endpoint_RFC Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
*** Bug 18102 has been marked as a duplicate of this bug. ***
Created attachment 84198 [details] [review] Bug 16497: (follow-up) GET operations require staff access With the introduction of the /public namespace all other endpoints, the rest of the endpoints are restricted to privileged users. This patch makes the GET /libraries and GET /libraries/:library_id endpoints require 'catalogue' permissions. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 84588 [details] [review] Bug 16497: Add /api/v1/libraries CRUD for libraries via REST API. GET /api/v1/libraries - List all libraries GET /api/v1/libraries/{branchcode} - Get one Library POST /api/v1/libraries - Add new Library DELETE /api/v1/libraries/{branchcode} - Delete Library Test plan: - apply patch - run tests: t/db_dependent/api/v1/libraries.t - test API with some API tool or simple curl e.g.: curl http://host:port/api/v1/libraries curl http://host:port/api/v1/libraries/cpl Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 84589 [details] [review] Bug 16497: Add missing field in definitions due to bug 18066 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 84590 [details] [review] Bug 16497: (follow-up) New column marcorgcode added Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 84591 [details] [review] Bug 16497: Add POD in Koha/REST/V1/Library.pm Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 84592 [details] [review] Bug 16497: (follow-up) Adapt to existing guidelines and RFC This patch makes the original implementation match what is specified on the RFC [1]. The controller is updated, and so the tests. To test: - Apply this patches: - Run: $ kshell k$ prove t/db_dependent/api/v1/libraries.t => SUCCESS: Tests pass! [1] https://wiki.koha-community.org/wiki/Libraries_endpoint_RFC Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 84593 [details] [review] Bug 16497: (follow-up) GET operations require staff access With the introduction of the /public namespace all other endpoints, the rest of the endpoints are restricted to privileged users. This patch makes the GET /libraries and GET /libraries/:library_id endpoints require 'catalogue' permissions. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
A solid implementation, well tested, passes qa scripts. Passing QA
Created attachment 84734 [details] [review] Bug 16497: (follow-up) GET operations require staff access With the introduction of the /public namespace all other endpoints, the rest of the endpoints are restricted to privileged users. This patch makes the GET /libraries and GET /libraries/:library_id endpoints require 'catalogue' permissions. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 84769 [details] [review] Bug 16497: Refactor tests using BASIC authentication Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Awesome work all! Pushed to master for 19.05
As per my policy of backporting new api routes to aid developers of plugins and other api reliant third party integrations I have pushed this to 18.11.x for 18.11.04
18.05.x is missing 22125 dependency, won't backport