From 63d68bab81f2a31073fd615535d14a7c302a461c 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 Signed-off-by: Andrew Fuerste-Henry Signed-off-by: Florian Bontemps --- C4/Reserves.pm | 15 +++++++++++---- Koha/Hold.pm | 29 +++++++++++++++++++---------- opac/opac-reserve.pl | 4 ++-- 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index a42168bc72..5184b0fbfe 100644 --- a/C4/Reserves.pm +++ b/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 (?) @@ -2099,6 +2105,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 9736dc527f..33ab06f839 100644 --- a/Koha/Hold.pm +++ b/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 diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index a7391e2327..5f7a09e50b 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.20.1