From 7870aa2c0a93aee0de1f7f98b5cf4018336dbdf7 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 29 Nov 2019 14:45:25 +0100 Subject: [PATCH] Bug 24138: Fix calculation of suspension days when a limit is set If there is a limit for the number of suspension days (maxsuspensiondays), the calculation is wrong. We are reducing the number of days before taking into account the suspension charging interval. For instance, a checkin is 1 year late and the circ rules are defined to charge 7 days every 15 days. It results in 365 * 7 / 15 days of suspension => 170 days Before this patch the calculation was: 365 * 7 limited to 333, 333 / 15 => 22 days Test plan: Given the examples in the commit messages and the description of the bug report, setup complex circulation rules and confirm that the debarment dates are calculated correctly --- C4/Circulation.pm | 15 +++--- .../Circulation/IssuingRules/maxsuspensiondays.t | 57 +++++++++++++++++++++- 2 files changed, 64 insertions(+), 8 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index c58eb7c3c2..3568a32639 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2266,9 +2266,17 @@ sub _calculate_new_debar_dt { my $deltadays = DateTime::Duration->new( days => $chargeable_units ); + if ( $deltadays->subtract($grace)->is_positive() ) { my $suspension_days = $deltadays * $finedays; + if ( $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 ) + ); + } + # If the max suspension days is < than the suspension days # the suspension days is limited to this maximum period. my $max_sd = $issuing_rule->maxsuspensiondays; @@ -2287,13 +2295,6 @@ sub _calculate_new_debar_dt { } } - if ( $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 ) - ); - } - my $new_debar_dt; # Use the calendar or not to calculate the debarment date if ( C4::Context->preference('SuspensionsCalendar') eq 'noSuspensionsWhenClosed' ) { diff --git a/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t b/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t index d2602263f3..a58739853d 100644 --- a/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t +++ b/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t @@ -1,5 +1,5 @@ use Modern::Perl; -use Test::More tests => 2; +use Test::More tests => 4; use MARC::Record; use MARC::Field; @@ -101,4 +101,59 @@ is( ); DelDebarment( $debarments->[0]->{borrower_debarment_id} ); +subtest "suspension_chargeperiod" => sub { + Koha::IssuingRules->search->delete; + $builder->build( + { + source => 'Issuingrule', + value => { + categorycode => '*', + itemtype => '*', + branchcode => '*', + firstremind => 0, + finedays => 7, + lengthunit => 'days', + suspension_chargeperiod => 15, + maxsuspensiondays => 333, + } + } + ); + my $patron = $builder->build_object({ class => 'Koha::Patrons' }); + my $item = $builder->build_sample_item; + + 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->unblessed, $item->unblessed, $last_year, $today ); + is( $new_debar_dt->truncate( to => 'day' ), + $today->clone->add( days => 365 / 15 * 7 )->truncate( to => 'day' ) ); + +}; + +subtest "maxsuspensiondays" => sub { + Koha::IssuingRules->search->delete; + $builder->build( + { + source => 'Issuingrule', + value => { + categorycode => '*', + itemtype => '*', + branchcode => '*', + firstremind => 0, + finedays => 15, + lengthunit => 'days', + suspension_chargeperiod => 7, + maxsuspensiondays => 333, + } + } + ); + my $patron = $builder->build_object({ class => 'Koha::Patrons' }); + my $item = $builder->build_sample_item; + + 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->unblessed, $item->unblessed, $last_year, $today ); + is( $new_debar_dt->truncate( to => 'day' ), + $today->clone->add( days => 333 )->truncate( to => 'day' ) ); +}; + $schema->storage->txn_rollback; -- 2.11.0