[Bug 30661] New: Able to update more hold parameters via REST API
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Bug ID: 30661 Summary: Able to update more hold parameters via REST API Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: johanna.raisa@koha-suomi.fi CC: tomascohen@gmail.com If the holds edit endpoint is wanted to use more effectively then it needs more parameters than priority, pickup_library and suspend_until. Is there a plan for PUT action since it has acted as PATCH and is now deprecated? There definitely should be also a real PUT endpoint as documented here https://wiki.koha-community.org/wiki/Holds_endpoint_RFC, now the situation is quite misleading. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Johanna Räisä <johanna.raisa@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Assignee|koha-bugs@lists.koha-commun |johanna.raisa@koha-suomi.fi |ity.org | --- Comment #1 from Johanna Räisä <johanna.raisa@koha-suomi.fi> --- Created attachment 139973 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=139973&action=edit Bug 30661: Allow to update more hold parameters via REST This patch adds hold_date and expiration_date to holds edit endpoint Test plan: 1) prove t/db_dependent/api/v1/holds.t Sponsored-by: Koha-Suomi Oy -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 --- Comment #2 from Johanna Räisä <johanna.raisa@koha-suomi.fi> --- Created attachment 139974 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=139974&action=edit Bug 30661: [FOLLOW-UP] add separate endpoints This patch adds separate endpoints for updating hold_date and expiration_date. Test plan: 1) prove t/db_dependent/api/v1/holds.t Sponsored-by: Koha-Suomi Oy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Johanna Räisä <johanna.raisa@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Change sponsored?|--- |Sponsored -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joonas.kylmala@iki.fi --- Comment #3 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- Is it right to allow changing the reservedate after it has been set as you cannot do that now either through staff or opac as far as I know? Did you intend to use this to change the reservedate for a hold with reservedate in the future? Other than that I cannot think of any use case for this. We would have to probably add some checks to make sure the API user is only able to edit reservedate in the future, not one that has already passed? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 --- Comment #4 from Owen Leonard <oleonard@myacpl.org> --- (In reply to Joonas Kylmälä from comment #3)
Is it right to allow changing the reservedate after it has been set as you cannot do that now either through staff or opac as far as I know?
You can update reservedate from the hold details page, e.g. /cgi-bin/koha/reserve/request.pl?biblionumber=XXX -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 --- Comment #5 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- (In reply to Owen Leonard from comment #4)
(In reply to Joonas Kylmälä from comment #3)
Is it right to allow changing the reservedate after it has been set as you cannot do that now either through staff or opac as far as I know?
You can update reservedate from the hold details page, e.g. /cgi-bin/koha/reserve/request.pl?biblionumber=XXX
How? I don't see such option. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 --- Comment #6 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 140221 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=140221&action=edit Changing the hold date of an existing hold -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Johanna Räisä <johanna.raisa@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |23269 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23269 [Bug 23269] Long hold queues are slowing the service -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 --- Comment #7 from Johanna Räisä <johanna.raisa@koha-suomi.fi> --- If we want to start using API in the frontend then we need these endpoints. Creating a pagination to holds is impossible, so I will use the endpoints for this bug: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23269 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 --- Comment #8 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- (In reply to Owen Leonard from comment #6)
Created attachment 140221 [details] Changing the hold date of an existing hold
Thanks, I figured it out, need to enable AllowHoldDateInFuture syspref in order for the date to be editable. It seems to be possible also to change the date back in the past through staff interface so having it being possible through API is not a problem. Although I would consider it a bug but it can be handled in a separate report. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #9 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- 1. The API seems to break after applying these patches: [ERROR] Warning: Could not load REST API spec bundle: Unable to load schema "file:///kohadevbox/koha/api/v1/swagger/parameters.yaml". at /usr/share/perl5/JSON/Validator/Store.pm line 53. Koha::REST::V1::catch {...} /kohadevbox/koha/Koha/REST/V1.pm (130) Cannot do any API requests. 2. Going back to the reservedate modification through API. There needs to be a check to disallow changing reservedate if the reserve has been already found. Actually reading the ModReserve code it says:
# The only column that can be updated for a found hold is the expiration date $hold->expirationdate($date)->store();
The current API for editing hold already does these checks as it calls ModReserve. So I'm thinking either the new API endpoints need to call ModReserve too or the checks from ModReserve need to be duplicated to to the API functions. I think it would be better to call ModReserve as then we don't risk only changing the conditions only either in the API side or ModReserve side. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 --- Comment #10 from Johanna Räisä <johanna.raisa@koha-suomi.fi> --- (In reply to Joonas Kylmälä from comment #9)
1. The API seems to break after applying these patches:
[ERROR] Warning: Could not load REST API spec bundle: Unable to load schema "file:///kohadevbox/koha/api/v1/swagger/parameters.yaml". at /usr/share/perl5/JSON/Validator/Store.pm line 53. Koha::REST::V1::catch {...} /kohadevbox/koha/Koha/REST/V1.pm (130)
Is your test environment updated? The parameters.yaml doesn't exist anymore, it is replaced with swagger.yaml.
Cannot do any API requests.
2. Going back to the reservedate modification through API. There needs to be a check to disallow changing reservedate if the reserve has been already found. Actually reading the ModReserve code it says:
# The only column that can be updated for a found hold is the expiration date $hold->expirationdate($date)->store();
The current API for editing hold already does these checks as it calls ModReserve. So I'm thinking either the new API endpoints need to call ModReserve too or the checks from ModReserve need to be duplicated to to the API functions. I think it would be better to call ModReserve as then we don't risk only changing the conditions only either in the API side or ModReserve side.
I will check this out. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 --- Comment #11 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- (In reply to Johanna Räisä from comment #10)
Is your test environment updated? The parameters.yaml doesn't exist anymore, it is replaced with swagger.yaml.
It is updated. The second patch refers to parameters.yaml. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Johanna Räisä <johanna.raisa@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #139974|0 |1 is obsolete| | --- Comment #12 from Johanna Räisä <johanna.raisa@koha-suomi.fi> --- Created attachment 140359 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=140359&action=edit Bug 30661: [FOLLOW-UP] add separate endpoints Here is the fixed follow-up patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Johanna Räisä <johanna.raisa@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Status|Needs Signoff |Failed QA --- Comment #13 from David Nind <david@davidnind.com> --- The tests fail. Also, I think the format for the follow-up in the bug title is Bug XXXX: (follow-up) ... prove t/db_dependent/api/v1/holds.t t/db_dependent/api/v1/holds.t .. 15/16 Use of uninitialized value $rank in string eq at /kohadevbox/koha/C4/Reserves.pm line 1115. Use of uninitialized value $rank in pattern match (m//) at /kohadevbox/koha/C4/Reserves.pm line 1125. # Failed test 'expiration date changed correctly' # at t/db_dependent/api/v1/holds.t line 1597. # got: undef # expected: '2022-01-01' # Looks like you failed 1 test of 7. # Failed test 'PUT /holds/{hold_id}/expiration_date tests' # at t/db_dependent/api/v1/holds.t line 1600. # Looks like you failed 1 test of 16. t/db_dependent/api/v1/holds.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/16 subtests Test Summary Report ------------------- t/db_dependent/api/v1/holds.t (Wstat: 256 Tests: 16 Failed: 1) Failed test: 16 Non-zero exit status: 1 Files=1, Tests=16, 25 wallclock secs ( 0.07 usr 0.01 sys + 21.83 cusr 1.58 csys = 23.49 CPU) Result: FAIL -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Hammat wele <hammat.wele@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hammat.wele@inlibro.com Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Hammat wele <hammat.wele@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Emmi Takkinen <emmi.takkinen@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |emmi.takkinen@koha-suomi.fi -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Johanna Räisä <johanna.raisa@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #139973|0 |1 is obsolete| | --- Comment #14 from Johanna Räisä <johanna.raisa@koha-suomi.fi> --- Created attachment 173116 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173116&action=edit Bug 30661: Allow to update more hold parameters via REST API This patch adds hold_date and expiration_date to holds edit endpoint Test plan: 1) prove t/db_dependent/api/v1/holds.t Sponsored-by: Koha-Suomi Oy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Johanna Räisä <johanna.raisa@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #140359|0 |1 is obsolete| | --- Comment #15 from Johanna Räisä <johanna.raisa@koha-suomi.fi> --- Created attachment 173117 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173117&action=edit Bug 30661: (follow-up) Add separate endpoints This patch adds separate endpoints for updating hold_date and expiration_date. Test plan: 1) prove t/db_dependent/api/v1/holds.t Sponsored-by: Koha-Suomi Oy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Johanna Räisä <johanna.raisa@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #16 from David Nind <david@davidnind.com> --- Tests are still failing for me 8-(: prove t/db_dependent/api/v1/holds.t t/db_dependent/api/v1/holds.t .. 9/16 # Failed test '200 OK' # at t/db_dependent/api/v1/holds.t line 1036. # got: '400' # expected: '200' # Failed test 'Hold date changed correctly' # at t/db_dependent/api/v1/holds.t line 1040. # got: '2022-01-01' # expected: '2022-01-02' # Failed test 'Expiration date changed correctly' # at t/db_dependent/api/v1/holds.t line 1041. # got: '2022-03-01' # expected: '2022-03-02' # Failed test '200 OK' # at t/db_dependent/api/v1/holds.t line 1116. # got: '400' # expected: '200' # Failed test 'Hold date changed correctly' # at t/db_dependent/api/v1/holds.t line 1119. # got: '2022-01-01' # expected: '2022-01-02' # Failed test 'Expiration date changed correctly' # at t/db_dependent/api/v1/holds.t line 1120. # got: '2022-03-01' # expected: '2022-03-02' # Looks like you failed 6 tests of 47. t/db_dependent/api/v1/holds.t .. 11/16 # Failed test 'edit() tests' # at t/db_dependent/api/v1/holds.t line 1124. t/db_dependent/api/v1/holds.t .. 14/16 # Failed test '403 Forbidden' # at t/db_dependent/api/v1/holds.t line 1624. # got: '404' # expected: '403' # Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/holds.t line 1624. # Structures begin differing at: # $got->{errors} = ARRAY(0x60ee6c5f4130) # $expected->{errors} = Does not exist # Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/holds.t line 1631. # Structures begin differing at: # $got->{status} = '404' # $expected->{status} = Does not exist # Failed test '200 OK' # at t/db_dependent/api/v1/holds.t line 1634. # got: '404' # expected: '200' # Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/holds.t line 1634. # Structures begin differing at: # $got->{status} = '404' # $expected->{status} = Does not exist # Failed test 'hold date changed correctly' # at t/db_dependent/api/v1/holds.t line 1638. # got: '2024-10-22' # expected: '2022-01-01' # Looks like you failed 6 tests of 10. # Failed test 'PUT /holds/{hold_id}/hold_date tests' # at t/db_dependent/api/v1/holds.t line 1641. # Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/holds.t line 1687. # Structures begin differing at: # $got->{errors} = ARRAY(0x60ee6ca3f200) # $expected->{errors} = Does not exist # Failed test '200 OK' # at t/db_dependent/api/v1/holds.t line 1691. # got: '404' # expected: '200' # Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/holds.t line 1691. # Structures begin differing at: # $got->{errors} = ARRAY(0x60ee6c5e0ab8) # $expected->{errors} = Does not exist # Failed test 'expiration date changed correctly' # at t/db_dependent/api/v1/holds.t line 1696. # got: undef # expected: '2022-01-01' # Looks like you failed 4 tests of 7. # Failed test 'PUT /holds/{hold_id}/expiration_date tests' # at t/db_dependent/api/v1/holds.t line 1699. # Looks like you failed 3 tests of 16. t/db_dependent/api/v1/holds.t .. Dubious, test returned 3 (wstat 768, 0x300) Failed 3/16 subtests Test Summary Report ------------------- t/db_dependent/api/v1/holds.t (Wstat: 768 (exited 3) Tests: 16 Failed: 3) Failed tests: 11, 15-16 Non-zero exit status: 3 Files=1, Tests=16, 11 wallclock secs ( 0.04 usr 0.01 sys + 8.69 cusr 2.04 csys = 10.78 CPU) Result: FAIL -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 --- Comment #17 from Johanna Räisä <johanna.raisa@koha-suomi.fi> --- (In reply to David Nind from comment #16)
Tests are still failing for me 8-(:
prove t/db_dependent/api/v1/holds.t t/db_dependent/api/v1/holds.t .. 9/16 # Failed test '200 OK' # at t/db_dependent/api/v1/holds.t line 1036. # got: '400' # expected: '200'
# Failed test 'Hold date changed correctly' # at t/db_dependent/api/v1/holds.t line 1040. # got: '2022-01-01' # expected: '2022-01-02'
# Failed test 'Expiration date changed correctly' # at t/db_dependent/api/v1/holds.t line 1041. # got: '2022-03-01' # expected: '2022-03-02'
# Failed test '200 OK' # at t/db_dependent/api/v1/holds.t line 1116. # got: '400' # expected: '200'
# Failed test 'Hold date changed correctly' # at t/db_dependent/api/v1/holds.t line 1119. # got: '2022-01-01' # expected: '2022-01-02'
# Failed test 'Expiration date changed correctly' # at t/db_dependent/api/v1/holds.t line 1120. # got: '2022-03-01' # expected: '2022-03-02' # Looks like you failed 6 tests of 47. t/db_dependent/api/v1/holds.t .. 11/16 # Failed test 'edit() tests' # at t/db_dependent/api/v1/holds.t line 1124. t/db_dependent/api/v1/holds.t .. 14/16 # Failed test '403 Forbidden' # at t/db_dependent/api/v1/holds.t line 1624. # got: '404' # expected: '403'
# Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/holds.t line 1624. # Structures begin differing at: # $got->{errors} = ARRAY(0x60ee6c5f4130) # $expected->{errors} = Does not exist
# Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/holds.t line 1631. # Structures begin differing at: # $got->{status} = '404' # $expected->{status} = Does not exist
# Failed test '200 OK' # at t/db_dependent/api/v1/holds.t line 1634. # got: '404' # expected: '200'
# Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/holds.t line 1634. # Structures begin differing at: # $got->{status} = '404' # $expected->{status} = Does not exist
# Failed test 'hold date changed correctly' # at t/db_dependent/api/v1/holds.t line 1638. # got: '2024-10-22' # expected: '2022-01-01' # Looks like you failed 6 tests of 10.
# Failed test 'PUT /holds/{hold_id}/hold_date tests' # at t/db_dependent/api/v1/holds.t line 1641.
# Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/holds.t line 1687. # Structures begin differing at: # $got->{errors} = ARRAY(0x60ee6ca3f200) # $expected->{errors} = Does not exist
# Failed test '200 OK' # at t/db_dependent/api/v1/holds.t line 1691. # got: '404' # expected: '200'
# Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/holds.t line 1691. # Structures begin differing at: # $got->{errors} = ARRAY(0x60ee6c5e0ab8) # $expected->{errors} = Does not exist
# Failed test 'expiration date changed correctly' # at t/db_dependent/api/v1/holds.t line 1696. # got: undef # expected: '2022-01-01' # Looks like you failed 4 tests of 7.
# Failed test 'PUT /holds/{hold_id}/expiration_date tests' # at t/db_dependent/api/v1/holds.t line 1699. # Looks like you failed 3 tests of 16. t/db_dependent/api/v1/holds.t .. Dubious, test returned 3 (wstat 768, 0x300) Failed 3/16 subtests
Test Summary Report ------------------- t/db_dependent/api/v1/holds.t (Wstat: 768 (exited 3) Tests: 16 Failed: 3) Failed tests: 11, 15-16 Non-zero exit status: 3 Files=1, Tests=16, 11 wallclock secs ( 0.04 usr 0.01 sys + 8.69 cusr 2.04 csys = 10.78 CPU) Result: FAIL
Interesting, I fetched main branch today and added the code on it. My tests go through, are you on main branch? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 --- Comment #18 from David Nind <david@davidnind.com> --- (In reply to Johanna Räisä from comment #17)
(In reply to David Nind from comment #16)
Tests are still failing for me 8-(:
prove t/db_dependent/api/v1/holds.t t/db_dependent/api/v1/holds.t .. 9/16 ... Interesting, I fetched main branch today and added the code on it. My tests go through, are you on main branch?
Using KTD - pulled everything (Koha clone, KTD clone, KTD pull). Tests pass before applying the patches. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 --- Comment #19 from Johanna Räisä <johanna.raisa@koha-suomi.fi> --- (In reply to David Nind from comment #18)
(In reply to Johanna Räisä from comment #17)
(In reply to David Nind from comment #16)
Tests are still failing for me 8-(:
prove t/db_dependent/api/v1/holds.t t/db_dependent/api/v1/holds.t .. 9/16 ... Interesting, I fetched main branch today and added the code on it. My tests go through, are you on main branch?
Using KTD - pulled everything (Koha clone, KTD clone, KTD pull). Tests pass before applying the patches.
# Failed test '403 Forbidden' # at t/db_dependent/api/v1/holds.t line 1624. # got: '404' # expected: '403' This indicates that the endpoints are not in place, since there is 404 error instead of 403. My koha-dev is installed to my computer and endpoints work after plack restart. Can you restart plack or is it done automatically? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 --- Comment #20 from David Nind <david@davidnind.com> --- (In reply to Johanna Räisä from comment #19) ..
Using KTD - pulled everything (Koha clone, KTD clone, KTD pull). Tests pass before applying the patches.
# Failed test '403 Forbidden' # at t/db_dependent/api/v1/holds.t line 1624. # got: '404' # expected: '403'
This indicates that the endpoints are not in place, since there is 404 error instead of 403. My koha-dev is installed to my computer and endpoints work after plack restart. Can you restart plack or is it done automatically?
I did a restart (restart_all) (I normally do this as a matter of course after applying patches). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173116|0 |1 is obsolete| | --- Comment #21 from David Nind <david@davidnind.com> --- Created attachment 173122 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173122&action=edit Bug 30661: Allow to update more hold parameters via REST API This patch adds hold_date and expiration_date to holds edit endpoint Test plan: 1) prove t/db_dependent/api/v1/holds.t Sponsored-by: Koha-Suomi Oy Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173117|0 |1 is obsolete| | --- Comment #22 from David Nind <david@davidnind.com> --- Created attachment 173123 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173123&action=edit Bug 30661: (follow-up) Add separate endpoints This patch adds separate endpoints for updating hold_date and expiration_date. Test plan: 1) prove t/db_dependent/api/v1/holds.t Sponsored-by: Koha-Suomi Oy Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 --- Comment #23 from David Nind <david@davidnind.com> --- Testing notes (using KTD): 1. Run the tests before applying the patches - tests should pass: prove t/db_dependent/api/v1/holds.t 2. Apply the patches. 3. Update the API specification: yarn api:bundle 4. Restart everything: restart_all 5. Re-run the tests - they should still pass. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pedro.amorim@ptfs-europe.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |pedro.amorim@ptfs-europe.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |38229 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38229 [Bug 38229] Clean up PATCH on /holds endpoints -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 --- Comment #24 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Hi, I'm looking here. Had a chat with Martin we agreed that due to existing pattern, there's no point in blocking this for adding specific 2 new specific PUT endpoints and we can address that in bug 38229. Though the first patch is also adding the attributes to the edit PATCH endpoint, I'm possibly missing something here but shouldn't we only need one or the other? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 --- Comment #25 from Johanna Räisä <johanna.raisa@koha-suomi.fi> --- (In reply to Pedro Amorim from comment #24)
Hi, I'm looking here. Had a chat with Martin we agreed that due to existing pattern, there's no point in blocking this for adding specific 2 new specific PUT endpoints and we can address that in bug 38229.
Though the first patch is also adding the attributes to the edit PATCH endpoint, I'm possibly missing something here but shouldn't we only need one or the other?
There are some time since I started doing this, but I think I have made this similar as the pickup_locations. It has it's own PUT endpoint and it is also included to the edit(). We can skip the follow-up since PATCH should allow updating a singular value. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173122|0 |1 is obsolete| | --- Comment #26 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 173209 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173209&action=edit Bug 30661: Allow to update more hold parameters via REST API This patch adds hold_date and expiration_date to holds edit endpoint Test plan: 1) 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173123|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #27 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- (In reply to Johanna Räisä from comment #25)
We can skip the follow-up since PATCH should allow updating a singular value.
Hi Johanna, I've dropped the follow-up patch as discussed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |24.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 --- Comment #28 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.11! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 --- Comment #29 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Could you have a look if these failures relate to this change and provide a fix? prove t/db_dependent/api/v1/holds.t t/db_dependent/api/v1/holds.t .. 1/14 No reserves HOLD_CANCELLATION letter transported by email at /kohadevbox/koha/C4/Letters.pm line 610. t/db_dependent/api/v1/holds.t .. 12/14 # Looks like you planned 37 tests but ran 3. t/db_dependent/api/v1/holds.t .. 13/14 # Failed test 'PUT /holds/{hold_id}/pickup_location tests' # at t/db_dependent/api/v1/holds.t line 1470. Can't locate object method "branchcode" via package "euRljh" (perhaps you forgot to load "euRljh"?) at t/db_dependent/api/v1/holds.t line 1377. # Looks like your test exited with 11 just after 13. t/db_dependent/api/v1/holds.t .. Dubious, test returned 11 (wstat 2816, 0xb00) Failed 2/14 subtests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 --- Comment #30 from Johanna Räisä <johanna.raisa@koha-suomi.fi> --- (In reply to Katrin Fischer from comment #29)
Could you have a look if these failures relate to this change and provide a fix?
prove t/db_dependent/api/v1/holds.t t/db_dependent/api/v1/holds.t .. 1/14 No reserves HOLD_CANCELLATION letter transported by email at /kohadevbox/koha/C4/Letters.pm line 610. t/db_dependent/api/v1/holds.t .. 12/14 # Looks like you planned 37 tests but ran 3. t/db_dependent/api/v1/holds.t .. 13/14 # Failed test 'PUT /holds/{hold_id}/pickup_location tests' # at t/db_dependent/api/v1/holds.t line 1470. Can't locate object method "branchcode" via package "euRljh" (perhaps you forgot to load "euRljh"?) at t/db_dependent/api/v1/holds.t line 1377. # Looks like your test exited with 11 just after 13. t/db_dependent/api/v1/holds.t .. Dubious, test returned 11 (wstat 2816, 0xb00) Failed 2/14 subtests
Not related to this, I think. I didn't get that HOLD_CANCELLATION error after I pulled main repo. The subtest "PUT /holds/{hold_id}/pickup_location tests" fails in holds.t There are some interesting branchcode fetching in "PUT /holds/{hold_id}/pickup_location tests" at least: $hold->discard_changes->branchcode->branchcode If those second branchcode values are removed the tests pass in that subtest. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 --- Comment #31 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Thanks for the quick reply, Johanna. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 --- Comment #32 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- (In reply to Katrin Fischer from comment #29)
Could you have a look if these failures relate to this change and provide a fix?
I think this was caused by bug 38273. I've left a fix patch there. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |RESOLVED Resolution|--- |FIXED CC| |lucas@bywatersolutions.com --- Comment #33 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Enhancement, no 24.05.x backport -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30661 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lisette@bywatersolutions.co | |m Keywords|release-notes-needed | Text to go in the| |This enhancement adds API release notes| |endpoints to update the | |hold_date and | |expiration_date via the | |API. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org