From bf93b6636ade87a827abe259a7a76087e5a90af8 Mon Sep 17 00:00:00 2001 From: Alex Buckley Date: Wed, 8 Dec 2021 18:06:12 +0000 Subject: [PATCH] Bug 23172: The holds queue should check if a patron category is eligible to reserve an itemtype Currently when mapping items to pending holds the holds queue does not check the circulation rules to determine if the reserving patron category is eligible to reserve the item. This patch fixes that by calling C4::Reserves->CanItemBeReserved() when mapping items to pending reserves. Test plan: 1. Create two patrons, with the following patron categories: - Patron A -> Patron - Patron B -> Student 2. Create two items attached to a single biblio record: - Book - Continuing resource 3. Create the following circ rules: Patron category, item type, Holds allowed (total), Holds allowed (daily), Holds per record (count) - Patron, Book, 0, 0, 0 - Student, Book, 1, 1, 1 - All, Continuing resources, 1, 1, 1 4. Place a title level hold on the biblio for patron A 5. Place a title level hold on the biblio for patron B 6. Run HoldsQueue.pl 7. Notice in 'Circulation' > 'Holds queue' Patron A is listed as having a hold on the Book 8. Checkin the Book and notice you are prompted to confirm the hold by Patron A - even though according to circ rules they cannot reserve Books 9. Apply patch and restart services 10. Run HoldsQueue.pl 11. Notice in 'Circulation' > 'Holds queue' Patron A is listed as having a hold on the Continuing resource. Patron B is listed as having a hold on the Book -> These holds conform to the circulation rules 12. Checkin the Book, and notice you're prompted to confirm the hold by Patron B Sponsored-By: Brimbank Library, Australia --- C4/HoldsQueue.pm | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm index b8abe21496..2850c3acbd 100644 --- a/C4/HoldsQueue.pm +++ b/C4/HoldsQueue.pm @@ -25,6 +25,7 @@ use warnings; use C4::Context; use C4::Search; use C4::Circulation qw( GetTransfers GetBranchItemRule ); +use C4::Reserves qw( CanItemBeReserved ); use Koha::DateUtils qw( dt_from_string ); use Koha::Items; use Koha::Patrons; @@ -409,6 +410,25 @@ sub _checkHoldPolicy { } +=head2 _checkCirculationRules + + _checkCirculationRules($item, $request); + + Check if the patron is eligible to reserve this item. + +=cut + +sub _checkCirculationRules { + + my ( $item, $request ) = @_; + + return 0 + if (CanItemBeReserved( $request->{borrowernumber}, $item->{itemnumber}, $request->{branchcode}, { ignore_hold_counts => 1 } )->{status} ne 'OK'); + + return 1; + +} + =head2 MapItemsToHoldRequests MapItemsToHoldRequests($hold_requests, $available_items, $branches, $transport_cost_matrix) @@ -461,6 +481,9 @@ sub MapItemsToHoldRequests { next unless _checkHoldPolicy($item, $request); + # Check the patron is eligible to reserve this item type + next unless _checkCirculationRules($item, $request); + next if $request->{itemnumber} && $request->{itemnumber} != $item->{itemnumber}; next unless $item->{_object}->can_be_transferred( { to => $libraries->{ $request->{branchcode} } } ); @@ -515,6 +538,7 @@ sub MapItemsToHoldRequests { exists $items_by_itemnumber{ $request->{itemnumber} } and not exists $allocated_items{ $request->{itemnumber} } and _checkHoldPolicy($items_by_itemnumber{ $request->{itemnumber} }, $request) # Don't fill item level holds that contravene the hold pickup policy at this time + and _checkCirculationRules($items_by_itemnumber{ $request->{itemnumber} }, $request) # Don't fill item level hold if the patron is not eligible to reserve this item and ( !$request->{itemtype} # If hold itemtype is set, item's itemtype must match || $items_by_itemnumber{ $request->{itemnumber} }->{itype} eq $request->{itemtype} ) @@ -572,6 +596,7 @@ sub MapItemsToHoldRequests { if ( $request->{borrowerbranch} eq $item->{homebranch} && _checkHoldPolicy($item, $request) # Don't fill item level holds that contravene the hold pickup policy at this time + && _checkCirculationRules($item, $request) # Don't fill item level hold if the patron is not eligible to reserve this item && ( !$request->{itemtype} # If hold itemtype is set, item's itemtype must match || ( $request->{itemnumber} && ( $items_by_itemnumber{ $request->{itemnumber} }->{itype} eq $request->{itemtype} ) ) ) ) @@ -595,6 +620,9 @@ sub MapItemsToHoldRequests { # Don't fill item level holds that contravene the hold pickup policy at this time next unless _checkHoldPolicy($item, $request); + # Don't fill item level hold if the patron is not eligible to reserve this item + next unless _checkCirculationRules($item, $request); + # If hold itemtype is set, item's itemtype must match next unless ( !$request->{itemtype} || $item->{itype} eq $request->{itemtype} ); @@ -636,6 +664,9 @@ sub MapItemsToHoldRequests { next unless ( !$request->{itemtype} || $item->{itype} eq $request->{itemtype} ); + # Check if the patron is eligible to reserve this item type + next unless _checkCirculationRules($item, $request); + $itemnumber = $item->{itemnumber}; $holdingbranch = $branch; last PULL_BRANCHES; @@ -653,6 +684,9 @@ sub MapItemsToHoldRequests { next unless $items_by_itemnumber{ $current_item->{itemnumber} }->{_object}->can_be_transferred( { to => $libraries->{ $request->{branchcode} } } ); + # Check if the patron is eligible to reserve this item + next unless _checkCirculationRules($current_item, $request); + $itemnumber = $current_item->{itemnumber}; last; # quit this loop as soon as we have a suitable item } @@ -675,6 +709,9 @@ sub MapItemsToHoldRequests { next unless $items_by_itemnumber{ $item->{itemnumber} }->{_object}->can_be_transferred( { to => $libraries->{ $request->{branchcode} } } ); + # Check if the patron is eligible to reserve this item + next unless _checkCirculationRules($item, $request); + $itemnumber = $item->{itemnumber}; $holdingbranch = $branch; last PULL_BRANCHES2; -- 2.11.0