From 4da5a74e48d3a258e69a8af7b20f786c2c01a9d0 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Fri, 16 Jan 2015 09:07:48 -0500 Subject: [PATCH] Bug 13590 - Add ability to charge fines at start of charge period --- C4/Overdues.pm | 99 +++++++------ Koha/Schema/Result/Issuingrule.pm | 12 ++- admin/smart-rules.pl | 40 ++++-- installer/data/mysql/kohastructure.sql | 1 + installer/data/mysql/updatedatabase.pl | 8 + .../prog/en/modules/admin/smart-rules.tt | 8 + t/db_dependent/Circulation_Issuingrule.t | 153 ++++++++++---------- t/db_dependent/Fines.t | 56 +++++++ 8 files changed, 244 insertions(+), 133 deletions(-) create mode 100644 t/db_dependent/Fines.t diff --git a/C4/Overdues.pm b/C4/Overdues.pm index 1797923..3a50274 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -24,6 +24,8 @@ use strict; use Date::Calc qw/Today Date_to_Days/; use Date::Manip qw/UnixDate/; use List::MoreUtils qw( uniq ); +use POSIX qw( floor ceil ); +use Readonly; use C4::Circulation; use C4::Context; @@ -33,46 +35,51 @@ use C4::Debug; use vars qw($VERSION @ISA @EXPORT); +Readonly::Scalar our $INTERVAL_START => 1; +Readonly::Scalar our $INTERVAL_END => 0; + BEGIN { - # set the version for version checking + # set the version for version checking $VERSION = 3.07.00.049; - require Exporter; - @ISA = qw(Exporter); - # subs to rename (and maybe merge some...) - push @EXPORT, qw( - &CalcFine - &Getoverdues - &checkoverdues - &NumberNotifyId - &AmountNotify - &UpdateFine - &GetFine - - &CheckItemNotify - &GetOverduesForBranch - &RemoveNotifyLine - &AddNotifyLine - &GetOverdueMessageTransportTypes - ); - # subs to remove - push @EXPORT, qw( - &BorType - ); - - # check that an equivalent don't exist already before moving - - # subs to move to Circulation.pm - push @EXPORT, qw( - &GetIssuesIteminfo - ); - - # &GetIssuingRules - delete. - # use C4::Circulation::GetIssuingRule instead. - - # subs to move to Biblio.pm - push @EXPORT, qw( - &GetItems - ); + require Exporter; + @ISA = qw(Exporter); + + # subs to rename (and maybe merge some...) + push @EXPORT, qw( + &CalcFine + &Getoverdues + &checkoverdues + &NumberNotifyId + &AmountNotify + &UpdateFine + &GetFine + + &CheckItemNotify + &GetOverduesForBranch + &RemoveNotifyLine + &AddNotifyLine + &GetOverdueMessageTransportTypes + ); + + # subs to remove + push @EXPORT, qw( + &BorType + ); + + # check that an equivalent don't exist already before moving + + # subs to move to Circulation.pm + push @EXPORT, qw( + &GetIssuesIteminfo + ); + + # &GetIssuingRules - delete. + # use C4::Circulation::GetIssuingRule instead. + + # subs to move to Biblio.pm + push @EXPORT, qw( + &GetItems + ); } =head1 NAME @@ -251,15 +258,13 @@ sub CalcFine { my $chargeable_units = _get_chargeable_units($fine_unit, $start_date, $end_date, $branchcode); my $units_minus_grace = $chargeable_units - $data->{firstremind}; my $amount = 0; - if ($data->{'chargeperiod'} && ($units_minus_grace > 0) ) { - if ( C4::Context->preference('FinesIncludeGracePeriod') ) { - $amount = int($chargeable_units / $data->{'chargeperiod'}) * $data->{'fine'};# TODO fine calc should be in cents - } else { - $amount = int($units_minus_grace / $data->{'chargeperiod'}) * $data->{'fine'}; - } - } else { - # a zero (or null) chargeperiod or negative units_minus_grace value means no charge. - } + if ( $data->{'chargeperiod'} && ( $units_minus_grace > 0 ) ) { + my $units = C4::Context->preference('FinesIncludeGracePeriod') ? $chargeable_units : $units_minus_grace; + my $charge_periods = $units / $data->{'chargeperiod'}; + $charge_periods = $data->{'chargeperiod_charge_at'} == $INTERVAL_START ? ceil($charge_periods) : floor($charge_periods); + $amount = $charge_periods * $data->{'fine'}; + } # else { # a zero (or null) chargeperiod or negative units_minus_grace value means no charge. } + $amount = $data->{overduefinescap} if $data->{overduefinescap} && $amount > $data->{overduefinescap}; $debug and warn sprintf("CalcFine returning (%s, %s, %s, %s)", $amount, $data->{'chargename'}, $units_minus_grace, $chargeable_units); return ($amount, $data->{'chargename'}, $units_minus_grace, $chargeable_units); diff --git a/Koha/Schema/Result/Issuingrule.pm b/Koha/Schema/Result/Issuingrule.pm index a8cf014..2983ad3 100644 --- a/Koha/Schema/Result/Issuingrule.pm +++ b/Koha/Schema/Result/Issuingrule.pm @@ -80,6 +80,12 @@ __PACKAGE__->table("issuingrules"); data_type: 'integer' is_nullable: 1 +=head2 chargeperiod_charge_at + + data_type: 'tinyint' + default_value: 0 + is_nullable: 0 + =head2 accountsent data_type: 'integer' @@ -184,6 +190,8 @@ __PACKAGE__->add_columns( { data_type => "integer", is_nullable => 1 }, "chargeperiod", { data_type => "integer", is_nullable => 1 }, + "chargeperiod_charge_at", + { data_type => "tinyint", default_value => 0, is_nullable => 0 }, "accountsent", { data_type => "integer", is_nullable => 1 }, "chargename", @@ -236,8 +244,8 @@ __PACKAGE__->add_columns( __PACKAGE__->set_primary_key("branchcode", "categorycode", "itemtype"); -# Created by DBIx::Class::Schema::Loader v0.07039 @ 2014-09-17 21:06:58 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:vmlv13CeQ99RO0VsviRABg +# Created by DBIx::Class::Schema::Loader v0.07040 @ 2015-01-16 07:20:41 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:6AKNiL/+m3EyTjvAAOz9Cw # You can replace this text with custom content, and it will be preserved on regeneration diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index 942e9bf..63c9919 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -27,6 +27,7 @@ use C4::Koha; use C4::Debug; use C4::Branch; # GetBranches use C4::Dates qw/format_date format_date_in_iso/; +use Koha::Database; my $input = CGI->new; my $dbh = C4::Context->dbh; @@ -100,10 +101,6 @@ elsif ($op eq 'delete-branch-item') { } # save the values entered elsif ($op eq 'add') { - my $sth_search = $dbh->prepare('SELECT COUNT(*) AS total FROM issuingrules WHERE branchcode=? AND categorycode=? AND itemtype=?'); - my $sth_insert = $dbh->prepare('INSERT INTO issuingrules (branchcode, categorycode, itemtype, maxissueqty, renewalsallowed, renewalperiod, norenewalbefore, auto_renew, reservesallowed, issuelength, lengthunit, hardduedate, hardduedatecompare, fine, finedays, maxsuspensiondays, firstremind, chargeperiod,rentaldiscount, overduefinescap) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)'); - my $sth_update=$dbh->prepare("UPDATE issuingrules SET fine=?, finedays=?, maxsuspensiondays=?, firstremind=?, chargeperiod=?, maxissueqty=?, renewalsallowed=?, renewalperiod=?, norenewalbefore=?, auto_renew=?, reservesallowed=?, issuelength=?, lengthunit = ?, hardduedate=?, hardduedatecompare=?, rentaldiscount=?, overduefinescap=? WHERE branchcode=? AND categorycode=? AND itemtype=?"); - my $br = $branch; # branch my $bor = $input->param('categorycode'); # borrower category my $cat = $input->param('itemtype'); # item type @@ -113,6 +110,7 @@ elsif ($op eq 'add') { $maxsuspensiondays = undef if $maxsuspensiondays eq q||; my $firstremind = $input->param('firstremind'); my $chargeperiod = $input->param('chargeperiod'); + my $chargeperiod_charge_at = $input->param('chargeperiod_charge_at'); my $maxissueqty = $input->param('maxissueqty'); my $renewalsallowed = $input->param('renewalsallowed'); my $renewalperiod = $input->param('renewalperiod'); @@ -131,13 +129,33 @@ elsif ($op eq 'add') { my $overduefinescap = $input->param('overduefinescap') || undef; $debug and warn "Adding $br, $bor, $cat, $fine, $maxissueqty"; - $sth_search->execute($br,$bor,$cat); - my $res = $sth_search->fetchrow_hashref(); - if ($res->{total}) { - $sth_update->execute($fine, $finedays, $maxsuspensiondays, $firstremind, $chargeperiod, $maxissueqty, $renewalsallowed, $renewalperiod, $norenewalbefore, $auto_renew, $reservesallowed, $issuelength,$lengthunit, $hardduedate,$hardduedatecompare,$rentaldiscount,$overduefinescap, $br,$bor,$cat); - } else { - $sth_insert->execute($br,$bor,$cat,$maxissueqty,$renewalsallowed, $renewalperiod, $norenewalbefore, $auto_renew, $reservesallowed,$issuelength,$lengthunit,$hardduedate,$hardduedatecompare,$fine,$finedays, $maxsuspensiondays, $firstremind,$chargeperiod,$rentaldiscount,$overduefinescap); - } + my $rs = Koha::Database->new()->schema()->resultset('Issuingrule'); + + $rs->update_or_create( + { + branchcode => $br, + categorycode => $bor, + itemtype => $cat, + fine => $fine, + finedays => $finedays, + maxsuspensiondays => $maxsuspensiondays, + firstremind => $firstremind, + chargeperiod => $chargeperiod, + chargeperiod_charge_at => $chargeperiod_charge_at, + maxissueqty => $maxissueqty, + renewalsallowed => $renewalsallowed, + renewalperiod => $renewalperiod, + norenewalbefore => $norenewalbefore, + auto_renew => $auto_renew, + reservesallowed => $reservesallowed, + issuelength => $issuelength, + lengthunit => $lengthunit, + hardduedate => $hardduedate, + hardduedatecompare => $hardduedatecompare, + rentaldiscount => $rentaldiscount, + overduefinescap => $overduefinescap, + } + ); } elsif ($op eq "set-branch-defaults") { my $categorycode = $input->param('categorycode'); diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index facde44..8897960 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1178,6 +1178,7 @@ CREATE TABLE `issuingrules` ( -- circulation and fine rules `maxsuspensiondays` int(11) default NULL, -- max suspension days `firstremind` int(11) default NULL, -- fine grace period `chargeperiod` int(11) default NULL, -- how often the fine amount is charged + `chargeperiod_charge_at` tinyint(1) NOT NULL DEFAULT '0', -- Should fine be given at the start ( 1 ) or the end ( 0 ) of the period `accountsent` int(11) default NULL, -- not used? always NULL `chargename` varchar(100) default NULL, -- not used? always NULL `maxissueqty` int(4) default NULL, -- total number of checkouts allowed diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index 4653c02..0c56701 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -9669,6 +9669,14 @@ if ( CheckVersion($DBversion) ) { SetVersion ($DBversion); } +$DBversion = "3.19.00.XXX"; +if ( CheckVersion($DBversion) ) { + $dbh->do("ALTER TABLE issuingrules ADD chargeperiod_charge_at BOOLEAN NOT NULL DEFAULT '0' AFTER chargeperiod"); + + print "Upgrade to $DBversion done (Bug 11944 - Convert DB tables to utf8_unicode_ci)\n"; + SetVersion ($DBversion); +} + =head1 FUNCTIONS =head2 TableExists($table) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt index 8bc5809..3ed1674 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt @@ -147,6 +147,7 @@ for="tobranch">Clone these rules to: Clone these rules to: [% rule.finedays %] @@ -261,6 +263,12 @@ for="tobranch">Clone these rules to: + + + diff --git a/t/db_dependent/Circulation_Issuingrule.t b/t/db_dependent/Circulation_Issuingrule.t index 6e4f44f..5f6623e 100644 --- a/t/db_dependent/Circulation_Issuingrule.t +++ b/t/db_dependent/Circulation_Issuingrule.t @@ -108,82 +108,85 @@ $dbh->do( #Test GetIssuingRule my $sampleissuingrule1 = { - reservecharge => '0.000000', - chargename => 'Null', - restrictedtype => 0, - accountsent => 0, - maxissueqty => 5, - finedays => 0, - lengthunit => 'Null', - renewalperiod => 5, - norenewalbefore => 6, - auto_renew => 0, - issuelength => 5, - chargeperiod => 0, - rentaldiscount => '2.000000', - reservesallowed => 0, - hardduedate => '2013-01-01', - branchcode => $samplebranch1->{branchcode}, - fine => '0.000000', - hardduedatecompare => 5, - overduefinescap => '0.000000', - renewalsallowed => 0, - firstremind => 0, - itemtype => 'BOOK', - categorycode => $samplecat->{categorycode}, - maxsuspensiondays => 0, + reservecharge => '0.000000', + chargename => 'Null', + restrictedtype => 0, + accountsent => 0, + maxissueqty => 5, + finedays => 0, + lengthunit => 'Null', + renewalperiod => 5, + norenewalbefore => 6, + auto_renew => 0, + issuelength => 5, + chargeperiod => 0, + chargeperiod_charge_at => 0, + rentaldiscount => '2.000000', + reservesallowed => 0, + hardduedate => '2013-01-01', + branchcode => $samplebranch1->{branchcode}, + fine => '0.000000', + hardduedatecompare => 5, + overduefinescap => '0.000000', + renewalsallowed => 0, + firstremind => 0, + itemtype => 'BOOK', + categorycode => $samplecat->{categorycode}, + maxsuspensiondays => 0, }; my $sampleissuingrule2 = { - branchcode => $samplebranch2->{branchcode}, - categorycode => $samplecat->{categorycode}, - itemtype => 'BOOK', - maxissueqty => 2, - renewalsallowed => 'Null', - renewalperiod => 2, - norenewalbefore => 7, - auto_renew => 0, - reservesallowed => 'Null', - issuelength => 2, - lengthunit => 'Null', - hardduedate => 2, - hardduedatecompare => 'Null', - fine => 'Null', - finedays => 'Null', - firstremind => 'Null', - chargeperiod => 'Null', - rentaldiscount => 2.00, - overduefinescap => 'Null', - accountsent => 'Null', - reservecharge => 'Null', - chargename => 'Null', - restrictedtype => 'Null', - maxsuspensiondays => 0, + branchcode => $samplebranch2->{branchcode}, + categorycode => $samplecat->{categorycode}, + itemtype => 'BOOK', + maxissueqty => 2, + renewalsallowed => 'Null', + renewalperiod => 2, + norenewalbefore => 7, + auto_renew => 0, + reservesallowed => 'Null', + issuelength => 2, + lengthunit => 'Null', + hardduedate => 2, + hardduedatecompare => 'Null', + fine => 'Null', + finedays => 'Null', + firstremind => 'Null', + chargeperiod => 'Null', + chargeperiod_charge_at => 0, + rentaldiscount => 2.00, + overduefinescap => 'Null', + accountsent => 'Null', + reservecharge => 'Null', + chargename => 'Null', + restrictedtype => 'Null', + maxsuspensiondays => 0, }; my $sampleissuingrule3 = { - branchcode => $samplebranch1->{branchcode}, - categorycode => $samplecat->{categorycode}, - itemtype => 'DVD', - maxissueqty => 3, - renewalsallowed => 'Null', - renewalperiod => 3, - norenewalbefore => 8, - auto_renew => 0, - reservesallowed => 'Null', - issuelength => 3, - lengthunit => 'Null', - hardduedate => 3, - hardduedatecompare => 'Null', - fine => 'Null', - finedays => 'Null', - firstremind => 'Null', - chargeperiod => 'Null', - rentaldiscount => 3.00, - overduefinescap => 'Null', - accountsent => 'Null', - reservecharge => 'Null', - chargename => 'Null', - restrictedtype => 'Null', - maxsuspensiondays => 0, + branchcode => $samplebranch1->{branchcode}, + categorycode => $samplecat->{categorycode}, + itemtype => 'DVD', + maxissueqty => 3, + renewalsallowed => 'Null', + renewalperiod => 3, + norenewalbefore => 8, + auto_renew => 0, + reservesallowed => 'Null', + issuelength => 3, + lengthunit => 'Null', + hardduedate => 3, + hardduedatecompare => 'Null', + fine => 'Null', + finedays => 'Null', + firstremind => 'Null', + chargeperiod => 'Null', + chargeperiod_charge_at => 0, + rentaldiscount => 3.00, + overduefinescap => 'Null', + accountsent => 'Null', + reservecharge => 'Null', + chargename => 'Null', + restrictedtype => 'Null', + maxsuspensiondays => 0, }; $query = 'INSERT INTO issuingrules ( branchcode, @@ -203,6 +206,7 @@ $query = 'INSERT INTO issuingrules ( finedays, firstremind, chargeperiod, + chargeperiod_charge_at, rentaldiscount, overduefinescap, accountsent, @@ -210,7 +214,7 @@ $query = 'INSERT INTO issuingrules ( chargename, restrictedtype, maxsuspensiondays - ) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)'; + ) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)'; my $sth = $dbh->prepare($query); $sth->execute( $sampleissuingrule1->{branchcode}, @@ -230,6 +234,7 @@ $sth->execute( $sampleissuingrule1->{finedays}, $sampleissuingrule1->{firstremind}, $sampleissuingrule1->{chargeperiod}, + $sampleissuingrule1->{chargeperiod_charge_at}, $sampleissuingrule1->{rentaldiscount}, $sampleissuingrule1->{overduefinescap}, $sampleissuingrule1->{accountsent}, @@ -256,6 +261,7 @@ $sth->execute( $sampleissuingrule2->{finedays}, $sampleissuingrule2->{firstremind}, $sampleissuingrule2->{chargeperiod}, + $sampleissuingrule2->{chargeperiod_charge_at}, $sampleissuingrule2->{rentaldiscount}, $sampleissuingrule2->{overduefinescap}, $sampleissuingrule2->{accountsent}, @@ -282,6 +288,7 @@ $sth->execute( $sampleissuingrule3->{finedays}, $sampleissuingrule3->{firstremind}, $sampleissuingrule3->{chargeperiod}, + $sampleissuingrule3->{chargeperiod_charge_at}, $sampleissuingrule3->{rentaldiscount}, $sampleissuingrule3->{overduefinescap}, $sampleissuingrule3->{accountsent}, diff --git a/t/db_dependent/Fines.t b/t/db_dependent/Fines.t new file mode 100644 index 0000000..1f273ba --- /dev/null +++ b/t/db_dependent/Fines.t @@ -0,0 +1,56 @@ +#!/usr/bin/perl + +use Modern::Perl; + +use C4::Context; +use C4::Overdues; +use Koha::Database; +use Koha::DateUtils; + +use Test::More tests => 5; + +#Start transaction +my $dbh = C4::Context->dbh; +my $schema = Koha::Database->new()->schema(); + +$dbh->{RaiseError} = 1; +$dbh->{AutoCommit} = 0; + +$dbh->do(q|DELETE FROM issuingrules|); + +my $issuingrule = $schema->resultset('Issuingrule')->create( + { + categorycode => '*', + itemtype => '*', + branchcode => '*', + fine => 1, + finedays => 0, + chargeperiod => 7, + chargeperiod_charge_at => 0, + lengthunit => 'days', + issuelength => 1, + } +); + +ok( $issuingrule, 'Issuing rule created' ); + +my $period_start = dt_from_string('2000-01-01'); +my $period_end = dt_from_string('2000-01-05'); + +my ( $fine ) = CalcFine( {}, q{}, q{}, $period_start, $period_end ); +is( $fine, 0, '4 days overdue, charge period 7 days, charge at end of interval gives fine of $0' ); + +$period_end = dt_from_string('2000-01-10'); +( $fine ) = CalcFine( {}, q{}, q{}, $period_start, $period_end ); +is( $fine, 1, '9 days overdue, charge period 7 days, charge at end of interval gives fine of $1' ); + +# Test charging fine at the *beginning* of each charge period +$issuingrule->update( { chargeperiod_charge_at => 1 } ); + +$period_end = dt_from_string('2000-01-05'); +( $fine ) = CalcFine( {}, q{}, q{}, $period_start, $period_end ); +is( $fine, 1, '4 days overdue, charge period 7 days, charge at start of interval gives fine of $1' ); + +$period_end = dt_from_string('2000-01-10'); +( $fine ) = CalcFine( {}, q{}, q{}, $period_start, $period_end ); +is( $fine, 2, '9 days overdue, charge period 7 days, charge at start of interval gives fine of $2' ); -- 1.7.2.5