Bug 21443: Remove the dependency on finesCalendar
authorKyle M Hall <kyle@bywatersolutions.com>
Mon, 23 Mar 2020 12:06:08 +0000 (08:06 -0400)
committerMartin Renvoize <martin.renvoize@ptfs-europe.com>
Tue, 24 Mar 2020 11:09:16 +0000 (11:09 +0000)
Considering that the the use of finesCalendar for this calculation is
already a binary choice, it makes sense to remove the use of
finesCalendar here. It is an uneccessary complication that could
introduce confusion. Allowing this new setting to directly control
the behavior makes it clear and obvious what is going on.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Koha/Charges/Fees.pm
installer/data/mysql/atomicupdate/bug_21443.perl
koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt
t/db_dependent/Koha/Charges/Fees.t

index a3d2a12..7eff76b 100644 (file)
@@ -112,7 +112,7 @@ sub accumulate_rentalcharge {
     my $calendar = Koha::Calendar->new( branchcode => $self->library->id );
 
     if ( $units eq 'hours' ) {
-        if ( $itemtype->rentalcharge_hourly_calendar && C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed' ) {
+        if ( $itemtype->rentalcharge_hourly_calendar ) {
             $duration = $calendar->hours_between(
                 $self->from_date->truncate( to => 'minute' ),
                 $self->to_date->truncate( to => 'minute' )
@@ -124,7 +124,7 @@ sub accumulate_rentalcharge {
         }
     }
     else {
-        if ( $itemtype->rentalcharge_daily_calendar && C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed' ) {
+        if ( $itemtype->rentalcharge_daily_calendar ) {
             $duration =
               $calendar->days_between( $self->from_date, $self->to_date );
         }
index 74cf57f..8232923 100644 (file)
@@ -16,6 +16,10 @@ if( CheckVersion( $DBversion ) ) {
         });
     }
 
+    my $finesCalendar = C4::Context->preference('finesCalendar');
+    my $value = $finesCalendar eq 'noFinesWhenClosed' ? 1 : 0;
+    $dbh->do("UPDATE itemtypes SET rentalcharge_hourly_calendar = $value, rentalcharge_daily_calendar = $value");
+
     # Always end with this (adjust the bug info)
     SetVersion( $DBversion );
     print "Upgrade to $DBversion done (Bug 21443: Add ability to exclude holidays when calculating rentals fees by time period)\n";
index 3ba8410..a024053 100644 (file)
@@ -248,7 +248,7 @@ Item types administration
                         [% ELSE %]
                             <input type="checkbox" id="rentalcharge_daily_calendar" name="rentalcharge_daily_calendar" value="1" />
                         [% END %]
-                        <span class="hint">If checked, daily charge will be calculated based on the value of the system preference <i>finesCalendar</i>. If not checked, the fee will be calculated based on the number of days until due, directly.</span>
+                        <span class="hint">If checked, daily charge will be calculated using the calendar to exclude holidays. If not checked, the fee will be calculated based on the number of days until due, directly.</span>
                 </li>
                 <li>
                     <label for="rentalcharge_hourly">Hourly rental charge: </label>
@@ -262,7 +262,7 @@ Item types administration
                         [% ELSE %]
                             <input type="checkbox" id="rentalcharge_hourly_calendar" name="rentalcharge_hourly_calendar" value="1" />
                         [% END %]
-                        <span class="hint">If checked, hourly charge will be calculated based on the value of the system preference <i>finesCalendar</i>. If not checked, the fee will be calculated based on the number of hours until due, directly.</span>
+                        <span class="hint">If checked, hourly charge will be calculated using the calendar to exclude holidays. If not checked, the fee will be calculated based on the number of hours until due, directly.</span>
                 </li>
                 <li>
                     <label for="defaultreplacecost">Default replacement cost: </label>
index a28cc48..a204521 100644 (file)
@@ -408,11 +408,11 @@ subtest 'accumulate_rentalcharge tests' => sub {
         }
     );
 
-    t::lib::Mocks::mock_preference( 'finesCalendar', 'ignoreCalendar' );
+    $itemtype->rentalcharge_hourly_calendar(0)->store();
     $charge = $fees->accumulate_rentalcharge();
     is( $charge, 24.00, 'Hourly rental charge calculated correctly (96h * 0.25u)' );
 
-    t::lib::Mocks::mock_preference( 'finesCalendar', 'noFinesWhenClosed' );
+    $itemtype->rentalcharge_hourly_calendar(1)->store();
     $charge = $fees->accumulate_rentalcharge();
     is( $charge, 18.00,
 "Hourly rental charge calculated correctly with finesCalendar = noFinesWhenClosed and closed $dayname (96h - 24h * 0.25u)"
@@ -423,8 +423,8 @@ subtest 'accumulate_rentalcharge tests' => sub {
     is( $charge, 24.00,
 "Hourly rental charge calculated correctly with finesCalendar = noFinesWhenClosed and closed $dayname (96h - 24h * 0.25u) and rentalcharge_hourly_calendar = 0"
     );
-    $itemtype->rentalcharge_hourly_calendar(1)->store();
 
+    $itemtype->rentalcharge_hourly_calendar(1)->store();
     $calendar->delete_holiday( weekday => $closed_day );
     $charge = $fees->accumulate_rentalcharge();
     is( $charge, 24.00, 'Hourly rental charge calculated correctly with finesCalendar = noFinesWhenClosed (96h - 0h * 0.25u)' );