Bug 38253

Summary: REST API: Toggle holds lowest priority via REST API
Product: Koha Reporter: Emmi Takkinen <emmi.takkinen>
Component: REST APIAssignee: Johanna Räisä <johanna.raisa>
Status: Passed QA --- QA Contact: Tomás Cohen Arazi (tcohen) <tomascohen>
Severity: enhancement    
Priority: P5 - low CC: david, johanna.raisa, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on:    
Bug Blocks: 23269    
Attachments: Bug 38253: Add toggle holds lowest priority via REST API
Bug 38253: Add toggle holds lowest priority via REST API
Bug 38253: Add lowest priority REST API endpoint for holds
Bug 38253: Add lowest priority REST API endpoint for holds
Bug 38253: Add lowest priority REST API endpoint for holds
Bug 38253: Add lowest priority REST API endpoint for holds

Description Emmi Takkinen 2024-10-24 08:53:42 UTC
We need an ability to toggle holds lowest priority via REST API, either from holds edit endpoint or from separate endpoint (e.g. like /holds/{hold_id}/priority).
Comment 1 Johanna Räisä 2024-10-24 09:51:14 UTC
Created attachment 173260 [details] [review]
Bug 38253: Add toggle holds lowest priority via REST API

This patch adds a new endpoint to toggle the lowest priority of a hold via the REST API.

To test:
1) Apply the patch
2) perl build-resources.PL
3) prove t/db_dependent/api/v1/holds.t

Sponsored-by: Koha-Suomi Oy
Comment 2 Lisette Scheer 2024-10-24 14:45:38 UTC
Created attachment 173279 [details] [review]
Bug 38253: Add toggle holds lowest priority via REST API

This patch adds a new endpoint to toggle the lowest priority of a hold via the REST API.

To test:
1) Apply the patch
2) perl build-resources.PL
3) prove t/db_dependent/api/v1/holds.t

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 3 Tomás Cohen Arazi (tcohen) 2024-10-25 11:16:48 UTC
I don't think having a verb in the resource path is RESTful, and we should avoid it for consistency.

This can be solved by getting rid of the 'toggle' nature of the route and the method used. For example:

```
PUT /holds/{hold_id}/lowest_priority
true
```

or

```
PUT /holds/{hold_id}/lowest_priority
false
```
Comment 4 Johanna Räisä 2024-10-29 06:21:59 UTC
Created attachment 173602 [details] [review]
Bug 38253: Add lowest priority REST API endpoint for holds

This patch adds a new endpoint to toggle the lowest priority of a hold via the REST API.

To test:
1) Apply the patch
2) perl build-resources.PL
3) prove t/db_dependent/api/v1/holds.t

Sponsored-by: Koha-Suomi Oy
Comment 5 Johanna Räisä 2024-10-29 06:24:00 UTC
I changed it to be just PUT /holds/{hold_id}/lowest_priority
Can you test again, please.
Comment 6 David Nind 2024-10-31 22:08:51 UTC
The tests fail for me (tested using koha-testing-docker). Is there anything that needs to be configured first?

prove t/db_dependent/api/v1/holds.t
t/db_dependent/api/v1/holds.t .. 1/15 No reserves HOLD_CANCELLATION letter transported by email at /kohadevbox/koha/C4/Letters.pm line 610.
t/db_dependent/api/v1/holds.t .. 14/15     # Looks like you planned 5 tests but ran 4.

#   Failed test 'PUT /holds/{hold_id}/lowest_priority tests'
#   at t/db_dependent/api/v1/holds.t line 1629.
The method Koha::Hold->lowest_priority is not covered by tests!

Trace begun at /kohadevbox/koha/Koha/Object.pm line 1040
Koha::Object::AUTOLOAD('Koha::Hold=HASH(0x5b0544e7cf28)') called at t/db_dependent/api/v1/holds.t line 1626
main::__ANON__ at /usr/share/perl/5.36/Test/Builder.pm line 374
eval {...} at /usr/share/perl/5.36/Test/Builder.pm line 374
Test::Builder::subtest('Test::Builder=HASH(0x5b05389595c0)', 'PUT /holds/{hold_id}/lowest_priority tests', 'CODE(0x5b05447cc908)') called at /usr/share/perl/5.36/Test/More.pm line 809
Test::More::subtest('PUT /holds/{hold_id}/lowest_priority tests', 'CODE(0x5b05447cc908)') called at t/db_dependent/api/v1/holds.t line 1629
# Looks like your test exited with 11 just after 15.
t/db_dependent/api/v1/holds.t .. Dubious, test returned 11 (wstat 2816, 0xb00)
Failed 1/15 subtests 

Test Summary Report
-------------------
t/db_dependent/api/v1/holds.t (Wstat: 2816 (exited 11) Tests: 15 Failed: 1)
  Failed test:  15
  Non-zero exit status: 11
Files=1, Tests=15, 11 wallclock secs ( 0.04 usr  0.00 sys +  8.77 cusr  1.94 csys = 10.75 CPU)
Result: FAIL
Comment 7 Johanna Räisä 2024-11-01 06:35:08 UTC
Created attachment 173852 [details] [review]
Bug 38253: Add lowest priority REST API endpoint for holds

This patch adds a new endpoint to toggle the lowest priority of a hold via the REST API.

To test:
1) Apply the patch
2) perl build-resources.PL
3) prove t/db_dependent/api/v1/holds.t

Sponsored-by: Koha-Suomi Oy
Comment 8 Johanna Räisä 2024-11-01 06:40:15 UTC
I fixed the "PUT /holds/{hold_id}/lowest_priority tests" subtest, the name in the object had changed. 

I fetched the latest main and I can't see this error, "No reserves HOLD_CANCELLATION letter transported by email".
Comment 9 Katrin Fischer 2024-11-01 07:40:54 UTC
(In reply to Johanna Räisä from comment #8)
> I fixed the "PUT /holds/{hold_id}/lowest_priority tests" subtest, the name
> in the object had changed. 
> 
> I fetched the latest main and I can't see this error, "No reserves
> HOLD_CANCELLATION letter transported by email".

I think we fixed this fail in the meantime as another patch had broken the test.
Comment 10 David Nind 2024-11-01 08:31:32 UTC
Created attachment 173857 [details] [review]
Bug 38253: Add lowest priority REST API endpoint for holds

This patch adds a new endpoint to toggle the lowest priority of a hold via the REST API.

To test:
1) Apply the patch
2) perl build-resources.PL
3) prove t/db_dependent/api/v1/holds.t

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Comment 11 David Nind 2024-11-01 08:32:31 UTC
Passed now, and have signed off - thanks for fixing!
Comment 12 Pedro Amorim 2025-01-24 12:28:21 UTC
Created attachment 177113 [details] [review]
Bug 38253: Add lowest priority REST API endpoint for holds

This patch adds a new endpoint to toggle the lowest priority of a hold via the REST API.

To test:
1) Apply the patch
2) perl build-resources.PL
3) prove t/db_dependent/api/v1/holds.t

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>