From d95b5ac27a3c8fc2e082844186f3676ce68bf716 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Mon, 22 Mar 2021 13:03:11 +0000 Subject: [PATCH] Bug 28013: Performance improvements to CanBookBeRenewed MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit In the case of 'AllowRenewalIfOtherItemsAvailable' we check all existing reserves against all existing items. This patchset reduces the number of DB/subroutine calls To test: 1 - Apply patch 2 - prove -v t/db_dependent/Circulation.t Signed-off-by: Amit Gupta Signed-off-by: Joonas Kylmälä Signed-off-by: Jonathan Druart (cherry picked from commit 0b8f4ec614f8e526f814c1d79a6729f7abcac874) Signed-off-by: Fridolin Somers (cherry picked from commit 81254e0db022d407e728d773b835d4f594388174) Signed-off-by: Andrew Fuerste-Henry --- C4/Circulation.pm | 50 ++++++++++++++++------------------------------- 1 file changed, 17 insertions(+), 33 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 1d61044ef0..06f4b252b2 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2804,16 +2804,14 @@ sub CanBookBeRenewed { } } - my ( $resfound, $resrec, undef ) = C4::Reserves::CheckReserves($itemnumber); + my ( $resfound, $resrec, $possible_reserves ) = C4::Reserves::CheckReserves($itemnumber); # This item can fill one or more unfilled reserve, can those unfilled reserves # all be filled by other available items? if ( $resfound && C4::Context->preference('AllowRenewalIfOtherItemsAvailable') ) { - my $schema = Koha::Database->new()->schema(); - - my $item_holds = $schema->resultset('Reserve')->search( { itemnumber => $itemnumber, found => undef } )->count(); + my $item_holds = Koha::Holds->search( { itemnumber => $itemnumber, found => undef } )->count(); if ($item_holds) { # There is an item level hold on this item, no other item can fill the hold $resfound = 1; @@ -2821,29 +2819,18 @@ sub CanBookBeRenewed { else { # Get all other items that could possibly fill reserves - my @itemnumbers = $schema->resultset('Item')->search( - { - biblionumber => $resrec->{biblionumber}, - onloan => undef, - notforloan => 0, - -not => { itemnumber => $itemnumber } - }, - { columns => 'itemnumber' } - )->get_column('itemnumber')->all(); + my $items = Koha::Items->search({ + biblionumber => $resrec->{biblionumber}, + onloan => undef, + notforloan => 0, + -not => { itemnumber => $itemnumber } + }); # Get all other reserves that could have been filled by this item - my @borrowernumbers; - while (1) { - my ( $reserve_found, $reserve, undef ) = - C4::Reserves::CheckReserves( $itemnumber, undef, undef, \@borrowernumbers ); - - if ($reserve_found) { - push( @borrowernumbers, $reserve->{borrowernumber} ); - } - else { - last; - } - } + my @borrowernumbers = map { $_->{borrowernumber} } @$possible_reserves; + my $patrons = Koha::Patrons->search({ + borrowernumber => { -in => \@borrowernumbers } + }); # If the count of the union of the lists of reservable items for each borrower # is equal or greater than the number of borrowers, we know that all reserves @@ -2851,15 +2838,12 @@ sub CanBookBeRenewed { # by pushing all the elements onto an array and removing the duplicates. my @reservable; my %patrons; - ITEM: foreach my $itemnumber (@itemnumbers) { - my $item = Koha::Items->find( $itemnumber ); - next if IsItemOnHoldAndFound( $itemnumber ); - for my $borrowernumber (@borrowernumbers) { - my $patron = $patrons{$borrowernumber} //= Koha::Patrons->find( $borrowernumber ); + ITEM: while ( my $item = $items->next ) { + next if IsItemOnHoldAndFound( $item->itemnumber ); + while ( my $patron = $patrons->next ) { next unless IsAvailableForItemLevelRequest($item, $patron); - next unless CanItemBeReserved($borrowernumber,$itemnumber); - - push @reservable, $itemnumber; + next unless CanItemBeReserved($patron->borrowernumber,$item->itemnumber); + push @reservable, $item->itemnumber; if (@reservable >= @borrowernumbers) { $resfound = 0; last ITEM; -- 2.39.5