Bug 21729: Keep expiration date set when placing a hold

The expiration date picked by the patron (or librarian) when placing a
hold is lost when a waiting hold is reverted.

We need a separate DB field to store this value and restore it when
needed: patron_expiration_date

The new behaviours are now:
Create a hold and specify an expiration date:
  expirationdate=patron_expiration_date

Fill the hold:
  expiration_date is calculated
  expiration_date set to the calculated value or to
patron_expiration_date if anterior
  patron_expiration_date not modified

Revert the waiting status:
  expirationdate set back to patron_expiration_date

Cancel expire reserves:
  if < expirationdate OR < patron_expiration_date
Note: This change should not be needed but won't hurt

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Florian Bontemps <florian.bontemps@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

C4/Reserves.pm

@@ -185,7 +185,7 @@ sub AddReserve {
     my $biblionumber   = $params->{biblionumber};
     my $priority       = $params->{priority};
     my $resdate        = $params->{reservation_date};
-    my $expdate        = $params->{expiration_date};
+    my $patron_expiration_date = $params->{expiration_date};
     my $notes          = $params->{notes};
     my $title          = $params->{title};
     my $checkitem      = $params->{itemnumber};
@@ -196,7 +196,7 @@ sub AddReserve {
     $resdate = output_pref( { str => dt_from_string( $resdate ), dateonly => 1, dateformat => 'iso' })
         or output_pref({ dt => dt_from_string, dateonly => 1, dateformat => 'iso' });
 
-    $expdate = output_pref({ str => $expdate, dateonly => 1, dateformat => 'iso' });
+    $patron_expiration_date = output_pref({ str => $patron_expiration_date, dateonly => 1, dateformat => 'iso' });
 
     # if we have an item selectionned, and the pickup branch is the same as the holdingbranch
     # of the document, we force the value $priority and $found .
@@ -252,7 +252,7 @@ sub AddReserve {
             itemnumber     => $checkitem,
             found          => $found,
             waitingdate    => $waitingdate,
-            expirationdate => $expdate,
+            patron_expiration_date => $patron_expiration_date,
             itemtype       => $itemtype,
             item_level_hold => $checkitem ? 1 : 0,
             non_priority   => $non_priority ? 1 : 0,
@@ -946,7 +946,13 @@ sub CancelExpiredReserves {
     my $expireWaiting = C4::Context->preference('ExpireReservesMaxPickUpDelay');
 
     my $dtf = Koha::Database->new->schema->storage->datetime_parser;
-    my $params = { expirationdate => { '<', $dtf->format_date($today) } };
+    my $params = {
+        -or => [
+            { expirationdate => { '<', $dtf->format_date($today) } },
+            { patron_expiration_date => { '<' => $dtf->format_date($today) } }
+        ]
+    };
+
     $params->{found} = [ { '!=', 'W' }, undef ]  unless $expireWaiting;
 
     # FIXME To move to Koha::Holds->search_expired (?)
@@ -2101,6 +2107,7 @@ sub RevertWaitingStatus {
             priority    => 1,
             found       => undef,
             waitingdate => undef,
+            expirationdate => $hold->patron_expiration_date,
             itemnumber  => $hold->item_level_hold ? $hold->itemnumber : undef,
         }
     )->store();

Koha/Hold.pm

@@ -185,16 +185,10 @@ sub set_waiting {
         desk_id => $desk_id,
     };
 
-    my $requested_expiration;
-    if ($self->expirationdate) {
-        $requested_expiration = dt_from_string($self->expirationdate);
-    }
-
     my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay");
     my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays');
 
-    my $expirationdate = $today->clone;
-    $expirationdate->add(days => $max_pickup_delay);
+    my $new_expiration_date = $today->clone->add(days => $max_pickup_delay);
 
     if ( C4::Context->preference("ExcludeHolidaysFromMaxPickUpDelay") ) {
         my $itemtype = $self->item ? $self->item->effective_itemtype : $self->biblio->itemtype;
@@ -207,13 +201,24 @@ sub set_waiting {
         );
         my $calendar = Koha::Calendar->new( branchcode => $self->branchcode, days_mode => $daysmode );
 
-        $expirationdate = $calendar->days_forward( dt_from_string(), $max_pickup_delay );
+        $new_expiration_date = $calendar->days_forward( dt_from_string(), $max_pickup_delay );
     }
 
     # If patron's requested expiration date is prior to the
     # calculated one, we keep the patron's one.
-    my $cmp = $requested_expiration ? DateTime->compare($requested_expiration, $expirationdate) : 0;
-    $values->{expirationdate} = $cmp == -1 ? $requested_expiration->ymd : $expirationdate->ymd;
+    if ( $self->patron_expiration_date ) {
+        my $requested_expiration = dt_from_string( $self->patron_expiration_date );
+
+        my $cmp =
+          $requested_expiration
+          ? DateTime->compare( $requested_expiration, $new_expiration_date )
+          : 0;
+
+        $new_expiration_date =
+          $cmp == -1 ? $requested_expiration : $new_expiration_date;
+    }
+
+    $values->{expirationdate} = $new_expiration_date->ymd;
 
     $self->set($values)->store();
 
@@ -585,6 +590,10 @@ sub store {
     my ($self) = @_;
 
     if ( !$self->in_storage ) {
+        if ( ! $self->expirationdate && $self->patron_expiration_date ) {
+            $self->expirationdate($self->patron_expiration_date);
+        }
+
         if (
             C4::Context->preference('DefaultHoldExpirationdate')
             and ( not defined $self->expirationdate

opac/opac-reserve.pl

@@ -275,7 +275,7 @@ if ( $query->param('place_reserve') ) {
             $startdate = $query->param("reserve_date_$biblioNum");
         }
 
-        my $expiration_date = $query->param("expiration_date_$biblioNum");
+        my $patron_expiration_date = $query->param("expiration_date_$biblioNum");
 
         my $rank = $biblioData->{rank};
         if ( $itemNum ne '' ) {
@@ -316,7 +316,7 @@ if ( $query->param('place_reserve') ) {
                     biblionumber     => $biblioNum,
                     priority         => $rank,
                     reservation_date => $startdate,
-                    expiration_date  => $expiration_date,
+                    expiration_date  => $patron_expiration_date,
                     notes            => $notes,
                     title            => $biblioData->{title},
                     itemnumber       => $itemNum,