From: Colin Campbell Date: Mon, 20 Jun 2011 14:47:10 +0000 (+0100) Subject: Bug 5549 : Fix calculation of duedates in fines.pl and advance_notices.pl X-Git-Url: http://git.rot13.org/?p=koha.git;a=commitdiff_plain;h=d55405047b952e2becac861f6ac7981558be9979 Bug 5549 : Fix calculation of duedates in fines.pl and advance_notices.pl Cleaned up some no longer used parameters in Overdues::CalcFine --- diff --git a/C4/Overdues.pm b/C4/Overdues.pm index afccdb5cd5..2935b24838 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -209,9 +209,9 @@ sub checkoverdues { =head2 CalcFine - ($amount, $chargename, $daycount, $daycounttotal) = &CalcFine($item, - $categorycode, $branch, $days_overdue, - $description, $start_date, $end_date ); + ($amount, $chargename, $daycounttotal) = &CalcFine($item, + $categorycode, $branch, + $start_dt, $end_dt ); Calculates the fine for a book. @@ -229,13 +229,8 @@ the book. C<$branchcode> is the library (string) whose issuingrules govern this transaction. -C<$days_overdue> is the number of days elapsed since the book's due date. - NOTE: supplying days_overdue is deprecated. - -C<$start_date> & C<$end_date> are C4::Dates objects +C<$start_date> & C<$end_date> are DateTime objects defining the date range over which to determine the fine. -Note that if these are defined, we ignore C<$difference> and C<$dues> , -but retain these for backwards-comptibility with extant fines scripts. Fines scripts should just supply the date range over which to calculate the fine. @@ -249,8 +244,6 @@ the categoryitem table, whatever that is. C<$daycount> is the number of days between start and end dates, Calendar adjusted (where needed), minus any applicable grace period. -C<$daycounttotal> is C<$daycount> without consideration of grace period. - FIXME - What is chargename supposed to be ? FIXME: previously attempted to return C<$message> as a text message, either "First Notice", "Second Notice", @@ -259,48 +252,38 @@ or "Final Notice". But CalcFine never defined any value. =cut sub CalcFine { - my ( $item, $bortype, $branchcode, $difference ,$dues , $start_date, $end_date ) = @_; - $debug and warn sprintf("CalcFine(%s, %s, %s, %s, %s, %s, %s)", - ($item ? '{item}' : 'UNDEF'), - ($bortype || 'UNDEF'), - ($branchcode || 'UNDEF'), - ($difference || 'UNDEF'), - ($dues || 'UNDEF'), - ($start_date ? ($start_date->output('iso') || 'Not a C4::Dates object') : 'UNDEF'), - ( $end_date ? ( $end_date->output('iso') || 'Not a C4::Dates object') : 'UNDEF') - ); + my ( $item, $bortype, $branchcode, $start_date, $end_date ) = @_; my $dbh = C4::Context->dbh; my $amount = 0; - my $daystocharge; - # get issuingrules (fines part will be used) - $debug and warn sprintf("CalcFine calling GetIssuingRule(%s, %s, %s)", $bortype, $item->{'itemtype'}, $branchcode); - my $data = C4::Circulation::GetIssuingRule($bortype, $item->{'itemtype'}, $branchcode); - if($difference) { - # if $difference is supplied, the difference has already been calculated, but we still need to adjust for the calendar. - # use copy-pasted functions from calendar module. (deprecated -- these functions will be removed from C4::Overdues ). - my $countspecialday = &GetSpecialHolidays($dues,$item->{itemnumber}); - my $countrepeatableday = &GetRepeatableHolidays($dues,$item->{itemnumber},$difference); - my $countalldayclosed = $countspecialday + $countrepeatableday; - $daystocharge = $difference - $countalldayclosed; - } else { - # if $difference is not supplied, we have C4::Dates objects giving us the date range, and we use the calendar module. - if(C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed') { - my $calendar = C4::Calendar->new( branchcode => $branchcode ); - $daystocharge = $calendar->daysBetween( $start_date, $end_date ); - } else { - $daystocharge = Date_to_Days(split('-',$end_date->output('iso'))) - Date_to_Days(split('-',$start_date->output('iso'))); - } - } - # correct for grace period. - my $days_minus_grace = $daystocharge - $data->{'firstremind'}; - if ($data->{'chargeperiod'} > 0 && $days_minus_grace > 0 ) { - $amount = int($daystocharge / $data->{'chargeperiod'}) * $data->{'fine'}; + 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->day; + } + my $days_minus_grace = $chargeable_units - $data->{firstremind}; + if ($data->{'chargeperiod'} && $days_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. } - $amount = C4::Context->preference('maxFine') if(C4::Context->preference('maxFine') && ( $amount > C4::Context->preference('maxFine'))); - $debug and warn sprintf("CalcFine returning (%s, %s, %s, %s)", $amount, $data->{'chargename'}, $days_minus_grace, $daystocharge); - return ($amount, $data->{'chargename'}, $days_minus_grace, $daystocharge); + if(C4::Context->preference('maxFine') && ( $amount > C4::Context->preference('maxFine'))) { + $amount = C4::Context->preference('maxFine'); + } + return ($amount, $data->{chargename}, $days_minus_grace); # FIXME: chargename is NEVER populated anywhere. } diff --git a/misc/cronjobs/advance_notices.pl b/misc/cronjobs/advance_notices.pl index 09c4017558..de00846039 100755 --- a/misc/cronjobs/advance_notices.pl +++ b/misc/cronjobs/advance_notices.pl @@ -54,7 +54,7 @@ use C4::Letters; use C4::Members; use C4::Members::Messaging; use C4::Overdues; -use C4::Dates qw/format_date/; +use Koha::DateUtils; # These are defaults for command line options. @@ -347,6 +347,12 @@ sub parse_letter { ); } +sub format_date { + my $date_string = shift; + my $dt=dt_from_string($date_string); + return output_pref($dt); +} + 1; __END__ diff --git a/misc/cronjobs/fines.pl b/misc/cronjobs/fines.pl index 73cdf9f81b..81e77a7e32 100755 --- a/misc/cronjobs/fines.pl +++ b/misc/cronjobs/fines.pl @@ -37,16 +37,19 @@ BEGIN { eval { require "$FindBin::Bin/kohalib.pl" }; } -use Date::Calc qw/Date_to_Days/; +#use Date::Calc qw/Date_to_Days/; use C4::Context; use C4::Circulation; use C4::Overdues; -use C4::Calendar qw(); # don't need any exports from Calendar use C4::Biblio; use C4::Debug; # supplying $debug and $cgi_debug use Getopt::Long; +use Koha::Calendar; +use Koha::DateUtils; + + my $help = 0; my $verbose = 0; my $output_dir; @@ -54,7 +57,7 @@ my $output_dir; GetOptions( 'h|help' => \$help, 'v|verbose' => \$verbose, 'o|out:s' => \$output_dir, - ); + ); my $usage = << 'ENDUSAGE'; This script calculates and charges overdue fines @@ -73,7 +76,7 @@ ENDUSAGE die $usage if $help; use vars qw(@borrower_fields @item_fields @other_fields); -use vars qw($fldir $libname $control $mode $delim $dbname $today $today_iso $today_days); +use vars qw($fldir $libname $control $mode $delim $dbname ); use vars qw($filename); CHECK { @@ -99,9 +102,7 @@ INIT { my $data = Getoverdues(); my $overdueItemsCounted = 0; my %calendars = (); -$today = C4::Dates->new(); -$today_iso = $today->output('iso'); -$today_days = Date_to_Days(split(/-/,$today_iso)); +my $today = DateTime->now( time_zone => C4::Context->tz() ); if($output_dir){ $fldir = $output_dir if( -d $output_dir ); } else { @@ -112,16 +113,14 @@ if (!-d $fldir) { } $filename = $dbname; $filename =~ s/\W//; -$filename = $fldir . '/'. $filename . '_' . $today_iso . ".log"; +$filename = $fldir . '/'. $filename . '_' . $today->ymd() . ".log"; print "writing to $filename\n" if $verbose; open (FILE, ">$filename") or die "Cannot write file $filename: $!"; print FILE join $delim, (@borrower_fields, @item_fields, @other_fields); print FILE "\n"; for (my $i=0; $inew($data->[$i]->{'date_due'},'iso'); - my $datedue_days = Date_to_Days(split(/-/,$datedue->output('iso'))); - my $due_str = $datedue->output(); + my $datedue = dt_from_string($data->[$i]->{'date_due'}); unless (defined $data->[$i]->{'borrowernumber'}) { print STDERR "ERROR in Getoverdues line $i: issues.borrowernumber IS NULL. Repair 'issues' table now! Skipping record.\n"; next; # Note: this doesn't solve everything. After NULL borrowernumber, multiple issues w/ real borrowernumbers can pile up. @@ -133,22 +132,24 @@ for (my $i=0; $inew(branchcode => $branchcode); + $calendars{$branchcode} = Koha::Calendar->new(branchcode => $branchcode); } $calendar = $calendars{$branchcode}; - my $isHoliday = $calendar->isHoliday(split '/', $today->output('metric')); + my $isHoliday = $calendar->is_holiday($today); - ($datedue_days <= $today_days) or next; # or it's not overdue, right? + if ( DateTime->compare($datedue, $today) != 1) { + next; # not overdue + } $overdueItemsCounted++; - my ($amount,$type,$daycounttotal,$daycount)= - CalcFine($data->[$i], $borrower->{'categorycode'}, $branchcode,undef,undef, $datedue, $today); + my ($amount,$type,$daycounttotal)= + CalcFine($data->[$i], $borrower->{'categorycode'}, $branchcode, $datedue, $today); # FIXME: $type NEVER gets populated by anything. - (defined $type) or $type = ''; + $type ||= ''; # Don't update the fine if today is a holiday. # This ensures that dropbox mode will remove the correct amount of fine. if ($mode eq 'production' and ! $isHoliday) { - UpdateFine($data->[$i]->{'itemnumber'},$data->[$i]->{'borrowernumber'},$amount,$type,$due_str) if( $amount > 0 ) ; + UpdateFine($data->[$i]->{'itemnumber'},$data->[$i]->{'borrowernumber'},$amount,$type,output_pref($datedue)) if( $amount > 0 ) ; } my @cells = (); push @cells, map {$borrower->{$_}} @borrower_fields; @@ -160,7 +161,7 @@ for (my $i=0; $iymd() -- Saved to $filename Number of Overdue Items: counted $overdueItemsCounted reported $numOverdueItems