From e8f4c176521e55e1b21333b2dcb7d9a6ed349236 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Wed, 13 Apr 2022 08:24:22 +0100 Subject: [PATCH] Bug 30518: Correct DateTime maths for needs_advancing The needs_advancing method prior to this patch used basic DateTime arithmatic, adding a DateTime::Duration in Days to the Arrival date of the item and then comparing that to today. This, however, can cause bugs when the arrival + duration date coincides with a DST boundary and as such may result in an invalid local date. See https://metacpan.org/pod/DateTime#Making-Things-Simple for further details. This patch updates the code to use the DST safe delta_days method to count the days between arrival and now instead and then compares this integer to the defined duration of the stage. To test: 1. Re-run the unit tests, they should now pass. --- Koha/StockRotationItem.pm | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/Koha/StockRotationItem.pm b/Koha/StockRotationItem.pm index d736aee1b6..66578d83b0 100644 --- a/Koha/StockRotationItem.pm +++ b/Koha/StockRotationItem.pm @@ -116,27 +116,29 @@ Return 1 if this item is ready to be moved on to the next stage in its rota. =cut sub needs_advancing { - my ( $self ) = @_; - return 0 if $self->item->get_transfer; # intransfer: don't advance. - return 1 if $self->fresh; # Just on rota: advance. + my ($self) = @_; + return 0 if $self->item->get_transfer; # intransfer: don't advance. + return 1 if $self->fresh; # Just on rota: advance. my $completed = $self->item->_result->branchtransfers->search( { 'reason' => "StockrotationAdvance" }, { order_by => { -desc => 'datearrived' } } ); + # Do maths on whether we need to be moved on. if ( $completed->count ) { - my $arrival = dt_from_string( - $completed->next->datearrived, 'iso' - ); - my $duration = DateTime::Duration - ->new( days => $self->stage->duration ); - if ( $arrival + $duration le dt_from_string() ) { + my $arrival = dt_from_string( $completed->next->datearrived ); + my $duration = $arrival->delta_days( dt_from_string() ); + if ( $duration->in_units('days') >= $self->stage->duration ) { return 1; - } else { + } + else { return 0; } - } else { - warn "We have no historical branch transfer for item " . $self->item->itemnumber . "; This should not have happened!"; + } + else { + warn "We have no historical branch transfer for item " + . $self->item->itemnumber + . "; This should not have happened!"; } } -- 2.20.1