Bugzilla – Attachment 159277 Details for
Bug 14293
Error in the calculation of the suspension of users per day
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 14293: The suspension should be calculated in days
Bug-14293-The-suspension-should-be-calculated-in-d.patch (text/plain), 10.02 KB, created by
Thibaud Guillot (thibaud_g)
on 2023-11-27 15:43:03 UTC
(
hide
)
Description:
Bug 14293: The suspension should be calculated in days
Filename:
MIME Type:
Creator:
Thibaud Guillot (thibaud_g)
Created:
2023-11-27 15:43:03 UTC
Size:
10.02 KB
patch
obsolete
>From e031afc6981200e837b674d7ed2236def3f5f87f Mon Sep 17 00:00:00 2001 >From: Thibaud Guillot <thibaud.guillot@biblibre.com> >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 >--- > C4/Circulation.pm | 10 +- > C4/Overdues.pm | 33 ++--- > .../Circulation/maxsuspensiondays.t | 121 +++++++++++++++++- > 3 files changed, 133 insertions(+), 31 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index ac28b2e74b..4fdea76536 100644 >--- a/C4/Circulation.pm >+++ b/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} ) > ); > } > >diff --git a/C4/Overdues.pm b/C4/Overdues.pm >index 41674a468a..eac4208e8d 100644 >--- a/C4/Overdues.pm >+++ b/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 ); > } > } > >diff --git a/t/db_dependent/Circulation/maxsuspensiondays.t b/t/db_dependent/Circulation/maxsuspensiondays.t >index 614a32303a..bcd25e1a48 100755 >--- a/t/db_dependent/Circulation/maxsuspensiondays.t >+++ b/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; >-- >2.30.2
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 14293
:
42387
|
42492
|
85210
|
85211
|
85251
|
85252
|
85255
|
85407
|
85408
|
85788
|
90955
|
90956
|
90957
|
159075
|
159076
|
159128
|
159129
|
159277
|
159504
|
160924
|
160925