From c2b07744a2d30a549640154c0a575cd157845572 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 4 Sep 2015 10:39:48 +0100 Subject: [PATCH] Bug 14293: The suspension should be calculated in days There was a mismatch between bug 13909 and bug 5549. The get_chargeable_units call from _debar_user_on_return should specify that we want a suspension in days. However I don't understand the comment let by the author of bug 5549 f61a9617184ec4b24100c1d99150bfd4ebf13336 1/ > finedays is in days, so hourly loans must multiply by 24 Yes but the suspension is in days, so no need to * 24 2/ And we should have a look on this one too: > grace period is measured in the same units as the loan But the grace period is always in days. On the circ rules the column is "Fine grace period (day)". I think we should replace DateTime::Duration->new( $unit => $issuingrule->{firstremind} ); with DateTime::Duration->new( days => $issuingrule->{firstremind} ); Anyway, we definitelly need more tests in this area! Test plan: 1/ Define an issuing rule with a unit=hour 2/ Set a suspension in days 3/ Check an item out and specify a past due date. 4/ Check the item in 5/ The patron should be debarred correctly Signed-off-by: Nick Clemens --- C4/Circulation.pm | 11 ++++------- .../Circulation/IssuingRules/maxsuspensiondays.t | 22 +++++++++++++++++++++- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 4ddd75f..b5189c4 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2158,21 +2158,18 @@ sub _debar_user_on_return { GetIssuingRule( $borrower->{categorycode}, $item->{itype}, $branchcode ); my $finedays = $issuingrule->{finedays}; my $unit = $issuingrule->{lengthunit}; - my $chargeable_units = C4::Overdues::get_chargeable_units($unit, $dt_due, $dt_today, $branchcode); if ($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 => $issuingrule->{firstremind} ); - my $deltadays = DateTime::Duration->new( - days => $chargeable_units + my $deltadays = C4::Overdues::get_chargeable_units('days', $dt_due, $dt_today, $branchcode); + $deltadays = DateTime::Duration->new( + days => $deltadays, ); + if ( $deltadays->subtract($grace)->is_positive() ) { my $suspension_days = $deltadays * $finedays; diff --git a/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t b/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t index d2cf9c4..366431d 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 => 3; use MARC::Record; use MARC::Field; @@ -92,6 +92,26 @@ is( ); DelDebarment( $debarments->[0]->{borrower_debarment_id} ); +# Test with lengthunit => 'hours' +$circulation_module->mock('GetIssuingRule', sub { + return { + firstremind => 0, + finedays => 2, + maxsuspensiondays => 100, + lengthunit => 'hours', + } +}); +$daysafter10 = dt_from_string->add_duration(DateTime::Duration->new(days => 10)); +AddIssue( $borrower, $barcode, $daysago20 ); +AddReturn( $barcode, $branchcode ); +$debarments = GetDebarments({borrowernumber => $borrower->{borrowernumber}}); +is( + $debarments->[0]->{expiration}, + output_pref({ dt => $daysafter40, dateformat => 'iso', dateonly => 1 }), + 'calculate suspension with lengthunit hours.' +); +DelDebarment( $debarments->[0]->{borrower_debarment_id} ); + # C4::Context->userenv sub Mock_userenv { -- 1.9.1