Bug 25786: Holds Queue building may target the wrong item for item level requests...

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>

diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm
index 18a9063..339156a 100755 (executable)
--- 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 323742c..be3adfa 100755 (executable)
--- 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;