Bug 39204

Summary: Problem with cancelling a hold via SIP without marking it as waiting returns an error
Product: Koha Reporter: Lucas Gass (lukeg) <lucas>
Component: SIP2Assignee: Laura Escamilla <Laura.escamilla>
Status: Signed Off --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: blawlor, Laura.escamilla, nick
Version: unspecified   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: --- Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 39204: Added ability to cancel item-level holds via SIP
Bug 39204: Added ability to cancel item-level holds via SIP

Description Lucas Gass (lukeg) 2025-02-26 15:39:31 UTC
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:
Comment 1 Nick Clemens (kidclamp) 2025-07-11 20:49:25 UTC
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;
Comment 2 Laura Escamilla 2025-08-29 12:48:28 UTC
Created attachment 185913 [details] [review]
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!
Comment 3 Brendan Lawlor 2025-08-29 13:35:53 UTC
Created attachment 185921 [details] [review]
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>
Comment 4 Brendan Lawlor 2025-08-29 13:36:59 UTC
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