@@ -, +, @@ 'If all unavailable' 100 items: 38 seconds 200 items: 2.5 minutes 300 items: 5.4 minutes 100 items: 3.7 seconds 200 items: 6.4 seconds 300 items: 8.7 seconds --- C4/Reserves.pm | 17 ++++++++++++++--- reserve/request.pl | 29 ++++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 4 deletions(-) --- a/C4/Reserves.pm +++ a/C4/Reserves.pm @@ -1188,7 +1188,12 @@ and canreservefromotherbranches. =cut sub IsAvailableForItemLevelRequest { - my ( $item, $patron, $pickup_branchcode ) = @_; + my $item = shift; + my $patron = shift; + my $pickup_branchcode = shift; + # items_any_available is precalculated status passed from request.pl when set of items + # looped outside of IsAvailableForItemLevelRequest to avoid nested loops: + my $items_any_available = shift; my $dbh = C4::Context->dbh; # must check the notforloan setting of the itemtype @@ -1205,8 +1210,6 @@ sub IsAvailableForItemLevelRequest { $item->withdrawn || ($item->damaged && !C4::Context->preference('AllowHoldsOnDamagedItems')); - my $on_shelf_holds = Koha::IssuingRules->get_onshelfholds_policy( { item => $item, patron => $patron } ); - if ($pickup_branchcode) { my $destination = Koha::Libraries->find($pickup_branchcode); return 0 unless $destination; @@ -1214,9 +1217,17 @@ sub IsAvailableForItemLevelRequest { return 0 unless $item->can_be_transferred( { to => $destination } ); } + my $on_shelf_holds = Koha::IssuingRules->get_onshelfholds_policy( { item => $item, patron => $patron } ); + if ( $on_shelf_holds == 1 ) { return 1; } elsif ( $on_shelf_holds == 2 ) { + + # if we have this param predefined from outer caller sub, we just need + # to return it, so we saving from having loop inside other loop: + return $items_any_available ? 0 : 1 + if defined $items_any_available; + my @items = Koha::Items->search( { biblionumber => $item->biblionumber } ); --- a/reserve/request.pl +++ a/reserve/request.pl @@ -444,6 +444,31 @@ foreach my $biblionumber (@biblionumbers) { $itemtypes->{ $biblioitem->{itemtype} }{imageurl} ); } + # iterating through all items first to check if any of them available + # to pass this value further inside down to IsAvailableForItemLevelRequest to + # it's complicated logic to analyse. + # (before this loop was inside that sub loop so it was O(n^2) ) + my $items_any_available; + if ( $patron ) { + my @items = Koha::Items->search( { biblionumber => $biblioitemnumber } ); + + foreach my $i (@items) { + my $reserves_control_branch = GetReservesControlBranch( $i->unblessed(), $patron->unblessed ); + my $branchitemrule = C4::Circulation::GetBranchItemRule( $reserves_control_branch, $i->itype ); + + $items_any_available = 1 + unless $i->itemlost + || $i->notforloan > 0 + || $i->withdrawn + || $i->onloan + || IsItemOnHoldAndFound( $i->id ) + || ( $i->damaged + && !C4::Context->preference('AllowHoldsOnDamagedItems') ) + || Koha::ItemTypes->find( $i->effective_itemtype() )->notforloan + || $branchitemrule->{holdallowed} == 1 && $patron->branchcode ne $i->homebranch; + } + } + foreach my $itemnumber ( @{ $itemnumbers_of_biblioitem{$biblioitemnumber} } ) { my $item = $iteminfos_of->{$itemnumber}; my $do_check; @@ -556,8 +581,10 @@ foreach my $biblionumber (@biblionumbers) { if ( !$item->{cantreserve} && !$exceeded_maxreserves - && IsAvailableForItemLevelRequest($item_object, $patron) && $can_item_be_reserved eq 'OK' + # items_any_available defined outside of the current loop, + # so we avoiding loop inside IsAvailableForItemLevelRequest: + && IsAvailableForItemLevelRequest($item_object, $patron, undef, $items_any_available) ) { $item->{available} = 1; --