I propose to add an endpoint for fetching a library's list of cash registers. Context is bug 36237.
Created attachment 164228 [details] [review] Bug 36481: Add GET /libraries/:library_id/cash_registers This patch adds the mentioned route. For the task it: * Adds Koha::Cash::Register->to_api_mapping with trivial mappings * Adds a cash_register object definition on the API spec * Adds a controller to handle requests * Adds tests for the new endpoint To test: 1. Apply this patch 2. Run: $ ktd --shell k$ qa => SUCCESS: All green! Tests pass! 3. Play with Postman! 4. Sign off :-D
Created attachment 164237 [details] [review] Bug 36481: Add GET /libraries/:library_id/cash_registers This patch adds the mentioned route. For the task it: * Adds Koha::Cash::Register->to_api_mapping with trivial mappings * Adds a cash_register object definition on the API spec * Adds a controller to handle requests * Adds tests for the new endpoint To test: 1. Apply this patch 2. Run: $ ktd --shell k$ qa => SUCCESS: All green! Tests pass! 3. Play with Postman! 4. Sign off :-D Signed-off-by: David Nind <david@davidnind.com>
Not able to the patch on current master without applying 36480 first. Assuming bug 36480 needs to be added as a dependency. Testing notes (using KTD) ========================= 1. Apply bug 36480 2. Apply the patches for this bug (and dependencies). 3. qa and tests should pass without any errors 4. Enable these system preferences: - UseCashRegisters - RESTBasicAuth 5. Test the new API endpoint using Postman (or browser): - In the staff interface, add some cash registers for different libraries, some with multiple registers - Add a get request to request the cash registers for a library, for example: add two cash registers to Centerville and one to Fairfield: . Set Basic Auth in Postman to use koha/koha . http://127.0.0.1:8080/api/v1/libraries/cpl/cash_registers . http://127.0.0.1:8080/api/v1/libraries/ffl/cash_registers - Results should return the cash register information for the library, for example: [ { "archived": false, "branch_default": false, "cash_register_id": 3, "description": "Test cash register for Centerville", "library_id": "CPL", "name": "TEST1", "starting_float": 0.0 }, { "archived": false, "branch_default": false, "cash_register_id": 4, "description": "Another test cash register for Centerville", "library_id": "CPL", "name": "TEST2", "starting_float": 0.0 } ] - Put an invalid library code in the request, for example xxx: . http://127.0.0.1:8080/api/v1/libraries/xxx/cash_registers . Should get "Library not found" error - Disable cash registers and send above request. Should get: { "error": "Feature disabled" } Note: If cash registers are enabled, but a library has none set up, then the response is blank. Should there be an error/response saying this library has no cash registers setup? . Response received where library doesn't have a cash register: [] Path doesn't apply messages =========================== git bz apply 36481 Bug 36481 Depends on bug 36237 (Signed Off) Follow? [(y)es, (n)o] y Bug 36237 - Improve set-library UI after 34478 163013 - Bug 36237: UI improvments for set-library page Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 36237: UI improvments for set-library page Bug 36481 - Add GET /libraries/:library_id/cash_registers 164228 - Bug 36481: Add GET /libraries/:library_id/cash_registers Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 36481: Add GET /libraries/:library_id/cash_registers Using index info to reconstruct a base tree... M Koha/REST/V1/Libraries.pm M api/v1/swagger/paths/libraries.yaml M api/v1/swagger/swagger.yaml M t/db_dependent/api/v1/libraries.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/api/v1/libraries.t CONFLICT (content): Merge conflict in t/db_dependent/api/v1/libraries.t Auto-merging api/v1/swagger/swagger.yaml CONFLICT (content): Merge conflict in api/v1/swagger/swagger.yaml Auto-merging api/v1/swagger/paths/libraries.yaml CONFLICT (content): Merge conflict in api/v1/swagger/paths/libraries.yaml Auto-merging Koha/REST/V1/Libraries.pm CONFLICT (content): Merge conflict in Koha/REST/V1/Libraries.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 36481: Add GET /libraries/:library_id/cash_registers
(In reply to David Nind from comment #3) > Not able to the patch on current master without applying 36480 first. > Assuming bug 36480 needs to be added as a dependency. That's correct. I forgot to add it. Thanks!
Created attachment 164415 [details] [review] Bug 36481: Add GET /libraries/:library_id/cash_registers This patch adds the mentioned route. For the task it: * Adds Koha::Cash::Register->to_api_mapping with trivial mappings * Adds a cash_register object definition on the API spec * Adds a controller to handle requests * Adds tests for the new endpoint To test: 1. Apply this patch 2. Run: $ ktd --shell k$ qa => SUCCESS: All green! Tests pass! 3. Play with Postman! 4. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Being a little nit-picky, but shouldn't this be "library default"? + branch_default: + type: boolean + description: If this till is the branch default Adding "additional_work_needed", meaning "Asking for feedback". Not a blocker.
Pushed for 24.05! Well done everyone, thank you!
Created attachment 165965 [details] [review] Bug 36481: (QA follow-up) Rename branch_default to library_default Use agreed terminology Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Follow-up created and pushed.. removing additional_work_needed
Pushed to 23.11.x for 23.11.06
Oupsy : + responses: + 200: + description: A list of desks for the library Copy/paste error :D
Enhancement will not be backported to 23.05.x
Created attachment 169285 [details] [review] Bug 36481: (follow-up) Add missing library_id parameter The /libraries/{library_id}/cash_registers endpoint was missing the library_id parameter definition from the swagger specification. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Follow-up pushed.