[Koha-patches] [PATCH] Bug 8486 Correct calculation of days_between
Colin Campbell
colin.campbell at ptfs-europe.com
Tue Jul 24 16:29:25 CEST 2012
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 495f8ce..298cdb2 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 d2690f7..65c2622 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' );
--
1.7.11.2.249.g31c7954
More information about the Koha-patches
mailing list