Attachment 116022 Details for Bug 27068 – Bug 27068: Control hold group logic in HoldsQueue

[patch] Bug 27068: Control hold group logic in HoldsQueue

Bug-27068-Control-hold-group-logic-in-HoldsQueue.patch (text/plain), 8.85 KB, created by Kyle M Hall (khall) on 2021-01-29 12:49:19 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2021-01-29 12:49:19 UTC

Size: 8.85 KB

patch

obsolete

>From 83c56c3340b23d098f1c20d00241a0bb3febf62b Mon Sep 17 00:00:00 2001
>From: Agustin Moyano <agustinmoyano@theke.io>
>Date: Wed, 2 Dec 2020 20:04:51 -0300
>Subject: [PATCH] Bug 27068: Control hold group logic in HoldsQueue
>
>Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
>---
> C4/HoldsQueue.pm | 81 ++++++++++++++++++++++++++++++------------------
> 1 file changed, 50 insertions(+), 31 deletions(-)
>
>diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm
>index 87144193ff..e2666aa4ab 100644
>--- a/C4/HoldsQueue.pm
>+++ b/C4/HoldsQueue.pm
>@@ -31,6 +31,7 @@ use C4::Biblio;
> use Koha::DateUtils;
> use Koha::Items;
> use Koha::Patrons;
>+use Koha::Libraries;
> 
> use List::Util qw(shuffle);
> use List::MoreUtils qw(any);
>@@ -361,6 +362,38 @@ sub GetItemsAvailableToFillHoldRequestsForBib {
>     } @items ];
> }
> 
>+=head2 _checkHoldPolicy
>+
>+    _checkHoldPolicy($item, $request)
>+
>+    check if item agrees with hold policies
>+
>+=cut
>+
>+sub _checkHoldPolicy {
>+    my ($item, $request) = @_;
>+
>+    return 0 unless $item->{holdallowed};
>+    return 0 if $item->{holdallowed} == 1 && $item->{homebranch} ne $request->{borrowerbranch};
>+
>+    my $library = Koha::Libraries->find($item->{homebranch});
>+
>+    return 0 if $item->{'holdallowed'} == 3 && !$library->validate_hold_sibling({branchcode => $request->{borrowerbranch}});
>+
>+    my $hold_fulfillment_policy = $item->{hold_fulfillment_policy};
>+
>+    return 0 if $hold_fulfillment_policy eq 'holdgroup' && !$library->validate_hold_sibling({branchcode => $request->{branchcode}});
>+    return 0 if $hold_fulfillment_policy eq 'homebranch' && $request->{branchcode} ne $item->$hold_fulfillment_policy;
>+    return 0 if $hold_fulfillment_policy eq 'holdingbranch' && $request->{branchcode} ne $item->$hold_fulfillment_policy;
>+
>+    my $patronLibrary = Koha::Libraries->find($request->{borrowerbranch});
>+
>+    return 0 if $hold_fulfillment_policy eq 'patrongroup' && !patronLibrary->validate_hold_sibling({branchcode => $request->{branchcode}});
>+
>+    return 1;
>+
>+}
>+
> =head2 MapItemsToHoldRequests
> 
>   MapItemsToHoldRequests($hold_requests, $available_items, $branches, $transport_cost_matrix)
>@@ -409,11 +442,9 @@ sub MapItemsToHoldRequests {
> 
>             my $local_hold_match;
>             foreach my $item (@$available_items) {
>-                next
>-                  if ( !$item->{holdallowed} )
>-                  || ( $item->{holdallowed} == 1
>-                    && $item->{homebranch} ne $request->{borrowerbranch} )
>-                  || $item->{_object}->exclude_from_local_holds_priority;
>+                next if $item->{_object}->exclude_from_local_holds_priority;
>+
>+                next unless _checkHoldPolicy($item, $request);
> 
>                 next if $request->{itemnumber} && $request->{itemnumber} != $item->{itemnumber};
> 
>@@ -468,12 +499,10 @@ sub MapItemsToHoldRequests {
>             if (
>                     exists $items_by_itemnumber{ $request->{itemnumber} }
>                 and not exists $allocated_items{ $request->{itemnumber} }
>-                and ( # Don't fill item level holds that contravene the hold pickup policy at this time
>-                    ( $items_by_itemnumber{ $request->{itemnumber} }->{hold_fulfillment_policy} eq 'any' )
>-                    || ( $request->{branchcode} eq $items_by_itemnumber{ $request->{itemnumber} }->{ $items_by_itemnumber{ $request->{itemnumber} }->{hold_fulfillment_policy} }  )
>+                and  _checkHoldPolicy($items_by_itemnumber{ $request->{itemnumber} }, $request) # Don't fill item level holds that contravene the hold pickup policy at this time
>                 and ( !$request->{itemtype} # If hold itemtype is set, item's itemtype must match
>                     || $items_by_itemnumber{ $request->{itemnumber} }->{itype} eq $request->{itemtype} )
>-                )
>+
>                 and $items_by_itemnumber{ $request->{itemnumber} }->{_object}->can_be_transferred( { to => $libraries->{ $request->{branchcode} } } )
> 
>               )
>@@ -527,8 +556,7 @@ sub MapItemsToHoldRequests {
> 
>                 if (
>                     $request->{borrowerbranch} eq $item->{homebranch}
>-                    && ( ( $item->{hold_fulfillment_policy} eq 'any' ) # Don't fill item level holds that contravene the hold pickup policy at this time
>-                        || $request->{branchcode} eq $item->{ $item->{hold_fulfillment_policy} } )
>+                    && _checkHoldPolicy($item, $request) # Don't fill item level holds that contravene the hold pickup policy at this time
>                     && ( !$request->{itemtype} # If hold itemtype is set, item's itemtype must match
>                         || ( $request->{itemnumber} && ( $items_by_itemnumber{ $request->{itemnumber} }->{itype} eq $request->{itemtype} ) ) )
>                   )
>@@ -550,8 +578,7 @@ sub MapItemsToHoldRequests {
>                     next unless $items_by_itemnumber{ $item->{itemnumber} }->{_object}->can_be_transferred( { to => $libraries->{ $request->{branchcode} } } );
> 
>                     # Don't fill item level holds that contravene the hold pickup policy at this time
>-                    next unless $item->{hold_fulfillment_policy} eq 'any'
>-                        || $request->{branchcode} eq $item->{ $item->{hold_fulfillment_policy} };
>+                    next unless _checkHoldPolicy($item, $request);
> 
>                     # If hold itemtype is set, item's itemtype must match
>                     next unless ( !$request->{itemtype}
>@@ -583,7 +610,7 @@ sub MapItemsToHoldRequests {
>                 $holdingbranch ||= $branch;
>                 foreach my $item (@$holding_branch_items) {
>                     next if $pickup_branch ne $item->{homebranch};
>-                    next if ( $item->{holdallowed} == 1 && $item->{homebranch} ne $request->{borrowerbranch} );
>+                    next unless _checkHoldPolicy($item, $request);
>                     next unless $items_by_itemnumber{ $item->{itemnumber} }->{_object}->can_be_transferred( { to => $libraries->{ $request->{branchcode} } } );
> 
>                     # Don't fill item level holds that contravene the hold pickup policy at this time
>@@ -601,23 +628,18 @@ sub MapItemsToHoldRequests {
>             }
> 
>             # Now try items from the least cost branch based on the transport cost matrix or StaticHoldsQueueWeight
>-            unless ( $itemnumber ) {
>+            unless ( $itemnumber || !$holdingbranch) {
>                 foreach my $current_item ( @{ $items_by_branch{$holdingbranch} } ) {
>-                    if ( $holdingbranch && ( $current_item->{holdallowed} == 2 || $request->{borrowerbranch} eq $current_item->{homebranch} ) ) {
>-
>-                        # Don't fill item level holds that contravene the hold pickup policy at this time
>-                        next unless $current_item->{hold_fulfillment_policy} eq 'any'
>-                            || $request->{branchcode} eq $current_item->{ $current_item->{hold_fulfillment_policy} };
>+                    next unless _checkHoldPolicy($current_item, $request); # Don't fill item level holds that contravene the hold pickup policy at this time
> 
>-                        # If hold itemtype is set, item's itemtype must match
>-                        next unless ( !$request->{itemtype}
>-                            || $current_item->{itype} eq $request->{itemtype} );
>+                    # If hold itemtype is set, item's itemtype must match
>+                    next unless ( !$request->{itemtype}
>+                        || $current_item->{itype} eq $request->{itemtype} );
> 
>-                        next unless $items_by_itemnumber{ $current_item->{itemnumber} }->{_object}->can_be_transferred( { to => $libraries->{ $request->{branchcode} } } );
>+                    next unless $items_by_itemnumber{ $current_item->{itemnumber} }->{_object}->can_be_transferred( { to => $libraries->{ $request->{branchcode} } } );
> 
>-                        $itemnumber = $current_item->{itemnumber};
>-                        last; # quit this loop as soon as we have a suitable item
>-                    }
>+                    $itemnumber = $current_item->{itemnumber};
>+                    last; # quit this loop as soon as we have a suitable item
>                 }
>             }
> 
>@@ -629,11 +651,8 @@ sub MapItemsToHoldRequests {
>                       or next;
> 
>                     foreach my $item (@$holding_branch_items) {
>-                        next if ( $item->{holdallowed} == 1 && $item->{homebranch} ne $request->{borrowerbranch} );
>-
>                         # Don't fill item level holds that contravene the hold pickup policy at this time
>-                        next unless $item->{hold_fulfillment_policy} eq 'any'
>-                            || $request->{branchcode} eq $item->{ $item->{hold_fulfillment_policy} };
>+                        next unless _checkHoldPolicy($item, $request);
> 
>                         # If hold itemtype is set, item's itemtype must match
>                         next unless ( !$request->{itemtype}
>-- 
>2.24.1 (Apple Git-126)

Actions: View | Diff | Splinter Review

Attachments on bug 27068: 113900 | 114125 | 114140 | 114141 | 114142 | 115987 | 116022 | 116023 | 116024 | 116025 | 116026 | 116607 | 116608 | 116609 | 116610 | 116814 | 116815 | 116816 | 116817 | 117215