Bugzilla – Attachment 96100 Details for
Bug 24138
suspension miscalculated when Suspension charging interval bigger than 1 and Max. suspension duration is defined
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 24138: Move the calculation out of the sub
Bug-24138-Move-the-calculation-out-of-the-sub.patch (text/plain), 8.31 KB, created by
Marcel de Rooy
on 2019-12-09 13:47:55 UTC
(
hide
)
Description:
Bug 24138: Move the calculation out of the sub
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2019-12-09 13:47:55 UTC
Size:
8.31 KB
patch
obsolete
>From 86990e9c9f0a72a41bf2727b78f70da0ff7fb85a Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Fri, 29 Nov 2019 13:55:27 +0100 >Subject: [PATCH] Bug 24138: Move the calculation out of the sub >Content-Type: text/plain; charset=utf-8 > >No changes expected here. For the next patch we are going to need to add >test and calculate the new debarment date. To ease the writing of these >tests the calculation is moved out of the existing subroutine. > >Signed-off-by: Hugo Agud <hagud@orex.es> > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >--- > C4/Circulation.pm | 143 +++++++++++++++++++++++++++++------------------------- > 1 file changed, 77 insertions(+), 66 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 815bc7a0a0..c58eb7c3c2 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -2233,14 +2233,15 @@ Internal function, called only by AddReturn that calculates and updates > > Should only be called for overdue returns > >+Calculation of the debarment date has been moved to a separate subroutine _calculate_new_debar_dt >+to ease testing. >+ > =cut > >-sub _debar_user_on_return { >+sub _calculate_new_debar_dt { > my ( $borrower, $item, $dt_due, $return_date ) = @_; > > my $branchcode = _GetCircControlBranch( $item, $borrower ); >- $return_date //= dt_from_string(); >- > my $circcontrol = C4::Context->preference('CircControl'); > my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule( > { categorycode => $borrower->{categorycode}, >@@ -2252,81 +2253,91 @@ sub _debar_user_on_return { > my $unit = $issuing_rule ? $issuing_rule->lengthunit : undef; > my $chargeable_units = C4::Overdues::get_chargeable_units($unit, $dt_due, $return_date, $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' ); >+ return unless $finedays; > >- # grace period is measured in the same units as the loan >- my $grace = >- DateTime::Duration->new( $unit => $issuing_rule->firstremind ); >+ # 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' ); > >- my $deltadays = DateTime::Duration->new( >- days => $chargeable_units >- ); >- if ( $deltadays->subtract($grace)->is_positive() ) { >- my $suspension_days = $deltadays * $finedays; >- >- # 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; >- if ( defined $max_sd ) { >- $max_sd = DateTime::Duration->new( days => $max_sd ); >- $suspension_days = $max_sd >- if DateTime::Duration->compare( $max_sd, $suspension_days ) < 0; >- } >+ # grace period is measured in the same units as the loan >+ my $grace = >+ DateTime::Duration->new( $unit => $issuing_rule->firstremind ); > >- my ( $has_been_extended, $is_a_reminder ); >- if ( C4::Context->preference('CumulativeRestrictionPeriods') and $borrower->{debarred} ) { >- my $debarment = @{ GetDebarments( { borrowernumber => $borrower->{borrowernumber}, type => 'SUSPENSION' } ) }[0]; >- if ( $debarment ) { >- $return_date = dt_from_string( $debarment->{expiration}, 'sql' ); >- $has_been_extended = 1; >- } >- } >+ my $deltadays = DateTime::Duration->new( >+ days => $chargeable_units >+ ); >+ if ( $deltadays->subtract($grace)->is_positive() ) { >+ my $suspension_days = $deltadays * $finedays; >+ >+ # 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; >+ if ( defined $max_sd ) { >+ $max_sd = DateTime::Duration->new( days => $max_sd ); >+ $suspension_days = $max_sd >+ if DateTime::Duration->compare( $max_sd, $suspension_days ) < 0; >+ } > >- 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 ( $has_been_extended ); >+ if ( C4::Context->preference('CumulativeRestrictionPeriods') and $borrower->{debarred} ) { >+ my $debarment = @{ GetDebarments( { borrowernumber => $borrower->{borrowernumber}, type => 'SUSPENSION' } ) }[0]; >+ if ( $debarment ) { >+ $return_date = dt_from_string( $debarment->{expiration}, 'sql' ); >+ $has_been_extended = 1; > } >+ } > >- my $new_debar_dt; >- # Use the calendar or not to calculate the debarment date >- if ( C4::Context->preference('SuspensionsCalendar') eq 'noSuspensionsWhenClosed' ) { >- my $calendar = Koha::Calendar->new( >- branchcode => $branchcode, >- days_mode => 'Calendar' >- ); >- $new_debar_dt = $calendar->addDate( $return_date, $suspension_days ); >- } >- else { >- $new_debar_dt = $return_date->clone()->add_duration($suspension_days); >- } >+ 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 ) >+ ); >+ } > >- Koha::Patron::Debarments::AddUniqueDebarment({ >- borrowernumber => $borrower->{borrowernumber}, >- expiration => $new_debar_dt->ymd(), >- type => 'SUSPENSION', >- }); >- # if borrower was already debarred but does not get an extra debarment >- my $patron = Koha::Patrons->find( $borrower->{borrowernumber} ); >- my $new_debarment_str; >- if ( $borrower->{debarred} eq $patron->is_debarred ) { >- $is_a_reminder = 1; >- $new_debarment_str = $borrower->{debarred}; >- } else { >- $new_debarment_str = $new_debar_dt->ymd(); >- } >- # FIXME Should return a DateTime object >- return $new_debarment_str, $is_a_reminder; >+ my $new_debar_dt; >+ # Use the calendar or not to calculate the debarment date >+ if ( C4::Context->preference('SuspensionsCalendar') eq 'noSuspensionsWhenClosed' ) { >+ my $calendar = Koha::Calendar->new( >+ branchcode => $branchcode, >+ days_mode => 'Calendar' >+ ); >+ $new_debar_dt = $calendar->addDate( $return_date, $suspension_days ); >+ } >+ else { >+ $new_debar_dt = $return_date->clone()->add_duration($suspension_days); > } >+ return $new_debar_dt; > } > return; > } > >+sub _debar_user_on_return { >+ my ( $borrower, $item, $dt_due, $return_date ) = @_; >+ >+ $return_date //= dt_from_string(); >+ >+ my $new_debar_dt = _calculate_new_debar_dt ($borrower, $item, $dt_due, $return_date); >+ >+ return unless $new_debar_dt; >+ >+ Koha::Patron::Debarments::AddUniqueDebarment({ >+ borrowernumber => $borrower->{borrowernumber}, >+ expiration => $new_debar_dt->ymd(), >+ type => 'SUSPENSION', >+ }); >+ # if borrower was already debarred but does not get an extra debarment >+ my $patron = Koha::Patrons->find( $borrower->{borrowernumber} ); >+ my ($new_debarment_str, $is_a_reminder); >+ if ( $borrower->{debarred} eq $patron->is_debarred ) { >+ $is_a_reminder = 1; >+ $new_debarment_str = $borrower->{debarred}; >+ } else { >+ $new_debarment_str = $new_debar_dt->ymd(); >+ } >+ # FIXME Should return a DateTime object >+ return $new_debarment_str, $is_a_reminder; >+} >+ > =head2 _FixOverduesOnReturn > > &_FixOverduesOnReturn($borrowernumber, $itemnumber, $exemptfine, $status); >-- >2.11.0
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 24138
:
95886
|
95887
|
95945
|
95946
| 96100 |
96101
|
103193