From c9e5e6c5cbcfbe3f48f6b7ccb2d370ee980eef43 Mon Sep 17 00:00:00 2001
From: Lucas Gass <lucas@bywatersolutions.com>
Date: Fri, 17 Jul 2026 20:11:04 +0000
Subject: [PATCH] Bug 41879: (follow-up) Add helper function to avoid repeating
 code

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
---
 Koha/Hold.pm | 41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/Koha/Hold.pm b/Koha/Hold.pm
index cad3cfb385..e8a96460dd 100644
--- a/Koha/Hold.pm
+++ b/Koha/Hold.pm
@@ -215,15 +215,7 @@ sub move_hold {
         $new_biblionumber = $item->biblionumber;
 
         # need to make sure the item is being moved to a valid pickup location
-        my $branchitemrule = C4::Circulation::GetBranchItemRule(
-            Koha::Policy::Holds->holds_control_library( $item, $patron ),
-            $item->effective_itemtype
-        );
-
-        my $policy = $branchitemrule->{hold_fulfillment_policy};
-        if ( ( $policy eq 'homebranch' || $policy eq 'holdingbranch' ) && $self->branchcode ne $item->$policy ) {
-            return { success => 0, error => 'invalidPickupBranch' };
-        }
+        return { success => 0, error => 'invalidPickupBranch' } unless $self->_pickup_valid_for_item( $item, $patron );
 
         $canReserve = C4::Reserves::CanItemBeReserved( $patron, $item, $self->branchcode, { ignore_hold_counts => 1 } );
     } elsif ( $args->{new_biblionumber} ) {
@@ -233,16 +225,10 @@ sub move_hold {
         my @items             = Koha::Items->search( { biblionumber => $new_biblionumber } )->as_list;
         my $valid_pickup_item = 0;
         foreach my $candidate_item (@items) {
-            my $branchitemrule = C4::Circulation::GetBranchItemRule(
-                Koha::Policy::Holds->holds_control_library( $candidate_item, $patron ),
-                $candidate_item->effective_itemtype
-            );
-            my $policy = $branchitemrule->{hold_fulfillment_policy};
-            next
-                if ( $policy eq 'homebranch' || $policy eq 'holdingbranch' )
-                && $self->branchcode ne $candidate_item->$policy;
-            $valid_pickup_item = 1;
-            last;
+            if ( $self->_pickup_valid_for_item( $candidate_item, $patron ) ) {
+                $valid_pickup_item = 1;
+                last;
+            }
         }
         return { success => 0, error => 'invalidPickupBranch' } unless $valid_pickup_item;
 
@@ -1726,6 +1712,23 @@ sub strings_map {
     return $strings;
 }
 
+sub _pickup_valid_for_item {
+    my ( $self, $item, $patron ) = @_;
+
+    my $branchitemrule = C4::Circulation::GetBranchItemRule(
+        Koha::Policy::Holds->holds_control_library( $item, $patron ),
+        $item->effective_itemtype
+    );
+
+    my $policy = $branchitemrule->{hold_fulfillment_policy};
+
+    return 0
+        if ( $policy eq 'homebranch' || $policy eq 'holdingbranch' )
+        && $self->branchcode ne $item->$policy;
+
+    return 1;
+}
+
 =head2 Internal methods
 
 =head3 _type