Bugzilla – Attachment 185214 Details for
Bug 40529
Update how hold groups work
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40529: (QA follow-up): Include target holds
Bug-40529-QA-follow-up-Include-target-holds.patch (text/plain), 4.02 KB, created by
Pedro Amorim
on 2025-08-07 10:44:47 UTC
(
hide
)
Description:
Bug 40529: (QA follow-up): Include target holds
Filename:
MIME Type:
Creator:
Pedro Amorim
Created:
2025-08-07 10:44:47 UTC
Size:
4.02 KB
patch
obsolete
>From c067fee7a8959e6c257d390cf0e2c913c46b8cfd Mon Sep 17 00:00:00 2001 >From: Pedro Amorim <pedro.amorim@openfifth.co.uk> >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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 40529
:
184868
|
184869
|
184870
|
184871
|
184872
|
185137
|
185138
|
185139
|
185140
|
185141
|
185210
|
185211
|
185212
|
185213
|
185214
|
185215
|
185303
|
185304
|
185305
|
185306
|
185307