From b6dc6502a7735c80e6b9f081dfd8267d4a7ea0cc Mon Sep 17 00:00:00 2001 From: Arthur Suzuki Date: Tue, 17 Nov 2020 14:24:36 +0100 Subject: [PATCH] Bug 20985: Add OnShelfHoldsAllowed checks to CanItemBeReserved Expected behaviour for "On shelf holds allowed" setting for the circulation rules (Koha administration > Patrons and circulation > Circulation and fines rules): - Allow holds only on items that are currently checked out or otherwise unavailable. - If set to "Yes", patrons can place holds on items currently checked in. - If set to "If any unavailable", patrons can only place holds on items that are not unavailable. - If set to "If all unavailable", patrons can only place holds on items where *all* items on the record are unavailable. (Adapted from https://bywatersolutions.com/education/preparing-for-library-closures) Testing using staff interface: 1. Go to Koha administration > Patrons and circulation > Circulation and fines rules. 2. Set "On shelf holds allowed" to "Yes" for all libraries/patron categories/item types. 3. Place a hold on a record with a single item (where the item is available for loan) . I placed a hold for an item and patron both in/belonging to the Centerville library. . Information column in the item table displays "Not on hold". 4. The hold is placed. 5. Cancel the hold. 6. Set "On shelf holds allowed" to either "If any unavailable" or "If all unavailable" for all patrons/patron categories/item types. 7. The hold is not placed: . Message is "Cannot place hold. No items are available to be placed on hold." . Item table: Hold column displays in red "X onShelfHoldsNotAllowed" and the information column displays "Not on hold". 8. Run the tests prove t/db_dependent/Reserves.t - these should pass Signed-off-by: Arthur Suzuki --- C4/Reserves.pm | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index fbd42f0be4..afcc8b90d6 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -349,6 +349,7 @@ sub CanBookBeReserved{ current checkout against the high holds threshold @RETURNS { status => OK }, if the Item can be reserved. + { status => onShelfHoldsNotAllowed }, if onShelfHoldsAllowed parameter and item availability combination doesn't allow holds. { status => ageRestricted }, if the Item is age restricted for this borrower. { status => damaged }, if the Item is damaged. { status => cannotReserveFromOtherBranches }, if syspref 'canreservefromotherbranches' is OK. @@ -370,9 +371,13 @@ sub CanItemBeReserved { my $allowedreserves = 0; # Total number of holds allowed across all records my $holds_per_record = 1; # Total number of holds allowed for this one given record my $holds_per_day; # Default to unlimited + my $opacitemholds = 'Y'; # Itemlevel holds default to allowed + my $on_shelf_holds = 0; # Default to "if any unavailable" + my $context = $params->{context} // ''; # we retrieve borrowers and items informations # # item->{itype} will come for biblioitems if necessery + my $item = Koha::Items->find($itemnumber); my $biblio = $item->biblio; my $patron = Koha::Patrons->find( $borrowernumber ); @@ -427,6 +432,8 @@ sub CanItemBeReserved { $allowedreserves = $rights->{reservesallowed} // $allowedreserves; $holds_per_record = $rights->{holds_per_record} // $holds_per_record; $holds_per_day = $rights->{holds_per_day}; + $opacitemholds = $rights->{opacitemholds}; + $on_shelf_holds = $rights->{onshelfholds}; } else { $ruleitemtype = undef; @@ -436,8 +443,13 @@ sub CanItemBeReserved { borrowernumber => $borrowernumber, biblionumber => $item->biblionumber, }; + $search_params->{found} = undef if $params->{ignore_found_holds}; + # Check for item on shelves and OnShelfHoldsAllowed + return { status => 'onShelfHoldsNotAllowed' } + unless IsAvailableForItemLevelRequest($item, $patron, $pickup_branchcode,1); + my $holds = Koha::Holds->search($search_params); if ( defined $holds_per_record && $holds_per_record ne '' && $holds->count() >= $holds_per_record ) { @@ -1285,6 +1297,7 @@ checks with CanItemBeReserved or CanBookBeReserved. =cut + sub IsAvailableForItemLevelRequest { my $item = shift; my $patron = shift; @@ -1326,7 +1339,6 @@ sub IsAvailableForItemLevelRequest { if ( $on_shelf_holds == 1 ) { return 1; } elsif ( $on_shelf_holds == 2 ) { - # if we have this param predefined from outer caller sub, we just need # to return it, so we saving from having loop inside other loop: return $items_any_available ? 0 : 1 @@ -2285,7 +2297,7 @@ sub GetHoldRule { itemtype => $itemtype, categorycode => $categorycode, branchcode => $branchcode, - rules => ['holds_per_record', 'holds_per_day'], + rules => ['holds_per_record', 'holds_per_day', 'opacitemholds', 'onshelfholds'], order_by => { -desc => [ 'categorycode', 'itemtype', 'branchcode' ] } @@ -2293,6 +2305,8 @@ sub GetHoldRule { ); $rules->{holds_per_record} = $holds_per_x_rules->{holds_per_record}; $rules->{holds_per_day} = $holds_per_x_rules->{holds_per_day}; + $rules->{opacitemholds} = $holds_per_x_rules->{opacitemholds} // 'Y'; + $rules->{onshelfholds} = $holds_per_x_rules->{onshelfholds} // '0'; return $rules; } -- 2.11.0