gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 41728: (follow-up) Add no_short_circuit option to checkin availability 

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 <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
Tomás Cohen Arazi 2026-04-15 11:56:58 -03:00 committed by Pedro Amorim
parent 84bcca727a
commit dece71cb3e
No known key found for this signature in database
GPG Key ID: 06B75BA7F3894D55
2 changed files with 66 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
Koha/Item/Checkin/Availability.pm
View File

@ -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

44
t/db_dependent/Koha/Item/Checkin/Availability.t
View File

@ -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;
};