Bug 28464 - Cancelling a waiting hold via SIP returns a failed response even when cancellation succeeds
Summary: Cancelling a waiting hold via SIP returns a failed response even when cancell...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 24553
Blocks:
  Show dependency treegraph
 
Reported: 2021-05-26 15:11 UTC by Kyle M Hall
Modified: 2022-06-06 20:27 UTC (History)
5 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:
21.11.00,21.05.05,20.11.11


Attachments
Bug 28464: Add unit tests (3.10 KB, patch)
2021-05-26 15:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 28464: Remove useless check that gives the incorrect error message (1.02 KB, patch)
2021-05-26 15:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 28464: Add unit tests (3.14 KB, patch)
2021-06-20 19:48 UTC, David Nind
Details | Diff | Splinter Review
Bug 28464: Remove useless check that gives the incorrect error message (1.05 KB, patch)
2021-06-20 19:48 UTC, David Nind
Details | Diff | Splinter Review
Bug 28464: Add unit tests (3.06 KB, patch)
2021-09-10 07:24 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28464: Remove useless check that gives the incorrect error message (1.15 KB, patch)
2021-09-10 07:24 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28464: (QA follow-up) Correct test plan (721 bytes, patch)
2021-09-10 07:24 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2021-05-26 15:11:26 UTC
Bug 24553 solved the issue related to non-waiting holds returning errors when canceled. However, if a hold is waiting and canceled, the SIP server will return the incorrect error message "No such hold on patron record." even though the hold canceled just fine.
Comment 1 Kyle M Hall 2021-05-26 15:16:40 UTC
Created attachment 121437 [details] [review]
Bug 28464: Add unit tests
Comment 2 Kyle M Hall 2021-05-26 15:17:42 UTC
Created attachment 121438 [details] [review]
Bug 28464: Remove useless check that gives the incorrect error message
Comment 3 David Nind 2021-06-20 19:48:24 UTC
Created attachment 122199 [details] [review]
Bug 28464: Add unit tests

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2021-06-20 19:48:27 UTC
Created attachment 122200 [details] [review]
Bug 28464: Remove useless check that gives the incorrect error message

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2021-06-20 19:49:54 UTC
Testing notes (koha-testing-docker):
- Ran prove t/db_dependent/SIP/ILS.t before and after patches applied
- Tests passed both before and after
- Hope that is sufficient to test!
Comment 6 Magnus Enger 2021-08-10 12:20:06 UTC
Is it a bit weird that waiting holds can be cancelled via SIP2, but not the UI? See bug 22456.
Comment 7 Marcel de Rooy 2021-09-10 07:24:19 UTC
Created attachment 124722 [details] [review]
Bug 28464: Add unit tests

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 8 Marcel de Rooy 2021-09-10 07:24:22 UTC
Created attachment 124723 [details] [review]
Bug 28464: Remove useless check that gives the incorrect error message

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Marcel de Rooy 2021-09-10 07:24:26 UTC
Created attachment 124724 [details] [review]
Bug 28464: (QA follow-up) Correct test plan

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Jonathan Druart 2021-09-20 11:52:46 UTC
First and last patches will be squashed when pushed.
Comment 11 Jonathan Druart 2021-09-20 12:47:28 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 12 Kyle M Hall 2021-10-01 18:04:41 UTC
Pushed to 21.05.x for 21.05.05
Comment 13 Fridolin Somers 2021-10-06 20:16:42 UTC
Pushed to 20.11.x for 20.11.11