Bug 13909: Suspension days calculation should respect finesCalendar
authorTomas Cohen Arazi <tomascohen@gmail.com>
Fri, 27 Mar 2015 17:32:35 +0000 (14:32 -0300)
committerTomas Cohen Arazi <tomascohen@gmail.com>
Fri, 10 Apr 2015 13:19:24 +0000 (10:19 -0300)
This patch make _debar_user_on_return respect the finesCalendar syspref.

It does so, by replacing the ad-hoc overdue days calculation in favor of
C4::Overdues::_get_chargeable_units (which is renamed C4::Overdues::get_chargeable_units
and exported). There's no behaviour change besides making the calculation simpler
and correct.

To test:
- Set finesCalendar = "directly"
- Have a circulation rule stating:
  interval for calculating fines = 1
  suspension days = 3
- Have the calendar set for sunday and saturday as holidays.
- Checkout an item with a branch/itype/borrower category that matches the defined circ rule with a hand-writen due date to (say) last friday.
- Check the item in
=> FAIL: Notice that the user is debarred using the calendar (skipping saturday and sunday).
- Apply the patch
- Repeat the previous steps
=> SUCCESS: calculation is correct (counting saturday and sunday as overdue days, i.e. 'directly').
- Set finesCalendar = "calendar"
- Repeat the test
=> SUCCESS: calculation is correct (skipping holidays).
- Sign off.

Sponsored-by: Universidad Nacional de Cordoba
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
C4/Circulation.pm
C4/Overdues.pm

index dc828da..3baccea 100644 (file)
@@ -41,7 +41,7 @@ use C4::Koha qw(
     GetAuthValCode
     GetKohaAuthorisedValueLib
 );
-use C4::Overdues qw(CalcFine UpdateFine);
+use C4::Overdues qw(CalcFine UpdateFine get_chargeable_units);
 use C4::RotatingCollections qw(GetCollectionItemBranches);
 use Algorithm::CheckDigits;
 
@@ -2136,16 +2136,13 @@ sub _debar_user_on_return {
     my ( $borrower, $item, $dt_due, $dt_today ) = @_;
 
     my $branchcode = _GetCircControlBranch( $item, $borrower );
-    my $calendar = Koha::Calendar->new( branchcode => $branchcode );
-
-    # $deltadays is a DateTime::Duration object
-    my $deltadays = $calendar->days_between( $dt_due, $dt_today );
 
     my $circcontrol = C4::Context->preference('CircControl');
     my $issuingrule =
       GetIssuingRule( $borrower->{categorycode}, $item->{itype}, $branchcode );
     my $finedays = $issuingrule->{finedays};
     my $unit     = $issuingrule->{lengthunit};
+    my $chargeable_units = get_chargeable_units($unit, $dt_due, $dt_today, $branchcode);
 
     if ($finedays) {
 
@@ -2157,6 +2154,9 @@ sub _debar_user_on_return {
         my $grace =
           DateTime::Duration->new( $unit => $issuingrule->{firstremind} );
 
+        my $deltadays = DateTime::Duration->new(
+            days => $chargeable_units
+        );
         if ( $deltadays->subtract($grace)->is_positive() ) {
             my $suspension_days = $deltadays * $finedays;
 
index ed8968d..914168b 100644 (file)
@@ -47,7 +47,7 @@ BEGIN {
         &AmountNotify
         &UpdateFine
         &GetFine
-        
+        &get_chargeable_units
         &CheckItemNotify
         &GetOverduesForBranch
         &RemoveNotifyLine
@@ -248,7 +248,7 @@ sub CalcFine {
     my $fine_unit = $data->{lengthunit};
     $fine_unit ||= 'days';
 
-    my $chargeable_units = _get_chargeable_units($fine_unit, $start_date, $end_date, $branchcode);
+    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)  ) {
@@ -267,9 +267,9 @@ sub CalcFine {
 }
 
 
-=head2 _get_chargeable_units
+=head2 get_chargeable_units
 
-    _get_chargeable_units($unit, $start_date_ $end_date, $branchcode);
+    get_chargeable_units($unit, $start_date_ $end_date, $branchcode);
 
 return integer value of units between C<$start_date> and C<$end_date>, factoring in holidays for C<$branchcode>.
 
@@ -281,7 +281,7 @@ C<$branchcode> is the branch whose calendar to use for finding holidays.
 
 =cut
 
-sub _get_chargeable_units {
+sub get_chargeable_units {
     my ($unit, $date_due, $date_returned, $branchcode) = @_;
 
     # If the due date is later than the return date