From 662da18be2ec7c6c9348a704d9ad8ac955292862 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Fri, 14 Jul 2023 07:36:47 -0400 Subject: [PATCH] Bug 34279: Don't enforce overduefinescap unless it is greater than 0 When creating a circ rule, we can set overduefinescap to blank or 0 and no cap is enforced. If we edit that rule, the blank/0 is converted to "0.00" which perl considers true, thus zero-ing out any calculated fine. Considering we've always ignored an overdue fines cap of 0, we should also ignore 0.00. However, perl is evaluating it as a string which makes it true instead of false as 0 is. Test Plan: 1) Apply the first patch ( unit tests ) 2) prove t/db_dependent/Circulation/CalcFine.t 3) Note the test fails 4) Apply the second patch as well 5) prove t/db_dependent/Circulation/CalcFine.t 6) Note the test passes Test Plan 2: 1) Create an all/all/all rule with an overduefinescap of 0.00, with a daily fine. Enable CalculateFinesOnReturn 2) Backdate a checkout so it is overdue 3) Return this item, note the lack of a fine 4) Apply this patch set 5) Backdate a checkout and return it again 6) Note the fine is generated! Signed-off-by: Katrin Fischer Signed-off-by: Martin Renvoize Signed-off-by: Tomas Cohen Arazi (cherry picked from commit 1763b136d1dcd3348ee26bca8663823b5a05f07c) Signed-off-by: Fridolin Somers --- C4/Overdues.pm | 5 ++++- t/db_dependent/Circulation/CalcFine.t | 25 +++++++++++++------------ 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/C4/Overdues.pm b/C4/Overdues.pm index 8074411a9a..41674a468a 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -282,7 +282,10 @@ sub CalcFine { } } # else { # a zero (or null) chargeperiod or negative units_minus_grace value means no charge. } - $amount = $issuing_rule->{overduefinescap} if $issuing_rule->{overduefinescap} && $amount > $issuing_rule->{overduefinescap}; + $amount = $issuing_rule->{overduefinescap} + if $issuing_rule->{overduefinescap} + && $issuing_rule->{overduefinescap} > 0 + && $amount > $issuing_rule->{overduefinescap}; # This must be moved to Koha::Item (see also similar code in C4::Accounts::chargelostitem $item->{replacementprice} ||= $itemtype->defaultreplacecost diff --git a/t/db_dependent/Circulation/CalcFine.t b/t/db_dependent/Circulation/CalcFine.t index 628e45b460..d372d17339 100755 --- a/t/db_dependent/Circulation/CalcFine.t +++ b/t/db_dependent/Circulation/CalcFine.t @@ -122,15 +122,15 @@ subtest 'Overdue fines cap should be disabled when value is 0' => sub { ); my $start_dt = DateTime->new( - year => 2000, - month => 1, - day => 1, + year => 2000, + month => 1, + day => 1, ); my $end_dt = DateTime->new( - year => 2000, - month => 1, - day => 30, + year => 2000, + month => 1, + day => 30, ); my ($amount) = CalcFine( $item->unblessed, $patron->{categorycode}, $branch->{branchcode}, $start_dt, $end_dt ); @@ -161,15 +161,15 @@ subtest 'Overdue fines cap should be disabled when value is 0.00' => sub { ); my $start_dt = DateTime->new( - year => 2000, - month => 1, - day => 1, + year => 2000, + month => 1, + day => 1, ); my $end_dt = DateTime->new( - year => 2000, - month => 1, - day => 30, + year => 2000, + month => 1, + day => 30, ); my ($amount) = CalcFine( $item->unblessed, $patron->{categorycode}, $branch->{branchcode}, $start_dt, $end_dt ); @@ -179,6 +179,7 @@ subtest 'Overdue fines cap should be disabled when value is 0.00' => sub { teardown(); }; + subtest 'Test with fine amount empty' => sub { plan tests => 1; -- 2.20.1