From dece71cb3eebe0be96cad19f3dcccfda88d486ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Cohen=20Arazi?= Date: Wed, 15 Apr 2026 11:56:58 -0300 Subject: [PATCH] Bug 41728: (follow-up) Add no_short_circuit option to checkin availability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This follow-up replaces the unconditional removal of short-circuit logic with a caller-controlled no_short_circuit parameter, following the same pattern established by Koha::Patron->can_place_holds. By default, check() short-circuits on the first blocker. This is the correct behavior for AddReturn, which processes blockers sequentially and only acts on the first one it finds (BlockedWithdrawn => early return, Wrongbranch => early return, BlockedLost => sets doreturn=0). Running all policy checks only to discard the results is wasteful, specially when some of them (e.g. can_be_returned_at) hit the database. When no_short_circuit => 1 is passed, all checks run and all blockers are collected. This is the behavior API consumers will need: present the full set of issues in a single response instead of requiring fix-one-recheck cycles. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Item/Checkin/Availability.t => SUCCESS: Tests pass! k$ prove t/db_dependent/Circul* => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomás Cohen Arazi Signed-off-by: Martin Renvoize Signed-off-by: Pedro Amorim --- Koha/Item/Checkin/Availability.pm | 31 ++++++++++--- .../Koha/Item/Checkin/Availability.t | 44 ++++++++++++++++++- 2 files changed, 66 insertions(+), 9 deletions(-) diff --git a/Koha/Item/Checkin/Availability.pm b/Koha/Item/Checkin/Availability.pm index fce658d762..0768435946 100644 --- a/Koha/Item/Checkin/Availability.pm +++ b/Koha/Item/Checkin/Availability.pm @@ -43,6 +43,15 @@ Koha::Item::Checkin::Availability - Check-in availability validation for items } ); + # Collect all blockers (e.g. for API responses) + my $availability = $item->checkin_availability( + { + library => $branchcode, + to_library => $destination_branchcode, + no_short_circuit => 1, + } + ); + =head1 DESCRIPTION This class provides validation logic for item check-in operations, extracting @@ -61,8 +70,9 @@ The result categorizes conditions as: my $availability = Koha::Item::Checkin::Availability->check( $item, { - library => $branchcode, - to_library => $destination_branchcode, + library => $branchcode, + to_library => $destination_branchcode, + no_short_circuit => 1, } ); @@ -71,9 +81,13 @@ Validates check-in availability for an item. Parameters: $item - Koha::Item object (required) $params - hashref with: - library => branchcode where the return takes place (required) - to_library => branchcode where the item should be sent after return - (optional, for transfer limit validation) + library => branchcode where the return takes place (required) + to_library => branchcode where the item should be sent after return + (optional, for transfer limit validation) + no_short_circuit => boolean, if true all checks are performed and all + blockers collected. Default: false (short-circuit + on first blocker). This follows the same pattern + as Koha::Patron->can_place_holds. Returns a Koha::Availability::Result object with: blockers => {} # Conditions that prevent check-in @@ -89,8 +103,9 @@ Returns a Koha::Availability::Result object with: sub check { my ( $class, $item, $params ) = @_; - my $library = $params->{library}; - my $to_library = $params->{to_library}; + my $library = $params->{library}; + my $to_library = $params->{to_library}; + my $no_short_circuit = $params->{no_short_circuit} // 0; my $result = Koha::Availability::Result->new(); @@ -110,6 +125,7 @@ sub check { if ( $item->withdrawn ) { if ( C4::Context->preference("BlockReturnOfWithdrawnItems") ) { $result->add_blocker( BlockedWithdrawn => 1 ); + return $result unless $no_short_circuit; } else { $result->add_warning( withdrawn => 1 ); } @@ -125,6 +141,7 @@ sub check { Rightbranch => $message } ); + return $result unless $no_short_circuit; } # Check if item is lost and blocked diff --git a/t/db_dependent/Koha/Item/Checkin/Availability.t b/t/db_dependent/Koha/Item/Checkin/Availability.t index 5f733a46a4..fdb3996e50 100755 --- a/t/db_dependent/Koha/Item/Checkin/Availability.t +++ b/t/db_dependent/Koha/Item/Checkin/Availability.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 9; +use Test::More tests => 10; use Test::NoWarnings; use t::lib::TestBuilder; @@ -349,7 +349,8 @@ subtest 'check() - multiple simultaneous blockers' => sub { my $result = $item->checkin_availability( { - library => $wrongbranch->branchcode, + library => $wrongbranch->branchcode, + no_short_circuit => 1, } ); @@ -361,3 +362,42 @@ subtest 'check() - multiple simultaneous blockers' => sub { $schema->storage->txn_rollback; }; + +subtest 'check() - default short-circuits on first blocker' => sub { + + plan tests => 3; + + $schema->storage->txn_begin; + + t::lib::Mocks::mock_preference( 'BlockReturnOfWithdrawnItems', 1 ); + t::lib::Mocks::mock_preference( 'BlockReturnOfLostItems', 1 ); + t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homebranch' ); + + my $homebranch = $builder->build_object( { class => 'Koha::Libraries' } ); + my $wrongbranch = $builder->build_object( { class => 'Koha::Libraries' } ); + + my $item = $builder->build_object( + { + class => 'Koha::Items', + value => { + homebranch => $homebranch->branchcode, + holdingbranch => $homebranch->branchcode, + withdrawn => 1, + itemlost => 1, + } + } + ); + + # Default behavior: short-circuit on first blocker + my $result = $item->checkin_availability( + { + library => $wrongbranch->branchcode, + } + ); + + ok( !$result->available, 'item is not available for check-in' ); + is( keys %{ $result->blockers }, 1, 'only first blocker reported (short-circuited)' ); + is( $result->blockers->{BlockedWithdrawn}, 1, 'BlockedWithdrawn is the first blocker' ); + + $schema->storage->txn_rollback; +};