From d3657c3b1c81d97fff6aa0090e5a915dfd66b0c1 Mon Sep 17 00:00:00 2001
From: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Date: Wed, 21 Jun 2023 14:36:08 +0300
Subject: [PATCH] Bug 34032: Set new expirationdate if waiting status is
 reverted

When one reverts holds waiting status holds expiration
date is not set even if DefaultHoldExpirationdate
syspref is enabled. This patch adds new param
hold_reverted to be used when RevertWaitingStatus is
used to determine if expiration date should be set again.

To test:
1) Make sure you have DefaultHoldExpirationdate syspref enabled.
2) Find hold with status "Waiting".
3) Revert waiting status.
=> Note that hold has no expiration date set.
4) Apply this patch.
5) Repeat steps 2 and 3.
=> Expiration date should now be set based on reserve
date.

Also prove t/db_dependent/Hold.t.

Signed-off-by: Sam Lau <samalau@gmail.com>
---
 C4/Reserves.pm        |  2 +-
 Koha/Hold.pm          | 14 +++++++++-----
 t/db_dependent/Hold.t | 21 ++++++++++++++++++++-
 3 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index c95a3d7ad42..f972109e86b 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -2167,7 +2167,7 @@ sub RevertWaitingStatus {
             expirationdate => $hold->patron_expiration_date,
             itemnumber  => $hold->item_level_hold ? $hold->itemnumber : undef,
         }
-    )->store();
+    )->store({ hold_reverted => 1 });
 
     logaction( 'HOLDS', 'MODIFY', $hold->id, $hold )
         if C4::Context->preference('HoldsLog');
diff --git a/Koha/Hold.pm b/Koha/Hold.pm
index 706732c15bb..382f40a6ddc 100644
--- a/Koha/Hold.pm
+++ b/Koha/Hold.pm
@@ -929,7 +929,9 @@ expirationdate for holds.
 =cut
 
 sub store {
-    my ($self) = @_;
+    my ($self, $params) = @_;
+
+    my $hold_reverted = $params->{hold_reverted} // 0;
 
     Koha::Exceptions::Hold::MissingPickupLocation->throw() unless $self->branchcode;
 
@@ -950,11 +952,13 @@ sub store {
 
         my %updated_columns = $self->_result->get_dirty_columns;
         return $self->SUPER::store unless %updated_columns;
-
-        if ( exists $updated_columns{reservedate} ) {
+        if ( exists $updated_columns{reservedate} || $hold_reverted ) {
             if (
-                C4::Context->preference('DefaultHoldExpirationdate')
-                && ! exists $updated_columns{expirationdate}
+                ( C4::Context->preference('DefaultHoldExpirationdate')
+                && ! exists $updated_columns{expirationdate} )
+                || ( C4::Context->preference('DefaultHoldExpirationdate')
+                && exists $updated_columns{expirationdate}
+                && $hold_reverted )
               )
             {
                 $self->_set_default_expirationdate;
diff --git a/t/db_dependent/Hold.t b/t/db_dependent/Hold.t
index 73007c18a08..ce8488c8447 100755
--- a/t/db_dependent/Hold.t
+++ b/t/db_dependent/Hold.t
@@ -146,7 +146,7 @@ $schema->storage->txn_rollback();
 
 subtest "store() tests" => sub {
 
-    plan tests => 5;
+    plan tests => 6;
 
     $schema->storage->txn_begin();
 
@@ -222,6 +222,25 @@ subtest "store() tests" => sub {
     is( $hold->expirationdate,
         $passed_date->ymd, 'Passed expiration date when updating hold correctly set (Passing both reservedate and expirationdate.' );
 
+    $passed_date = dt_from_string('2023-06-20');
+    $hold->set(
+        {
+            reservedate    => $passed_date->ymd,
+            waitingdate    => $passed_date->ymd,
+        }
+    )->store();
+    $hold->discard_changes;
+
+    $hold->set_waiting;
+    C4::Reserves::RevertWaitingStatus(
+        { itemnumber => $item->itemnumber }
+    );
+    $hold->discard_changes;
+
+    $expected_date = dt_from_string( $hold->reservedate )->add( years => 2 )->ymd;
+    is( $hold->expirationdate,
+        $expected_date, 'Expiration date set after reverting holds waiting status.' );
+
     $schema->storage->txn_rollback();
 };
 
-- 
2.30.2