From b64f72bf7b091445b1ad1ad1176d4f037c5e6631 Mon Sep 17 00:00:00 2001 From: Pedro Amorim 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