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>
This commit is contained in:
Marcel de Rooy 2017-10-26 09:31:50 +02:00 committed by Jonathan Druart
parent c398cfa377
commit 64899db254
2 changed files with 14 additions and 18 deletions

View file

@ -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)
) {

View file

@ -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" );