Bug 36482 - Make it possible to embed desks and cash_registers on /libraries
Summary: Make it possible to embed desks and cash_registers on /libraries
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact:
URL:
Keywords:
Depends on: 36481
Blocks: 36495
  Show dependency treegraph
 
Reported: 2024-04-01 20:57 UTC by Tomás Cohen Arazi
Modified: 2024-04-26 16:07 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00


Attachments
Bug 36482: Allow embedding desks and cash_registers on libraries endpoints (1.71 KB, patch)
2024-04-01 21:11 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 36482: Make embedding work for GET /libraries/:library_id (839 bytes, patch)
2024-04-01 21:11 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 36482: Allow embedding desks and cash_registers on libraries endpoints (1.76 KB, patch)
2024-04-03 06:31 UTC, David Nind
Details | Diff | Splinter Review
Bug 36482: Make embedding work for GET /libraries/:library_id (885 bytes, patch)
2024-04-03 06:31 UTC, David Nind
Details | Diff | Splinter Review
Bug 36482: Allow embedding desks and cash_registers on libraries endpoints (1.82 KB, patch)
2024-04-04 11:15 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 36482: Make embedding work for GET /libraries/:library_id (946 bytes, patch)
2024-04-04 11:15 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 36482: Add embed tests (1.77 KB, patch)
2024-04-04 11:15 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 36482: Allow embedding desks and cash_registers on libraries endpoints (1.83 KB, patch)
2024-04-16 12:59 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 36482: Make embedding work for GET /libraries/:library_id (946 bytes, patch)
2024-04-16 12:59 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 36482: Add embed tests (1.77 KB, patch)
2024-04-16 12:59 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2024-04-01 20:57:32 UTC

    
Comment 1 Tomás Cohen Arazi 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 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 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 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 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 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 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 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 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 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 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!