Attachment 129092 Details for Bug 21729 – Bug 21729: Keep expiration date set when placing a hold

[patch] Bug 21729: Keep expiration date set when placing a hold

Bug-21729-Keep-expiration-date-set-when-placing-a-.patch (text/plain), 7.11 KB, created by Martin Renvoize (ashimema) on 2022-01-06 14:12:54 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2022-01-06 14:12:54 UTC

Size: 7.11 KB

patch

obsolete

>From ac3a582dabceb410acbf40ae641ce329717aa06a Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>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 <andrew@bywatersolutions.com>
>Signed-off-by: Florian Bontemps <florian.bontemps@biblibre.com>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>---
> 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

Actions: View | Diff | Splinter Review

Attachments on bug 21729: 126160 | 126161 | 126162 | 126203 | 126258 | 127797 | 127798 | 127799 | 127800 | 127867 | 127868 | 127869 | 127870 | 127877 | 127878 | 127879 | 127880 | 128761 | 129090 | 129091 | 129092 | 129093 | 129094