Bugzilla – Attachment 185138 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: Behavior change: Main patch
Bug-40529-Behavior-change-Main-patch.patch (text/plain), 9.94 KB, created by
Pedro Amorim
on 2025-08-06 09:14:59 UTC
(
hide
)
Description:
Bug 40529: Behavior change: Main patch
Filename:
MIME Type:
Creator:
Pedro Amorim
Created:
2025-08-06 09:14:59 UTC
Size:
9.94 KB
patch
obsolete
>From 7f7c501caa0af8ca8193563dd438f2945d1addfe Mon Sep 17 00:00:00 2001 >From: Pedro Amorim <pedro.amorim@openfifth.co.uk> >Date: Thu, 17 Jul 2025 15:24:10 +0000 >Subject: [PATCH] Bug 40529: Behavior change: Main patch > >When a hold that belongs to a hold group becomes 'In transit' or 'Waiting', >it now becomes that hold group's target hold. >Subsequent check-ins that would satisfy other holds in the same hold >group are skipped, as they are part of a group that already has a >target. > >The behavior change introduced here that differs from the groundwork >from bug 15516 are: >1) Before, only 'Waiting' holds would consider an action based on hold > groups i.e. cancel the other holds in the group. Now 'In transit' > also does this. >2) This hold groups related action used to be to cancel other holds in > the group. This is no longer the case. Now, the respective hold is > set as the hold group's target. This causes the holds in the same group that > are not the target to be skipped when a check-in that would otherwise > satisfy one of those holds happens. This behavior change allows the following > scenario which wasn't possible before: > > A hold is marked 'Waiting'. It becomes its group target. For any reason, > that hold is then cancelled. Any subsequent check-in of an item of a > hold in this group may now becomes that group's new target. > Similarly, the same use case above is also true for 'In transit' holds. > >---- > > The 'skip non-target holds' behavior has been added to check-in logic >(Reserves::_Findgroupreserve), 'holds to pull' logic >(pendingreserves.pl) and holds queue (HoldsQueue.pm). >--- > C4/HoldsQueue.pm | 5 +++++ > C4/Reserves.pm | 22 ++++++++++++---------- > Koha/Hold.pm | 19 +++++++++++++++++++ > Koha/HoldGroup.pm | 35 +++++++++++++++++++++++++++++++++++ > circ/pendingreserves.pl | 10 +++++++++- > 5 files changed, 80 insertions(+), 11 deletions(-) > >diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm >index c0ffd43ac2f..a9735b845f0 100644 >--- a/C4/HoldsQueue.pm >+++ b/C4/HoldsQueue.pm >@@ -26,6 +26,7 @@ use C4::Context; > use C4::Circulation qw( GetBranchItemRule ); > use Koha::DateUtils qw( dt_from_string ); > use Koha::Hold::HoldsQueueItems; >+use Koha::HoldGroup; > use Koha::Items; > use Koha::Libraries; > use Koha::Logger; >@@ -250,6 +251,9 @@ that have one or more unfilled hold requests. > sub GetBibsWithPendingHoldRequests { > my $dbh = C4::Context->dbh; > >+ my $skip_non_target_holds_query = Koha::HoldGroup::skip_non_target_holds_query('sql'); >+ my $skip_non_target_holds_query_sql = $skip_non_target_holds_query ? " $skip_non_target_holds_query" : ''; >+ > my $bib_query = "SELECT DISTINCT biblionumber > FROM reserves > WHERE found IS NULL >@@ -257,6 +261,7 @@ sub GetBibsWithPendingHoldRequests { > AND reservedate <= CURRENT_DATE() > AND suspend = 0 > AND reserve_id NOT IN (SELECT reserve_id FROM hold_fill_targets) >+ $skip_non_target_holds_query_sql > "; > my $sth = $dbh->prepare($bib_query); > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 52ac6c6cea0..aca7116ad30 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -40,6 +40,7 @@ use Koha::CirculationRules; > use Koha::Database; > use Koha::DateUtils qw( dt_from_string output_pref ); > use Koha::Holds; >+use Koha::HoldGroup; > use Koha::ItemTypes; > use Koha::Items; > use Koha::Libraries; >@@ -1244,6 +1245,7 @@ sub ModReserveAffect { > > if ($transferToDo) { > $hold->set_transfer(); >+ $hold->set_as_hold_group_target(); > } elsif ( C4::Context->preference('HoldsNeedProcessingSIP') > && C4::Context->interface eq 'sip' > && !$already_on_shelf ) >@@ -1257,14 +1259,7 @@ sub ModReserveAffect { > my $transfer = $hold->item->get_transfer; > $transfer->receive if $transfer; > >- # if this hold was part of a group, cancel other holds in the group >- if ( $hold->hold_group_id ) { >- my @holds = $hold->hold_group->holds->as_list; >- foreach my $h (@holds) { >- push @reserve_ids, $h->reserve_id; >- $h->cancel unless $h->reserve_id == $hold->reserve_id; >- } >- } >+ $hold->set_as_hold_group_target(); > } > > _koha_notify_hold_changed($hold) if $notify_library; >@@ -1753,6 +1748,9 @@ sub _Findgroupreserve { > my ( $biblionumber, $itemnumber, $lookahead, $ignore_borrowers ) = @_; > my $dbh = C4::Context->dbh; > >+ my $skip_non_target_holds_query = Koha::HoldGroup::skip_non_target_holds_query('sql'); >+ my $skip_non_target_holds_query_sql = $skip_non_target_holds_query ? " $skip_non_target_holds_query" : ''; >+ > # check for targeted match form the holds queue > my $hold_target_query = qq{ > SELECT reserves.biblionumber AS biblionumber, >@@ -1769,7 +1767,8 @@ sub _Findgroupreserve { > reserves.reserve_id AS reserve_id, > reserves.itemtype AS itemtype, > reserves.non_priority AS non_priority, >- reserves.item_group_id AS item_group_id >+ reserves.item_group_id AS item_group_id, >+ reserves.hold_group_id AS hold_group_id > FROM reserves > JOIN biblioitems USING (biblionumber) > JOIN hold_fill_targets USING (reserve_id) >@@ -1778,6 +1777,7 @@ sub _Findgroupreserve { > AND hold_fill_targets.itemnumber = ? > AND reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY) > AND suspend = 0 >+ $skip_non_target_holds_query_sql > ORDER BY priority > }; > my $sth = $dbh->prepare($hold_target_query); >@@ -1804,12 +1804,14 @@ sub _Findgroupreserve { > reserves.reserve_id AS reserve_id, > reserves.itemtype AS itemtype, > reserves.non_priority AS non_priority, >- reserves.item_group_id AS item_group_id >+ reserves.item_group_id AS item_group_id, >+ reserves.hold_group_id AS hold_group_id > FROM reserves > WHERE reserves.biblionumber = ? > AND (reserves.itemnumber IS NULL OR reserves.itemnumber = ?) > AND reserves.reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY) > AND suspend = 0 >+ $skip_non_target_holds_query_sql > ORDER BY priority > }; > $sth = $dbh->prepare($query); >diff --git a/Koha/Hold.pm b/Koha/Hold.pm >index 465b9cbd5b7..a4fadda4f4d 100644 >--- a/Koha/Hold.pm >+++ b/Koha/Hold.pm >@@ -200,6 +200,25 @@ sub cleanup_hold_group { > } > } > >+=head3 set_as_hold_group_target >+ >+$self->set_as_hold_group_target; >+ >+Sets this hold and its' hold group's target >+ >+=cut >+ >+sub set_as_hold_group_target { >+ my ($self) = @_; >+ >+ if ( $self->hold_group && !$self->hold_group->target_hold_id && C4::Context->preference("DisplayAddHoldGroups") ) { >+ $self->_result->create_related( >+ 'hold_group_target_hold', >+ { hold_group_id => $self->hold_group->hold_group_id } >+ ); >+ } >+} >+ > =head3 is_hold_group_target > > $self->is_hold_group_target; >diff --git a/Koha/HoldGroup.pm b/Koha/HoldGroup.pm >index 028ae70516d..581536ea322 100644 >--- a/Koha/HoldGroup.pm >+++ b/Koha/HoldGroup.pm >@@ -94,6 +94,41 @@ sub target_hold_id { > return $self->_result->hold_group_target_hold->reserve_id; > } > >+=head3 skip_non_target_holds_query >+ >+ $where{'me.hold_group_id'} = Koha::HoldGroup::skip_non_target_holds_query('dbix') >+ >+Static method, returns the query part be added if DisplayAddHoldGroups >+Accepts a $query_type of 'dbix' or 'sql' and returns the respective query >+The query is designed to skip non target holds that are part of a hold group which already has a hold target >+ >+=cut >+ >+sub skip_non_target_holds_query { >+ my ($query_type) = @_; >+ >+ return unless C4::Context->preference('DisplayAddHoldGroups'); >+ >+ my $query_types = { >+ 'dbix' => [ >+ -or => { '=' => undef }, >+ [ >+ -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' >+ } >+ ] >+ ], >+ '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))' >+ }; >+ >+ return unless $query_type && exists $query_types->{$query_type}; >+ >+ return $query_types->{$query_type}; >+} >+ > =head3 _type > > =cut >diff --git a/circ/pendingreserves.pl b/circ/pendingreserves.pl >index 5ce792a1a91..046ebea87bb 100755 >--- a/circ/pendingreserves.pl >+++ b/circ/pendingreserves.pl >@@ -31,6 +31,7 @@ use C4::Items; > use Koha::Biblios; > use Koha::DateUtils qw( dt_from_string ); > use Koha::Holds; >+use Koha::HoldGroup; > use DateTime::Duration; > > my $input = CGI->new; >@@ -182,6 +183,9 @@ if ( C4::Context->only_my_library() ) { > $where{'me.branchcode'} = C4::Context->userenv->{'branch'}; > } > >+my $skip_non_target_holds_query = Koha::HoldGroup::skip_non_target_holds_query('dbix'); >+$where{'me.hold_group_id'} = $skip_non_target_holds_query if $skip_non_target_holds_query; >+ > # get all distinct unfulfilled reserves > my $holds = Koha::Holds->search( > {%where}, >@@ -200,7 +204,11 @@ if ( $holds->count ) { > # patrons count per biblio > my $patrons_count = { > map { $_->{biblionumber} => $_->{patrons_count} } @{ Koha::Holds->search( >- { 'suspend' => 0, 'found' => undef }, >+ { >+ 'suspend' => 0, >+ 'found' => undef, >+ $skip_non_target_holds_query ? ( 'hold_group_id' => $skip_non_target_holds_query ) : () >+ }, > { > select => [ 'biblionumber', { count => { distinct => 'borrowernumber' } } ], > as => [qw( biblionumber patrons_count )], >-- >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