Bug 23909: Extend SIP to distinguish pending vs waiting holds

This patch extends the SIP checkout handler to honour the new
three-valued AllowItemsOnHoldCheckoutSIP preference:

* 0 - Don't allow checkout of any held item
* 1 - Allow checkout when the hold is still pending (not attached)
* 2 - Also allow checkout when the hold is waiting, in transit or
      being processed for another patron

Previously the preference was a simple YesNo toggle that, when
enabled, allowed the checkout regardless of whether the hold was
pending or already attached to an item on the hold shelf. Libraries
could not block the more disruptive case (checking out a waiting
hold destined for another patron) without also blocking pending
holds.

The existing post-confirmation filter is reworked into a single
branch per confirmation type that consults the preference value.

Test plan:
1. prove t/db_dependent/SIP/Transaction.t
2. With AllowItemsOnHoldCheckoutSIP=0, place a pending hold for
   patron A on an item, attempt SIP checkout to patron B: refused.
3. Repeat with the hold marked waiting: refused.
4. Set AllowItemsOnHoldCheckoutSIP=1, pending hold: allowed;
   waiting hold: refused.
5. Set AllowItemsOnHoldCheckoutSIP=2, waiting hold: allowed.

Signed-off-by: Kristi Krueger <kkrueger@cuyahogalibrary.org>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Martin Renvoize 2026-04-20 17:48:56 +01:00 committed by Lucas Gass
parent b336cf88d5
commit cd3b0770c5
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
2 changed files with 47 additions and 9 deletions

View File

@ -74,18 +74,23 @@ sub do_checkout {
last;
}
} else {
my $hold_pref = C4::Context->preference("AllowItemsOnHoldCheckoutSIP") || 0;
foreach my $confirmation ( keys %{$needsconfirmation} ) {
if ( $confirmation eq 'RENEW_ISSUE' ) {
$self->screen_msg("Item already checked out to you: renewing item.");
} elsif ( $confirmation eq 'RESERVED' and !C4::Context->preference("AllowItemsOnHoldCheckoutSIP") ) {
} elsif ( $confirmation eq 'RESERVED' ) {
if ( $hold_pref >= 1 ) {
next;
}
$self->screen_msg("Item is reserved for another patron upon return.");
$noerror = 0;
} elsif ( $confirmation eq 'RESERVED' and C4::Context->preference("AllowItemsOnHoldCheckoutSIP") ) {
next;
} elsif ( $confirmation eq 'RESERVE_WAITING'
or $confirmation eq 'TRANSFERRED'
or $confirmation eq 'PROCESSING' )
{
if ( $hold_pref >= 2 ) {
next;
}
$self->screen_msg("Item is on hold for another patron.");
$noerror = 0;
} elsif ( $confirmation eq 'ISSUED_TO_ANOTHER' and C4::Context->preference("AllowItemsOnLoanCheckoutSIP") )

View File

@ -22,7 +22,8 @@ use C4::SIP::ILS::Transaction::Hold;
use C4::SIP::ILS::Transaction::Checkout;
use C4::SIP::ILS::Transaction::Checkin;
use C4::Reserves qw( AddReserve ModReserve ModReserveAffect );
use C4::Circulation qw( AddReturn );
use C4::Reserves qw( AddReserve ModReserve ModReserveAffect );
use Koha::CirculationRules;
use Koha::Item::Transfer;
use Koha::DateUtils qw( dt_from_string output_pref );
@ -1141,7 +1142,7 @@ subtest do_checkout_with_noblock => sub {
};
subtest do_checkout_with_holds => sub {
plan tests => 7;
plan tests => 9;
my $library = $builder->build_object( { class => 'Koha::Libraries' } );
my $patron = $builder->build_object(
@ -1189,6 +1190,9 @@ subtest do_checkout_with_holds => sub {
$sip_item, "Item assigned to transaction"
);
# Default pref value is 0 (don't allow): every hold state blocks checkout
t::lib::Mocks::mock_preference( 'AllowItemsOnHoldCheckoutSIP', '0' );
# Test attached holds
ModReserveAffect( $item->itemnumber, $patron->borrowernumber, 0, $reserve ); # Mark waiting (W)
my $hold = Koha::Holds->find($reserve);
@ -1203,16 +1207,45 @@ subtest do_checkout_with_holds => sub {
$co_transaction->do_checkout();
is( $patron->checkouts->count, 0, 'Checkout was not done due to attached hold (P)' );
# Test non-attached holds
# Pending (non-attached) hold: pref value 0 blocks, 1 allows
$hold->set_waiting();
$hold->revert_found();
t::lib::Mocks::mock_preference( 'AllowItemsOnHoldCheckoutSIP', '0' );
$co_transaction->do_checkout();
is( $patron->checkouts->count, 0, 'Checkout refused due to hold and AllowItemsOnHoldCheckoutSIP' );
is(
$patron->checkouts->count, 0,
'Pending hold blocks checkout with AllowItemsOnHoldCheckoutSIP=0'
);
t::lib::Mocks::mock_preference( 'AllowItemsOnHoldCheckoutSIP', '1' );
$co_transaction->do_checkout();
is( $patron->checkouts->count, 1, 'Checkout allowed due to hold and AllowItemsOnHoldCheckoutSIP' );
is(
$patron->checkouts->count, 1,
'Pending hold allowed with AllowItemsOnHoldCheckoutSIP=1 (pending only)'
);
# Return the item and re-attach the waiting hold
AddReturn( $item->barcode, $library->branchcode );
ModReserveAffect( $item->itemnumber, $patron2->borrowernumber, 0, $reserve );
$hold->discard_changes;
$sip_item = C4::SIP::ILS::Item->new( $item->barcode );
$co_transaction = C4::SIP::ILS::Transaction::Checkout->new();
$co_transaction->patron($sip_patron);
$co_transaction->item($sip_item);
# Waiting hold is still blocked when pref=1 (pending only)
$co_transaction->do_checkout();
is(
$patron->checkouts->count, 0,
'Waiting hold blocked with AllowItemsOnHoldCheckoutSIP=1 (pending only)'
);
# Waiting hold is allowed when pref=2 (pending and waiting)
t::lib::Mocks::mock_preference( 'AllowItemsOnHoldCheckoutSIP', '2' );
$co_transaction->do_checkout();
is(
$patron->checkouts->count, 1,
'Waiting hold allowed with AllowItemsOnHoldCheckoutSIP=2 (pending and waiting)'
);
};
subtest checkin_lost => sub {