From 0b8f4ec614f8e526f814c1d79a6729f7abcac874 Mon Sep 17 00:00:00 2001
From: Nick Clemens <nick@bywatersolutions.com>
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 <amitddng135@gmail.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 C4/Circulation.pm | 50 ++++++++++++++++-------------------------------
 1 file changed, 17 insertions(+), 33 deletions(-)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 1c1ba520a0..e403c3b012 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -2872,7 +2872,7 @@ sub CanBookBeRenewed {
         }
     }
 
-    my ( $resfound, $resrec, undef ) = C4::Reserves::CheckReserves($itemnumber);
+    my ( $resfound, $resrec, $possible_reserves ) = C4::Reserves::CheckReserves($itemnumber);
 
     # If next hold is non priority, then check if any hold with priority (non_priority = 0) exists for the same biblionumber.
     if ( $resfound && $resrec->{non_priority} ) {
@@ -2888,9 +2888,7 @@ sub CanBookBeRenewed {
     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;
@@ -2898,29 +2896,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
@@ -2928,15 +2915,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