From 6768615db564375fc97cb29459fe925ee482c142 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 13 Oct 2021 11:30:42 +0200 Subject: [PATCH] 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 --- C4/Reserves.pm | 15 +++++++++++---- Koha/Hold.pm | 25 ++++++++++++++++--------- opac/opac-reserve.pl | 4 ++-- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index f2ee4639797..03be209b12b 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -186,7 +186,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}; @@ -197,7 +197,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 . @@ -253,7 +253,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, @@ -947,7 +947,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 (?) @@ -2100,6 +2106,7 @@ sub RevertWaitingStatus { priority => 1, found => undef, waitingdate => undef, + expirationdate => $hold->patron_expiration_date, itemnumber => $hold->item_level_hold ? $hold->itemnumber : undef, } )->store(); diff --git a/Koha/Hold.pm b/Koha/Hold.pm index 26c41d7924d..df18fa8bd71 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -186,16 +186,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 $expirationdate = $today->clone->add(days => $max_pickup_delay); if ( C4::Context->preference("ExcludeHolidaysFromMaxPickUpDelay") ) { my $itemtype = $self->item ? $self->item->effective_itemtype : $self->biblio->itemtype; @@ -213,8 +207,17 @@ sub set_waiting { # 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, $expirationdate ) + : 0; + + $values->{expirationdate} = + $cmp == -1 ? $requested_expiration->ymd : $expirationdate->ymd; + } $self->set($values)->store(); @@ -613,6 +616,10 @@ sub store { } } + unless ( $self->expirationdate ) { + $self->expirationdate($self->patron_expiration_date); + } + $self = $self->SUPER::store; } diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index 721981bb2a0..06cbbaa18f5 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -273,7 +273,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 '' ) { @@ -314,7 +314,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, -- 2.25.1