@@ -, +, @@ Bug 8486 Correct calculation of days_between There 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 Bug 8486 Merge Calendar test files Moves the tests previously in t/Kalendar.t to t/Calendar.t and removes the old file. Note the existing minimal test of C4/Calendar still called in t/Calendar --- Koha/Calendar.pm | 87 +++++++++++++++++++++++++++++-------- t/Calendar.t | 129 ++++++++++++++++++++++++++++++++++++++++++++++++++++--- t/Kalendar.t | 62 -------------------------- 3 files changed, 192 insertions(+), 86 deletions(-) delete mode 100755 t/Kalendar.t --- a/Koha/Calendar.pm +++ a/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 - my $duration = $dateend_temp->delta_days($datestart_temp); - $datestart_temp->truncate( to => 'days' ); - $dateend_temp->truncate( to => 'days' ); - 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 => 'days' ); $end_dt->truncate( to => 'days' ); @@ -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 --- a/t/Calendar.t +++ a/t/Calendar.t @@ -1,14 +1,129 @@ -#!/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 => 21; +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', 'Calendar class returned' ); + +my $saturday = DateTime->new( + year => 2011, + month => 6, + day => 25, + time_zone => 'Europe/London', +); +my $sunday = DateTime->new( + year => 2011, + month => 6, + day => 26, + time_zone => 'Europe/London', +); +my $monday = DateTime->new( + year => 2011, + month => 6, + day => 27, + time_zone => 'Europe/London', +); +my $bloomsday = DateTime->new( + year => 2011, + month => 6, + day => 16, + time_zone => 'Europe/London', +); # should be a holiday +my $special = DateTime->new( + year => 2011, + month => 6, + day => 1, + time_zone => 'Europe/London', +); # should be a holiday +my $notspecial = DateTime->new( + year => 2011, + month => 6, + day => 2, + time_zone => 'Europe/London', +); # should NOT be a holiday + +is( $cal->is_holiday($sunday), 1, 'Sunday is a closed day' ); # wee free test; +is( $cal->is_holiday($monday), 0, 'Monday is not a closed day' ); # alas +is( $cal->is_holiday($bloomsday), 1, 'month/day closed day test' ); +is( $cal->is_holiday($special), 1, 'special closed day test' ); +is( $cal->is_holiday($notspecial), 0, 'open day test' ); + +my $dt = $cal->addDate( $saturday, 1, 'days' ); +is( $dt->day_of_week, 1, 'addDate skips closed Sunday' ); + +$dt = $cal->addDate( $bloomsday, -1 ); +is( $dt->ymd(), '2011-06-15', 'Negative call to addDate' ); + +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' ); --- a/t/Kalendar.t +++ a/t/Kalendar.t @@ -1,62 +0,0 @@ -use strict; -use warnings; -use 5.010; -use DateTime; -use DateTime::TimeZone; - -use C4::Context; -use Test::More tests => 9; - -BEGIN { use_ok('Koha::Calendar'); } - -my $cal = Koha::Calendar->new( TEST_MODE => 1 ); - -isa_ok( $cal, 'Koha::Calendar', 'Calendar class returned' ); - -my $saturday = DateTime->new( - year => 2011, - month => 6, - day => 25, - time_zone => 'Europe/London', -); -my $sunday = DateTime->new( - year => 2011, - month => 6, - day => 26, - time_zone => 'Europe/London', -); -my $monday = DateTime->new( - year => 2011, - month => 6, - day => 27, - time_zone => 'Europe/London', -); -my $bloomsday = DateTime->new( - year => 2011, - month => 6, - day => 16, - time_zone => 'Europe/London', -); # should be a holiday -my $special = DateTime->new( - year => 2011, - month => 6, - day => 1, - time_zone => 'Europe/London', -); # should be a holiday -my $notspecial = DateTime->new( - year => 2011, - month => 6, - day => 2, - time_zone => 'Europe/London', -); # should NOT be a holiday -is( $cal->is_holiday($sunday), 1, 'Sunday is a closed day' ); # wee free test; -is( $cal->is_holiday($monday), 0, 'Monday is not a closed day' ); # alas -is( $cal->is_holiday($bloomsday), 1, 'month/day closed day test' ); -is( $cal->is_holiday($special), 1, 'special closed day test' ); -is( $cal->is_holiday($notspecial), 0, 'open day test' ); - -my $dt = $cal->addDate( $saturday, 1, 'days' ); -is( $dt->day_of_week, 1, 'addDate skips closed Sunday' ); - -$dt = $cal->addDate( $bloomsday, -1 ); -is( $dt->ymd(), '2011-06-15', 'Negative call to addDate' ); --