Attachment 103185 Details for Bug 25184 – Bug 25184: Items with a negative notforloan status should not be captured for holds

[patch] Bug 25184: Items with a negative notforloan status should not be captured for holds

Bug-25184-Items-with-a-negative-notforloan-status-.patch (text/plain), 4.04 KB, created by Andrew Fuerste-Henry on 2020-04-17 23:51:50 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Fuerste-Henry

Created: 2020-04-17 23:51:50 UTC

Size: 4.04 KB

patch

obsolete

>From 17e84886d3baa4b4b56355486c278f6112a1e7e7 Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>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 <andrew@bywatersolutions.com>
>
>Signed-off-by: Catherine Ingram <Catherine.Ingram@cedarparktexas.gov>
>---
> 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

Actions: View | Diff | Splinter Review

Attachments on bug 25184: 103177 | 103180 | 103185 | 103396 | 103397 | 104449 | 104450 | 104666