From d5490f50e306c1b942b3316df4f60158b2c2f877 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy 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 Signed-off-by: Nick Clemens --- 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 f6a276f..c66ab45 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 b443420..3cfe961 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.1.4