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.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

diff --git a/Koha/StockRotationItem.pm b/Koha/StockRotationItem.pm
index d736aee1b6689de3785434a03a9707b75d7bf375..66578d83b014eb7807f11493ad951895f69e4f70 100644 (file)
--- 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!";
     }
 }