From 5de6bebbc838eddf1b62dd76b12e5e0f5c6d5533 Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@gmail.com>
Date: Fri, 21 Sep 2012 12:50:26 -0300
Subject: [PATCH] Bug 8800 - useDaysMode=Datedue wrong behaviour (revisited)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
useDaysMode=Datedue wasn't used as advertised in the docs. Added
next_open_day and prev_open_day subs to Koha::Calendar and some tests for them.
- CalcDateDue was modified to be consistent with the intended behaviour
- Koha::Calendar->addDate was rewritten in a more sane way (also split into
addHours and addDays for convenience).
Regards
To+
Sponsored-by: Universidad Nacional de Córdoba
---
C4/Circulation.pm | 7 ++-
Koha/Calendar.pm | 179 +++++++++++++++++++++++++++++++++++++++++------------
t/Calendar.t | 52 +++++++++++++++-
3 files changed, 196 insertions(+), 42 deletions(-)
diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 7485de5..7ddb66a 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -3007,20 +3007,21 @@ sub CalcDateDue {
} else {
# otherwise, calculate the datedue as normal
- if ( C4::Context->preference('useDaysMode') eq 'Days' )
+ if ( C4::Context->preference('useDaysMode') ne 'Calendar' )
{ # ignoring calendar
my $dt =
DateTime->now( time_zone => C4::Context->tz() )
->truncate( to => 'minute' );
if ( $loanlength->{lengthunit} eq 'hours' ) {
$dt->add( hours => $loanlength->{issuelength} );
- return $dt;
} else { # days
$dt->add( days => $loanlength->{issuelength} );
$dt->set_hour(23);
$dt->set_minute(59);
- return $dt;
}
+
+ return $dt;
+
} else {
my $dur;
if ($loanlength->{lengthunit} eq 'hours') {
diff --git a/Koha/Calendar.pm b/Koha/Calendar.pm
index 2a2e062..1573aec 100644
--- a/Koha/Calendar.pm
+++ b/Koha/Calendar.pm
@@ -85,57 +85,93 @@ sub _init {
sub addDate {
my ( $self, $startdate, $add_duration, $unit ) = @_;
- my $base_date = $startdate->clone();
+
+ # Default to days duration (legacy support I guess)
if ( ref $add_duration ne 'DateTime::Duration' ) {
$add_duration = DateTime::Duration->new( days => $add_duration );
}
- $unit ||= q{}; # default days ?
- my $days_mode = $self->{days_mode};
- Readonly::Scalar my $return_by_hour => 10;
- my $day_dur = DateTime::Duration->new( days => 1 );
- if ( $add_duration->is_negative() ) {
- $day_dur = DateTime::Duration->new( days => -1 );
+
+ $unit ||= 'days'; # default days ?
+ my $dt;
+
+ if ( $unit eq 'hours' ) {
+ # Fixed for legacy support. Should be set as a branch parameter
+ Readonly::Scalar my $return_by_hour => 10;
+
+ $dt = $self->addHours($startdate, $add_duration, $return_by_hour);
+ } else {
+ # days
+ $dt = $self->addDays($startdate, $add_duration);
}
- if ( $days_mode eq 'Datedue' ) {
- my $dt = $base_date + $add_duration;
- while ( $self->is_holiday($dt) ) {
+ return $dt;
+}
- $dt->add_duration($day_dur);
- if ( $unit eq 'hours' ) {
- $dt->set_hour($return_by_hour); # Staffs specific
- }
+sub addHours {
+ my ( $self, $startdate, $hours_duration, $return_by_hour ) = @_;
+ my $base_date = $startdate->clone();
+
+ $base_date->add_duration($hours_duration);
+
+ # If we are using the calendar behave for now as if Datedue
+ # was the chosen option (current intended behaviour)
+
+ if ( $self->{days_mode} ne 'Days' &&
+ $self->is_holiday($base_date) ) {
+
+ if ( $hours_duration->is_negative() ) {
+ $base_date = $self->prev_open_day($base_date);
+ } else {
+ $base_date = $self->next_open_day($base_date);
}
- return $dt;
- } elsif ( $days_mode eq 'Calendar' ) {
- if ( $unit eq 'hours' ) {
- $base_date->add_duration($add_duration);
- while ( $self->is_holiday($base_date) ) {
- $base_date->add_duration($day_dur);
- }
+ $base_date->set_hour($return_by_hour);
+
+ }
+
+ return $base_date;
+}
+sub addDays {
+ my ( $self, $startdate, $days_duration ) = @_;
+ my $base_date = $startdate->clone();
+
+ if ( $self->{days_mode} eq 'Calendar' ) {
+ # use the calendar to skip all days the library is closed
+ # when adding
+ my $days = abs $days_duration->in_units('days');
+
+ if ( $days_duration->is_negative() ) {
+ while ($days) {
+ $base_date = $self->prev_open_day($base_date);
+ --$days;
+ }
} else {
- my $days = abs $add_duration->in_units('days');
while ($days) {
- $base_date->add_duration($day_dur);
- if ( $self->is_holiday($base_date) ) {
- next;
- } else {
- --$days;
- }
+ $base_date = $self->next_open_day($base_date);
+ --$days;
}
}
- if ( $unit eq 'hours' ) {
- my $dt = $base_date->clone()->subtract( days => 1 );
- if ( $self->is_holiday($dt) ) {
- $base_date->set_hour($return_by_hour); # Staffs specific
+
+ } else { # Days or Datedue
+ # use straight days, then use calendar to push
+ # the date to the next open day if Datedue
+ $base_date->add_duration($days_duration);
+
+ if ( $self->{days_mode} eq 'Datedue' ) {
+ # Datedue, then use the calendar to push
+ # the date to the next open day if holiday
+ if ( $self->is_holiday($base_date) ) {
+ if ( $days_duration->is_negative() ) {
+ $base_date = $self->prev_open_day($base_date);
+ } else {
+ $base_date = $self->next_open_day($base_date);
+ }
}
}
- return $base_date;
- } else { # Days
- return $base_date + $add_duration;
}
+
+ return $base_date;
}
sub is_holiday {
@@ -164,6 +200,32 @@ sub is_holiday {
return 0;
}
+sub next_open_day {
+ my ( $self, $dt ) = @_;
+ my $base_date = $dt->clone();
+
+ $base_date->add(days => 1);
+
+ while ($self->is_holiday($base_date)) {
+ $base_date->add(days => 1);
+ }
+
+ return $base_date;
+}
+
+sub prev_open_day {
+ my ( $self, $dt ) = @_;
+ my $base_date = $dt->clone();
+
+ $base_date->add(days => -1);
+
+ while ($self->is_holiday($base_date)) {
+ $base_date->add(days => -1);
+ }
+
+ return $base_date;
+}
+
sub days_between {
my $self = shift;
my $start_dt = shift;
@@ -273,9 +335,9 @@ This documentation refers to Koha::Calendar version 0.0.1
=head1 SYNOPSIS
- use Koha::Calendat
+ use Koha::Calendar
- my $c = Koha::Calender->new( branchcode => 'MAIN' );
+ my $c = Koha::Calendar->new( branchcode => 'MAIN' );
my $dt = DateTime->now();
# are we open
@@ -311,11 +373,36 @@ Currently unit is only used to invoke Staffs return Monday at 10 am rule this
parameter will be removed when issuingrules properly cope with that
+=head2 addHours
+
+ my $dt = $calendar->addHours($date, $dur, $return_by_hour )
+
+C<$date> is a DateTime object representing the starting date of the interval.
+
+C<$offset> is a DateTime::Duration to add to it
+
+C<$return_by_hour> is an integer value representing the opening hour for the branch
+
+
+=head2 addDays
+
+ my $dt = $calendar->addDays($date, $dur)
+
+C<$date> is a DateTime object representing the starting date of the interval.
+
+C<$offset> is a DateTime::Duration to add to it
+
+C<$unit> is a string value 'days' or 'hours' toflag granularity of duration
+
+Currently unit is only used to invoke Staffs return Monday at 10 am rule this
+parameter will be removed when issuingrules properly cope with that
+
+
=head2 is_holiday
$yesno = $calendar->is_holiday($dt);
-passed at DateTime object returns 1 if it is a closed day
+passed a DateTime object returns 1 if it is a closed day
0 if not according to the calendar
=head2 days_between
@@ -326,6 +413,22 @@ Passed two dates returns a DateTime::Duration object measuring the length betwee
ignoring closed days. Always returns a positive number irrespective of the
relative order of the parameters
+=head2 next_open_day
+
+$datetime = $calendar->next_open_day($duedate_dt)
+
+Passed a Datetime returns another Datetime representing the next open day. It is
+intended for use to calculate the due date when useDaysMode syspref is set to either
+'Datedue' or 'Calendar'.
+
+=head2 prev_open_day
+
+$datetime = $calendar->prev_open_day($duedate_dt)
+
+Passed a Datetime returns another Datetime representing the previous open day. It is
+intended for use to calculate the due date when useDaysMode syspref is set to either
+'Datedue' or 'Calendar'.
+
=head2 set_daysmode
For testing only allows the calling script to change days mode
diff --git a/t/Calendar.t b/t/Calendar.t
index 283300e..a01e246 100755
--- a/t/Calendar.t
+++ b/t/Calendar.t
@@ -3,7 +3,8 @@
use strict;
use warnings;
use DateTime;
-use Test::More tests => 21;
+use DateTime::Duration;
+use Test::More tests => 26;
use Koha::DateUtils;
BEGIN {
@@ -127,3 +128,52 @@ $cal->add_holiday( dt_from_string('2012-07-07','iso') );
$daycount = $cal->days_between( dt_from_string("2012-07-01",'iso'),
dt_from_string("2012-07-15",'iso') )->in_units('days');
cmp_ok( $daycount, '==', 12, 'multiple holidays correctly recognized' );
+
+# next_open_day tests
+$dt = dt_from_string('2012-07-07','iso');
+cmp_ok( $dt->add(days => 1), '==', $cal->next_open_day(dt_from_string('2012-07-07','iso')), 'Next open day correctly calculated' );
+
+# prev_open_day tests
+# 2012-07-07 and 2012-07-06 are set to holidays previously, so previous open day should match 2012-07-05
+$dt = dt_from_string('2012-07-05','iso');
+cmp_ok( $dt, '==', $cal->prev_open_day(dt_from_string('2012-07-08','iso')), 'Previous open day correctly calculated' );
+
+######################
+# useDaysMode='Datedue' case tests
+subtest 'useDaysMode=Datedue' => sub {
+ my $cal = Koha::Calendar->new( TEST_MODE => 1 );
+ my $dur = DateTime::Duration->new( days => 1 );
+ $cal->set_daysmode( 'Datedue' );
+ $cal->add_holiday( dt_from_string('2012-07-04','iso') );
+ $dt = dt_from_string( '2012-07-03','iso' );
+ is($cal->addDate( $dt, $dur, 'days' ), dt_from_string('2012-07-05','iso'), 'Single day add (Datedue, matches holiday, shift)' );
+ $dur = DateTime::Duration->new( days => 2 );
+ is($cal->addDate( $dt, $dur, 'days' ), dt_from_string('2012-07-05','iso'), 'Two days add, skips holiday (Datedue)' );
+
+};
+#######################
+
+#######################
+# useDaysMode='Calendar' case tests
+subtest 'useDaysMode=Calendar' => sub {
+ my $cal = Koha::Calendar->new( TEST_MODE => 1 );
+ my $dur = DateTime::Duration->new( days => 1 );
+ $cal->set_daysmode('Calendar');
+ $cal->add_holiday( dt_from_string('2012-07-04','iso') );
+ $dt = dt_from_string('2012-07-03','iso');
+ is($cal->addDate( $dt, $dur, 'days' ), dt_from_string('2012-07-05','iso'), 'Single day add (Calendar)' );
+};
+#######################
+
+#######################
+# useDaysMode='Days' case tests
+subtest 'useDaysMode=Days' => sub {
+ my $cal = Koha::Calendar->new( TEST_MODE => 1 );
+ my $dur = DateTime::Duration->new( days => 1 );
+ $cal->set_daysmode('Days');
+ $cal->add_holiday( dt_from_string('2012-07-04','iso') );
+ $dt = dt_from_string('2012-07-03','iso');
+ is($cal->addDate( $dt, $dur, 'days' ), dt_from_string('2012-07-04','iso'), 'Single day add (Days)' );
+};
+#######################
+
--
1.7.9.5