From cb03909af62585aa7a280cd49b2008b0e7b2cbb8 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Thu, 11 Mar 2021 16:11:15 +0100
Subject: [PATCH] Bug 27921: Log correct timestamp for HOLD MODIFY when set
 waiting

The HOLD MODIFY log at the end of ModReserveAffect is not using an
up-to-date $hold object.

$hold is modified at
1201         $hold->set_waiting($desk_id);
But not refreshed before logged (and so the timestamp is not logged
correctly).

Test plan:
Turn on HoldsLog
Place an item on hold
Check it in to mark it waiting

Confirm that the timestamp logged is the one from the check in, not when
you created the hold

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 C4/Reserves.pm            |  2 +-
 t/db_dependent/Reserves.t | 47 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index 54ccc7457d..87a425008f 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -1225,7 +1225,7 @@ sub ModReserveAffect {
     });
     $std->execute($hold->reserve_id);
 
-    logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, Dumper($hold->unblessed) )
+    logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, Dumper($hold->get_from_storage->unblessed) )
         if C4::Context->preference('HoldsLog');
 
     return;
diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t
index 7422cb9817..0f2bec8b61 100755
--- a/t/db_dependent/Reserves.t
+++ b/t/db_dependent/Reserves.t
@@ -17,7 +17,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 66;
+use Test::More tests => 67;
 use Test::MockModule;
 use Test::Warn;
 
@@ -32,6 +32,7 @@ use C4::Items;
 use C4::Biblio;
 use C4::Members;
 use C4::Reserves;
+use Koha::ActionLogs;
 use Koha::Caches;
 use Koha::DateUtils;
 use Koha::Holds;
@@ -1218,6 +1219,50 @@ subtest 'MergeHolds' => sub {
     is( $biblio_2->holds->count, 1, 'Hold has been transferred' );
 };
 
+subtest 'ModReserveAffect logging' => sub {
+
+    plan tests => 4;
+
+    my $item = $builder->build_sample_item;
+    my $patron = $builder->build_object(
+        {
+            class => "Koha::Patrons",
+            value => { branchcode => $item->homebranch }
+        }
+    );
+
+    t::lib::Mocks::mock_userenv({ patron => $patron });
+    t::lib::Mocks::mock_preference('HoldsLog', 1);
+
+    my $reserve_id = AddReserve(
+        {
+            branchcode     => $item->homebranch,
+            borrowernumber => $patron->borrowernumber,
+            biblionumber   => $item->biblionumber,
+            priority       => 1,
+            itemnumber     => $item->itemnumber,
+        }
+    );
+
+    my $hold = Koha::Holds->find($reserve_id);
+    my $previous_timestamp = '1970-01-01 12:34:56';
+    $hold->timestamp($previous_timestamp)->store;
+
+    $hold = Koha::Holds->find($reserve_id);
+    is( $hold->timestamp, $previous_timestamp, 'Make sure the previous timestamp has been used' );
+
+    # Mark it waiting
+    ModReserveAffect( $item->itemnumber, $patron->borrowernumber );
+
+    $hold = Koha::Holds->find($reserve_id);
+    is( $hold->found, 'W', 'Hold has been set waiting' );
+    isnt( $hold->timestamp, $previous_timestamp, 'The timestamp has been modified' );
+
+    my $log = Koha::ActionLogs->search({ module => 'HOLDS', action => 'MODIFY', object => $hold->reserve_id })->next;
+    my $expected = sprintf q{'timestamp' => '%s'}, $hold->timestamp;
+    like( $log->info, qr{$expected}, 'Timestamp logged is the current one' );
+};
+
 sub count_hold_print_messages {
     my $message_count = $dbh->selectall_arrayref(q{
         SELECT COUNT(*)
-- 
2.39.5