From 3b694b9b1c80477d50991c2e9204f9e155fdb378 Mon Sep 17 00:00:00 2001 From: Colin Campbell Date: Tue, 24 Jul 2012 15:16:10 +0100 Subject: [PATCH] Bug 8486 Correct calculation of days_between Thee were errors in the calculation used for days_between which caused incorrect values to be returned Added tests to validate calculation NB Tests still need to be provided for the hourly calculations --- Koha/Calendar.pm | 87 ++++++++++++++++++++++++++++++++++++++---------- t/Calendar.t | 80 ++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 143 insertions(+), 24 deletions(-) diff --git a/Koha/Calendar.pm b/Koha/Calendar.pm index 495f8ce67e..298cdb258c 100644 --- a/Koha/Calendar.pm +++ b/Koha/Calendar.pm @@ -79,6 +79,7 @@ sub _init { } $self->{single_holidays} = DateTime::Set->from_datetimes( dates => $dates ); $self->{days_mode} = C4::Context->preference('useDaysMode'); + $self->{test} = 0; return; } @@ -100,7 +101,6 @@ sub addDate { my $dt = $base_date + $add_duration; while ( $self->is_holiday($dt) ) { - # TODOP if hours set to 10 am $dt->add_duration($day_dur); if ( $unit eq 'hours' ) { $dt->set_hour($return_by_hour); # Staffs specific @@ -169,25 +169,25 @@ sub days_between { my $start_dt = shift; my $end_dt = shift; - my $datestart_temp = $start_dt->clone(); - my $dateend_temp = $end_dt->clone(); # start and end should not be closed days - $datestart_temp->truncate( to => 'day' ); - $dateend_temp->truncate( to => 'day' ); - my $duration = $dateend_temp - $datestart_temp; - while ( DateTime->compare( $datestart_temp, $dateend_temp ) == -1 ) { - $datestart_temp->add( days => 1 ); - if ( $self->is_holiday($datestart_temp) ) { - $duration->subtract( days => 1 ); + my $days = $start_dt->delta_days($end_dt)->delta_days; + for (my $dt = $start_dt->clone(); + $dt <= $end_dt; + $dt->add(days => 1) + ) { + if ($self->is_holiday($dt)) { + $days--; } } - return $duration; + return DateTime::Duration->new( days => $days ); } sub hours_between { - my ($self, $start_dt, $end_dt) = @_; + my ($self, $start_date, $end_date) = @_; + my $start_dt = $start_date->clone(); + my $end_dt = $end_date->clone(); my $duration = $end_dt->delta_ms($start_dt); $start_dt->truncate( to => 'day' ); $end_dt->truncate( to => 'day' ); @@ -195,12 +195,19 @@ sub hours_between { # However for hourly loans the logic should be expanded to # take into account open/close times then it would be a duration # of library open hours - while ( DateTime->compare( $start_dt, $end_dt ) == -1 ) { - $start_dt->add( days => 1 ); - if ( $self->is_holiday($start_dt) ) { - $duration->subtract( hours => 24 ); + my $skipped_days = 0; + for (my $dt = $start_dt->clone(); + $dt <= $end_dt; + $dt->add(days => 1) + ) { + if ($self->is_holiday($dt)) { + ++$skipped_days; } } + if ($skipped_days) { + $duration->subtract_duration(DateTime::Duration->new( hours => 24 * $skipped_days)); + } + return $duration; } @@ -221,6 +228,35 @@ sub _mockinit { push @{$dates}, $special; $self->{single_holidays} = DateTime::Set->from_datetimes( dates => $dates ); $self->{days_mode} = 'Calendar'; + $self->{test} = 1; + return; +} + +sub set_daysmode { + my ( $self, $mode ) = @_; + + # if not testing this is a no op + if ( $self->{test} ) { + $self->{days_mode} = $mode; + } + + return; +} + +sub clear_weekly_closed_days { + my $self = shift; + $self->{weekly_closed_days} = [ 0, 0, 0, 0, 0, 0, 0 ]; # Sunday only + return; +} + +sub add_holiday { + my $self = shift; + my $new_dt = shift; + my @dt = $self->{exception_holidays}->as_list; + push @dt, $new_dt; + $self->{exception_holidays} = + DateTime::Set->from_datetimes( dates => \@dt ); + return; } @@ -287,7 +323,24 @@ passed at DateTime object returns 1 if it is a closed day $duration = $calendar->days_between($start_dt, $end_dt); Passed two dates returns a DateTime::Duration object measuring the length between them -ignoring closed days +ignoring closed days. Always returns a positive number irrespective of the +relative order of the parameters + +=head2 set_daysmode + +For testing only allows the calling script to change days mode + +=head2 clear_weekly_closed_days + +In test mode changes the testing set of closed days to a new set with +no closed days. TODO passing an array of closed days to this would +allow testing of more configurations + +=head2 add_holiday + +Passed a datetime object this will add it to the calendar's list of +closed days. This is for testing so that we can alter the Calenfar object's +list of specified dates =head1 DIAGNOSTICS diff --git a/t/Calendar.t b/t/Calendar.t index d2690f78f6..65c2622343 100755 --- a/t/Calendar.t +++ b/t/Calendar.t @@ -1,14 +1,80 @@ -#!/usr/bin/perl -# -# This Koha test module is a stub! -# Add more tests here!!! +#!/usr/bin/env perl use strict; use warnings; - -use Test::More tests => 1; +use DateTime; +use Test::More tests => 14; +use Koha::DateUtils; BEGIN { - use_ok('C4::Calendar'); + use_ok('Koha::Calendar'); + + # This was the only test C4 had + # Remove when no longer used + use_ok('C4::Calendar'); } +my $cal = Koha::Calendar->new( TEST_MODE => 1 ); + +isa_ok( $cal, 'Koha::Calendar' ); + +my $test_dt = DateTime->new( # Monday + year => 2012, + month => 7, + day => 23, + hour => 11, + minute => 53, + time_zone => 'Europe/London', +); + +my $later_dt = DateTime->new( # Monday + year => 2012, + month => 9, + day => 17, + hour => 17, + minute => 30, + time_zone => 'Europe/London', +); + +my $daycount = $cal->days_between( $test_dt, $later_dt ); +cmp_ok( $daycount->in_units('days'), + '==', 48, 'days_between calculates correctly' ); + +my $ret = $cal->addDate( $test_dt, 1, 'days' ); + +cmp_ok( $ret->ymd(), 'eq', '2012-07-24', 'Simple Single Day Add (Calendar)`' ); + +$ret = $cal->addDate( $test_dt, 7, 'days' ); +cmp_ok( $ret->ymd(), 'eq', '2012-07-31', 'Add 7 days Calendar mode' ); +$cal->set_daysmode('Datedue'); +$ret = $cal->addDate( $test_dt, 7, 'days' ); +cmp_ok( $ret->ymd(), 'eq', '2012-07-30', 'Add 7 days Datedue mode' ); +$cal->set_daysmode('Days'); +$ret = $cal->addDate( $test_dt, 7, 'days' ); +cmp_ok( $ret->ymd(), 'eq', '2012-07-30', 'Add 7 days Days mode' ); +$cal->set_daysmode('Calendar'); + +# example tests for bug report +$cal->clear_weekly_closed_days(); + +$daycount = $cal->days_between( dt_from_string('2012-01-10'), + dt_from_string("2012-05-05") )->in_units('days'); +cmp_ok( $daycount, '==', 116, 'test larger intervals' ); +$daycount = $cal->days_between( dt_from_string("2012-01-01"), + dt_from_string("2012-05-05") )->in_units('days'); +cmp_ok( $daycount, '==', 125, 'test positive intervals' ); +my $daycount2 = $cal->days_between( dt_from_string("2012-05-05"), + dt_from_string("2012-01-01") )->in_units('days'); +cmp_ok( $daycount2, '==', $daycount, 'test parameter order not relevant' ); +$daycount = $cal->days_between( dt_from_string("2012-07-01"), + dt_from_string("2012-07-15") )->in_units('days'); +cmp_ok( $daycount, '==', 14, 'days_between calculates correctly' ); +$cal->add_holiday( dt_from_string('2012-07-06') ); +$daycount = $cal->days_between( dt_from_string("2012-07-01"), + dt_from_string("2012-07-15") )->in_units('days'); +cmp_ok( $daycount, '==', 13, 'holiday correctly recognized' ); + +$cal->add_holiday( dt_from_string('2012-07-07') ); +$daycount = $cal->days_between( dt_from_string("2012-07-01"), + dt_from_string("2012-07-15") )->in_units('days'); +cmp_ok( $daycount, '==', 12, 'multiple holidays correctly recognized' ); -- 2.39.5