Bugzilla – Attachment 43406 Details for
Bug 13590
Add ability to charge fines at start of charge period
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 13590 - Add ability to charge fines at start of charge period
Bug-13590---Add-ability-to-charge-fines-at-start-o.patch (text/plain), 17.31 KB, created by
Kyle M Hall (khall)
on 2015-10-14 17:18:50 UTC
(
hide
)
Description:
Bug 13590 - Add ability to charge fines at start of charge period
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2015-10-14 17:18:50 UTC
Size:
17.31 KB
patch
obsolete
>From 6a31993d37b8a683bc037fe23e11ee7fe330aba4 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Fri, 16 Jan 2015 09:07:48 -0500 >Subject: [PATCH] Bug 13590 - Add ability to charge fines at start of charge period > >Right now, Koha only charges fines at the end of a given charge period. >For example, let us assume a circulation rule has a charge period of one >week ( 7 days ) and a fine of $5. This means that an item can be overdue >for 6 days without accruing a fine. Koha should allow circulation rules >to be configured to place the charge at the start of the end of the >charge period so the library can decide when the fine should accrue. > >Test Plan: >1) Apply this patch >2) Run updatedatabase.pl >3) prove t/db_dependent/Circulation_Issuingrule.t >4) prove t/db_dependent/Circulation.t >5) prove t/db_dependent/Fines.t >6) Ensure you can still create/edit circulation rules >--- > C4/Overdues.pm | 97 ++++++++++---------- > Koha/Schema/Result/Issuingrule.pm | 8 ++ > admin/smart-rules.pl | 48 +++++----- > installer/data/mysql/atomicupdate/bug_13590.sql | 1 + > installer/data/mysql/kohastructure.sql | 1 + > .../prog/en/modules/admin/smart-rules.tt | 8 ++ > t/db_dependent/Circulation_Issuingrule.t | 10 ++- > t/db_dependent/Fines.t | 56 +++++++++++ > 8 files changed, 158 insertions(+), 71 deletions(-) > create mode 100644 installer/data/mysql/atomicupdate/bug_13590.sql > create mode 100644 t/db_dependent/Fines.t > >diff --git a/C4/Overdues.pm b/C4/Overdues.pm >index 6eeb6cf..80c6be0 100644 >--- a/C4/Overdues.pm >+++ b/C4/Overdues.pm >@@ -24,6 +24,7 @@ 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 C4::Circulation; > use C4::Context; >@@ -34,45 +35,47 @@ use C4::Debug; > use vars qw($VERSION @ISA @EXPORT); > > 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 >- &get_chargeable_units >- &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 >+ &get_chargeable_units >+ &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 +254,15 @@ 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'}; >+ # If chargeperiod_charge_at = 1, we charge a fine at the start of each charge period >+ # if chargeperiod_charge_at = 0, we charge at the end of each charge period >+ $charge_periods = $data->{'chargeperiod_charge_at'} == 1 ? 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 53de573..43a836a 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' >@@ -202,6 +208,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", >diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl >index bedaef3..de090cb 100755 >--- a/admin/smart-rules.pl >+++ b/admin/smart-rules.pl >@@ -110,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 $maxonsiteissueqty = $input->param('maxonsiteissueqty'); > my $renewalsallowed = $input->param('renewalsallowed'); >@@ -137,29 +138,30 @@ elsif ($op eq 'add') { > my $rs = $schema->resultset('Issuingrule'); > > my $params = { >- branchcode => $br, >- categorycode => $bor, >- itemtype => $itemtype, >- fine => $fine, >- finedays => $finedays, >- maxsuspensiondays => $maxsuspensiondays, >- firstremind => $firstremind, >- chargeperiod => $chargeperiod, >- maxissueqty => $maxissueqty, >- maxonsiteissueqty => $maxonsiteissueqty, >- renewalsallowed => $renewalsallowed, >- renewalperiod => $renewalperiod, >- norenewalbefore => $norenewalbefore, >- auto_renew => $auto_renew, >- reservesallowed => $reservesallowed, >- issuelength => $issuelength, >- lengthunit => $lengthunit, >- hardduedate => $hardduedate, >- hardduedatecompare => $hardduedatecompare, >- rentaldiscount => $rentaldiscount, >- onshelfholds => $onshelfholds, >- opacitemholds => $opacitemholds, >- overduefinescap => $overduefinescap, >+ branchcode => $br, >+ categorycode => $bor, >+ itemtype => $itemtype, >+ fine => $fine, >+ finedays => $finedays, >+ maxsuspensiondays => $maxsuspensiondays, >+ firstremind => $firstremind, >+ chargeperiod => $chargeperiod, >+ chargeperiod_charge_at => $chargeperiod_charge_at, >+ maxissueqty => $maxissueqty, >+ maxonsiteissueqty => $maxonsiteissueqty, >+ renewalsallowed => $renewalsallowed, >+ renewalperiod => $renewalperiod, >+ norenewalbefore => $norenewalbefore, >+ auto_renew => $auto_renew, >+ reservesallowed => $reservesallowed, >+ issuelength => $issuelength, >+ lengthunit => $lengthunit, >+ hardduedate => $hardduedate, >+ hardduedatecompare => $hardduedatecompare, >+ rentaldiscount => $rentaldiscount, >+ onshelfholds => $onshelfholds, >+ opacitemholds => $opacitemholds, >+ overduefinescap => $overduefinescap, > }; > > $rs->update_or_create($params); >diff --git a/installer/data/mysql/atomicupdate/bug_13590.sql b/installer/data/mysql/atomicupdate/bug_13590.sql >new file mode 100644 >index 0000000..6cea6c2 >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/bug_13590.sql >@@ -0,0 +1 @@ >+ALTER TABLE issuingrules ADD chargeperiod_charge_at BOOLEAN NOT NULL DEFAULT '0' AFTER chargeperiod; >diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql >index c36a792..693b3aa 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/installer/data/mysql/kohastructure.sql >@@ -1172,6 +1172,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/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt >index 01fe3ff..ff49786 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 >@@ -148,6 +148,7 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde > <th>Hard due date</th> > <th>Fine amount</th> > <th>Fine charging interval</th> >+ <th>Charge when?</th> > <th>Fine grace period</th> > <th>Overdue fines cap (amount)</th> > <th>Suspension in days (day)</th> >@@ -212,6 +213,7 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde > </td> > <td>[% rule.fine %]</td> > <td>[% rule.chargeperiod %]</td> >+ <td>[% IF rule.chargeperiod_charge_at %]Start of interval[% ELSE %]End of interval[% END %]</td> > <td>[% rule.firstremind %]</td> > <td>[% rule.overduefinescap FILTER format("%.2f") %]</td> > <td>[% rule.finedays %]</td> >@@ -273,6 +275,12 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde > </td> > <td><input type="text" name="fine" id="fine" size="4" /></td> > <td><input type="text" name="chargeperiod" id="chargeperiod" size="2" /></td> >+ <td> >+ <select name="chargeperiod_charge_at" id="chargeperiod_charge_at"> >+ <option value="0">End of interval</option> >+ <option value="1">Start of interval</option> >+ </select> >+ </td> > <td><input type="text" name="firstremind" id="firstremind" size="2" /> </td> > <td><input type="text" name="overduefinescap" id="overduefinescap" size="6" /> </td> > <td><input type="text" name="finedays" id="fined" size="3" /> </td> >diff --git a/t/db_dependent/Circulation_Issuingrule.t b/t/db_dependent/Circulation_Issuingrule.t >index a89624b..2e8f47a 100644 >--- a/t/db_dependent/Circulation_Issuingrule.t >+++ b/t/db_dependent/Circulation_Issuingrule.t >@@ -121,6 +121,7 @@ my $sampleissuingrule1 = { > auto_renew => 0, > issuelength => 5, > chargeperiod => 0, >+ chargeperiod_charge_at => 0, > rentaldiscount => '2.000000', > reservesallowed => 0, > hardduedate => '2013-01-01', >@@ -155,6 +156,7 @@ my $sampleissuingrule2 = { > finedays => 'Null', > firstremind => 'Null', > chargeperiod => 'Null', >+ chargeperiod_charge_at => 0, > rentaldiscount => 2.00, > overduefinescap => 'Null', > accountsent => 'Null', >@@ -184,6 +186,7 @@ my $sampleissuingrule3 = { > finedays => 'Null', > firstremind => 'Null', > chargeperiod => 'Null', >+ chargeperiod_charge_at => 0, > rentaldiscount => 3.00, > overduefinescap => 'Null', > accountsent => 'Null', >@@ -194,6 +197,7 @@ my $sampleissuingrule3 = { > onshelfholds => 1, > opacitemholds => 'F', > }; >+ > $query = 'INSERT INTO issuingrules ( > branchcode, > categorycode, >@@ -213,6 +217,7 @@ $query = 'INSERT INTO issuingrules ( > finedays, > firstremind, > chargeperiod, >+ chargeperiod_charge_at, > rentaldiscount, > overduefinescap, > accountsent, >@@ -220,7 +225,7 @@ $query = 'INSERT INTO issuingrules ( > chargename, > restrictedtype, > maxsuspensiondays >- ) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)'; >+ ) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)'; > my $sth = $dbh->prepare($query); > $sth->execute( > $sampleissuingrule1->{branchcode}, >@@ -241,6 +246,7 @@ $sth->execute( > $sampleissuingrule1->{finedays}, > $sampleissuingrule1->{firstremind}, > $sampleissuingrule1->{chargeperiod}, >+ $sampleissuingrule1->{chargeperiod_charge_at}, > $sampleissuingrule1->{rentaldiscount}, > $sampleissuingrule1->{overduefinescap}, > $sampleissuingrule1->{accountsent}, >@@ -268,6 +274,7 @@ $sth->execute( > $sampleissuingrule2->{finedays}, > $sampleissuingrule2->{firstremind}, > $sampleissuingrule2->{chargeperiod}, >+ $sampleissuingrule2->{chargeperiod_charge_at}, > $sampleissuingrule2->{rentaldiscount}, > $sampleissuingrule2->{overduefinescap}, > $sampleissuingrule2->{accountsent}, >@@ -295,6 +302,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
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=43406">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 13590
:
35326
|
35327
|
35328
|
35329
|
35330
|
35415
|
37909
|
38732
|
41149
|
41150
|
41151
|
43406
|
43407
|
43424
|
43425