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).

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>

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);
 

C4/Reserves.pm

@@ -42,6 +42,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;
@@ -1373,6 +1374,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 )
@@ -1386,14 +1388,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;
@@ -1882,6 +1877,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,
@@ -1898,7 +1896,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)
@@ -1907,6 +1906,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);
@@ -1933,12 +1933,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);

Koha/Hold.pm

@@ -271,6 +271,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;

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

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 )],