Bugzilla – Attachment 159075 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), 9.17 KB, created by
Thibaud Guillot
on 2023-11-17 10:18:09 UTC
(
hide
)
Description:
Bug 14293: The suspension should be calculated in days
Filename:
MIME Type:
Creator:
Thibaud Guillot
Created:
2023-11-17 10:18:09 UTC
Size:
9.17 KB
patch
obsolete
>From 964fa43f610c77c1d9bb6276fdd787ea7ee86d05 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >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 <nick@quecheelibrary.org> >Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com> > >Signed-off-by: Mark Tompsett <mtompset@hotmail.com> >--- > C4/Circulation.pm | 10 +- > C4/Overdues.pm | 17 +-- > .../IssuingRules/maxsuspensiondays.t | 133 ++++++++++++++++++ > 3 files changed, 142 insertions(+), 18 deletions(-) > create mode 100644 t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t > >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..3cc7c256b9 100644 >--- a/C4/Overdues.pm >+++ b/C4/Overdues.pm >@@ -321,7 +321,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 ); >@@ -331,16 +330,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 >new file mode 100644 >index 0000000000..b969baa960 >--- /dev/null >+++ b/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t >@@ -0,0 +1,133 @@ >+use Modern::Perl; >+use Test::More tests => 3; >+ >+use MARC::Record; >+use MARC::Field; >+use C4::Context; >+ >+use C4::Circulation qw( AddIssue AddReturn ); >+use C4::Items qw( AddItem ); >+use C4::Biblio qw( AddBiblio ); >+use Koha::Database; >+use Koha::DateUtils; >+use Koha::Patron::Debarments qw( GetDebarments DelDebarment ); >+use Koha::Patrons; >+ >+use t::lib::TestBuilder; >+use t::lib::Mocks; >+ >+my $schema = Koha::Database->schema; >+$schema->storage->txn_begin; >+my $builder = t::lib::TestBuilder->new; >+my $dbh = C4::Context->dbh; >+$dbh->{RaiseError} = 1; >+ >+my $branchcode = $builder->build({ source => 'Branch' })->{branchcode}; >+my $itemtype = $builder->build({ source => 'Itemtype' })->{itemtype}; >+my $patron_category = $builder->build({ source => 'Category' }); >+ >+t::lib::Mocks::mock_userenv({ branchcode => $branchcode }); >+ >+# Test without maxsuspensiondays set >+Koha::IssuingRules->search->delete; >+$builder->build( >+ { >+ source => 'Issuingrule', >+ value => { >+ categorycode => '*', >+ itemtype => '*', >+ branchcode => '*', >+ firstremind => 0, >+ finedays => 2, >+ lengthunit => 'days', >+ suspension_chargeperiod => 1, >+ } >+ } >+); >+ >+my $borrowernumber = Koha::Patron->new({ >+ firstname => 'my firstname', >+ surname => 'my surname', >+ categorycode => $patron_category->{categorycode}, >+ branchcode => $branchcode, >+})->store->borrowernumber; >+my $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; >+ >+my $record = MARC::Record->new(); >+$record->append_fields( >+ MARC::Field->new('100', ' ', ' ', a => 'My author'), >+ MARC::Field->new('245', ' ', ' ', a => 'My title'), >+); >+ >+my $barcode = 'bc_maxsuspensiondays'; >+my ($biblionumber, $biblioitemnumber) = AddBiblio($record, ''); >+my (undef, undef, $itemnumber) = AddItem({ >+ homebranch => $branchcode, >+ holdingbranch => $branchcode, >+ barcode => $barcode, >+ itype => $itemtype >+ } , $biblionumber); >+ >+# clear any holidays to avoid throwing off the suspension day >+# calculations >+$dbh->do('DELETE FROM special_holidays'); >+$dbh->do('DELETE FROM repeatable_holidays'); >+ >+my $daysago20 = dt_from_string->add_duration(DateTime::Duration->new(days => -20)); >+my $daysafter40 = dt_from_string->add_duration(DateTime::Duration->new(days => 40)); >+ >+AddIssue( $borrower, $barcode, $daysago20 ); >+AddReturn( $barcode, $branchcode ); >+my $debarments = GetDebarments({borrowernumber => $borrower->{borrowernumber}}); >+is( >+ $debarments->[0]->{expiration}, >+ output_pref({ dt => $daysafter40, dateformat => 'iso', dateonly => 1 }), >+ 'calculate suspension with no maximum set' >+); >+DelDebarment( $debarments->[0]->{borrower_debarment_id} ); >+ >+# Test with maxsuspensiondays = 10 days >+my $issuing_rule = Koha::IssuingRules->search->next; >+$issuing_rule->maxsuspensiondays( 10 )->store; >+ >+my $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 => $daysafter10, dateformat => 'iso', dateonly => 1 }), >+ 'calculate suspension with a maximum set' >+); >+DelDebarment( $debarments->[0]->{borrower_debarment_id} ); >+ >+# Test with lengthunit => 'hours' >+Koha::IssuingRules->search->delete; >+$builder->build( >+ { >+ source => 'Issuingrule', >+ value => { >+ categorycode => '*', >+ itemtype => '*', >+ branchcode => '*', >+ firstremind => 0, >+ finedays => 2, >+ lengthunit => 'hours', >+ suspension_chargeperiod => 1, >+ maxsuspensiondays => 100, >+ } >+ } >+); >+ >+$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} ); >+ >+$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