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).
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
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>
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 ```
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
I changed it to be just PUT /holds/{hold_id}/lowest_priority Can you test again, please.
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
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
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".
(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.
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>
Passed now, and have signed off - thanks for fixing!