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>
This commit is contained in:
Kyle M Hall 2020-06-17 12:53:36 -04:00 committed by Jonathan Druart
parent 679dcd1574
commit b17a04dd07
2 changed files with 67 additions and 1 deletions

View File

@ -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 =

View File

@ -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;