[Bug 39204] New: Problem with cancelling a hold via SIP without marking it as waiting returns an error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39204 Bug ID: 39204 Summary: Problem with cancelling a hold via SIP without marking it as waiting returns an error Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: SIP2 Assignee: koha-bugs@lists.koha-community.org Reporter: lucas@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org
From Bug 38615:
"When testing I noticed something both with and without the patch (so this is a general SIP question) If you place a hold and then try to cancel the hold with this command before checking the item into the hold shelf the SIP cancel request returns |AFError with transaction drop_hold:|" Easy to recreate with SIP emulator: 1. /kohadevbox/koha/misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m hold --patron 23529000035676 --item 39999000001310 --hold-mode + 2. /kohadevbox/koha/misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m hold --patron 23529000035676 --item 39999000001310 --hold-mode - 3. See the error: AFError with transaction drop_hold: -- 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=39204 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |blawlor@clamsnet.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39204 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com --- Comment #1 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- This is a combined problem of do_hold and drop_hold not handling items correctly. After step 1 of Lucas' plan, view the patron in the staff interface, they have a 'next available' hold in the biblio, not on the item In C4/SIP/ILS/Transaction/Hold.pm in do_hold: 64 my $canReserve = CanItemBeReserved( $patron, $item, $branch ); 65 if ( $canReserve->{status} eq 'OK' ) { 66 my $priority = C4::Reserves::CalculatePriority( $item->biblionumber ); 67 AddReserve( 68 { 69 priority => $priority, 70 branchcode => $branch, 71 borrowernumber => $patron->borrowernumber, 72 biblionumber => $item->biblionumber 73 } 74 ); We don't pass the itemnumber forward Then in drop_hold - we use the itemnumber to cancel, but there is no item level hold: 93 my $item = Koha::Items->find( { barcode => $self->{item}->id } ); 94 my $holds = $item->holds->search( { borrowernumber => $patron->borrowernumber } ); 95 96 return $self unless $holds->count; -- 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=39204 Laura Escamilla <Laura.escamilla@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=39204 --- Comment #2 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- Created attachment 185913 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185913&action=edit Bug 39204: Added ability to cancel item-level holds via SIP Test plan: 1. /kohadevbox/koha/misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m hold --patron 23529000035676 --item 39999000001310 --hold-mode + 2. /kohadevbox/koha/misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m hold --patron 23529000035676 --item 39999000001310 --hold-mode - 3. See the error: 1. AFError with transaction drop_hold: 4. Apply the patch & restart_all 5. Try step 2 again. See confirmation: 1. AFHold Cancelled. 6. prove t/db_dependent/Koha/Hold.t 7. Sign off — have an amazing day! -- 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=39204 Laura Escamilla <Laura.escamilla@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |Laura.escamilla@bywatersolu |ity.org |tions.com CC| |Laura.escamilla@bywatersolu | |tions.com -- 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=39204 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39204 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #185913|0 |1 is obsolete| | --- Comment #3 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 185921 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185921&action=edit Bug 39204: Added ability to cancel item-level holds via SIP Test plan: 1. /kohadevbox/koha/misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m hold --patron 23529000035676 --item 39999000001310 --hold-mode + 2. /kohadevbox/koha/misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m hold --patron 23529000035676 --item 39999000001310 --hold-mode - 3. See the error: 1. AFError with transaction drop_hold: 4. Apply the patch & restart_all 5. Try step 2 again. See confirmation: 1. AFHold Cancelled. 6. prove t/db_dependent/Koha/Hold.t 7. Sign off — have an amazing day! Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39204 --- Comment #4 from Brendan Lawlor <blawlor@clamsnet.org> --- This works and the tests pass: perl misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m hold --patron 23529000035676 --item 39999000001310 --hold-mode - Attempting socket connection to localhost:6001...connected! SEND: 9300CNterm1|COterm1|CPCPL| READ: 941 Trying 'hold' SEND: 15-20250829 133245AOCPL|AA23529000035676|AB39999000001310|ACterm1| READ: 161N20250829 133245AA23529000035676|BR0|AB39999000001310|AJGairm.|AOCPL|AFHold Cancelled.| prove t/db_dependent/Koha/Hold.t t/db_dependent/Koha/Hold.t .. ok All tests successful. Files=1, Tests=17, 9 wallclock secs ( 0.04 usr 0.01 sys + 6.47 cusr 1.45 csys = 7.97 CPU) Result: PASS -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39204 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |martin.renvoize@openfifth.c | |o.uk --- Comment #5 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Can we have a unit test for this one please? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39204 Laura Escamilla <Laura.escamilla@bywatersolutions.com> 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=39204 --- Comment #6 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- Created attachment 188009 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=188009&action=edit Bug 39204: Unit test -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39204 --- Comment #7 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- (In reply to Martin Renvoize (ashimema) from comment #5)
Can we have a unit test for this one please?
I've added a test! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39204 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |david@davidnind.com --- Comment #8 from David Nind <david@davidnind.com> --- After applying the patch, I don't get the 'AFHold Cancelled' in the response: /kohadevbox/koha/misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m hold --patron 23529000035676 --item 39999000001310 --hold-mode - Attempting socket connection to localhost:6001...connected! SEND: 9300CNterm1|COterm1|CPCPL| READ: 941 Trying 'hold' SEND: 15-20251128 224811AOCPL|AA23529000035676|AB39999000001310|ACterm1| READ: 160N20251128 224811AA23529000035676|AOCPL|AFError with transaction drop_hold: | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39204 --- Comment #9 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- (In reply to David Nind from comment #8) David - did you restart the services (restart_all) after applying the patch? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39204 Laura Escamilla <Laura.escamilla@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #10 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- I’ve confirmed this works on my end and the unit test is attached. I’m moving it back to needs signoff. Happy to revisit if there’s a reproducible case where it’s still failing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39204 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39204 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #185921|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=39204 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #188009|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=39204 --- Comment #11 from David Nind <david@davidnind.com> --- Created attachment 192504 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=192504&action=edit Bug 39204: Added ability to cancel item-level holds via SIP Test plan: 1. /kohadevbox/koha/misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m hold --patron 23529000035676 --item 39999000001310 --hold-mode + 2. /kohadevbox/koha/misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m hold --patron 23529000035676 --item 39999000001310 --hold-mode - 3. See the error: 1. AFError with transaction drop_hold: 4. Apply the patch & restart_all 5. Try step 2 again. See confirmation: 1. AFHold Cancelled. 6. prove t/db_dependent/Koha/Hold.t 7. Sign off — have an amazing day! Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> 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=39204 --- Comment #12 from David Nind <david@davidnind.com> --- Created attachment 192505 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=192505&action=edit Bug 39204: Unit test 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=39204 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |release-notes-needed --- Comment #13 from David Nind <david@davidnind.com> --- (In reply to Laura Escamilla from comment #9)
(In reply to David Nind from comment #8)
David - did you restart the services (restart_all) after applying the patch?
For whatever reason, a restart_all didn't work for me, but a reset_all did. Testing notes (using KTD): 1. Output from step 1: /kohadevbox/koha/misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m hold --patron 23529000035676 --item 39999000001310 --hold-mode + Attempting socket connection to localhost:6001...connected! SEND: 9300CNterm1|COterm1|CPCPL| READ: 941 Trying 'hold' SEND: 15+20260204 210609AOCPL|AA23529000035676|AB39999000001310|ACterm1| READ: 161N20260204 210609AA23529000035676|BR0|AB39999000001310|AJGairm.|AOCPL| 2. Output from step 2: /kohadevbox/koha/misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m hold --patron 23529000035676 --item 39999000001310 --hold-mode - Attempting socket connection to localhost:6001...connected! SEND: 9300CNterm1|COterm1|CPCPL| READ: 941 Trying 'hold' SEND: 15-20260204 210707AOCPL|AA23529000035676|AB39999000001310|ACterm1| READ: 160N20260204 210707AA23529000035676|AOCPL|AFError with transaction drop_hold: | 3. After applying the patch a restart_all didn't work for (maybe it doesn't restart the SIP emulator?). Instead, I did a reset_all and then repeated steps 1 and 2: Output after patch from step 1: /kohadevbox/koha/misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m hold --patron 23529000035676 --item 39999000001310 --hold-mode + Attempting socket connection to localhost:6001...connected! SEND: 9300CNterm1|COterm1|CPCPL| READ: 941 Trying 'hold' SEND: 15+20260204 211241AOCPL|AA23529000035676|AB39999000001310|ACterm1| READ: 161N20260204 211241AA23529000035676|BR0|AB39999000001310|AJGairm.|AOCPL| Output after patch from step 2: /kohadevbox/koha/misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m hold --patron 23529000035676 --item 39999000001310 --hold-mode - Attempting socket connection to localhost:6001...connected! SEND: 9300CNterm1|COterm1|CPCPL| READ: 941 Trying 'hold' SEND: 15-20260204 211250AOCPL|AA23529000035676|AB39999000001310|ACterm1| READ: 161N20260204 211250AA23529000035676|BR0|AB39999000001310|AJGairm.|AOCPL|AFHold Cancelled.| -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39204 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #14 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Laura Escamilla from comment #7)
(In reply to Martin Renvoize (ashimema) from comment #5)
Can we have a unit test for this one please?
I've added a test!
Thats great. But it would be nice to see the changed SIP routine being tested. We need something that tests handle_hold and friends in SIP::MsgType? See e.g. SIP/Message.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39204 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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=39204 Laura Escamilla <Laura.escamilla@bywatersolutions.com> 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=39204 --- Comment #15 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- Created attachment 209630 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209630&action=edit Bug 39204: Add SIP item-level hold creation and cancellation tests Added a new regression test in `t/db_dependent/SIP/Message.t` to address the feedback regarding SIP-level test coverage. The test exercises SIP `15+` hold creation and SIP `15-` hold cancellation through `handle_hold`, verifying that an item-level hold is created and successfully removed. I also verified that the test fails when the `itemnumber` assignment is removed from `C4/SIP/ILS/Transaction/Hold.pm` and passes when it is restored. All 25 tests in `t/db_dependent/SIP/Message.t` pass, with no warnings. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39204 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39204 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #192504|0 |1 is obsolete| | Attachment #192505|0 |1 is obsolete| | Attachment #209630|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=39204 --- Comment #16 from David Nind <david@davidnind.com> --- Created attachment 210040 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=210040&action=edit Bug 39204: Added ability to cancel item-level holds via SIP Test plan: 1. /kohadevbox/koha/misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m hold --patron 23529000035676 --item 39999000001310 --hold-mode + 2. /kohadevbox/koha/misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m hold --patron 23529000035676 --item 39999000001310 --hold-mode - 3. See the error: 1. AFError with transaction drop_hold: 4. Apply the patch & restart_all 5. Try step 2 again. See confirmation: 1. AFHold Cancelled. 6. prove t/db_dependent/Koha/Hold.t 7. Sign off — have an amazing day! Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> 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=39204 --- Comment #17 from David Nind <david@davidnind.com> --- Created attachment 210041 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=210041&action=edit Bug 39204: Unit test 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=39204 --- Comment #18 from David Nind <david@davidnind.com> --- Created attachment 210042 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=210042&action=edit Bug 39204: Add SIP item-level hold creation and cancellation tests Added a new regression test in `t/db_dependent/SIP/Message.t` to address the feedback regarding SIP-level test coverage. The test exercises SIP `15+` hold creation and SIP `15-` hold cancellation through `handle_hold`, verifying that an item-level hold is created and successfully removed. I also verified that the test fails when the `itemnumber` assignment is removed from `C4/SIP/ILS/Transaction/Hold.pm` and passes when it is restored. All 25 tests in `t/db_dependent/SIP/Message.t` pass, with no warnings. 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=39204 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |paul.derscheid@lmscloud.de |y.org | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org