From 4d22edb600106418aac02b2e1b3e84cccc113e52 Mon Sep 17 00:00:00 2001 From: Nick Clemens 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 --- 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.11.0