Bug 14297: Holds Queue building ignoring holds where pickup & home branch don't match...
authorKyle M Hall <kyle@bywatersolutions.com>
Fri, 29 May 2015 10:46:24 +0000 (06:46 -0400)
committerTomas Cohen Arazi <tomascohen@theke.io>
Fri, 25 Sep 2015 15:00:03 +0000 (12:00 -0300)
If a record has a hold on it where the pickup and home branch do not
match, the holds queue builder will only look at items from the least
cost branch ( as defined by the transport cost matrix or the sys pref
StaticHoldsQueueWeight.

Test Plan:
1) Create a record with two items, one for library A and one for library B
2) Set your circulation rules such that the book from library A is
   holdable by all and the book from library B is holdable only by library
   B patrons
3) Create a hold for a Library C patron for pickup at library C
4) Set the syspref StaticHoldsQueueWeight to by Library B, Library A,
   Library C in that order
5) Rebuild the holds queue
6) Note the hold wasn't picked up even though the item from library A
   could have filled the hold
7) Apply this patch
8) Rebuild the holds queue
9) View the holds queue again
10) Note the hold now displays

Signed-off-by: Nora Blake <nblake@masslibsystem.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
C4/HoldsQueue.pm

index 13cb295..bae6f82 100755 (executable)
@@ -457,6 +457,8 @@ sub MapItemsToHoldRequests {
             } else {
                 $pull_branches = [keys %items_by_branch];
             }
+
+            # Try picking items where the home and pickup branch match first
             PULL_BRANCHES:
             foreach my $branch (@$pull_branches) {
                 my $holding_branch_items = $items_by_branch{$branch}
@@ -473,6 +475,7 @@ sub MapItemsToHoldRequests {
                 }
             }
 
+            # Now try items from the least cost branch based on the transport cost matrix or StaticHoldsQueueWeight
             unless ( $itemnumber ) {
                 foreach my $current_item ( @{ $items_by_branch{$holdingbranch} } ) {
                     if ( $holdingbranch && ( $current_item->{holdallowed} == 2 || $request->{borrowerbranch} eq $current_item->{homebranch} ) ) {
@@ -481,6 +484,24 @@ sub MapItemsToHoldRequests {
                     }
                 }
             }
+
+            # Now try for items for any item that can fill this hold
+            unless ( $itemnumber ) {
+                PULL_BRANCHES2:
+                foreach my $branch (@$pull_branches) {
+                    my $holding_branch_items = $items_by_branch{$branch}
+                      or next;
+
+                    $holdingbranch ||= $branch;
+                    foreach my $item (@$holding_branch_items) {
+                        next if ( $item->{holdallowed} == 1 && $item->{homebranch} ne $request->{borrowerbranch} );
+
+                        $itemnumber = $item->{itemnumber};
+                        $holdingbranch = $branch;
+                        last PULL_BRANCHES2;
+                    }
+                }
+            }
         }
 
         if ($itemnumber) {