Bug 41878: Add Hold logging when grouping/ungrouping

Test plan:
-Enable DisplayAddHoldGroups and HoldsLog sys prefs
-Place 2 or more holds for a patron (regular, not grouped)
-Find the reserve_id for one of the holds, and view it in Tools > Logs. There will be an entry for the hold's creation.
-From the patron's account, select the holds and click 'Group selected'
-Run the log for the reserve_id, note that there is an entry generated by putting the hold into a group
-Suspend the hold, then view the log for that reserve_id. Note that the log entry for the suspension includes the hold_group_id.
-From the patron's hold tab, click on the number in the 'Hold group' column, then select 'Ungroup hold'
-Run the log for the reserve_id, and note that there is a new entry for ungrouping the hold

Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Koha/HoldGroup.pm

@@ -19,6 +19,9 @@ package Koha::HoldGroup;
 
 use Modern::Perl;
 
+use C4::Context;
+use C4::Log qw( logaction );
+
 use base qw(Koha::Object);
 
 =head1 NAME
@@ -31,6 +34,89 @@ Koha::HoldGroup - Koha Hold Group object class
 
 =cut
 
+=head3 store
+
+    store override
+
+Logs changes to related holds
+
+=cut
+
+sub store {
+    my ( $self, $params ) = @_;
+
+    my $is_new           = !$self->in_storage;
+    my $associated_holds = $params->{holds} || [];
+
+    my %prior_states;
+    foreach my $hold (@$associated_holds) {
+        my $unblessed = $hold->unblessed;
+        $unblessed->{hold_group_id} = undef if $is_new;
+        $prior_states{ $hold->id } = $unblessed;
+    }
+
+    my $result = $self->SUPER::store();
+
+    foreach my $hold (@$associated_holds) {
+        $hold->hold_group_id( $self->hold_group_id )->store;
+    }
+
+    if ( $result && C4::Context->preference('HoldsLog') ) {
+        foreach my $hold (@$associated_holds) {
+            eval { $hold->discard_changes; };
+            next if $@;
+
+            logaction(
+                'HOLDS',
+                'MODIFY',
+                $hold->reserve_id,
+                $hold,
+                undef,
+                $prior_states{ $hold->id }
+            );
+        }
+    }
+
+    return $result;
+}
+
+=head3 delete
+
+    delete override
+
+Log changes to related holds
+
+=cut
+
+sub delete {
+    my ($self) = @_;
+
+    my $hold_group_id    = $self->hold_group_id;
+    my $borrowernumber   = $self->borrowernumber;
+    my @associated_holds = $self->holds->as_list;
+
+    my $result = $self->SUPER::delete();
+
+    if ( $result && C4::Context->preference('HoldsLog') ) {
+        foreach my $hold (@associated_holds) {
+            my $hold_prior_to_delete = $hold->unblessed;
+            eval { $hold->discard_changes; };
+            next if $@;
+
+            logaction(
+                'HOLDS',
+                'MODIFY',
+                $hold->reserve_id,
+                $hold,
+                undef,
+                $hold_prior_to_delete
+            );
+        }
+    }
+
+    return $result;
+}
+
 =head3 to_api
 
     my $json = $hold_group->to_api;

Koha/Patron.pm

@@ -1951,21 +1951,33 @@ sub create_hold_group {
         $next_available_visual_hold_group_id++;
     }
 
-    my $hold_group_rs = $self->_result->create_related(
-        'hold_groups',
-        { visual_hold_group_id => $next_available_visual_hold_group_id }
-    );
-    foreach my $hold_id (@$hold_ids) {
-        my $hold                   = Koha::Holds->find($hold_id);
-        my $previous_hold_group_id = $hold->hold_group_id;
+    my @holds_to_group = Koha::Holds->search( { reserve_id => { -in => $hold_ids } } )->as_list;
 
-        $hold->hold_group_id( $hold_group_rs->hold_group_id )->store;
-        if ( $previous_hold_group_id && $previous_hold_group_id != $hold_group_rs->hold_group_id ) {
-            $hold->cleanup_hold_group($previous_hold_group_id);
+    my %cleanup_map;
+    foreach my $hold (@holds_to_group) {
+        if ( $hold->hold_group_id ) {
+            $cleanup_map{ $hold->id } = $hold->hold_group_id;
         }
     }
 
-    return Koha::HoldGroup->_new_from_dbic($hold_group_rs);
+    my $hold_group = Koha::HoldGroup->new(
+        {
+            borrowernumber       => $self->borrowernumber,
+            visual_hold_group_id => $next_available_visual_hold_group_id
+        }
+    );
+    $hold_group->store( { holds => \@holds_to_group } );
+
+    foreach my $hold_id ( keys %cleanup_map ) {
+        my $old_group_id = $cleanup_map{$hold_id};
+
+        if ( $old_group_id != $hold_group->hold_group_id ) {
+            my $hold = Koha::Holds->find($hold_id);
+            $hold->cleanup_hold_group($old_group_id);
+        }
+    }
+
+    return $hold_group;
 }
 
 =head3 curbside_pickups