From c067fee7a8959e6c257d390cf0e2c913c46b8cfd Mon Sep 17 00:00:00 2001 From: Pedro Amorim Date: Wed, 6 Aug 2025 14:35:12 +0000 Subject: [PATCH] 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 --- Koha/HoldGroup.pm | 4 ++-- t/db_dependent/Koha/Hold.t | 35 ++++++++++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/Koha/HoldGroup.pm b/Koha/HoldGroup.pm index 581536ea322..4f87aec6dd7 100644 --- a/Koha/HoldGroup.pm +++ b/Koha/HoldGroup.pm @@ -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}; diff --git a/t/db_dependent/Koha/Hold.t b/t/db_dependent/Koha/Hold.t index 646fbcef555..cb48d1331bf 100755 --- a/t/db_dependent/Koha/Hold.t +++ b/t/db_dependent/Koha/Hold.t @@ -1389,7 +1389,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; @@ -1461,6 +1461,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" ); @@ -1481,6 +1490,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( -- 2.39.5