From 560efd018bcfab0e97ddbef3da18e1f85592a33f Mon Sep 17 00:00:00 2001
From: Lucas Gass <lucas@bywatersolutions.com>
Date: Thu, 7 May 2026 16:03:03 +0000
Subject: [PATCH] Bug 41879: Disallow moves to invalid pickup locations for
 items

To test:

1. Set ReservesControlBranch is set to 'item's home library'
2. Set the deafult 'Holds and bookings policies by item type' for the itype BK to allow holds from patrons from any library, but pickup at CPL only
3. Have a bib owned by another library ( I used MPL ), and bib owned by CPL. Both are the item type BK, and each has one item on the bib.
4. Place a hold for a patron on bib A for pickup at MPL
5. Move the hold to bib B
6. Note that the hold moves successfully, and is still set for pickup at MPL
7. Login and try to trigger the hold, nothing happens
8. APPLY PATCH, restart_all
9. Try spets 3 - 6 again. This time on step 6 you should see a message that the hold cannot be moved.

Signed-off-by: Eric Phetteplace <phette23@gmail.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
---
 Koha/Hold.pm                                        | 13 +++++++++++++
 .../prog/en/modules/reserve/request.tt              |  2 ++
 2 files changed, 15 insertions(+)

diff --git a/Koha/Hold.pm b/Koha/Hold.pm
index 03bc85dfbf..9a06833193 100644
--- a/Koha/Hold.pm
+++ b/Koha/Hold.pm
@@ -214,6 +214,16 @@ sub move_hold {
         $new_itemnumber   = $item->itemnumber;
         $new_biblionumber = $item->biblionumber;
 
+        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' };
+        }
+
         $canReserve = C4::Reserves::CanItemBeReserved( $patron, $item, $self->branchcode, { ignore_hold_counts => 1 } );
     } elsif ( $args->{new_biblionumber} ) {
         $new_biblionumber = $args->{new_biblionumber};
@@ -222,6 +232,9 @@ sub move_hold {
             $patron->borrowernumber, $new_biblionumber, $self->branchcode,
             { ignore_hold_counts => 1 }
         );
+
+        # for record level holds we must check to make sure there is an item on the record that can fufill the new hold
+
     } else {
         return { success => 0, error => 'Missing itemnumber or biblionumber' };
     }
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
index 8726265631..af8123ee97 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
@@ -225,6 +225,8 @@
                             <span>Target item is not for loan</span>
                         [% CASE 'damaged' %]
                             <span>Target item is damaged</span>
+                        [% CASE 'invalidPickupBranch' %]
+                            <span>Target item is an invalid pickup location</span>
                         [% CASE %]
                             <span>Error: [% fail.error | html %]</span>
                         [% END %]