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
 
 
 =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.
 
 
 Calculates the fine for a book.
 


@@ -229,13 +229,8 @@ the book.
 
 C<$branchcode> is the library (string) whose issuingrules govern this transaction.
 
 
 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.
 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.
 
 
 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<$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",
 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 {
 =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 $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.
     }
     } 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.
 }
 
     # 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::Members;
 use C4::Members::Messaging;
 use C4::Overdues;

-use C4::Dates qw/format_date/;
+use Koha::DateUtils;

 
 
 # These are defaults for command line options.
 
 
 # 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__
 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" };
 }
 
     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::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 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;
 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,
 GetOptions( 'h|help'    => \$help,
             'v|verbose' => \$verbose,
             'o|out:s'   => \$output_dir,

-       );
+    );

 my $usage = << 'ENDUSAGE';
 
 This script calculates and charges overdue fines
 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);
 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 {
 use vars qw($filename);
 
 CHECK {


@@ -99,9 +102,7 @@ INIT {
 my $data = Getoverdues();
 my $overdueItemsCounted = 0;
 my %calendars = ();
 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 {
 if($output_dir){
     $fldir = $output_dir if( -d $output_dir );
 } else {


@@ -112,16 +113,14 @@ if (!-d $fldir) {
 }
 $filename = $dbname;
 $filename =~ s/\W//;
 }
 $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++) {
 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.
     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})) {
     # 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};
     }
     $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++;
 
     $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.
         # 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) {
        # 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;
        }
     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;
 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
 Number of Overdue Items:
      counted $overdueItemsCounted
     reported $numOverdueItems