Bug 8800 - useDaysMode=Datedue wrong behaviour (revisited)

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.

- Koha::Calendar->addDate was rewritten in a more sane way (also split into
  addHours and addDays for convenience).
- Fixed a bug introduced in Bug 8966 regarding dt truncation and dtSets->contains
- Minor docs typos
- Use the passed Calendar mode or default to 'Calendar' in Koha::Calendar->_mockinit.
- Tests

I'm writing some db-dependent tests for is_holiday, and hopefully for CalcDateDue so any
rewrite/followup doesn't break things.

Regards
To+

Sponsored-by: Universidad Nacional de Córdoba
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passed-QA-by: Paul Poulain <paul.poulain@biblibre.com>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
This commit is contained in:
Tomas Cohen Arazi 2012-09-21 12:50:26 -03:00 committed by Jared Camins-Esakov
parent be19f11e97
commit 7b18e2146c
3 changed files with 214 additions and 56 deletions

View File

@ -3014,13 +3014,14 @@ sub CalcDateDue {
->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;
}
# break
return $dt;
} else {
my $dur;
if ($loanlength->{lengthunit} eq 'hours') {

View File

@ -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);
}
$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 {
while ($days) {
$base_date = $self->next_open_day($base_date);
--$days;
}
}
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);
}
} 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);
} else {
my $days = abs $add_duration->in_units('days');
while ($days) {
$base_date->add_duration($day_dur);
if ( $self->is_holiday($base_date) ) {
next;
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 {
--$days;
$base_date = $self->next_open_day($base_date);
}
}
}
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
}
}
return $base_date;
} else { # Days
return $base_date + $add_duration;
}
return $base_date;
}
sub is_holiday {
@ -154,10 +190,10 @@ sub is_holiday {
if ( exists $self->{day_month_closed_days}->{$month}->{$day} ) {
return 1;
}
if ( $self->{exception_holidays}->contains($dt) ) {
if ( $self->{exception_holidays}->contains($localdt) ) {
return 1;
}
if ( $self->{single_holidays}->contains($dt) ) {
if ( $self->{single_holidays}->contains($localdt) ) {
return 1;
}
@ -165,6 +201,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;
@ -228,7 +290,12 @@ sub _mockinit {
);
push @{$dates}, $special;
$self->{single_holidays} = DateTime::Set->from_datetimes( dates => $dates );
$self->{days_mode} = 'Calendar';
# if not defined, days_mode defaults to 'Calendar'
if ( !defined($self->{days_mode}) ) {
$self->{days_mode} = 'Calendar';
}
$self->{test} = 1;
return;
}
@ -274,9 +341,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
@ -312,11 +379,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
@ -327,6 +419,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

View File

@ -3,7 +3,8 @@
use strict;
use warnings;
use DateTime;
use Test::More tests => 23;
use DateTime::Duration;
use Test::More tests => 22;
use Koha::DateUtils;
BEGIN {
@ -89,18 +90,8 @@ 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' );
my $ret;
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');
# see bugzilla #8966
@ -131,3 +122,61 @@ $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' );
my $one_day_dur = DateTime::Duration->new( days => 1 );
my $two_day_dur = DateTime::Duration->new( days => 2 );
my $seven_day_dur = DateTime::Duration->new( days => 7 );
subtest '\'Datedue\' tests' => sub {
my $cal = Koha::Calendar->new( TEST_MODE => 1 ,
days_mode => 'Datedue');
$cal->add_holiday( dt_from_string('2012-07-04','iso') );
$dt = dt_from_string( '2012-07-03','iso' );
is($cal->addDate( $dt, $one_day_dur, 'days' ),
dt_from_string('2012-07-05','iso'),
'Single day add (Datedue, matches holiday, shift)' );
is($cal->addDate( $dt, $two_day_dur, 'days' ),
dt_from_string('2012-07-05','iso'),
'Two days add, skips holiday (Datedue)' );
cmp_ok($cal->addDate( $test_dt, $seven_day_dur, 'days' ), 'eq',
'2012-07-30T11:53:00',
'Add 7 days (Datedue)' );
};
subtest '\'Calendar\' tests' => sub {
my $cal = Koha::Calendar->new( TEST_MODE => 1,
days_mode => 'Calendar' );
$cal->add_holiday( dt_from_string('2012-07-04','iso') );
$dt = dt_from_string('2012-07-03','iso');
is($cal->addDate( $dt, $one_day_dur, 'days' ),
dt_from_string('2012-07-05','iso'),
'Single day add (Calendar)' );
cmp_ok($cal->addDate( $test_dt, $seven_day_dur, 'days' ), 'eq',
'2012-07-31T11:53:00',
'Add 7 days (Calendar)' );
};
subtest '\'Days\' tests' => sub {
my $cal = Koha::Calendar->new( TEST_MODE => 1,
days_mode => 'Days' );
$cal->add_holiday( dt_from_string('2012-07-04','iso') );
$dt = dt_from_string('2012-07-03','iso');
is($cal->addDate( $dt, $one_day_dur, 'days' ),
dt_from_string('2012-07-04','iso'),
'Single day add (Days)' );
cmp_ok($cal->addDate( $test_dt, $seven_day_dur, 'days' ),'eq',
'2012-07-30T11:53:00',
'Add 7 days (Days)' );
};