From b17a04dd077bb118086ad9e4bb58eee81ade2cdd Mon Sep 17 00:00:00 2001
From: Kyle M Hall <kyle@bywatersolutions.com>
Date: Wed, 17 Jun 2020 12:53:36 -0400
Subject: [PATCH] Bug 25786: Holds Queue building may target the wrong item for
 item level requests that match holds queue priority

Bug 23934 removed the limitation that prevented item level holds from
getting local holds priority. The problem is the code has never checked
if the item level hold matches the given item! This means the wrong item
may be requested to fill an item level hold.

Test Plan:
1) Create 3 items on a record
2) Place a hold for the 2nd item you created
4) Ensure that hold would be picked up by local holds priority
5) Build the holds queue
6) Note the holds queue is asking for the wrong item!
7) Apply this patch
8) Rebuild the holds queue
9) Holds queue should now be asking for the correct item!

Signed-off-by: Kim Peine <kim@williston.lib.vt.us>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 C4/HoldsQueue.pm            |  2 ++
 t/db_dependent/HoldsQueue.t | 66 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm
index 18a9063e53..339156a5eb 100755
--- a/C4/HoldsQueue.pm
+++ b/C4/HoldsQueue.pm
@@ -409,6 +409,8 @@ sub MapItemsToHoldRequests {
                   || ( $item->{holdallowed} == 1
                     && $item->{homebranch} ne $request->{borrowerbranch} );
 
+                next if $request->{itemnumber} && $request->{itemnumber} != $item->{itemnumber};
+
                 next unless $item->{_object}->can_be_transferred( { to => $libraries->{ $request->{branchcode} } } );
 
                 my $local_holds_priority_item_branchcode =
diff --git a/t/db_dependent/HoldsQueue.t b/t/db_dependent/HoldsQueue.t
index 323742cc46..be3adfaab3 100755
--- a/t/db_dependent/HoldsQueue.t
+++ b/t/db_dependent/HoldsQueue.t
@@ -8,7 +8,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 52;
+use Test::More tests => 53;
 use Data::Dumper;
 
 use C4::Calendar;
@@ -1098,6 +1098,70 @@ subtest "Test Local Holds Priority - Item level hold over Record level hold (Bug
     );
 };
 
+subtest "Test Local Holds Priority - Get correct item for item level hold" => sub {
+    plan tests => 3;
+
+    Koha::Biblios->delete();
+    t::lib::Mocks::mock_preference( 'LocalHoldsPriority', 1 );
+    t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'PickupLibrary' );
+    t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'homebranch' );
+    my $branch  = $builder->build_object( { class => 'Koha::Libraries' } );
+    my $branch2 = $builder->build_object( { class => 'Koha::Libraries' } );
+    my $local_patron = $builder->build_object(
+        {
+            class => "Koha::Patrons",
+            value => {
+                branchcode => $branch->branchcode
+            }
+        }
+    );
+    my $other_patron = $builder->build_object(
+        {
+            class => "Koha::Patrons",
+            value => {
+                branchcode => $branch2->branchcode
+            }
+        }
+    );
+    my $biblio = $builder->build_sample_biblio();
+
+    my $item1 = $builder->build_sample_item(
+        {
+            biblionumber  => $biblio->biblionumber,
+            library    => $branch->branchcode,
+        }
+    );
+    my $item2 = $builder->build_sample_item(
+        {
+            biblionumber  => $biblio->biblionumber,
+            library    => $branch->branchcode,
+        }
+    );
+    my $item3 = $builder->build_sample_item(
+        {
+            biblionumber  => $biblio->biblionumber,
+            library    => $branch->branchcode,
+        }
+    );
+
+    my $reserve_id2 =
+        AddReserve( $item2->homebranch, $local_patron->borrowernumber,
+            $biblio->biblionumber, '', 2, undef, undef, undef, undef, $item2->id, undef, undef );
+
+    C4::HoldsQueue::CreateQueue();
+
+    my $queue_rs = $schema->resultset('TmpHoldsqueue');
+    my $q = $queue_rs->next;
+    is( $queue_rs->count(), 1,
+        "Hold queue contains one hold" );
+    is(
+        $q->borrowernumber,
+        $local_patron->borrowernumber,
+        "We should pick the local hold over the next available"
+    );
+    is( $q->itemnumber->id, $item2->id, "Got the correct item for item level local holds priority" );
+};
+
 subtest "Test Local Holds Priority - Ensure no duplicate requests in holds queue (Bug 18001)" => sub {
     plan tests => 1;
 
-- 
2.39.5