From bafe6594d40ce6cd42234356b63dceea9e9a1d43 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Wed, 17 Nov 2021 14:03:10 +0000 Subject: [PATCH] Bug 29043: Don't load items unless a p[atron has been chosen on request.pl This patch moves the code that we sh9ould run if there is no patron to the head of the biblio loop. If we have no patron we execute this and go to the next loop. A few variables that were being passed to the template in the loop are moved outside - no reason to overwrite each loop To test: 1 - Browse to a bib in the staff client 2 - Click on the holds tab 3 - Note loading time 4 - Add 500 items to bib 5 - Note loading time - it takes longer 6 - Note items are not displayed 7 - Apply patch 8 - Reload page - is much faster 9 - Find/choose a patron 10 - Confirm items are loaded correctly Signed-off-by: Samu Heiskanen --- reserve/request.pl | 413 +++++++++++++++++++++++---------------------- 1 file changed, 207 insertions(+), 206 deletions(-) diff --git a/reserve/request.pl b/reserve/request.pl index 10fa575f11..6affcecc3d 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -313,6 +313,7 @@ if ($patron) { my $itemdata_enumchron = 0; my $itemdata_ccode = 0; my @biblioloop = (); +my $reserveloop = 0; my $no_reserves_allowed = 0; foreach my $biblionumber (@biblionumbers) { next unless $biblionumber =~ m|^\d+$|; @@ -326,64 +327,6 @@ foreach my $biblionumber (@biblionumbers) { last; } - my $force_hold_level; - if ( $patron ) { - { # CanBookBeReserved - my $canReserve = CanBookBeReserved( $patron->borrowernumber, $biblionumber ); - if ( $canReserve->{status} eq 'OK' ) { - - #All is OK and we can continue - } - elsif ( $canReserve->{status} eq 'noReservesAllowed' || $canReserve->{status} eq 'notReservable' ) { - $no_reserves_allowed = 1; - } - elsif ( $canReserve->{status} eq 'tooManyReserves' ) { - $exceeded_maxreserves = 1; - $template->param( maxreserves => $canReserve->{limit} ); - } - elsif ( $canReserve->{status} eq 'tooManyHoldsForThisRecord' ) { - $exceeded_holds_per_record = 1; - $biblioloopiter{ $canReserve->{status} } = 1; - } - elsif ( $canReserve->{status} eq 'ageRestricted' ) { - $template->param( $canReserve->{status} => 1 ); - $biblioloopiter{ $canReserve->{status} } = 1; - } - elsif ( $canReserve->{status} eq 'alreadypossession' ) { - $template->param( $canReserve->{status} => 1); - $biblioloopiter{ $canReserve->{status} } = 1; - } - else { - $biblioloopiter{ $canReserve->{status} } = 1; - } - } - - # For multiple holds per record, if a patron has previously placed a hold, - # the patron can only place more holds of the same type. That is, if the - # patron placed a record level hold, all the holds the patron places must - # be record level. If the patron placed an item level hold, all holds - # the patron places must be item level - my $holds = Koha::Holds->search( - { - borrowernumber => $patron->borrowernumber, - biblionumber => $biblionumber, - found => undef, - } - ); - $force_hold_level = $holds->forced_hold_level(); - $biblioloopiter{force_hold_level} = $force_hold_level; - $template->param( force_hold_level => $force_hold_level ); - - # For a librarian to be able to place multiple record holds for a patron for a record, - # we must find out what the maximum number of holds they can place for the patron is - my $max_holds_for_record = GetMaxPatronHoldsForRecord( $patron->borrowernumber, $biblionumber ); - my $remaining_holds_for_record = $max_holds_for_record - $holds->count(); - $biblioloopiter{remaining_holds_for_record} = $max_holds_for_record; - $template->param( max_holds_for_record => $max_holds_for_record ); - $template->param( remaining_holds_for_record => $remaining_holds_for_record ); - } - - my $count = Koha::Holds->search( { biblionumber => $biblionumber } )->count(); my $totalcount = $count; @@ -407,6 +350,146 @@ foreach my $biblionumber (@biblionumbers) { $biblioloopiter{noitems} = 1; } + $biblioloopiter{biblionumber} = $biblionumber; + $biblioloopiter{title} = $biblio->title; + $biblioloopiter{rank} = $fixedRank; + + # get the time for the form name... + my $time = time(); + + $template->param( + time => $time, + fixedRank => $fixedRank, + ); + + unless( $patron ){ + + # existingreserves building + my @reserveloop; + my @reserves = Koha::Holds->search( { biblionumber => $biblionumber }, { order_by => 'priority' } ); + foreach my $res ( + sort { + my $a_found = $a->found() || ''; + my $b_found = $a->found() || ''; + $a_found cmp $b_found; + } @reserves + ) + { + my %reserve; + if ( $res->is_found() ) { + $reserve{'holdingbranch'} = $res->item()->holdingbranch(); + $reserve{'biblionumber'} = $res->item()->biblionumber(); + $reserve{'barcodenumber'} = $res->item()->barcode(); + $reserve{'wbrcode'} = $res->branchcode(); + $reserve{'itemnumber'} = $res->itemnumber(); + $reserve{'wbrname'} = $res->branch()->branchname(); + $reserve{'atdestination'} = $res->is_at_destination(); + $reserve{'desk_name'} = ( $res->desk() ) ? $res->desk()->desk_name() : '' ; + $reserve{'found'} = $res->is_found(); + $reserve{'inprocessing'} = $res->is_in_processing(); + $reserve{'intransit'} = $res->is_in_transit(); + } + elsif ( $res->priority() > 0 ) { + if ( my $item = $res->item() ) { + $reserve{'itemnumber'} = $item->id(); + $reserve{'barcodenumber'} = $item->barcode(); + $reserve{'item_level_hold'} = 1; + } + } + + $reserve{'expirationdate'} = $res->expirationdate; + $reserve{'date'} = $res->reservedate; + $reserve{'borrowernumber'} = $res->borrowernumber(); + $reserve{'biblionumber'} = $res->biblionumber(); + $reserve{'patron'} = $res->borrower; + $reserve{'notes'} = $res->reservenotes(); + $reserve{'waiting_date'} = $res->waitingdate(); + $reserve{'ccode'} = $res->item() ? $res->item()->ccode() : undef; + $reserve{'barcode'} = $res->item() ? $res->item()->barcode() : undef; + $reserve{'priority'} = $res->priority(); + $reserve{'lowestPriority'} = $res->lowestPriority(); + $reserve{'suspend'} = $res->suspend(); + $reserve{'suspend_until'} = $res->suspend_until(); + $reserve{'reserve_id'} = $res->reserve_id(); + $reserve{itemtype} = $res->itemtype(); + $reserve{branchcode} = $res->branchcode(); + $reserve{non_priority} = $res->non_priority(); + $reserve{object} = $res; + + push( @reserveloop, \%reserve ); + } + + $biblioloopiter{reserveloop} = \@reserveloop; + $reserveloop = 1 if (@reserveloop); + + push @biblioloop, \%biblioloopiter; + next; + } + + my $force_hold_level; + my $items_any_available; + + { # CanBookBeReserved + my $canReserve = CanBookBeReserved( $patron->borrowernumber, $biblionumber ); + if ( $canReserve->{status} eq 'OK' ) { + + #All is OK and we can continue + } + elsif ( $canReserve->{status} eq 'noReservesAllowed' || $canReserve->{status} eq 'notReservable' ) { + $no_reserves_allowed = 1; + } + elsif ( $canReserve->{status} eq 'tooManyReserves' ) { + $exceeded_maxreserves = 1; + $template->param( maxreserves => $canReserve->{limit} ); + } + elsif ( $canReserve->{status} eq 'tooManyHoldsForThisRecord' ) { + $exceeded_holds_per_record = 1; + $biblioloopiter{ $canReserve->{status} } = 1; + } + elsif ( $canReserve->{status} eq 'ageRestricted' ) { + $template->param( $canReserve->{status} => 1 ); + $biblioloopiter{ $canReserve->{status} } = 1; + } + elsif ( $canReserve->{status} eq 'alreadypossession' ) { + $template->param( $canReserve->{status} => 1); + $biblioloopiter{ $canReserve->{status} } = 1; + } + else { + $biblioloopiter{ $canReserve->{status} } = 1; + } + } + + # Add the valid pickup locations + my @pickup_locations = $biblio->pickup_locations({ patron => $patron }); + $biblioloopiter{pickup_locations} = \@pickup_locations; + $biblioloopiter{pickup_locations_codes} = [ map { $_->branchcode } @pickup_locations ]; + + # For multiple holds per record, if a patron has previously placed a hold, + # the patron can only place more holds of the same type. That is, if the + # patron placed a record level hold, all the holds the patron places must + # be record level. If the patron placed an item level hold, all holds + # the patron places must be item level + my $holds = Koha::Holds->search( + { + borrowernumber => $patron->borrowernumber, + biblionumber => $biblionumber, + found => undef, + } + ); + $force_hold_level = $holds->forced_hold_level(); + $biblioloopiter{force_hold_level} = $force_hold_level; + $template->param( force_hold_level => $force_hold_level ); + + # For a librarian to be able to place multiple record holds for a patron for a record, + # we must find out what the maximum number of holds they can place for the patron is + my $max_holds_for_record = GetMaxPatronHoldsForRecord( $patron->borrowernumber, $biblionumber ); + my $remaining_holds_for_record = $max_holds_for_record - $holds->count(); + $biblioloopiter{remaining_holds_for_record} = $max_holds_for_record; + $template->param( max_holds_for_record => $max_holds_for_record ); + $template->param( remaining_holds_for_record => $remaining_holds_for_record ); + + my @bibitemloop; + ## Here we go backwards again to create hash of biblioitemnumber to itemnumbers ## this is important when we have analytic items which may be on another record my ( $iteminfos_of ); @@ -431,7 +514,7 @@ foreach my $biblionumber (@biblionumbers) { } }; - my @bibitemloop; + my @available_itemtypes; foreach my $biblioitemnumber (@biblioitemnumbers) { @@ -463,9 +546,7 @@ foreach my $biblionumber (@biblionumbers) { # 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; + $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblioitem->{biblionumber}, patron => $patron }); foreach my $itemnumber ( @{ $itemnumbers_of_biblioitem{$biblioitemnumber} } ) { my $item = $iteminfos_of->{$itemnumber}; @@ -563,34 +644,54 @@ foreach my $biblionumber (@biblionumbers) { } } - if ( $patron ) { - my $patron_unblessed = $patron->unblessed; - my $branch = C4::Circulation::_GetCircControlBranch($item, $patron_unblessed); + my $patron_unblessed = $patron->unblessed; + my $branch = C4::Circulation::_GetCircControlBranch($item, $patron_unblessed); + + my $branchitemrule = GetBranchItemRule( $branch, $item->{'itype'} ); - my $branchitemrule = GetBranchItemRule( $branch, $item->{'itype'} ); + $item->{'holdallowed'} = $branchitemrule->{'holdallowed'}; - $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' ); - 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 } ); - $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 }); + $item->{pickup_locations_count} = $pickup_locations->count; + if ( $item->{pickup_locations_count} > 0 ) { + $num_available++; + $item->{available} = 1; + # 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"; + } - 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) - ) - { + 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 ) { - $num_available++; - $item->{available} = 1; + $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; @@ -599,48 +700,26 @@ foreach my $biblionumber (@biblionumbers) { $item->{available} = 0; $item->{not_holdable} = "no_valid_pickup_location"; } + } else { $num_alreadyheld++ } - 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++ } - - push( @available_itemtypes, $item->{itype} ); - } + push( @available_itemtypes, $item->{itype} ); + } - # If none of the conditions hold true, then neither override nor available is set and the item cannot be checked + # 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; - } + # 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; } - # While we can't override an alreay held item, we should be able to override the others + # While we can't override an already 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 @@ -657,102 +736,24 @@ foreach my $biblionumber (@biblionumbers) { @available_itemtypes = uniq( @available_itemtypes ); $template->param( available_itemtypes => \@available_itemtypes ); + $template->param(bibitemloop => \@bibitemloop); - # existingreserves building - my @reserveloop; - my @reserves = Koha::Holds->search( { biblionumber => $biblionumber }, { order_by => 'priority' } ); - foreach my $res ( - sort { - my $a_found = $a->found() || ''; - my $b_found = $a->found() || ''; - $a_found cmp $b_found; - } @reserves - ) - { - my %reserve; - if ( $res->is_found() ) { - $reserve{'holdingbranch'} = $res->item()->holdingbranch(); - $reserve{'biblionumber'} = $res->item()->biblionumber(); - $reserve{'barcodenumber'} = $res->item()->barcode(); - $reserve{'wbrcode'} = $res->branchcode(); - $reserve{'itemnumber'} = $res->itemnumber(); - $reserve{'wbrname'} = $res->branch()->branchname(); - $reserve{'atdestination'} = $res->is_at_destination(); - $reserve{'desk_name'} = ( $res->desk() ) ? $res->desk()->desk_name() : '' ; - $reserve{'found'} = $res->is_found(); - $reserve{'inprocessing'} = $res->is_in_processing(); - $reserve{'intransit'} = $res->is_in_transit(); - } - elsif ( $res->priority() > 0 ) { - if ( my $item = $res->item() ) { - $reserve{'itemnumber'} = $item->id(); - $reserve{'barcodenumber'} = $item->barcode(); - $reserve{'item_level_hold'} = 1; - } - } - - $reserve{'expirationdate'} = $res->expirationdate; - $reserve{'date'} = $res->reservedate; - $reserve{'borrowernumber'} = $res->borrowernumber(); - $reserve{'biblionumber'} = $res->biblionumber(); - $reserve{'patron'} = $res->borrower; - $reserve{'notes'} = $res->reservenotes(); - $reserve{'waiting_date'} = $res->waitingdate(); - $reserve{'ccode'} = $res->item() ? $res->item()->ccode() : undef; - $reserve{'barcode'} = $res->item() ? $res->item()->barcode() : undef; - $reserve{'priority'} = $res->priority(); - $reserve{'lowestPriority'} = $res->lowestPriority(); - $reserve{'suspend'} = $res->suspend(); - $reserve{'suspend_until'} = $res->suspend_until(); - $reserve{'reserve_id'} = $res->reserve_id(); - $reserve{itemtype} = $res->itemtype(); - $reserve{branchcode} = $res->branchcode(); - $reserve{non_priority} = $res->non_priority(); - $reserve{object} = $res; - - push( @reserveloop, \%reserve ); - } - - # get the time for the form name... - my $time = time(); - - $template->param( - time => $time, - fixedRank => $fixedRank, - ); - - # display infos - $template->param( - bibitemloop => \@bibitemloop, - itemdata_enumchron => $itemdata_enumchron, - itemdata_ccode => $itemdata_ccode, - date => $date, - biblionumber => $biblionumber, - findborrower => $findborrower, - biblio => $biblio, - holdsview => 1, - C4::Search::enabled_staff_search_views, - ); - - $biblioloopiter{biblionumber} = $biblionumber; - $biblioloopiter{title} = $biblio->title; - $biblioloopiter{rank} = $fixedRank; - $biblioloopiter{reserveloop} = \@reserveloop; - - if (@reserveloop) { - $template->param( reserveloop => \@reserveloop ); - } - - if ( $patron ) { - # Add the valid pickup locations - my @pickup_locations = $biblio->pickup_locations({ patron => $patron }); - $biblioloopiter{pickup_locations} = \@pickup_locations; - $biblioloopiter{pickup_locations_codes} = [ map { $_->branchcode } @pickup_locations ]; - } push @biblioloop, \%biblioloopiter; } +# display infos +$template->param( + reserveloop => 1, + itemdata_enumchron => $itemdata_enumchron, + itemdata_ccode => $itemdata_ccode, + date => $date, + biblionumber => $biblionumber, + findborrower => $findborrower, + holdsview => 1, + C4::Search::enabled_staff_search_views, +); + $template->param( biblioloop => \@biblioloop ); $template->param( no_reserves_allowed => $no_reserves_allowed ); $template->param( biblionumbers => join('/', @biblionumbers) ); -- 2.25.1