From 515df43af8dddd493616dfe9efc83373c8d2f50b Mon Sep 17 00:00:00 2001
From: Kyle M Hall <kyle@bywatersolutions.com>
Date: Tue, 3 Dec 2019 11:03:18 -0500
Subject: [PATCH] Bug 21944: Improve efficiency of code

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 C4/Circulation.pm            |  7 +++----
 C4/Reserves.pm               | 26 ++++++++++----------------
 t/db_dependent/Circulation.t |  5 +++--
 3 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 6b2c91eec5..860d965cf1 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -2079,11 +2079,10 @@ sub AddReturn {
     ($resfound, $resrec, undef) = C4::Reserves::CheckReserves( $item->itemnumber, undef, $lookahead ) unless ( $item->withdrawn );
     # if a hold is found and is waiting at another branch, change the priority back to 1 and trigger the hold (this will trigger a transfer and update the hold status properly)
     if ( $resfound eq "Waiting" and $branch ne $resrec->{branchcode} ) {
-        C4::Reserves::RevertWaitingStatus( { itemnumber => $item->itemnumber } );
-        #If the hold is reverted we need to refetch for the return values
-        ($resfound, $resrec, undef) = C4::Reserves::CheckReserves( $item->itemnumber, undef, $lookahead ) unless ( $item->withdrawn );
+        my $hold = C4::Reserves::RevertWaitingStatus( { itemnumber => $item->itemnumber } );
+        $resfound = 'Reserved';
+        $resrec = $hold->unblessed;
     }
-    ($resfound, $resrec, undef) = C4::Reserves::CheckReserves( $item->itemnumber, undef, $lookahead ) unless ( $item->withdrawn );
     if ($resfound) {
           $resrec->{'ResFound'} = $resfound;
         $messages->{'ResFound'} = $resrec;
diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index ca354f8f47..00d921d77e 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -1987,24 +1987,18 @@ sub RevertWaitingStatus {
     $sth = $dbh->prepare( $query );
     $sth->execute( $reserve->{'biblionumber'} );
 
-    ## Fix up the currently waiting reserve
-    $query = "
-    UPDATE reserves
-    SET
-      priority = 1,
-      found = NULL,
-      waitingdate = NULL
-    WHERE
-      reserve_id = ?
-    ";
-    $sth = $dbh->prepare( $query );
-    $sth->execute( $reserve->{'reserve_id'} );
-
-    unless ( $hold->item_level_hold ) {
-        $hold->itemnumber(undef)->store;
-    }
+    $hold->set(
+        {
+            priority    => 1,
+            found       => undef,
+            waitingdate => undef,
+            itemnumber  => $hold->item_level_hold ? $hold->itemnumber : undef,
+        }
+    )->store();
 
     _FixPriority( { biblionumber => $reserve->{biblionumber} } );
+
+    return $hold;
 }
 
 =head2 ReserveSlip
diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
index 537fa17278..121c1767b4 100755
--- a/t/db_dependent/Circulation.t
+++ b/t/db_dependent/Circulation.t
@@ -2909,7 +2909,7 @@ subtest '_FixAccountForLostAndFound returns undef if patron is deleted' => sub {
 };
 
 subtest 'Set waiting flag' => sub {
-    plan tests => 9;
+    plan tests => 11;
 
     my $library_1 = $builder->build( { source => 'Branch' } );
     my $patron_1  = $builder->build( { source => 'Borrower', value => { branchcode => $library_1->{branchcode}, categorycode => $patron_category->{categorycode} } } );
@@ -2958,10 +2958,11 @@ subtest 'Set waiting flag' => sub {
     $hold = Koha::Holds->find( $reserve_id );
     is( $hold->found, undef, 'Hold is no longer marked waiting' );
     is( $hold->priority, 1,  "Hold is now priority one again");
+    is( $hold->waitingdate, undef, "Hold no longer has a waiting date");
+    is( $hold->itemnumber, $item->{itemnumber}, "Hold has retained its' itemnumber");
     is( $messages->{ResFound}->{ResFound}, "Reserved", "Hold is still returned");
     is( $messages->{ResFound}->{found}, undef, "Hold is no longer marked found in return message");
     is( $messages->{ResFound}->{priority}, 1, "Hold is priority 1 in return message");
-
 };
 
 subtest 'Cancel transfers on lost items' => sub {
-- 
2.39.5