Bug 40529: (QA follow-up): Include target holds

The skipping of non target holds was skipping all holds that are part of a hold group which has a target, including the target hold itself. This was wrong. Target holds need to be included as they are considered to be a regular 'found' hold.

Tests that prove the bug and its fix added to:
prove ./t/db_dependent/Koha/Hold.t

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Pedro Amorim 2025-08-06 14:35:12 +00:00 committed by Lucas Gass
parent c9c8ce74b5
commit fc52ac9a45
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
2 changed files with 36 additions and 3 deletions

View File

@ -116,12 +116,12 @@ sub skip_non_target_holds_query {
-and => { '!=' => undef },
{
-not_in => \
'SELECT hold_group_id FROM hold_group_target_holds WHERE reserve_id IS NOT NULL AND hold_group_id IS NOT NULL'
'SELECT hold_group_id FROM hold_group_target_holds WHERE reserve_id IS NOT NULL AND reserve_id != me.reserve_id'
}
]
],
'sql' =>
'AND (hold_group_id IS NULL OR hold_group_id IS NOT NULL AND reserves.hold_group_id NOT IN (SELECT hold_group_id FROM hold_group_target_holds WHERE reserve_id IS NOT NULL AND hold_group_id IS NOT NULL))'
'AND (hold_group_id IS NULL OR reserves.hold_group_id NOT IN (SELECT hold_group_id FROM hold_group_target_holds WHERE reserve_id IS NOT NULL AND reserve_id != reserves.reserve_id))'
};
return unless $query_type && exists $query_types->{$query_type};

View File

@ -1977,7 +1977,7 @@ subtest 'is_hold_group_target, cleanup_hold_group and set_as_hold_group_target t
};
subtest '_Findgroupreserve in the context of hold groups' => sub {
plan tests => 13;
plan tests => 17;
$schema->storage->txn_begin;
@ -2049,6 +2049,15 @@ subtest '_Findgroupreserve in the context of hold groups' => sub {
}
);
my $reserve_4_id = AddReserve(
{
branchcode => $library_2->{branchcode},
borrowernumber => $patron_2->borrowernumber,
biblionumber => $item->biblionumber,
priority => 2,
}
);
my @reserves = C4::Reserves::_Findgroupreserve( $item_2->biblionumber, $item_2->id, 0, [] );
is( scalar @reserves, 2, "No hold group created yet. We should get both holds" );
is( $reserves[0]->{reserve_id}, $reserve_2_id, "We got the expected reserve" );
@ -2069,6 +2078,30 @@ subtest '_Findgroupreserve in the context of hold groups' => sub {
is( $hold->is_hold_group_target, 1, 'First hold is the hold group target' );
my @reserves_with_target_hold = C4::Reserves::_Findgroupreserve( $item->biblionumber, $item->id, 0, [] );
is(
$reserves_with_target_hold[0]->{hold_group_id}, $new_hold_group->hold_group_id,
'First eligible hold is part of a hold group'
);
my $target_hold = Koha::Holds->find( $reserves_with_target_hold[0]->{reserve_id} );
is(
$target_hold->is_hold_group_target, 1,
'First eligible hold is the hold group target'
);
is(
$reserves_with_target_hold[1]->{hold_group_id}, undef,
'Second eligible hold is not part of a hold group'
);
is(
$reserves_with_target_hold[1]->{reserve_id}, $reserve_4_id,
'Second eligible hold is reserve_4'
);
my @new_reserves = C4::Reserves::_Findgroupreserve( $item_2->biblionumber, $item_2->id, 0, [] );
is( scalar @new_reserves, 1, "reserve_2_id is now part of a group that has a target. Should not be here." );
is(