From 2d30534aaf2dfb72f954e5f9dc4c1da1cce9ac0a Mon Sep 17 00:00:00 2001
From: Nick Clemens <nick@bywatersolutions.com>
Date: Mon, 4 Nov 2019 16:06:02 +0000
Subject: [PATCH] Bug 23964: Unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 t/db_dependent/Reserves.t | 94 +++++++++++++++++++++++++++++++----------------
 1 file changed, 63 insertions(+), 31 deletions(-)

diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t
index 43aff8bddf..e362f11adc 100755
--- a/t/db_dependent/Reserves.t
+++ b/t/db_dependent/Reserves.t
@@ -672,54 +672,72 @@ subtest '_koha_notify_reserve() tests' => sub {
 };
 
 subtest 'ReservesNeedReturns' => sub {
-    plan tests => 4;
+    plan tests => 10;
 
-    my $biblioitem = $builder->build_object( { class => 'Koha::Biblioitems' } );
     my $library    = $builder->build_object( { class => 'Koha::Libraries' } );
-    my $itemtype   = $builder->build_object( { class => 'Koha::ItemTypes', value => { rentalcharge => 0 } } );
     my $item_info  = {
-        biblionumber     => $biblioitem->biblionumber,
-        biblioitemnumber => $biblioitem->biblioitemnumber,
         homebranch       => $library->branchcode,
         holdingbranch    => $library->branchcode,
-        itype            => $itemtype->itemtype,
     };
-    my $item = $builder->build_object( { class => 'Koha::Items', value => $item_info } );
+    my $item = $builder->build_sample_item($item_info);
     my $patron   = $builder->build_object(
         {
             class => 'Koha::Patrons',
             value => { branchcode => $library->branchcode, }
         }
     );
-
-    my $priority = 1;
-    my ( $hold_id, $hold );
-
-    t::lib::Mocks::mock_preference('ReservesNeedReturns', 0); # '0' means 'Automatically mark a hold as found and waiting'
-    $hold_id = C4::Reserves::AddReserve(
-        $library->branchcode, $patron->borrowernumber,
-        $item->biblionumber,  '',
-        $priority,            undef,
-        undef,                '',
-        "title for fee",      $item->itemnumber,
+    my $patron_2   = $builder->build_object(
+        {
+            class => 'Koha::Patrons',
+            value => { branchcode => $library->branchcode, }
+        }
     );
-    $hold = Koha::Holds->find($hold_id);
-    is( $hold->priority, 0, 'If ReservesNeedReturns is 0, priority must have been set to 0' );
-    is( $hold->found, 'W', 'If ReservesNeedReturns is 0, found must have been set waiting' );
 
-    $hold->delete; # cleanup
+    my $priority = 1;
 
-    t::lib::Mocks::mock_preference('ReservesNeedReturns', 1); # '0' means "Don't automatically mark a hold as found and waiting"
-    $hold_id = C4::Reserves::AddReserve(
-        $library->branchcode, $patron->borrowernumber,
-        $item->biblionumber,  '',
-        $priority,            undef,
-        undef,                '',
-        "title for fee",      $item->itemnumber,
-    );
-    $hold = Koha::Holds->find($hold_id);
+    t::lib::Mocks::mock_preference('ReservesNeedReturns', 1); # Test with feature disabled
+    my $hold = place_item_hold( $patron, $item, $library, $priority );
     is( $hold->priority, $priority, 'If ReservesNeedReturns is 1, priority must not have been set to changed' );
     is( $hold->found, undef, 'If ReservesNeedReturns is 1, found must not have been set waiting' );
+    $hold->delete;
+
+    t::lib::Mocks::mock_preference('ReservesNeedReturns', 0); # '0' means 'Automatically mark a hold as found and waiting'
+    $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->delete;
+
+    $item->onloan('2010-01-01')->store;
+    $hold = place_item_hold( $patron, $item, $library, $priority );
+    isnt( $hold->priority, 0, 'If ReservesNeedReturns is 0 but item onloan priority must not be set to 0' );
+    $hold->delete;
+
+    t::lib::Mocks::mock_preference('AllowHoldsOnDamagedItems', 0); # '0' means damaged holds not allowed
+    $item->onloan(undef)->damaged(1)->store;
+    $hold = place_item_hold( $patron, $item, $library, $priority );
+    isnt( $hold->priority, 0, 'If ReservesNeedReturns is 0 but item damaged and not allowed holds on damaged items priority must not be set to 0' );
+    $hold->delete;
+    t::lib::Mocks::mock_preference('AllowHoldsOnDamagedItems', 1); # '0' means damaged holds not allowed
+    $hold = place_item_hold( $patron, $item, $library, $priority );
+    is( $hold->priority, 0, 'If ReservesNeedReturns is 0 and damaged holds allowed, priority must have been set to 0' );
+    is( $hold->found, 'W', 'If ReservesNeedReturns is 0 and damaged holds allowed, found must have been set waiting' );
+
+    my $hold_1 = place_item_hold( $patron, $item, $library, $priority );
+    $hold = place_item_hold( $patron_2, $item, $library, $priority );
+    isnt( $hold->priority, 0, 'If ReservesNeedReturns is 0 but item already on hold priority must not be set to 0' );
+    $hold->delete;
+    $hold_1->delete;
+
+    $hold->delete;
+    $builder->build({ source => "Branchtransfer", value => {
+        itemnumber  => $item->itemnumber,
+        datearrived => undef,
+    } });
+    $item->damaged(0)->store;
+    $hold = place_item_hold( $patron, $item, $library, $priority );
+    isnt( $hold->priority, 0, 'If ReservesNeedReturns is 0 but item in transit the hold must not be set to waiting' );
+
+    t::lib::Mocks::mock_preference('ReservesNeedReturns', 1); # Don't affect other tests
 };
 
 subtest 'ChargeReserveFee tests' => sub {
@@ -845,5 +863,19 @@ sub count_hold_print_messages {
     return $message_count->[0]->[0];
 }
 
+sub place_item_hold {
+    my ($patron,$item,$library,$priority) = @_;
+
+    my $hold_id = C4::Reserves::AddReserve(
+        $library->branchcode, $patron->borrowernumber,
+        $item->biblionumber,  '',
+        $priority,            undef,
+        undef,                '',
+        "title for fee",      $item->itemnumber,
+    );
+    my $hold = Koha::Holds->find($hold_id);
+    return $hold;
+}
+
 # we reached the finish
 $schema->storage->txn_rollback();
-- 
2.11.0