Bug 36482

Summary: Make it possible to embed desks and cash_registers on /libraries
Product: Koha Reporter: Tomás Cohen Arazi (tcohen) <tomascohen>
Component: REST APIAssignee: Tomás Cohen Arazi (tcohen) <tomascohen>
Status: RESOLVED FIXED QA Contact:
Severity: enhancement    
Priority: P5 - low CC: david, fridolin.somers, jonathan.druart, lucas, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36483
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.06
Circulation function:
Bug Depends on: 36481    
Bug Blocks: 36495    
Attachments: Bug 36482: Allow embedding desks and cash_registers on libraries endpoints
Bug 36482: Make embedding work for GET /libraries/:library_id
Bug 36482: Allow embedding desks and cash_registers on libraries endpoints
Bug 36482: Make embedding work for GET /libraries/:library_id
Bug 36482: Allow embedding desks and cash_registers on libraries endpoints
Bug 36482: Make embedding work for GET /libraries/:library_id
Bug 36482: Add embed tests
Bug 36482: Allow embedding desks and cash_registers on libraries endpoints
Bug 36482: Make embedding work for GET /libraries/:library_id
Bug 36482: Add embed tests

Description Tomás Cohen Arazi (tcohen) 2024-04-01 20:57:32 UTC

    
Comment 1 Tomás Cohen Arazi (tcohen) 2024-04-01 20:58:13 UTC
Now that we have specs thanks to bug 36480 and bug 36481, we should add the option.
Comment 2 Tomás Cohen Arazi (tcohen) 2024-04-01 21:11:24 UTC
Created attachment 164229 [details] [review]
Bug 36482: Allow embedding desks and cash_registers on libraries endpoints

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Tomás Cohen Arazi (tcohen) 2024-04-01 21:11:26 UTC
Created attachment 164230 [details] [review]
Bug 36482: Make embedding work for GET /libraries/:library_id

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 David Nind 2024-04-02 17:31:22 UTC
Not sure how to test this one.
Comment 5 Tomás Cohen Arazi (tcohen) 2024-04-02 19:19:06 UTC
(In reply to David Nind from comment #4)
> Not sure how to test this one.

Sorry for that!
Comment 6 Tomás Cohen Arazi (tcohen) 2024-04-02 19:23:53 UTC
To test:
1. Have UseCirculationDesks and UseCashRegisters enabled
2. Add some cash registers and desks to a known KTD library. For example 'MPL'.
3. Use Postman to make:
   * GET /libraries
   * GET /libraries/MPL
in both cases add the request header 'x-koha-embed: desks,cash_registers'
=> FAIL: In both cases the requests are denied because embedding those is not allowed.
4. Apply the first patch
5. Repeat 3
=> PARTIAL SUCCESS: GET /libraries does the job!
=> FAIL: GET /libraries/MPL fails!
6. Apply the second patch and repeat 3
=> SUCCESS: Both cases succeed!
7. Sign off :-D
Comment 7 David Nind 2024-04-03 06:31:12 UTC
Created attachment 164304 [details] [review]
Bug 36482: Allow embedding desks and cash_registers on libraries endpoints

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Comment 8 David Nind 2024-04-03 06:31:15 UTC
Created attachment 164305 [details] [review]
Bug 36482: Make embedding work for GET /libraries/:library_id

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Comment 9 David Nind 2024-04-03 06:33:27 UTC
Thanks Tomás!

Testing notes (using KTD):

1. Also needed to enable RESTBasicAuth system preference.
Comment 10 Pedro Amorim 2024-04-04 11:15:43 UTC
Created attachment 164416 [details] [review]
Bug 36482: Allow embedding desks and cash_registers on libraries endpoints

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 11 Pedro Amorim 2024-04-04 11:15:46 UTC
Created attachment 164417 [details] [review]
Bug 36482: Make embedding work for GET /libraries/:library_id

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 12 Pedro Amorim 2024-04-04 11:15:48 UTC
Created attachment 164418 [details] [review]
Bug 36482: Add embed tests

prove t/db_dependent/api/v1/libraries.t

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 13 Tomás Cohen Arazi (tcohen) 2024-04-04 11:31:02 UTC
(In reply to Pedro Amorim from comment #12)
> Created attachment 164418 [details] [review] [review]
> Bug 36482: Add embed tests
> 
> prove t/db_dependent/api/v1/libraries.t
> 
> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>

Thanks for the missed tests!
Comment 14 Jonathan Druart 2024-04-16 07:39:20 UTC
Applying: Bug 36482: Allow embedding desks and cash_registers on libraries endpoints
error: sha1 information is lacking or useless (api/v1/swagger/paths/libraries.yaml).
Comment 15 Martin Renvoize (ashimema) 2024-04-16 09:52:34 UTC
I let Tomas know about this yesterday, I think he's working on it once his jetlag lets his brain work again.
Comment 16 Tomás Cohen Arazi (tcohen) 2024-04-16 12:59:17 UTC
Created attachment 164929 [details] [review]
Bug 36482: Allow embedding desks and cash_registers on libraries endpoints

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 17 Tomás Cohen Arazi (tcohen) 2024-04-16 12:59:20 UTC
Created attachment 164930 [details] [review]
Bug 36482: Make embedding work for GET /libraries/:library_id

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 18 Tomás Cohen Arazi (tcohen) 2024-04-16 12:59:22 UTC
Created attachment 164931 [details] [review]
Bug 36482: Add embed tests

prove t/db_dependent/api/v1/libraries.t

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 19 Tomás Cohen Arazi (tcohen) 2024-04-16 13:00:32 UTC
Back to PQA, trivial conflict with 'library hours' fixed.
Comment 20 Katrin Fischer 2024-04-26 15:29:57 UTC
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.
Comment 21 Katrin Fischer 2024-04-26 16:07:28 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 22 Fridolin Somers 2024-05-24 08:18:15 UTC
Pushed to 23.11.x for 23.11.06
Comment 23 Lucas Gass (lukeg) 2024-05-28 20:41:58 UTC
Enhancement will not be backported to 23.05.x