From 0f4bc4fd812b34bf15f593d769363dde1ca0247b Mon Sep 17 00:00:00 2001
From: Nick Clemens <nick@bywatersolutions.com>
Date: Mon, 25 Nov 2019 11:32:32 +0000
Subject: [PATCH] Bug 23964: (follow-up) We should check for only active holds
 when determining to set waiting

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 C4/Reserves.pm            |  2 +-
 t/db_dependent/Reserves.t | 29 ++++++++++++++++++++++++-----
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index 0f7e58ac5f..7a3785724a 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -189,7 +189,7 @@ sub AddReserve {
             && ( $item->damaged && C4::Context->preference('AllowHoldsOnDamagedItems') || !$item->damaged )
 
             # Lastly, if this already has holds, we shouldn't make it waiting for the new hold
-            && !$item->holds->count )
+            && !$item->current_holds->count )
         {
             $priority = 0;
             $found = 'W';
diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t
index 36149668ed..618f0598c9 100755
--- a/t/db_dependent/Reserves.t
+++ b/t/db_dependent/Reserves.t
@@ -672,7 +672,7 @@ subtest '_koha_notify_reserve() tests' => sub {
 };
 
 subtest 'ReservesNeedReturns' => sub {
-    plan tests => 13;
+    plan tests => 18;
 
     my $library    = $builder->build_object( { class => 'Koha::Libraries' } );
     my $item_info  = {
@@ -731,14 +731,33 @@ subtest 'ReservesNeedReturns' => sub {
     $hold->delete;
     $hold_1->delete;
 
-    $builder->build({ source => "Branchtransfer", value => {
-        itemnumber  => $item->itemnumber,
-        datearrived => undef,
-    } });
+    my $transfer = $builder->build_object({
+        class => "Koha::Item::Transfers",
+        value => {
+          itemnumber  => $item->itemnumber,
+          datearrived => undef,
+        }
+    });
     $item->damaged(0)->store;
     $hold = place_item_hold( $patron, $item, $library, $priority );
     is( $hold->found, undef, 'If ReservesNeedReturns is 0 but item in transit the hold must not be set to waiting' );
     is( $hold->priority, 1,  'If ReservesNeedReturns is 0 but item in transit the hold must not be set to waiting' );
+    $hold->delete;
+    $transfer->delete;
+
+    $hold = place_item_hold( $patron, $item, $library, $priority );
+    is( $hold->priority, 0, 'If ReservesNeedReturns is 0 and no other status, priority must have been set to 0' );
+    is( $hold->found, 'W', 'If ReservesNeedReturns is 0 and no other status, found must have been set waiting' );
+    $hold_1 = place_item_hold( $patron, $item, $library, $priority );
+    is( $hold_1->priority, 1, 'If ReservesNeedReturns is 0 but item has a hold priority is 1' );
+    $hold_1->suspend(1)->store; # We suspend the hold
+    $hold->delete; # Delete the waiting hold
+    $hold = place_item_hold( $patron, $item, $library, $priority );
+    is( $hold->priority, 0, 'If ReservesNeedReturns is 0 and other hold(s) suspended, priority must have been set to 0' );
+    is( $hold->found, 'W', 'If ReservesNeedReturns is 0 and other  hold(s) suspended, found must have been set waiting' );
+
+
+
 
     t::lib::Mocks::mock_preference('ReservesNeedReturns', 1); # Don't affect other tests
 };
-- 
2.39.5