From 5c8ec18bfc2c904e69f988277f580465674252f0 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 17 Jan 2022 15:14:20 +0100 Subject: [PATCH] Bug 29043: Don't fetch items if when are not on the "Place a hold on" form --- reserve/request.pl | 470 ++++++++++++++++++++++++++------------------- 1 file changed, 274 insertions(+), 196 deletions(-) diff --git a/reserve/request.pl b/reserve/request.pl index 02464249af4..3c43b3dfb88 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -437,240 +437,319 @@ if ( ( $findborrower && $borrowernumber_hold || $findclub && $club_hold ) } }; - my @bibitemloop; - - my @available_itemtypes; - foreach my $biblioitemnumber (@biblioitemnumbers) { - my $biblioitem = $biblioiteminfos_of->{$biblioitemnumber}; - my $num_available = 0; - my $num_override = 0; - my $hiddencount = 0; - my $num_alreadyheld = 0; - - $biblioitem->{force_hold_level} = $force_hold_level; + if ( $club_hold or $borrowernumber_hold ) { + my @bibitemloop; + my @available_itemtypes; + foreach my $biblioitemnumber (@biblioitemnumbers) { + my $biblioitem = $biblioiteminfos_of->{$biblioitemnumber}; + my $num_available = 0; + my $num_override = 0; + my $hiddencount = 0; + my $num_alreadyheld = 0; + + $biblioitem->{force_hold_level} = $force_hold_level; + + if ( $biblioitem->{biblioitemnumber} ne $biblionumber ) { + $biblioitem->{hostitemsflag} = 1; + } - if ( $biblioitem->{biblioitemnumber} ne $biblionumber ) { - $biblioitem->{hostitemsflag} = 1; - } + $biblioloopiter{description} = $biblioitem->{description}; + $biblioloopiter{itypename} = $biblioitem->{description}; + if ( $biblioitem->{itemtype} ) { - $biblioloopiter{description} = $biblioitem->{description}; - $biblioloopiter{itypename} = $biblioitem->{description}; - if ( $biblioitem->{itemtype} ) { + $biblioitem->{description} = + $itemtypes->{ $biblioitem->{itemtype} }{description}; - $biblioitem->{description} = - $itemtypes->{ $biblioitem->{itemtype} }{description}; + $biblioloopiter{imageurl} = + getitemtypeimagelocation( 'intranet', + $itemtypes->{ $biblioitem->{itemtype} }{imageurl} ); + } - $biblioloopiter{imageurl} = - getitemtypeimagelocation( 'intranet', - $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; + $items_any_available = ItemsAnyAvailableAndNotRestricted( + { + biblionumber => $biblioitem->{biblionumber}, + patron => $patron + } + ) if $patron; - # 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; - $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblioitem->{biblionumber}, patron => $patron }) - if $patron; - - foreach my $itemnumber ( @{ $itemnumbers_of_biblioitem{$biblioitemnumber} } ) { - my $item = $iteminfos_of->{$itemnumber}; - my $do_check; - if ( $patron ) { - $do_check = $patron->do_check_for_previous_checkout($item) if $wants_check; - if ( $do_check && $wants_check ) { - $item->{checked_previously} = $do_check; - if ( $multi_hold ) { - $biblioloopiter{checked_previously} = $do_check; - } else { - $template->param( checked_previously => $do_check ); + foreach my $itemnumber ( + @{ $itemnumbers_of_biblioitem{$biblioitemnumber} } ) + { + my $item = $iteminfos_of->{$itemnumber}; + my $do_check; + if ($patron) { + $do_check = + $patron->do_check_for_previous_checkout($item) + if $wants_check; + if ( $do_check && $wants_check ) { + $item->{checked_previously} = $do_check; + if ($multi_hold) { + $biblioloopiter{checked_previously} = $do_check; + } + else { + $template->param( + checked_previously => $do_check ); + } } } - } - $item->{force_hold_level} = $force_hold_level; - - unless (C4::Context->preference('item-level_itypes')) { - $item->{itype} = $biblioitem->{itemtype}; - } - - $item->{itypename} = $itemtypes->{ $item->{itype} }{description}; - $item->{imageurl} = getitemtypeimagelocation( 'intranet', $itemtypes->{ $item->{itype} }{imageurl} ); - $item->{homebranch} = $item->{homebranch}; + $item->{force_hold_level} = $force_hold_level; - # if the holdingbranch is different than the homebranch, we show the - # holdingbranch of the document too - if ( $item->{homebranch} ne $item->{holdingbranch} ) { - $item->{holdingbranch} = $item->{holdingbranch}; - } - - if($item->{biblionumber} ne $biblionumber){ - $item->{hostitemsflag} = 1; - $item->{hosttitle} = Koha::Biblios->find( $item->{biblionumber} )->title; - } + unless ( C4::Context->preference('item-level_itypes') ) { + $item->{itype} = $biblioitem->{itemtype}; + } - # if the item is currently on loan, we display its return date and - # change the background color - my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } ); - if ( $issue ) { - $item->{date_due} = $issue->date_due; - $item->{backgroundcolor} = 'onloan'; - } + $item->{itypename} = + $itemtypes->{ $item->{itype} }{description}; + $item->{imageurl} = getitemtypeimagelocation( 'intranet', + $itemtypes->{ $item->{itype} }{imageurl} ); + $item->{homebranch} = $item->{homebranch}; - # checking reserve - my $item_object = Koha::Items->find( $itemnumber ); - my $holds = $item_object->current_holds; - if ( my $first_hold = $holds->next ) { - my $p = Koha::Patrons->find( $first_hold->borrowernumber ); - - $item->{backgroundcolor} = 'reserved'; - $item->{reservedate} = output_pref({ dt => dt_from_string( $first_hold->reservedate ), dateonly => 1 }); # FIXME Should be formatted in the template - $item->{ReservedFor} = $p; - $item->{ExpectedAtLibrary} = $first_hold->branchcode; - $item->{waitingdate} = $first_hold->waitingdate; - } + # if the holdingbranch is different than the homebranch, we show the + # holdingbranch of the document too + if ( $item->{homebranch} ne $item->{holdingbranch} ) { + $item->{holdingbranch} = $item->{holdingbranch}; + } - # Management of the notforloan document - if ( $item->{notforloan} ) { - $item->{backgroundcolor} = 'other'; - } + if ( $item->{biblionumber} ne $biblionumber ) { + $item->{hostitemsflag} = 1; + $item->{hosttitle} = + Koha::Biblios->find( $item->{biblionumber} )->title; + } - # Management of lost or long overdue items - if ( $item->{itemlost} ) { - $item->{backgroundcolor} = 'other'; - if ($logged_in_patron->category->hidelostitems && !$showallitems) { - $item->{hide} = 1; - $hiddencount++; + # if the item is currently on loan, we display its return date and + # change the background color + my $issue = + Koha::Checkouts->find( { itemnumber => $itemnumber } ); + if ($issue) { + $item->{date_due} = $issue->date_due; + $item->{backgroundcolor} = 'onloan'; } - } - # Check the transit status - my ( $transfertwhen, $transfertfrom, $transfertto ) = - GetTransfers($itemnumber); + # checking reserve + my $item_object = Koha::Items->find($itemnumber); + my $holds = $item_object->current_holds; + if ( my $first_hold = $holds->next ) { + my $p = + Koha::Patrons->find( $first_hold->borrowernumber ); + + $item->{backgroundcolor} = 'reserved'; + $item->{reservedate} = output_pref( + { + dt => + dt_from_string( $first_hold->reservedate ), + dateonly => 1 + } + ); # FIXME Should be formatted in the template + $item->{ReservedFor} = $p; + $item->{ExpectedAtLibrary} = $first_hold->branchcode; + $item->{waitingdate} = $first_hold->waitingdate; + } - if ( defined $transfertwhen && $transfertwhen ne '' ) { - $item->{transfertwhen} = output_pref({ dt => dt_from_string( $transfertwhen ), dateonly => 1 }); - $item->{transfertfrom} = $transfertfrom; - $item->{transfertto} = $transfertto; - $item->{nocancel} = 1; - } + # Management of the notforloan document + if ( $item->{notforloan} ) { + $item->{backgroundcolor} = 'other'; + } - # If there is no loan, return and transfer, we show a checkbox. - $item->{notforloan} ||= 0; - - # if independent branches is on we need to check if the person can reserve - # for branches they arent logged in to - if ( C4::Context->preference("IndependentBranches") ) { - if (! C4::Context->preference("canreservefromotherbranches")){ - # can't reserve items so need to check if item homebranch and userenv branch match if not we can't reserve - my $userenv = C4::Context->userenv; - unless ( C4::Context->IsSuperLibrarian ) { - $item->{cantreserve} = 1 if ( $item->{homebranch} ne $userenv->{branch} ); + # Management of lost or long overdue items + if ( $item->{itemlost} ) { + $item->{backgroundcolor} = 'other'; + if ( $logged_in_patron->category->hidelostitems + && !$showallitems ) + { + $item->{hide} = 1; + $hiddencount++; } } - } - if ( $patron ) { - my $patron_unblessed = $patron->unblessed; - my $branch = C4::Circulation::_GetCircControlBranch($item, $patron_unblessed); + # Check the transit status + my ( $transfertwhen, $transfertfrom, $transfertto ) = + GetTransfers($itemnumber); - my $branchitemrule = GetBranchItemRule( $branch, $item->{'itype'} ); - - $item->{'holdallowed'} = $branchitemrule->{'holdallowed'}; - - my $can_item_be_reserved = CanItemBeReserved( $patron->borrowernumber, $itemnumber )->{status}; - $item->{not_holdable} = $can_item_be_reserved unless ( $can_item_be_reserved eq 'OK' ); - - $item->{item_level_holds} = Koha::CirculationRules->get_opacitemholds_policy( { item => $item_object, patron => $patron } ); - - if ( - !$item->{cantreserve} - && !$exceeded_maxreserves - && $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) - ) - { - # Send the pickup locations count to the UI, the pickup locations will be pulled using the API - my @pickup_locations = $item_object->pickup_locations({ patron => $patron })->as_list; - $item->{pickup_locations_count} = scalar @pickup_locations; - - if ( @pickup_locations ) { - $num_available++; - $item->{available} = 1; - - my $default_pickup_location; - - # Default to logged-in, if valid - if ( C4::Context->userenv->{branch} ) { - ($default_pickup_location) = grep { $_->branchcode eq C4::Context->userenv->{branch} } @pickup_locations; + if ( defined $transfertwhen && $transfertwhen ne '' ) { + $item->{transfertwhen} = output_pref( + { + dt => dt_from_string($transfertwhen), + dateonly => 1 } + ); + $item->{transfertfrom} = $transfertfrom; + $item->{transfertto} = $transfertto; + $item->{nocancel} = 1; + } - $item->{default_pickup_location} = $default_pickup_location; - } - else { - $item->{available} = 0; - $item->{not_holdable} = "no_valid_pickup_location"; + # If there is no loan, return and transfer, we show a checkbox. + $item->{notforloan} ||= 0; + + # if independent branches is on we need to check if the person can reserve + # for branches they arent logged in to + if ( C4::Context->preference("IndependentBranches") ) { + if ( + !C4::Context->preference( + "canreservefromotherbranches") + ) + { +# can't reserve items so need to check if item homebranch and userenv branch match if not we can't reserve + my $userenv = C4::Context->userenv; + unless ( C4::Context->IsSuperLibrarian ) { + $item->{cantreserve} = 1 + if ( + $item->{homebranch} ne $userenv->{branch} ); + } } - - push( @available_itemtypes, $item->{itype} ); } - elsif ( C4::Context->preference('AllowHoldPolicyOverride') ) { - # If AllowHoldPolicyOverride is set, it should override EVERY restriction, not just branch item rules - # with the exception of itemAlreadyOnHold because, you know, the item is already on hold - if ( $can_item_be_reserved ne 'itemAlreadyOnHold' ) { - # Send the pickup locations count to the UI, the pickup locations will be pulled using the API - my $pickup_locations = $item_object->pickup_locations({ patron => $patron }); - $item->{pickup_locations_count} = $pickup_locations->count; - if ( $item->{pickup_locations_count} > 0 ) { - $item->{override} = 1; - $num_override++; - # pass the holding branch for use as default - my $default_pickup_location = $pickup_locations->search({ branchcode => $item->{holdingbranch} })->next; - $item->{default_pickup_location} = $default_pickup_location; + + if ($patron) { + my $patron_unblessed = $patron->unblessed; + my $branch = + C4::Circulation::_GetCircControlBranch( $item, + $patron_unblessed ); + + my $branchitemrule = + GetBranchItemRule( $branch, $item->{'itype'} ); + + $item->{'holdallowed'} = + $branchitemrule->{'holdallowed'}; + + my $can_item_be_reserved = + CanItemBeReserved( $patron->borrowernumber, + $itemnumber )->{status}; + $item->{not_holdable} = $can_item_be_reserved + unless ( $can_item_be_reserved eq 'OK' ); + + $item->{item_level_holds} = + Koha::CirculationRules->get_opacitemholds_policy( + { item => $item_object, patron => $patron } ); + + if ( + !$item->{cantreserve} + && !$exceeded_maxreserves + && $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 + ) + ) + { +# Send the pickup locations count to the UI, the pickup locations will be pulled using the API + my @pickup_locations = + $item_object->pickup_locations( + { patron => $patron } )->as_list; + $item->{pickup_locations_count} = + scalar @pickup_locations; + + if (@pickup_locations) { + $num_available++; + $item->{available} = 1; + + my $default_pickup_location; + + # Default to logged-in, if valid + if ( C4::Context->userenv->{branch} ) { + ($default_pickup_location) = grep { + $_->branchcode eq + C4::Context->userenv->{branch} + } @pickup_locations; + } + + $item->{default_pickup_location} = + $default_pickup_location; } else { $item->{available} = 0; - $item->{not_holdable} = "no_valid_pickup_location"; + $item->{not_holdable} = + "no_valid_pickup_location"; } - } else { $num_alreadyheld++ } - push( @available_itemtypes, $item->{itype} ); - } + push( @available_itemtypes, $item->{itype} ); + } + elsif ( + C4::Context->preference('AllowHoldPolicyOverride') ) + { +# If AllowHoldPolicyOverride is set, it should override EVERY restriction, not just branch item rules +# with the exception of itemAlreadyOnHold because, you know, the item is already on hold + if ( $can_item_be_reserved ne 'itemAlreadyOnHold' ) + { +# Send the pickup locations count to the UI, the pickup locations will be pulled using the API + my $pickup_locations = + $item_object->pickup_locations( + { patron => $patron } ); + $item->{pickup_locations_count} = + $pickup_locations->count; + if ( $item->{pickup_locations_count} > 0 ) { + $item->{override} = 1; + $num_override++; + + # pass the holding branch for use as default + my $default_pickup_location = + $pickup_locations->search( + { + branchcode => $item->{holdingbranch} + } + )->next; + $item->{default_pickup_location} = + $default_pickup_location; + } + else { + $item->{available} = 0; + $item->{not_holdable} = + "no_valid_pickup_location"; + } + } + else { $num_alreadyheld++ } - # If none of the conditions hold true, then neither override nor available is set and the item cannot be checked + push( @available_itemtypes, $item->{itype} ); + } - # Show serial enumeration when needed - if ($item->{enumchron}) { - $itemdata_enumchron = 1; - } - # Show collection when needed - if ($item->{ccode}) { - $itemdata_ccode = 1; +# If none of the conditions hold true, then neither override nor available is set and the item cannot be checked + + # Show serial enumeration when needed + if ( $item->{enumchron} ) { + $itemdata_enumchron = 1; + } + + # Show collection when needed + if ( $item->{ccode} ) { + $itemdata_ccode = 1; + } } + + push @{ $biblioitem->{itemloop} }, $item; } - push @{ $biblioitem->{itemloop} }, $item; - } +# While we can't override an alreay held item, we should be able to override the others +# Unless all items are already held + if ( $num_override > 0 + && ( $num_override + $num_alreadyheld ) == + scalar( @{ $biblioitem->{itemloop} } ) ) + { + # That is, if all items require an override + $template->param( override_required => 1 ); + } + elsif ( $num_available == 0 ) { + $template->param( none_available => 1 ); + $biblioloopiter{warn} = 1; + $biblioloopiter{none_avail} = 1; + } + $template->param( hiddencount => $hiddencount ); - # While we can't override an alreay held item, we should be able to override the others - # Unless all items are already held - if ( $num_override > 0 && ($num_override + $num_alreadyheld) == scalar( @{ $biblioitem->{itemloop} } ) ) { - # That is, if all items require an override - $template->param( override_required => 1 ); - } elsif ( $num_available == 0 ) { - $template->param( none_available => 1 ); - $biblioloopiter{warn} = 1; - $biblioloopiter{none_avail} = 1; + push @bibitemloop, $biblioitem; } - $template->param( hiddencount => $hiddencount); - push @bibitemloop, $biblioitem; + @available_itemtypes = uniq(@available_itemtypes); + $template->param( + bibitemloop => \@bibitemloop, + available_itemtypes => \@available_itemtypes + ); } - @available_itemtypes = uniq( @available_itemtypes ); - $template->param( available_itemtypes => \@available_itemtypes ); - # existingreserves building my @reserveloop; my @reserves = Koha::Holds->search( { biblionumber => $biblionumber }, { order_by => 'priority' } ); @@ -736,7 +815,6 @@ if ( ( $findborrower && $borrowernumber_hold || $findclub && $club_hold ) # display infos $template->param( - bibitemloop => \@bibitemloop, itemdata_enumchron => $itemdata_enumchron, itemdata_ccode => $itemdata_ccode, date => $date, -- 2.25.1