From d2af63343e06b36871f144fe4a8e5769aaca04a6 Mon Sep 17 00:00:00 2001 From: Colin Campbell Date: Fri, 27 Jul 2012 17:31:30 +0100 Subject: [PATCH] Bug 8486 Correct calculation of days_between` This patch backports the following patches from master to 3.8.x: commit d786d7c7b6c2894cca6231cae3deee9c9d43120b Author: Colin Campbell Date: Tue Jul 24 15:16:10 2012 +0100 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 commit 8379a9781dbf16bcb53b55b20f15dcd55ff948Oc2 Author: Colin Campbell Date: Wed Jul 25 10:55:56 2012 +0100 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 diff --git a/Koha/Calendar.pm b/Koha/Calendar.pm index 03146fa..0a66c52 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 - 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 diff --git a/t/Calendar.t b/t/Calendar.t index d2690f7..859d60b 100755 --- a/t/Calendar.t +++ b/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' ); diff --git a/t/Kalendar.t b/t/Kalendar.t deleted file mode 100755 index 5d219b2..0000000 --- a/t/Kalendar.t +++ /dev/null @@ -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' ); -- 1.7.11.2.249.g31c7954