From 4576eb73da90f22b16398ab758608ced6013db1a Mon Sep 17 00:00:00 2001
From: Kyle M Hall <kyle@bywatersolutions.com>
Date: Tue, 18 Jul 2023 09:07:59 -0400
Subject: [PATCH] Bug 25159: Implement diffs in action logs for holds

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Restart all the things!
4) Enable HoldsLog
5) Perform various hold related actions
6) Observe the diff column is populated by a JSON string
   of the diff format generated by Struct::Diff

Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
---
 C4/Reserves.pm | 19 ++++++++++++-------
 Koha/Hold.pm   | 16 ++++++++++++----
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index 97374d9514..08ab5669bc 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -1129,20 +1129,20 @@ sub ModReserve {
     # FIXME Other calls may fail
     Koha::Exceptions::ObjectNotFound->throw( 'No hold with id ' . $reserve_id ) unless $hold;
 
+    my $original = C4::Context->preference('HoldsLog') ? $hold->unblessed : undef;
+
     if ( $rank eq "del" ) {
         $hold->cancel({ cancellation_reason => $cancellation_reason });
     }
     elsif ($hold->found && $hold->priority eq '0' && $date) {
-        logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, $hold )
-            if C4::Context->preference('HoldsLog');
 
         # The only column that can be updated for a found hold is the expiration date
         $hold->expirationdate($date)->store();
+
+        logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, $hold, undef, $original )
+            if C4::Context->preference('HoldsLog');
     }
     elsif ($rank =~ /^\d+/ and $rank > 0) {
-        logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, $hold )
-            if C4::Context->preference('HoldsLog');
-
         my $properties = {
             priority    => $rank,
             branchcode  => $branchcode,
@@ -1169,7 +1169,10 @@ sub ModReserve {
             }
         }
 
-        _FixPriority({ reserve_id => $reserve_id, rank =>$rank });
+        _FixPriority( { reserve_id => $reserve_id, rank => $rank } );
+
+        logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, $hold, undef, $original )
+            if C4::Context->preference('HoldsLog');
     }
 }
 
@@ -1243,6 +1246,8 @@ sub ModReserveAffect {
 
     return unless $hold;
 
+    my $original = $hold->unblessed;
+
     my $already_on_shelf = $hold->found && $hold->found eq 'W';
 
     $hold->itemnumber($itemnumber);
@@ -1283,7 +1288,7 @@ sub ModReserveAffect {
     });
     $std->execute($hold->reserve_id);
 
-    logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, $hold )
+    logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, $hold, undef, $original )
         if C4::Context->preference('HoldsLog');
 
     return;
diff --git a/Koha/Hold.pm b/Koha/Hold.pm
index 4017e56417..12109ad573 100644
--- a/Koha/Hold.pm
+++ b/Koha/Hold.pm
@@ -92,6 +92,8 @@ my $hold = $hold->suspend_hold( $suspend_until );
 sub suspend_hold {
     my ( $self, $date ) = @_;
 
+    my $original = C4::Context->preference('HoldsLog') ? $self->unblessed : undef;
+
     $date &&= dt_from_string($date)->truncate( to => 'day' )->datetime;
 
     if ( $self->is_found ) {    # We can't suspend found holds
@@ -126,7 +128,7 @@ sub suspend_hold {
         }
     );
 
-    logaction( 'HOLDS', 'SUSPEND', $self->reserve_id, $self )
+    logaction( 'HOLDS', 'SUSPEND', $self->reserve_id, $self, undef, $original )
         if C4::Context->preference('HoldsLog');
 
     Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue(
@@ -147,6 +149,8 @@ my $hold = $hold->resume();
 sub resume {
     my ( $self ) = @_;
 
+    my $original = C4::Context->preference('HoldsLog') ? $self->unblessed : undef;
+
     $self->suspend(0);
     $self->suspend_until( undef );
 
@@ -160,7 +164,7 @@ sub resume {
         }
     );
 
-    logaction( 'HOLDS', 'RESUME', $self->reserve_id, $self )
+    logaction( 'HOLDS', 'RESUME', $self->reserve_id, $self, undef, $original )
         if C4::Context->preference('HoldsLog');
 
     Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue(
@@ -709,6 +713,8 @@ sub cancel {
 
     my $autofill_next = $params->{autofill} && $self->itemnumber && $self->found && $self->found eq 'W';
 
+    my $original = C4::Context->preference('HoldsLog') ? $self->unblessed : undef;
+
     $self->_result->result_source->schema->txn_do(
         sub {
             my $patron = $self->patron;
@@ -796,7 +802,7 @@ sub cancel {
                 );
             }
 
-            C4::Log::logaction( 'HOLDS', 'CANCEL', $self->reserve_id, $self )
+            C4::Log::logaction( 'HOLDS', 'CANCEL', $self->reserve_id, $self, undef, $original )
                 if C4::Context->preference('HoldsLog');
 
             Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue(
@@ -836,6 +842,8 @@ sub fill {
         sub {
             my $patron = $self->patron;
 
+            my $original = C4::Context->preference('HoldsLog') ? $self->unblessed : undef;
+
             $self->set(
                 {
                     found    => 'F',
@@ -880,7 +888,7 @@ sub fill {
                 }
             }
 
-            C4::Log::logaction( 'HOLDS', 'FILL', $self->id, $self )
+            C4::Log::logaction( 'HOLDS', 'FILL', $self->id, $self, undef, $original )
                 if C4::Context->preference('HoldsLog');
 
             Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue(
-- 
2.39.5