From 8af1c737370346f58352c215b56bdd587598dc29 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                                     |   98 ++++++------
 Koha/Schema/Result/Issuingrule.pm                  |    8 +
 admin/smart-rules.pl                               |   46 +++---
 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           |  165 ++++++++++----------
 t/db_dependent/Fines.t                             |   56 +++++++
 8 files changed, 235 insertions(+), 148 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..0d134df 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;
@@ -34,45 +36,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 +255,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 438c23b..fe220c8 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'
@@ -197,6 +203,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 9ef6ef6..100ed37 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 $renewalsallowed  = $input->param('renewalsallowed');
     my $renewalperiod    = $input->param('renewalperiod');
@@ -134,28 +135,29 @@ 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,
-        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,
+        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 df123ee..f1ce7f1 100644
--- a/installer/data/mysql/kohastructure.sql
+++ b/installer/data/mysql/kohastructure.sql
@@ -1181,6 +1181,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 84ffcd8..05736e6 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"><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 (day)</th>
                 <th>Overdue fines cap (amount)</th>
                 <th>Suspension in days (day)</th>
@@ -205,6 +206,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>
@@ -265,6 +267,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 b1d9d72..73ba4c9 100644
--- a/t/db_dependent/Circulation_Issuingrule.t
+++ b/t/db_dependent/Circulation_Issuingrule.t
@@ -108,88 +108,91 @@ $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,
-    onshelfholds       => 0,
-    opacitemholds      => 'N',
+    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,
+    onshelfholds           => 0,
+    opacitemholds          => 'N',
 };
 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,
-    onshelfholds       => 1,
-    opacitemholds      => 'Y',
+    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,
+    onshelfholds           => 1,
+    opacitemholds          => 'Y',
 };
 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,
-    onshelfholds       => 1,
-    opacitemholds      => 'F',
+    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,
+    onshelfholds           => 1,
+    opacitemholds          => 'F',
 };
 $query = 'INSERT INTO issuingrules (
                 branchcode,
@@ -209,6 +212,7 @@ $query = 'INSERT INTO issuingrules (
                 finedays,
                 firstremind,
                 chargeperiod,
+                chargeperiod_charge_at,
                 rentaldiscount,
                 overduefinescap,
                 accountsent,
@@ -216,7 +220,7 @@ $query = 'INSERT INTO issuingrules (
                 chargename,
                 restrictedtype,
                 maxsuspensiondays
-                ) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)';
+                ) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)';
 my $sth = $dbh->prepare($query);
 $sth->execute(
     $sampleissuingrule1->{branchcode},
@@ -236,6 +240,7 @@ $sth->execute(
     $sampleissuingrule1->{finedays},
     $sampleissuingrule1->{firstremind},
     $sampleissuingrule1->{chargeperiod},
+    $sampleissuingrule1->{chargeperiod_charge_at},
     $sampleissuingrule1->{rentaldiscount},
     $sampleissuingrule1->{overduefinescap},
     $sampleissuingrule1->{accountsent},
@@ -262,6 +267,7 @@ $sth->execute(
     $sampleissuingrule2->{finedays},
     $sampleissuingrule2->{firstremind},
     $sampleissuingrule2->{chargeperiod},
+    $sampleissuingrule2->{chargeperiod_charge_at},
     $sampleissuingrule2->{rentaldiscount},
     $sampleissuingrule2->{overduefinescap},
     $sampleissuingrule2->{accountsent},
@@ -288,6 +294,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