Summary: | Add GET /libraries/:library_id/desks | ||
---|---|---|---|
Product: | Koha | Reporter: | Tomás Cohen Arazi <tomascohen> |
Component: | REST API | Assignee: | Tomás Cohen Arazi <tomascohen> |
Status: | Pushed to main --- | QA Contact: | |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | david, lucas, martin.renvoize, nick, pedro.amorim, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36237 | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: |
This enhancement adds an API endpoint for requesting a list of desks for a library. For example: http://127.0.0.1:8080/api/v1/libraries/cpl/desks
|
Version(s) released in: |
24.05.00
|
Bug Depends on: | |||
Bug Blocks: | 36481 | ||
Attachments: |
Bug 36480: Add Koha::Library->desks
Bug 36480: Add GET /libraries/:library_id/desks Bug 36480: Add Koha::Library->desks Bug 36480: Add GET /libraries/:library_id/desks Bug 36480: Add Koha::Library->desks Bug 36480: Add GET /libraries/:library_id/desks |
Description
Tomás Cohen Arazi
2024-04-01 20:21:24 UTC
Created attachment 164226 [details] [review] Bug 36480: Add Koha::Library->desks We add an accessor for the related desks. Tests are added. To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Library.t => SUCCESS: Tests pass! 3. Sign off :-D Created attachment 164227 [details] [review] Bug 36480: Add GET /libraries/:library_id/desks This patch adds the mentioned endpoint. For it, it does: * Add Koha::Desk->to_api_mapping * Add desk.yaml with the correct data structure for desks * Add the route to the spec * Add tests Note: Lucas and I had doubts about the right return value for when the feature is disabled. I opted for returning 404 with a message telling the feature is disabled. This can be discussed. To test: 1. Apply this patches 2. Run: $ ktd k$ qa => SUCCESS: All green, all tests pass! 3. Play with this using Postman. 4. Sign off :-D Created attachment 164233 [details] [review] Bug 36480: Add Koha::Library->desks We add an accessor for the related desks. Tests are added. To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Library.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Created attachment 164234 [details] [review] Bug 36480: Add GET /libraries/:library_id/desks This patch adds the mentioned endpoint. For it, it does: * Add Koha::Desk->to_api_mapping * Add desk.yaml with the correct data structure for desks * Add the route to the spec * Add tests Note: Lucas and I had doubts about the right return value for when the feature is disabled. I opted for returning 404 with a message telling the feature is disabled. This can be discussed. To test: 1. Apply this patches 2. Run: $ ktd k$ qa => SUCCESS: All green, all tests pass! 3. Play with this using Postman. 4. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Testing notes (using KTD): 1. Tests pass before and after patches applied: - prove prove t/db_dependent/Koha/Library.t - prove t/db_dependent/api/v1/libraries.t 2. Enable these system preferences: - UseCirculationDesks - RESTBasicAuth 3. Test the new API endpoint using Postman: - In the staff interface, add some desks for different libraries, some with multiple desks - Add a get request to request the desks for a library, for example add two desks to Centerville and one to Fairfield: . Set Basic Auth in Postamn use koha/koha . http://127.0.0.1:8080/api/v1/libraries/cpl/desks . http://127.0.0.1:8080/api/v1/libraries/ffl/desks - Results should return the desk information for the library, for example: [ { "desk_id": 11, "library_id": "CPL", "name": "Desk 1" }, { "desk_id": 13, "library_id": "CPL", "name": "Desk 2" } ] Created attachment 164413 [details] [review] Bug 36480: Add Koha::Library->desks We add an accessor for the related desks. Tests are added. To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Library.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 164414 [details] [review] Bug 36480: Add GET /libraries/:library_id/desks This patch adds the mentioned endpoint. For it, it does: * Add Koha::Desk->to_api_mapping * Add desk.yaml with the correct data structure for desks * Add the route to the spec * Add tests Note: Lucas and I had doubts about the right return value for when the feature is disabled. I opted for returning 404 with a message telling the feature is disabled. This can be discussed. To test: 1. Apply this patches 2. Run: $ ktd k$ qa => SUCCESS: All green, all tests pass! 3. Play with this using Postman. 4. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> I'm unsure about the 404 when the UseCirculationDesks feature is disabled. Having said that, I don't know what to propose as an alternative, either. Looking at REST/V1/Patrons.pm, it renders a 403 response if AllowPatronToSetCheckoutsVisibilityForGuarantor is disabled. We should at least agree on a pattern and stick to it. There doesn't seem to exist a 100% fit of an HTTP code for "Feature disabled". Doh. Only now read the "Note: Lucas and I had doubts about the right return value for when the feature is disabled." in the original commit. Yeah, not a blocker, but deffo a pattern we should agree and stick to it going forward imo. (In reply to Pedro Amorim from comment #9) > Doh. Only now read the "Note: Lucas and I had doubts about the right return > value for when the feature is disabled." in the original commit. Yeah, not a > blocker, but deffo a pattern we should agree and stick to it going forward > imo. Do we need to follow-up on this or was it covered by our recent discussion on return values? Pushed for 24.05! Well done everyone, thank you! |