Attachment 128817 Details for Bug 23172 – Bug 23172: The holds queue should check if a patron category is eligible to reserve an itemtype

[patch] Bug 23172: The holds queue should check if a patron category is eligible to reserve an itemtype

Bug-23172-The-holds-queue-should-check-if-a-patron.patch (text/plain), 6.75 KB, created by Alex Buckley on 2021-12-21 04:33:12 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Alex Buckley

Created: 2021-12-21 04:33:12 UTC

Size: 6.75 KB

patch

obsolete

>From bf93b6636ade87a827abe259a7a76087e5a90af8 Mon Sep 17 00:00:00 2001
>From: Alex Buckley <alexbuckley@catalyst.net.nz>
>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

Actions: View | Diff | Splinter Review

Attachments on bug 23172: 90868 | 91133 | 91301 | 92250 | 92251 | 92252 | 102099 | 102100 | 102101 | 102169 | 118930 | 118931 | 118932 | 118933 | 128817 | 128857