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 afccdb5..2935b24 100644 (file)
--- 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 09c4017..de00846 100755 (executable)
--- 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 73cdf9f..81e77a7 100755 (executable)
--- 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; $i<scalar(@$data); $i++) {
-    my $datedue = C4::Dates->new($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; $i<scalar(@$data); $i++) {
     # In final case, CircControl must be PickupLibrary. (branchcode comes from issues table here).
     my $calendar;
     unless (defined ($calendars{$branchcode})) {
-        $calendars{$branchcode} = C4::Calendar->new(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; $i<scalar(@$data); $i++) {
 my $numOverdueItems = scalar(@$data);
 if ($verbose) {
    print <<EOM;
-Fines assessment -- $today_iso -- Saved to $filename
+Fines assessment -- $today->ymd() -- Saved to $filename
 Number of Overdue Items:
      counted $overdueItemsCounted
     reported $numOverdueItems