Attachment 133296 Details for Bug 30518 – Bug 30518: Correct DateTime maths for needs_advancing

[patch] Bug 30518: Correct DateTime maths for needs_advancing

Bug-30518-Correct-DateTime-maths-for-needsadvancin.patch (text/plain), 2.79 KB, created by Nick Clemens (kidclamp) on 2022-04-14 10:44:34 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2022-04-14 10:44:34 UTC

Size: 2.79 KB

patch

obsolete

>From dc6ef0e4284de9e85fe600f1eab078ef094985cc Mon Sep 17 00:00:00 2001
>From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>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.
>
>Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
>---
> 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.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 30518: 133233 | 133234 | 133295 | 133296