From dcbc717397963806c3fbf1e08f7a72b2bcd314bb Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Fri, 17 Apr 2020 14:06:32 -0400 Subject: [PATCH] Bug 25184: Items with a negative notforloan status should not be captured for holds Negative notforloan statuses should allow holds to be placed but not captured. Due to coronavirus, we have libraries setting all returned materials to a negative notforloan value of Quarantine for several days. They're using UpdateNotForLoanStatusOnCheckin to set that status automatically. However, those items are still capturing for holds, even though those items cannot be checked out until the notforloan status is removed. In cases like an On Order item where we do want the hold to fill at checkin, UpdateNotForLoanStatusOnCheckin should be used to clear that notforloan status so the hold can fill. In master, if I set an item to a not for loan but holdable status ( < 0 ) I can place the hold, capture the hold and set it to waiting, but *not* check it out to the patron! This does not make sense. I should not be able to trap an item for checkout unless it can be checked out. Test Plan: 1) Set an item's notforloan value to -1 2) Place a hold on that item 3) Check in the item 4) Trap the item for that hold 5) Attempt to check the item out to the patron, you will be unable to because it is notforloan 6) Apply this patch 7) Restart all the things! 8) Repeat steps 1-3 9) The screen should no longer ask if the item should be trapped to fill the hold! Signed-off-by: Andrew Fuerste-Henry --- C4/Reserves.pm | 2 +- t/db_dependent/Holds.t | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 9c040bdc12..d320d9d880 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -793,7 +793,7 @@ sub CheckReserves { return unless $itemnumber; # bail if we got nothing. # if item is not for loan it cannot be reserved either..... # except where items.notforloan < 0 : This indicates the item is holdable. - return if ( $notforloan_per_item > 0 ) or $notforloan_per_itemtype; + return if $notforloan_per_item or $notforloan_per_itemtype; # Find this item in the reserves my @reserves = _Findgroupreserve( $bibitem, $biblio, $itemnumber, $lookahead_days, $ignore_borrowers); diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 69b2071284..3cc2dc7e0f 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -7,7 +7,7 @@ use t::lib::TestBuilder; use C4::Context; -use Test::More tests => 61; +use Test::More tests => 63; use MARC::Record; use C4::Biblio; @@ -344,6 +344,20 @@ t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 0 ); ok( CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status} eq 'damaged', "Patron cannot reserve damaged item with AllowHoldsOnDamagedItems disabled" ); ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for damaged item with AllowHoldsOnDamagedItems disabled" ); +# Items that are not for loan, but holdable should not be trapped until they are available for loan +Koha::Items->find($itemnumber)->damaged(0)->notforloan(-1)->store; +Koha::Holds->search({ biblionumber => $biblio->id })->delete(); +is( CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status}, 'OK', "Patron can place hold on item that is not for loan but holdable ( notforloan < 0 )" ); +$hold = Koha::Hold->new( + { + borrowernumber => $borrowernumbers[0], + itemnumber => $itemnumber, + biblionumber => $biblio->biblionumber, + } +)->store(); +ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for item that is not for loan but holdable ( notforloan < 0 )" ); +$hold->delete(); + # Regression test for bug 9532 $biblio = $builder->build_sample_biblio({ itemtype => 'CANNOT' }); $item = $builder->build_sample_item({ library => $branch_1, itype => 'CANNOT', biblionumber => $biblio->biblionumber}); -- 2.11.0