[Koha-bugs] [Bug 34886] Regression in when hold button appears

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34886

--- Comment #46 from Nick Clemens <nick at bywatersolutions.com> ---
(In reply to Marcel de Rooy from comment #44)
> On 34886 we did:
> -        $can_item_be_reserved = $can_item_be_reserved ||
> IsAvailableForItemLevelRequest($item, $patron, undef);
> +        $can_item_be_reserved = $can_item_be_reserved || $patron &&
> IsAvailableForItemLevelRequest($item, $patron, undef);
> 
> And there was noted that this causes the regression repeated here in
> description? Could you explain that? This is just a very logical check
> before calling IsAvailableForItemLevelRequest? Which you should not for
> anonymous..

Go back to 30486 - then we only called:
Koha::CirculationRules->get_onshelfholds_policy({ item => $item, patron =>
$patron } )

Which could operate with or without a patron, so sometimes returned 1

We changed it to:
IsAvailableForItemLevelRequest

which sometimes requires a patron or crashes, so we then only called it if we
had a patron, which meant when not logged in $can_item_be_reserved was always
set to 0



(In reply to Marcel de Rooy from comment #45)
> Now we remove it:
> -if( $can_item_be_reserved || CountItemsIssued($biblionumber) ||
> $biblio->has_items_waiting_or_intransit ) {
> -    $template->param( ReservableItems => 1 );
> -}
> 
> And actually replace it by $holdable_items for anonymous. This is just
> another guess. What makes it better than what we had ?

Because currently - without a patron, we are never setting can_item_be_reserved
to 1 - it is always 0
CountItemsIssued  is a meaningless check on its own without checking on-shelf
hold policies
Same for has_items_waiting_or_intransit

Read the code comments for what the checks do now, I think they make more sense

-- 
You are receiving this mail because:
You are watching all bug changes.