@@ -, +, @@ +finedays is in days, so hourly loans must multiply by 24 +grace period is measured in the same units as the loan DateTime::Duration->new( $unit => $issuingrule->{firstremind} ); DateTime::Duration->new( days => $issuingrule->{firstremind} ); --- C4/Circulation.pm | 10 +- C4/Overdues.pm | 33 ++--- .../Circulation/maxsuspensiondays.t | 121 +++++++++++++++++- 3 files changed, 133 insertions(+), 31 deletions(-) --- a/C4/Circulation.pm +++ a/C4/Circulation.pm @@ -20,7 +20,7 @@ package C4::Circulation; use Modern::Perl; use DateTime; -use POSIX qw( floor ); +use POSIX qw( floor ceil ); use YAML::XS; use Encode; @@ -2699,14 +2699,10 @@ sub _calculate_new_debar_dt { ); my $finedays = $issuing_rule ? $issuing_rule->{finedays} : undef; my $unit = $issuing_rule ? $issuing_rule->{lengthunit} : undef; - my $chargeable_units = C4::Overdues::get_chargeable_units($unit, $dt_due, $return_date, $branchcode); + my $chargeable_units = C4::Overdues::get_chargeable_units('days', $dt_due, $return_date, $branchcode); return unless $finedays; - # finedays is in days, so hourly loans must multiply by 24 - # thus 1 hour late equals 1 day suspension * finedays rate - $finedays = $finedays * 24 if ( $unit eq 'hours' ); - # grace period is measured in the same units as the loan my $grace = DateTime::Duration->new( $unit => $issuing_rule->{firstremind} // 0); @@ -2721,7 +2717,7 @@ sub _calculate_new_debar_dt { if ( defined $issuing_rule->{suspension_chargeperiod} && $issuing_rule->{suspension_chargeperiod} > 1 ) { # No need to / 1 and do not consider / 0 $suspension_days = DateTime::Duration->new( - days => floor( $suspension_days->in_units('days') / $issuing_rule->{suspension_chargeperiod} ) + days => ceil( $suspension_days->in_units('days') / $issuing_rule->{suspension_chargeperiod} ) ); } --- a/C4/Overdues.pm +++ a/C4/Overdues.pm @@ -321,26 +321,21 @@ sub get_chargeable_units { my $charge_units = 0; my $charge_duration; - if ($unit eq 'hours') { - if(C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed') { - my $calendar = Koha::Calendar->new( branchcode => $branchcode ); - $charge_duration = $calendar->hours_between( $date_due, $date_returned ); - } else { - $charge_duration = $date_returned->delta_ms( $date_due ); - } - if($charge_duration->in_units('hours') == 0 && $charge_duration->in_units('seconds') > 0){ - return 1; - } - return $charge_duration->in_units('hours'); + if(C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed') { + my $calendar = Koha::Calendar->new( branchcode => $branchcode ); + $charge_duration = $calendar->hours_between( $date_due, $date_returned ); + } else { + $charge_duration = $date_returned->delta_ms( $date_due ); } - else { # days - if(C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed') { - my $calendar = Koha::Calendar->new( branchcode => $branchcode ); - $charge_duration = $calendar->days_between( $date_due, $date_returned ); - } else { - $charge_duration = $date_returned->delta_days( $date_due ); - } - return $charge_duration->in_units('days'); + if($charge_duration->in_units('hours') == 0 && $charge_duration->in_units('seconds') > 0){ + return 1; + } + + my $hours = $charge_duration->in_units('hours'); + if ($unit eq 'hours') { + return $hours; + } else { + return ( $hours > 0 ? ceil($hours / 24) : 0 ); } } --- a/t/db_dependent/Circulation/maxsuspensiondays.t +++ a/t/db_dependent/Circulation/maxsuspensiondays.t @@ -15,6 +15,8 @@ use Koha::Patrons; use t::lib::TestBuilder; use t::lib::Mocks; +use POSIX qw( ceil ); + my $schema = Koha::Database->schema; $schema->storage->txn_begin; my $builder = t::lib::TestBuilder->new; @@ -129,9 +131,12 @@ subtest "suspension_chargeperiod" => sub { my $last_year = dt_from_string->clone->subtract( years => 1 ); my $today = dt_from_string; my $new_debar_dt = C4::Circulation::_calculate_new_debar_dt( $patron, $item, $last_year, $today ); - is( $new_debar_dt->truncate( to => 'day' ), - $today->clone->add( days => 365 / 15 * 7 )->truncate( to => 'day' ) ); - + is( + $new_debar_dt->truncate( to => 'day' ), + $today->clone->add( days => ceil( 365 / 15 * 7 ) ) + ->truncate( to => 'day' ) + ), + 'the truncated dates are equal.' }; subtest "maxsuspensiondays" => sub { @@ -156,7 +161,113 @@ subtest "maxsuspensiondays" => sub { my $today = dt_from_string; my $new_debar_dt = C4::Circulation::_calculate_new_debar_dt( $patron, $item, $last_year, $today ); is( $new_debar_dt->truncate( to => 'day' ), - $today->clone->add( days => 333 )->truncate( to => 'day' ) ); -}; + $today->clone->add( days => 333 )->truncate( to => 'day' )); + + + Koha::CirculationRules->set_rules( + { + categorycode => undef, + itemtype => undef, + branchcode => undef, + rules => { + firstremind => 0, + finedays => 2, + lengthunit => 'hours', + suspension_chargeperiod => 1, + maxsuspensiondays => 100, + } + } + ); + + my $yesterday = dt_from_string->clone->subtract( days => 1 ); + my $daysafter2 = dt_from_string->clone->add( days => 2 ); + AddIssue( $patron, $barcode, $yesterday ); + AddReturn( $barcode, $branchcode ); + $debarments = $patron->restrictions; + $THE_debarment = $debarments->next; + is( + $THE_debarment->expiration, + output_pref( + { dt => $daysafter2, dateformat => 'iso', dateonly => 1 } + ), + 'calculate suspension with lengthunit hours.' + ); + + DelDebarment( $THE_debarment->borrower_debarment_id ); + + my $hoursago2 = dt_from_string->clone->subtract( hours => 2 ); + AddIssue( $patron, $barcode, $hoursago2 ); + AddReturn( $barcode, $branchcode ); + $debarments = $patron->restrictions; + $THE_debarment = $debarments->next; + is( + $THE_debarment->expiration, + output_pref( + { dt => $daysafter2, dateformat => 'iso', dateonly => 1 } + ), + 'calculate suspension with lengthunit hours.' + ); + DelDebarment( $THE_debarment->borrower_debarment_id ); + + my $hoursafter2 = dt_from_string->clone->add( hours => 2 ); + AddIssue( $patron, $barcode, $hoursafter2 ); + AddReturn( $barcode, $branchcode ); + $debarments = $patron->restrictions; + $THE_debarment = $debarments->next; + is_deeply( $THE_debarment, undef, 'No debarment if in time' ); + + # Add 1 day every 2 days + Koha::CirculationRules->set_rules( + { + categorycode => undef, + itemtype => undef, + branchcode => undef, + rules => { + firstremind => 0, + finedays => 1, + lengthunit => 'hours', + suspension_chargeperiod => 2, + maxsuspensiondays => 100, + } + } + ); + + # 20 days late => 20/2 * 1 => 10 days of suspension + AddIssue( $patron, $barcode, $daysago20 ); + AddReturn( $barcode, $branchcode ); + $debarments = $patron->restrictions; + $THE_debarment = $debarments->next; + is( + $THE_debarment->expiration, + output_pref( + { dt => $daysafter10, dateformat => 'iso', dateonly => 1 } + ), + 'calculate suspension with lengthunit hours.' + ); + + DelDebarment( $THE_debarment->borrower_debarment_id ); + + # 1 day late => ceil(1/2) * 1 => 1 day of suspension + my $tomorrow = dt_from_string->clone->add( days => 1 ); + AddIssue( $patron, $barcode, $yesterday ); + AddReturn( $barcode, $branchcode ); + $debarments = $patron->restrictions; + $THE_debarment = $debarments->next; + is( + $THE_debarment->expiration, + output_pref( { dt => $tomorrow, dateformat => 'iso', dateonly => 1 } ), + 'calculate suspension with lengthunit hours.' + ); + + DelDebarment( $THE_debarment->borrower_debarment_id ); + + # 2 hours late => ceil(.x/2) * 1 => 1 day of suspension + AddIssue( $patron, $barcode, $hoursafter2 ); + AddReturn( $barcode, $branchcode ); + $debarments = $patron->restrictions; + $THE_debarment = $debarments->next; + is_deeply( $THE_debarment, undef, 'No debarment if in time' ); + +}; $schema->storage->txn_rollback; --