From b4e25240cd67560d5abd643c8c2445f31865225d Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Mon, 23 Jan 2023 16:48:53 +0000 Subject: [PATCH] Bug 32702: Move item status checks to CanItemBeReserved This patch moves item status checks from IsAvailableForItemLevelRequest to CanItemBeReserved. Changes to existing calls highlighted below. Existing Calls for CanItemBeReserved/CanBookBeReserved: Call in Circulation.pm checkHighHolds - previously would have allowed damaged, notforloan, etc items to fill holds CanBookBeRenewed - Already a combination of CanItemBeReserved and IsAvailableForItemLevelRequest Call in ILSDI/Services.pm HoldItem - would have allowed placing holds on damaged/notforloan/etc items C4/Reserves: What is ItemsAnyAvailableAndNotRestricted used for C4/SIP/ILS/Transaction/Hold.pm: do_hold - would have allowed hold on lost/damaged/etc. Koha/Club/Hold/pm: add - would have allowed etc. Koha/REST/V1/Holds.pm: add - would have allowed ... opac/opac-reserve.pl: Was already combo of CanItemBeReserved and IsAvailableForItemLevelRequest reserve/placerequest: Items would have been filtered on reserve/request.pl by IsAvailableForItemLevelRequest In this case and opac, I believe before this a hold could have been forced with the right parameters Existing Calls for IsAvailableForItemLevelRequest C4/Circulation - see above Call in ILSDI/Services.pm: GetServices - Already a combination of CanItemBeReserved and IsAvailableForItemLevelRequest Call in opac/opac-reserve.pl: Already a combination of CanItemBeReserved and IsAvailableForItemLevelRequest Call in reserve/request.pl: Already a combination of CanItemBeReserved and IsAvailableForItemLevelRequest To test: 1 - Setup a record with items to have: 1 available item 1 lost item 1 item with positive not for loan status 1 item with negative not for loan status 1 item withdrawn 1 item damaged 1 item not for loan by itemtype 2 - Attempt to place hold on staff and opac and note the statuses 3 - Apply patch 4 - Confirm the statuses have not changed 5 - Attempt to hold lost/damaged/withdrawn/notforloan items via API 6 - You should not be able to place the hold 7 - Attempt to place club holds on these items - it should not be possible 8 - Attempt to place holds via ILSDI - it should not be possible --- C4/Reserves.pm | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 6bcae548d4..c3d86b2ebe 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -446,15 +446,27 @@ sub CanItemBeReserved { } } - # we retrieve borrowers and items informations # - # item->{itype} will come for biblioitems if necessery - my $borrower = $patron->unblessed; + # must check the notforloan setting of the itemtype + # FIXME - a lot of places in the code do this + # or something similar - need to be + # consolidated + my $itemtype = $item->effective_itemtype; + return { status => 'missing_itemtype' } + unless defined $itemtype; + my $notforloan_per_itemtype = Koha::ItemTypes->find($itemtype)->notforloan; + + return { status => 'notforloan' } if ( $item->notforloan || $notforloan_per_itemtype ); + return { status => 'itemlost' } if $item->itemlost; + return { status => 'withdrawn' } if $item->withdrawn; # If an item is damaged and we don't allow holds on damaged items, we can stop right here return { status =>'damaged' } if ( $item->damaged && !C4::Context->preference('AllowHoldsOnDamagedItems') ); + # we retrieve borrowers information # + my $borrower = $patron->unblessed; + if( GetMarcFromKohaField('biblioitems.agerestriction') ){ my $biblio = $item->biblio; # Check for the age restriction @@ -1370,23 +1382,6 @@ sub IsAvailableForItemLevelRequest { # looped outside of IsAvailableForItemLevelRequest to avoid nested loops: my $items_any_available = shift; - my $dbh = C4::Context->dbh; - # must check the notforloan setting of the itemtype - # FIXME - a lot of places in the code do this - # or something similar - need to be - # consolidated - my $itemtype = $item->effective_itemtype; - return 0 - unless defined $itemtype; - my $notforloan_per_itemtype = Koha::ItemTypes->find($itemtype)->notforloan; - - return 0 if - $notforloan_per_itemtype || - $item->itemlost || - $item->notforloan > 0 || # item with negative or zero notforloan value is holdable - $item->withdrawn || - ($item->damaged && !C4::Context->preference('AllowHoldsOnDamagedItems')); - if ($pickup_branchcode) { my $destination = Koha::Libraries->find($pickup_branchcode); return 0 unless $destination; -- 2.30.2