Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Fines fixes: apparent problems with fines prevent processing.

Browse source 
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.

Signed-off-by: Galen Charlton <galen.charlton@liblime.com>
This commit is contained in:
Joe Atzberger 2008-08-25 22:57:01 -05:00 committed by Galen Charlton
parent 9eb1465b28
commit aa4c6ff62f
4 changed files with 154 additions and 160 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

139
C4/Calendar.pm
View file

@ -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
@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 {

29
C4/Overdues.pm
View file

@ -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.
}

144
misc/cronjobs/fines.pl
View file

@ -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);
my $fldir = "/tmp" ;
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";
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') ) );
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'});
# 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.
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";
}
$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
}
my $numOverdueItems=scalar(@$data);
if ($DEBUG) {
print <<EOM
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";
}
Number of Overdue Items counted $overdueItemsCounted
Number of Overdue Items reported $numOverdueItems
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'));
($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 ($mode eq 'production' and ! $isHoliday) {
UpdateFine($data->[$i]->{'itemnumber'},$data->[$i]->{'borrowernumber'},$amount,$type,$due_str) if( $amount > 0 ) ;
}
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 ($summary) {
print <<EOM;
Fines assessment -- $today_iso -- Saved to $filename
Number of Overdue Items:
counted $overdueItemsCounted
reported $numOverdueItems
EOM
}

2
t/lib/KohaTest/Calendar.pm
View file

@ -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