From 123cc2e6d9a38707d98cb3c71a15472818d47725 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 Signed-off-by: Arthur Suzuki --- C4/Overdues.pm | 17 +++++--------- .../Circulation/IssuingRules/maxsuspensiondays.t | 26 ++++++++++++++++++++-- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/C4/Overdues.pm b/C4/Overdues.pm index 958fdd0077..ae92fbe865 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -306,7 +306,6 @@ 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 ); @@ -316,16 +315,12 @@ sub get_chargeable_units { if($charge_duration->in_units('hours') == 0 && $charge_duration->in_units('seconds') > 0){ return 1; } - return $charge_duration->in_units('hours'); - } - 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'); + + my $hours = $charge_duration->in_units('hours'); + if ($unit eq 'hours') { + return $hours; + } else { + return ( $hours > 0 ? ceil($hours / 24) : 0 ); } } diff --git a/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t b/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t index abdef9daad..fcdf44d845 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 => 3; +use Test::More tests => 5; use MARC::Record; use MARC::Field; @@ -119,7 +119,6 @@ $builder->build( } ); -$daysafter10 = dt_from_string->add_duration(DateTime::Duration->new(days => 10)); AddIssue( $borrower, $barcode, $daysago20 ); AddReturn( $barcode, $branchcode ); $debarments = GetDebarments({borrowernumber => $borrower->{borrowernumber}}); @@ -130,6 +129,29 @@ is( ); DelDebarment( $debarments->[0]->{borrower_debarment_id} ); +my $yesterday = dt_from_string->subtract_duration(DateTime::Duration->new( days => 1 )); +my $daysafter2 = dt_from_string->add_duration(DateTime::Duration->new( days => 2 )); +AddIssue( $borrower, $barcode, $yesterday); +AddReturn( $barcode, $branchcode ); +$debarments = GetDebarments({borrowernumber => $borrower->{borrowernumber}}); +is( + $debarments->[0]->{expiration}, + output_pref({ dt => $daysafter2, dateformat => 'iso', dateonly => 1 }), + 'calculate suspension with lengthunit hours.' +); +DelDebarment( $debarments->[0]->{borrower_debarment_id} ); + +my $hoursago2= dt_from_string->subtract_duration(DateTime::Duration->new(hours => 2)); +AddIssue( $borrower, $barcode, $hoursago2); +AddReturn( $barcode, $branchcode ); +$debarments = GetDebarments({borrowernumber => $borrower->{borrowernumber}}); +is( + $debarments->[0]->{expiration}, + output_pref({ dt => $daysafter2, dateformat => 'iso', dateonly => 1 }), + 'calculate suspension with lengthunit hours.' +); +DelDebarment( $debarments->[0]->{borrower_debarment_id} ); + $schema->storage->txn_rollback; # C4::Context->userenv -- 2.13.7