From 64899db254375c39bd8e50583861a5e16f4616e8 Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Thu, 26 Oct 2017 09:31:50 +0200
Subject: [PATCH] Bug 9031: (QA follow-up) Final changes to
 Calendar::days_between

The crash is caused by comparing two datetimes where one datetime is
floating and the other one was not. In that case the floating is
converted. Note too that DateTime overloads comparison operators.

This patch clones the two dates first. Puts them in floating both. And
just after that starts comparing etc.

Similar small change in hours_between.

Adding a test where the parameters are swapped for days_between.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 Koha/Calendar.pm          | 28 +++++++++++-----------------
 t/db_dependent/Calendar.t |  4 +++-
 2 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/Koha/Calendar.pm b/Koha/Calendar.pm
index f6a276f21c..c66ab45092 100644
--- a/Koha/Calendar.pm
+++ b/Koha/Calendar.pm
@@ -317,32 +317,26 @@ sub days_between {
     my $start_dt = shift;
     my $end_dt   = shift;
 
-    if ( $start_dt->compare($end_dt) > 0 ) {
-        # swap dates
-        my $int_dt = $end_dt;
-        $end_dt = $start_dt;
-        $start_dt = $int_dt;
+    # Change time zone for date math and swap if needed
+    $start_dt = $start_dt->clone->set_time_zone('floating');
+    $end_dt = $end_dt->clone->set_time_zone('floating');
+    if( $start_dt->compare($end_dt) > 0 ) {
+        ( $start_dt, $end_dt ) = ( $end_dt, $start_dt );
     }
 
-
     # start and end should not be closed days
     my $days = $start_dt->delta_days($end_dt)->delta_days;
-    for (my $dt = $start_dt->clone()->set_time_zone('floating');
-        $dt <= $end_dt;
-        $dt->add(days => 1)
-    ) {
-        if ($self->is_holiday($dt)) {
-            $days--;
-        }
+    while( $start_dt->compare($end_dt) < 1 ) {
+        $days-- if $self->is_holiday($start_dt);
+        $start_dt->add( days => 1 );
     }
     return DateTime::Duration->new( days => $days );
-
 }
 
 sub hours_between {
     my ($self, $start_date, $end_date) = @_;
-    my $start_dt = $start_date->clone();
-    my $end_dt = $end_date->clone();
+    my $start_dt = $start_date->clone()->set_time_zone('floating');
+    my $end_dt = $end_date->clone()->set_time_zone('floating');
     my $duration = $end_dt->delta_ms($start_dt);
     $start_dt->truncate( to => 'day' );
     $end_dt->truncate( to => 'day' );
@@ -351,7 +345,7 @@ sub hours_between {
     # take into account open/close times then it would be a duration
     # of library open hours
     my $skipped_days = 0;
-    for (my $dt = $start_dt->clone()->set_time_zone('floating');
+    for (my $dt = $start_dt->clone();
         $dt <= $end_dt;
         $dt->add(days => 1)
     ) {
diff --git a/t/db_dependent/Calendar.t b/t/db_dependent/Calendar.t
index b443420dd1..3cfe961e20 100644
--- a/t/db_dependent/Calendar.t
+++ b/t/db_dependent/Calendar.t
@@ -68,13 +68,15 @@ is($forwarded_dt->ymd, $today->ymd, 'negative day should return start dt');
 
 subtest 'crossing_DST' => sub {
 
-    plan tests => 2;
+    plan tests => 3;
 
     my $tz = DateTime::TimeZone->new( name => 'America/New_York' );
     my $start_date = dt_from_string( "2016-03-09 02:29:00",undef,$tz );
     my $end_date = dt_from_string( "2017-01-01 00:00:00", undef, $tz );
     my $days_between = $calendar->days_between($start_date,$end_date);
     is( $days_between->delta_days, 298, "Days calculated correctly" );
+    $days_between = $calendar->days_between($end_date,$start_date);
+    is( $days_between->delta_days, 298, "Swapping returns the same" );
     my $hours_between = $calendar->hours_between($start_date,$end_date);
     is( $hours_between->delta_minutes, 298 * 24 * 60 - 149, "Hours (in minutes) calculated correctly" );
 
-- 
2.39.5