From 1bc658e0470322a2d45da3f27f5f32eb8891b8aa Mon Sep 17 00:00:00 2001
From: Nick Clemens <nick@bywatersolutions.com>
Date: Wed, 29 Nov 2023 15:00:18 +0000
Subject: [PATCH] Bug 35432: Remove PULL_BRANCHES and use ifs over unless

PULL_BRANCHES and PULL_BRANCHES2 felt a little arbitrary, moved
to using a last in the outer loop.

Swapped 'next unless' for an if to more directly show when we are acting

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
---
 C4/HoldsQueue.pm | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm
index a27487f09c..8fbab8fec9 100644
--- a/C4/HoldsQueue.pm
+++ b/C4/HoldsQueue.pm
@@ -585,24 +585,25 @@ sub MapItemsToHoldRequests {
             } else {
                 $pull_branches = [keys %items_by_branch];
             }
+            $holdingbranch ||= $pull_branches->[0];    # We set this as the first from the list of pull branches
+                 # unless we set it above to the pickupbranch or the least cost branch
+                 # FIXME: The itention is to follow StaticHoldsQueueWeight, but we don't check that pref
 
             # 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}
-                  or next;
+                    or next;
 
-                $holdingbranch ||= $branch; # We set this as the first from the list of pull branches
-                # unless we set it above to the pickupbranch or the least cost branch
-                # FIXME: The itention is to follow StaticHoldsQueueWeight, but we don't check that pref
                 foreach my $item (@$holding_branch_items) {
-                    next if $pickup_branch ne $item->{homebranch};
-                    next unless _can_item_fill_request( $item, $request, $libraries );
-
-                    $itemnumber = $item->{itemnumber};
-                    $holdingbranch = $branch;
-                    last PULL_BRANCHES;
+                    if ( $pickup_branch eq $item->{homebranch}
+                        && _can_item_fill_request( $item, $request, $libraries ) )
+                    {
+                        $itemnumber    = $item->{itemnumber};
+                        $holdingbranch = $branch;
+                        last;
+                    }
                 }
+                last if $itemnumber;
             }
 
             # Now try items from the least cost branch based on the transport cost matrix or StaticHoldsQueueWeight
@@ -617,19 +618,18 @@ 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;
 
                     foreach my $item (@$holding_branch_items) {
-
-                        next unless _can_item_fill_request( $item, $request, $libraries );
-
-                        $itemnumber = $item->{itemnumber};
-                        $holdingbranch = $branch;
-                        last PULL_BRANCHES2;
+                        if( _can_item_fill_request( $item, $request, $libraries ) ){
+                            $itemnumber = $item->{itemnumber};
+                            $holdingbranch = $branch;
+                            last;
+                        }
                     }
+                    last if $itemnumber;
                 }
             }
         }
-- 
2.39.5