https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42999
Bug ID: 42999
Summary: $valid_items flag contamination in reserve/request.pl
causes non-holdable items to appear as available after
the first holdable item in the loop
Initiative type: ---
Sponsorship ---
status:
Product: Koha
Version: Main
Hardware: All
OS: All
Status: NEW
Severity: major
Priority: P5 - low
Component: Hold requests
Assignee: [email protected]
Reporter: [email protected]
QA Contact: [email protected]
CC: [email protected], [email protected],
[email protected]
Target Milestone: ---
Bug 38412 (commit 1900effc9b) introduced $valid_items as a flag to track
whether any item passes all availability conditions. However, the flag is also
used as a per-item gate in the same loop without being reset between
iterations. Once any item sets $valid_items = 1, all subsequent items —
regardless of their individual availability — enter the if ($valid_items)
branch and are assigned $item->{available} = 1.
Root cause (reserve/request.pl):
The commit split what was a single if block into two consecutive, separate
blocks:
# Line 455 — initialized once, outside the loop
my $valid_items = 0;
for my $item_object (@items) {
...
# Block 1 (line 570): per-item condition check
if (
!$item->{cantreserve}
&& !$exceeded_maxreserves
&& $can_item_be_reserved eq 'OK'
&& IsAvailableForItemLevelRequest( $item_object, $patron, undef )
) {
$valid_items = 1; # set but never reset between iterations
}
# Block 2 (line 583): uses $valid_items as per-item gate — BUG
if ($valid_items) { # true for ALL items after the first valid one
$num_items_available++;
$item->{available} = 1;
...
} elsif ( C4::Context->preference('AllowHoldPolicyOverride') ) {
...
} else {
$item->{available} = 0; # never reached after first valid item
}
}
# Line 661 — post-loop: this use of $valid_items is correct
if ( $valid_items == 0 ) {
$template->param( none_available_override => 1 );
}
The variable serves two conflicting roles:
Global accumulator (correct, line 661): "did any item pass all checks?"
Per-item gate (incorrect, line 583): "does this item pass all checks?" — but it
carries over from previous iterations.
Steps to reproduce:
Create a biblio with at least two items: one with a holdable item type (e.g.
Books) and one with a not-for-loan item type (e.g. Reference,
itemtypes.notforloan = 1).
The not-for-loan item must have a higher itemnumber than at least one holdable
item (i.e., it is processed later in the loop).
Open the place-hold staff form:
reserve/request.pl?biblionumber=X&borrowernumber=Y
Expected: The not-for-loan item shows no radio button / is not selectable for
holds.
Actual: The not-for-loan item has a radio button and available = 1, despite the
Information column correctly showing "Not for loan (Itemtype not for loan)".
This is reproducible with AllowHoldPolicyOverride either on or off.
Diagnostic trace on a local instance running main (as of 2026-07-06,
biblionumber 47160, patron borrowernumber 2):
AllowHoldPolicyOverride: NO
itemno barcode itype ifl can_reserve cond_passes valid_items
available branch_taken
---------------------------------------------------------------------------------------------------
27924 FFL24040002 BK 0 OK YES 0->1 1
if($valid_items)
27926 CPL24040004 BK 0 OK YES 1->1 1
if($valid_items)
27936 CPL24050001 BK 0 OK YES 1->1 1
if($valid_items)
28043 CPL24110003 REF 1 OK NO 1->1 1
if($valid_items) <-- BUG
28046 CPL25030002 BK 0 OK YES 1->1 1
if($valid_items)
...
Legend:
ifl = itemtype notforloan flag
cond_passes = item individually passes all availability conditions
valid_items = flag value before->after this iteration
<-- BUG = actual result differs from correct result
Note: in this test case CanItemBeReserved returns OK for the REF item
(depending on circulation rules, it may also return notforloan); either way,
IsAvailableForItemLevelRequest returns false due to the itemtype flag, so
cond_passes = NO. The bug manifests regardless.
Proposed fix:
Merge the two if blocks back into one, so the pickup-location logic and
$item->{available} assignment only execute when the current item individually
passes all conditions. The global $valid_items accumulator for the post-loop
check at line 661 is preserved:
if (
!$item->{cantreserve}
&& !$exceeded_maxreserves
&& $can_item_be_reserved eq 'OK'
&& IsAvailableForItemLevelRequest( $item_object, $patron, undef )
) {
$valid_items = 1;
my $pickup_locations = $item_object->pickup_locations( { patron => $patron
} );
$item->{pickup_locations_count} = $pickup_locations->count;
if ( $item->{pickup_locations_count} > 0 ||
C4::Context->preference('AllowHoldPolicyOverride') ) {
$num_items_available++;
$item->{available} = 1;
...
} else {
$item->{available} = 0;
$item->{not_holdable} = "no_valid_pickup_location";
}
} elsif ( C4::Context->preference('AllowHoldPolicyOverride') ) {
...
} else {
$item->{available} = 0;
}
--
You are receiving this mail because:
You are the assignee for the bug.
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
