[Koha-patches] [PATCH] Fines fixes: apparent problems with fines prevent processing.
Joe Atzberger
joe.atzberger at liblime.com
Tue Aug 26 05:57:01 CEST 2008
CalcFine returned values that mismatched expectations in fines.pl.
fines.pl refactored: added debugging, prevent needless recreation of
Calendar objects by storing them in hash by branch.
Still outstanding problems with fines, including the output of a field
that has no other references in Koha (so is always undef) and the
incorrect description of FinesMode.
Calendar exported "new" erroneously. I also cleaned up the queries to
avoid needlessly compiling additional statement handles.
Please test and consider application to 3.0 maintenance.
---
C4/Calendar.pm | 139 +++++++++++++++++---------------------------
C4/Overdues.pm | 29 +++++----
misc/cronjobs/fines.pl | 136 +++++++++++++++++++++++++------------------
t/lib/KohaTest/Calendar.pm | 2 -
4 files changed, 150 insertions(+), 156 deletions(-)
diff --git a/C4/Calendar.pm b/C4/Calendar.pm
index a279e2f..8accf2b 100644
--- a/C4/Calendar.pm
+++ b/C4/Calendar.pm
@@ -16,13 +16,32 @@ package C4::Calendar;
# Suite 330, Boston, MA 02111-1307 USA
use strict;
-require Exporter;
use vars qw($VERSION @EXPORT);
+use Carp;
use Date::Calc qw( Date_to_Days );
-# set the version for version checking
-$VERSION = 3.00;
+use C4::Context;
+
+BEGIN {
+ # set the version for version checking
+ $VERSION = 3.01;
+ require Exporter;
+ @EXPORT = qw(
+ &get_week_days_holidays
+ &get_day_month_holidays
+ &get_exception_holidays
+ &get_single_holidays
+ &insert_week_day_holiday
+ &insert_day_month_holiday
+ &insert_single_holiday
+ &insert_exception_holiday
+ &delete_holiday
+ &isHoliday
+ &addDate
+ &daysBetween
+ );
+}
=head1 NAME
@@ -40,123 +59,73 @@ This package is used to deal with holidays. Through this package, you can set al
=over 2
-=cut
-
- at EXPORT = qw(&new
- &change_branchcode
- &get_week_days_holidays
- &get_day_month_holidays
- &get_exception_holidays
- &get_single_holidays
- &insert_week_day_holiday
- &insert_day_month_holiday
- &insert_single_holiday
- &insert_exception_holiday
- &delete_holiday
- &isHoliday
- &addDate
- &daysBetween);
-
=item new
$calendar = C4::Calendar->new(branchcode => $branchcode);
-C<$branchcode> Is the branch code wich you want to use calendar.
+Each library branch has its own Calendar.
+C<$branchcode> specifies which Calendar you want.
=cut
sub new {
my $classname = shift @_;
my %options = @_;
-
- my %hash;
- my $self = bless(\%hash, $classname);
-
+ my $self = bless({}, $classname);
foreach my $optionName (keys %options) {
$self->{lc($optionName)} = $options{$optionName};
}
-
- $self->_init;
-
+ defined($self->{branchcode}) or croak "No branchcode argument to new. Should be C4::Calendar->new(branchcode => \$branchcode)";
+ $self->_init($self->{branchcode});
return $self;
}
sub _init {
my $self = shift @_;
-
+ my $branch = shift;
+ defined($branch) or die "No branchcode sent to _init"; # must test for defined here and above to allow ""
my $dbh = C4::Context->dbh();
- my $week_days_sql = $dbh->prepare( 'SELECT weekday, title, description
- FROM repeatable_holidays
- WHERE ( branchcode = ? )
- AND (NOT(ISNULL(weekday)))' );
- $week_days_sql->execute( $self->{'branchcode'} );
+ my $repeatable = $dbh->prepare( 'SELECT *
+ FROM repeatable_holidays
+ WHERE ( branchcode = ? )
+ AND (ISNULL(weekday) = ?)' );
+ $repeatable->execute($branch,0);
my %week_days_holidays;
- while (my ($weekday, $title, $description) = $week_days_sql->fetchrow) {
- $week_days_holidays{$weekday}{title} = $title;
- $week_days_holidays{$weekday}{description} = $description;
+ while (my $row = $repeatable->fetchrow_hashref) {
+ my $key = $row->{weekday};
+ $week_days_holidays{$key}{title} = $row->{title};
+ $week_days_holidays{$key}{description} = $row->{description};
}
- $week_days_sql->finish;
$self->{'week_days_holidays'} = \%week_days_holidays;
- my $day_month_sql = $dbh->prepare( 'SELECT day, month, title, description
- FROM repeatable_holidays
- WHERE ( branchcode = ? )
- AND ISNULL(weekday)' );
- $day_month_sql->execute( $self->{'branchcode'} );
+ $repeatable->execute($branch,1);
my %day_month_holidays;
- while (my ($day, $month, $title, $description) = $day_month_sql->fetchrow) {
- $day_month_holidays{"$month/$day"}{title} = $title;
- $day_month_holidays{"$month/$day"}{description} = $description;
+ while (my $row = $repeatable->fetchrow_hashref) {
+ my $key = $row->{month} . "/" . $row->{day};
+ $day_month_holidays{$key}{title} = $row->{title};
+ $day_month_holidays{$key}{description} = $row->{description}
}
- $day_month_sql->finish;
$self->{'day_month_holidays'} = \%day_month_holidays;
- my $exception_holidays_sql = $dbh->prepare( 'SELECT day, month, year, title, description
- FROM special_holidays
- WHERE ( branchcode = ? )
- AnD (isexception = 1)' );
- $exception_holidays_sql->execute( $self->{'branchcode'} );
+ my $special = $dbh->prepare( 'SELECT day, month, year, title, description
+ FROM special_holidays
+ WHERE ( branchcode = ? )
+ AND (isexception = ?)' );
+ $special->execute($branch,1);
my %exception_holidays;
- while (my ($day, $month, $year, $title, $description) = $exception_holidays_sql->fetchrow) {
+ while (my ($day, $month, $year, $title, $description) = $special->fetchrow) {
$exception_holidays{"$year/$month/$day"}{title} = $title;
$exception_holidays{"$year/$month/$day"}{description} = $description;
}
- $exception_holidays_sql->finish;
$self->{'exception_holidays'} = \%exception_holidays;
- my $holidays_sql = $dbh->prepare( 'SELECT day, month, year, title, description
- FROM special_holidays
- WHERE ( branchcode = ? )
- AND (isexception = 0)' );
- $holidays_sql->execute( $self->{'branchcode'} );
+ $special->execute($branch,0);
my %single_holidays;
- while (my ($day, $month, $year, $title, $description) = $holidays_sql->fetchrow) {
+ while (my ($day, $month, $year, $title, $description) = $special->fetchrow) {
$single_holidays{"$year/$month/$day"}{title} = $title;
$single_holidays{"$year/$month/$day"}{description} = $description;
}
- $holidays_sql->finish;
$self->{'single_holidays'} = \%single_holidays;
-}
-
-=item change_branchcode
-
- $calendar->change_branchcode(branchcode => $branchcode)
-
-Change the calendar branch code. This means to change the holidays structure.
-
-C<$branchcode> Is the branch code wich you want to use calendar.
-
-=cut
-
-sub change_branchcode {
- my ($self, $branchcode) = @_;
- my %options = @_;
-
- foreach my $optionName (keys %options) {
- $self->{lc($optionName)} = $options{$optionName};
- }
- $self->_init;
-
return $self;
}
@@ -456,10 +425,10 @@ sub isHoliday {
$year=$year+0;
$day=$day+0;
my $weekday = &Date::Calc::Day_of_Week($year, $month, $day) % 7;
- my $weekDays = $self->get_week_days_holidays();
- my $dayMonths = $self->get_day_month_holidays();
+ my $weekDays = $self->get_week_days_holidays();
+ my $dayMonths = $self->get_day_month_holidays();
my $exceptions = $self->get_exception_holidays();
- my $singles = $self->get_single_holidays();
+ my $singles = $self->get_single_holidays();
if (defined($exceptions->{"$year/$month/$day"})) {
return 0;
} else {
diff --git a/C4/Overdues.pm b/C4/Overdues.pm
index bacd80a..63b0842 100644
--- a/C4/Overdues.pm
+++ b/C4/Overdues.pm
@@ -116,21 +116,22 @@ Koha database.
#'
sub Getoverdues {
my $params = shift;
-
my $dbh = C4::Context->dbh;
my $statement;
if ( C4::Context->preference('item-level_itypes') ) {
$statement = "
-SELECT issues.*,items.itype as itemtype, items.homebranch FROM issues
-LEFT JOIN items USING (itemnumber)
-WHERE date_due < now()
+ SELECT issues.*, items.itype as itemtype, items.homebranch, items.barcode
+ FROM issues
+LEFT JOIN items USING (itemnumber)
+ WHERE date_due < now()
";
} else {
$statement = "
-SELECT issues.*,biblioitems.itemtype,items.itype, items.homebranch FROM issues
- LEFT JOIN items USING (itemnumber)
- LEFT JOIN biblioitems USING (biblioitemnumber)
- WHERE date_due < now()
+ SELECT issues.*, biblioitems.itemtype, items.itype, items.homebranch, items.barcode
+ FROM issues
+LEFT JOIN items USING (itemnumber)
+LEFT JOIN biblioitems USING (biblioitemnumber)
+ WHERE date_due < now()
";
}
@@ -237,11 +238,10 @@ 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'),
+ ($item ? '{item}' : 'UNDEF'),
($bortype || 'UNDEF'),
($branchcode || 'UNDEF'),
($difference || 'UNDEF'),
@@ -253,7 +253,8 @@ sub CalcFine {
my $amount = 0;
my $daystocharge;
# get issuingrules (fines part will be used)
- my $data = C4::Circulation::GetIssuingRule($bortype, $item->{'itemtype'},$branchcode);
+ $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 ).
@@ -264,7 +265,7 @@ sub CalcFine {
} 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 );
+ 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')));
@@ -278,7 +279,9 @@ sub CalcFine {
# a zero (or null) chargeperiod means no charge.
}
$amount = C4::Context->preference('maxFine') if(C4::Context->preference('maxFine') && ( $amount > C4::Context->preference('maxFine')));
- return ( $amount, $data->{'chargename'}, $days_minus_grace, $daystocharge);
+ $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);
+ # FIXME: chargename is NEVER populated anywhere.
}
diff --git a/misc/cronjobs/fines.pl b/misc/cronjobs/fines.pl
index afc4dca..0e28357 100755
--- a/misc/cronjobs/fines.pl
+++ b/misc/cronjobs/fines.pl
@@ -26,83 +26,107 @@
# Suite 330, Boston, MA 02111-1307 USA
+# FIXME: use FinesMode as described or change syspref description
use strict;
+
BEGIN {
# find Koha's Perl modules
# test carefully before changing this
use FindBin;
eval { require "$FindBin::Bin/kohalib.pl" };
}
+
+use Date::Calc qw/Date_to_Days/;
+
use C4::Context;
use C4::Circulation;
use C4::Overdues;
-use C4::Calendar;
-use Date::Calc qw/Date_to_Days/;
+use C4::Calendar qw(); # don't need any exports from Calendar
use C4::Biblio;
+use C4::Debug; # supplying $debug and $cgi_debug
+
+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($filename $summary);
+
+CHECK {
+ @borrower_fields = qw(cardnumber categorycode surname firstname email phone address citystate);
+ @item_fields = qw(itemnumber barcode date_due);
+ @other_fields = qw(type days_overdue fine);
+ $libname = C4::Context->preference('LibraryName');
+ $control = C4::Context->preference('CircControl');
+ $mode = C4::Context->preference('finesMode');
+ $dbname = C4::Context->config('database');
+ $delim = "\t"; # ? C4::Context->preference('delimiter') || "\t";
+
+ $today = C4::Dates->new();
+ $today_iso = $today->output('iso');
+ $today_days = Date_to_Days(split(/-/,$today_iso));
+ $fldir = $ENV{TMPDIR} || "/tmp"; # TODO: use GetOpt
+ $filename = $dbname;
+ $filename =~ s/\W//;
+ $filename = $fldir . '/'. $filename . '_' . $today_iso . ".log";
+ $summary = 1; # TODO: use GetOpt
+}
+INIT {
+ $debug and print "Each line will contain the following fields:\n",
+ "From borrowers : ", join(', ', @borrower_fields), "\n",
+ "From items : ", join(', ', @item_fields), "\n",
+ "Per overdue: ", join(', ', @other_fields), "\n",
+ "Delimiter: '$delim'\n";
+}
-my $fldir = "/tmp" ;
-
-my $libname=C4::Context->preference('LibraryName');
-my $dbname= C4::Context->config('database');
-
-my $today = C4::Dates->new();
-my $datestr = $today->output('iso');
-my $today_days= Date_to_Days(split(/-/,$today->output('iso')));
-my $filename= $dbname;
-$filename =~ s/\W//;
-$filename = $fldir . '/'. $filename . $datestr . ".log";
-open (FILE,">$filename") || die "Can't open LOG";
-print FILE "cardnumber\tcategory\tsurname\tfirstname\temail\tphone\taddress\tcitystate\tbarcode\tdate_due\ttype\titemnumber\tdays_overdue\tfine\n";
-
-
-my $DEBUG =1;
-
-my $data=Getoverdues();
-my $overdueItemsCounted=0 ;
-my $borrowernumber;
-
-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 $borrower=BorType($data->[$i]->{'borrowernumber'});
- my $branchcode;
- if ( C4::Context->preference('CircControl') eq 'ItemHomeLibrary' ) {
- $branchcode = $data->[$i]->{'homebranch'};
- } elsif ( C4::Context->preference('CircControl') eq 'PatronLibrary' ) {
- $branchcode = $borrower->{'branchcode'};
-} else {
- # CircControl must be PickupLibrary. (branchcode comes from issues table here).
- $branchcode = $data->[$i]->{'branchcode'};
- }
- my $calendar = C4::Calendar->new( branchcode => $branchcode );
-
- my $isHoliday = $calendar->isHoliday( split( '/', C4::Dates->new()->output('metric') ) );
+open (FILE, ">$filename") or die "Cannot write file $filename: $!";
+print FILE join $delim, (@borrower_fields, @item_fields, @other_fields);
+print FILE "\n";
+
+my $data = Getoverdues();
+my $overdueItemsCounted = 0;
+my %calendars = ();
+
+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 $borrower = BorType($data->[$i]->{'borrowernumber'});
+ my $branchcode = ($control eq 'ItemHomeLibrary') ? $data->[$i]->{homebranch} :
+ ($control eq 'PatronLibrary' ) ? $borrower->{branchcode} :
+ $data->[$i]->{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);
+ }
+ $calendar = $calendars{$branchcode};
+ my $isHoliday = $calendar->isHoliday(split '/', $today->output('metric'));
- if ($datedue_days <= $today_days){
- $overdueItemsCounted++ if $DEBUG;
- my $difference=$today_days - $datedue_days;
- my ($amount,$type,$printout,$daycounttotal,$daycount)=
- CalcFine($data->[$i], $borrower->{'categorycode'}, $branchcode,undef,undef, $datedue ,$today);
- my ($delays1,$delays2,$delays3)=GetOverdueDelays($borrower->{'categorycode'});
+ ($datedue_days <= $today_days) or next; # or it's not overdue, right?
+ $overdueItemsCounted++;
+ my ($amount,$type,$daycounttotal,$daycount)=
+ CalcFine($data->[$i], $borrower->{'categorycode'}, $branchcode,undef,undef, $datedue, $today);
+ # FIXME: $type NEVER gets populated by anything.
+ (defined $type) or $type = '';
# Don't update the fine if today is a holiday.
# This ensures that dropbox mode will remove the correct amount of fine.
- if( (C4::Context->preference('finesMode') eq 'production') && ! $isHoliday ) {
- # FIXME - $type is always null, afaict.
+ if ($mode eq 'production' and ! $isHoliday) {
UpdateFine($data->[$i]->{'itemnumber'},$data->[$i]->{'borrowernumber'},$amount,$type,$due_str) if( $amount > 0 ) ;
}
- print FILE "$printout\t$borrower->{'cardnumber'}\t$borrower->{'categorycode'}\t$borrower->{'surname'}\t$borrower->{'firstname'}\t$borrower->{'email'}\t$borrower->{'phone'}\t$borrower->{'address'}\t$borrower->{'city'}\t$data->[$i]->{'barcode'}\t$data->[$i]->{'date_due'}\t$type\t$data->[$i]->{'itemnumber'}\t$daycounttotal\t$amount\n";
- }
+ my @cells = ();
+ push @cells, map {$borrower->{$_}} @borrower_fields;
+ push @cells, map {$data->[$i]->{$_}} @item_fields;
+ push @cells, $type, $daycounttotal, $amount;
+ print FILE join($delim, @cells), "\n";
}
-my $numOverdueItems=scalar(@$data);
-if ($DEBUG) {
- print <<EOM
-
-Number of Overdue Items counted $overdueItemsCounted
-Number of Overdue Items reported $numOverdueItems
+my $numOverdueItems = scalar(@$data);
+if ($summary) {
+ print <<EOM;
+Fines assessment -- $today_iso -- Saved to $filename
+Number of Overdue Items:
+ counted $overdueItemsCounted
+ reported $numOverdueItems
EOM
}
diff --git a/t/lib/KohaTest/Calendar.pm b/t/lib/KohaTest/Calendar.pm
index f1825af..8b1cda7 100644
--- a/t/lib/KohaTest/Calendar.pm
+++ b/t/lib/KohaTest/Calendar.pm
@@ -13,8 +13,6 @@ sub testing_class { 'C4::Calendar' };
sub methods : Test( 1 ) {
my $self = shift;
my @methods = qw( new
- _init
- change_branchcode
get_week_days_holidays
get_day_month_holidays
get_exception_holidays
--
1.5.5.GIT
More information about the Koha-patches
mailing list