From 32c5ef613d8805304c1b7f97e597116226b7bbe8 Mon Sep 17 00:00:00 2001 From: Colin Campbell Date: Thu, 15 Sep 2011 12:42:12 +0100 Subject: [PATCH] Use hour or day deltas to calculate overdue durations If durations are calculated by subtraction they will use units larger than those we care about and these are not convertable to the smaller units we are attempting to enumerate Use the appropriate delta methods to calculate theee fines Adds a separate hours_between method to calendar This should strictly be checking opening hours (of which closed days are a special case) of the relevant branch These need adding to branches http://bugs.koha-community.org/show_bug.cgi?id=7852 Signed-off-by: Kyle M Hall Signed-off-by: Ian Walls QA comment: renamed "days_minus_grace" variable to "units_minus_grace" Also added POD to _get_chargeable_units Signed-off-by: Paul Poulain --- C4/Overdues.pm | 65 +++++++++++++++++++++++++++++++++--------------- Koha/Calendar.pm | 23 ++++++++++++++--- 2 files changed, 65 insertions(+), 23 deletions(-) diff --git a/C4/Overdues.pm b/C4/Overdues.pm index 7676a87283..23e75f5653 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -254,29 +254,15 @@ or "Final Notice". But CalcFine never defined any value. sub CalcFine { my ( $item, $bortype, $branchcode, $due_dt, $end_date ) = @_; my $start_date = $due_dt->clone(); - my $dbh = C4::Context->dbh; - my $amount = 0; - my $charge_duration; # get issuingrules (fines part will be used) my $data = C4::Circulation::GetIssuingRule($bortype, $item->{itemtype}, $branchcode); - if(C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed') { - my $calendar = Koha::Calendar->new( branchcode => $branchcode ); - $charge_duration = $calendar->days_between( $start_date, $end_date ); - } else { - $charge_duration = $end_date - $start_date; - } - # correct for grace period. my $fine_unit = $data->{lengthunit}; $fine_unit ||= 'days'; - my $chargeable_units; - if ($fine_unit eq 'hours') { - $chargeable_units = $charge_duration->hours(); # TODO closed times??? - } - else { - $chargeable_units = $charge_duration->days; - } - my $days_minus_grace = $chargeable_units - $data->{firstremind}; - if ($data->{'chargeperiod'} && $days_minus_grace ) { + + my $chargeable_units = _get_chargeable_units($fine_unit, $start_date, $end_date, $branchcode); + my $units_minus_grace = $chargeable_units - $data->{firstremind}; + my $amount = 0; + if ($data->{'chargeperiod'} && $units_minus_grace ) { $amount = int($chargeable_units / $data->{'chargeperiod'}) * $data->{'fine'};# TODO fine calc should be in cents } else { # a zero (or null) chargeperiod means no charge. @@ -284,11 +270,50 @@ sub CalcFine { if(C4::Context->preference('maxFine') && ( $amount > C4::Context->preference('maxFine'))) { $amount = C4::Context->preference('maxFine'); } - return ($amount, $data->{chargename}, $days_minus_grace); + return ($amount, $data->{chargename}, $units_minus_grace); # FIXME: chargename is NEVER populated anywhere. } +=head2 _get_chargeable_units + + _get_chargeable_units($unit, $start_date_ $end_date, $branchcode); + +return integer value of units between C<$start_date> and C<$end_date>, factoring in holidays for C<$branchcode>. + +C<$unit> is 'days' or 'hours' (default is 'days'). + +C<$start_date> and C<$end_date> are the two DateTimes to get the number of units between. + +C<$branchcode> is the branch whose calendar to use for finding holidays. + +=cut + +sub _get_chargeable_units { + my ($unit, $dt1, $dt2, $branchcode) = @_; + my $charge_units = 0; + my $charge_duration; + if ($unit eq 'hours') { + if(C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed') { + my $calendar = Koha::Calendar->new( branchcode => $branchcode ); + $charge_duration = $calendar->hours_between( $dt1, $dt2 ); + } else { + $charge_duration = $dt2->delta_ms( $dt1 ); + } + return $charge_duration->in_units('hours'); + } + else { # days + if(C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed') { + my $calendar = Koha::Calendar->new( branchcode => $branchcode ); + $charge_duration = $calendar->days_between( $dt1, $dt2 ); + } else { + $charge_duration = $dt2->delta_days( $dt1 ); + } + return $charge_duration->in_units('days'); + } +} + + =head2 GetSpecialHolidays &GetSpecialHolidays($date_dues,$itemnumber); diff --git a/Koha/Calendar.pm b/Koha/Calendar.pm index 1e7299c083..75c5c9ebb1 100644 --- a/Koha/Calendar.pm +++ b/Koha/Calendar.pm @@ -168,11 +168,9 @@ sub days_between { my $self = shift; my $start_dt = shift; my $end_dt = shift; - $start_dt->truncate( to => 'hours' ); - $end_dt->truncate( to => 'hours' ); # start and end should not be closed days - my $duration = $end_dt - $start_dt; + my $duration = $end_dt->delta_days($start_dt); $start_dt->truncate( to => 'days' ); $end_dt->truncate( to => 'days' ); while ( DateTime->compare( $start_dt, $end_dt ) == -1 ) { @@ -185,6 +183,25 @@ sub days_between { } +sub hours_between { + my ($self, $start_dt, $end_dt) = @_; + my $duration = $end_dt->delta_ms($start_dt); + $start_dt->truncate( to => 'days' ); + $end_dt->truncate( to => 'days' ); + # NB this is a kludge in that it assumes all days are 24 hours + # 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 ); + } + } + return $duration; + +} + sub _mockinit { my $self = shift; $self->{weekly_closed_days} = [ 1, 0, 0, 0, 0, 0, 0 ]; # Sunday only -- 2.39.5