Bug 15741 - Incorrect rounding in total fines calculations
Summary: Incorrect rounding in total fines calculations
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 17140
  Show dependency treegraph
 
Reported: 2016-02-04 10:51 UTC by Jacek Ablewicz
Modified: 2018-06-04 20:10 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 15741: Fix rounding in total fines calculations (1.20 KB, patch)
2016-03-08 12:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15741: Fix rounding in total fines calculations (1.28 KB, patch)
2016-03-14 10:25 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
[PASSED QA] Bug 15741: Fix rounding in total fines calculations (1.34 KB, patch)
2016-03-23 22:45 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 15741: (regression tests) Rounding is not calculated correctly (1.91 KB, patch)
2016-03-23 23:36 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 15741: Fix rounding in total fines calculations (1.38 KB, patch)
2016-03-23 23:36 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jacek Ablewicz 2016-02-04 10:51:18 UTC
Our librarians discovered recently that recording payments for some specific fine amounts (64.60, for example) are not working properly - they are getting errors like:

    You must pay a value less than or equal to 64.60

It's not just 64.60, same thing happens for:

1.14
1.36
1.39
1.57
1.59
1.61
1.64
1.66
1.68
1.82
1.84
1.86
1.89
1.91
1.93
2.01
2.03
2.28
2.47
2.53
2.72
2.78
2.97
...

32.30
64.10
64.60
65.10
128.20
128.70
129.20
129.70
130.20
130.70
...

and so on. This error seems to be caused by incorrect rounding in C4::Members::GetMemberAccountRecords(). To replicate:

1) Create manual invoice for 64.60 (or 1.14, 1.36, ...)
2) Try to pay it using "Pay amount" or "Pay selected" buttons

As a temporary workaround, one may:

- pay such amount in 2+ steps (eg. 64.00 + 0.60)
- pay individual fines using 'Pay' button[s] on the left

but, at first glance at least, an underlying problem for this error may possibly have some more severe consequences than just minor troubles with making payments - eg., if patron has 12+ fines (or credits) of such kind, total account balance calculation may be wrong as well.
Comment 1 Jonathan Druart 2016-03-08 12:01:22 UTC Comment hidden (obsolete)
Comment 2 Sally 2016-03-09 11:14:48 UTC
I was keen to sign this patch off, as this causes terrible problems for us.  

Test plan:
1) Create manual invoice for 64.60 (or 1.14, 1.36, ...)
2) Try to pay it using "Pay amount" or "Pay selected" buttons

Using 'Pay Amount' button:

1)  Create manual invoice for a single payment:  64.60 (or 1.14, 1.36, ...)
2)  Pay it using 'Pay Amount' button

Pass:  this works as expected.

1)  Create manual invoice for multiple lines which total:  64.60 (or 1.14, 1.36, ...)
2)  Pay all lines using 'Pay Amount' button

Pass:  this works as expected.

1)  Create manual invoice for multiple lines.  
2)  Decide to pay an amount which totals:  64.60 (or 1.14, 1.36, ...)
3)  Pay using 'Pay Amount' button

Pass:  this works as expected.

Using 'Pay Selected' button:

1)  Create manual invoice for a single payment:  64.60 (or 1.14, 1.36, ...)
2)  Pay it using 'Pay Selected' button

Fail:  'You must pay a value less than or equal to 64.60 (or 1.14, 1.36, ...)' displays

1)  Create manual invoice for multiple lines which total:  64.60 (or 1.14, 1.36, ...)
2)  Pay all lines using 'Pay Selected' button

Pass:  this works as expected

1)  Create manual invoice for multiple lines.  
2)  Choose some lines which total:  64.60 (or 1.14, 1.36, ...)
3)  Pay chosen lines using 'Pay Selected' button

Pass:  the error line does not appear.

However, there seems to be a major error with the 'Pay Selected' function, which is reported in the following bug:  https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15397

Can we sign off this patch as it mends the functionality of the 'Pay Amount' button and focus on the 'Pay Selected' issue in a different bug - which appears to be a much bigger problem?
Comment 3 Jonathan Druart 2016-03-11 12:34:17 UTC
(In reply to Sally Healey from comment #2)
> Can we sign off this patch as it mends the functionality of the 'Pay Amount'
> button and focus on the 'Pay Selected' issue in a different bug - which
> appears to be a much bigger problem?

Yes, definitely.
Comment 4 Biblibre Sandboxes 2016-03-14 10:25:16 UTC
Patch tested with a sandbox, by Sally Healey <sally.healey@cheshiresharedservices.gov.uk>
Comment 5 Biblibre Sandboxes 2016-03-14 10:25:39 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2016-03-23 22:45:57 UTC Comment hidden (obsolete)
Comment 7 Tomás Cohen Arazi 2016-03-23 23:36:00 UTC
Created attachment 49495 [details] [review]
Bug 15741: (regression tests) Rounding is not calculated correctly

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 8 Tomás Cohen Arazi 2016-03-23 23:36:11 UTC
Created attachment 49496 [details] [review]
Bug 15741: Fix rounding in total fines calculations

C4::Members::GetMemberAccountRecords wrongly casts float to integer
It's common to use sprintf in Perl to do this job.

% perl -e 'print int(1000*64.60)."\n"';
64599
% perl -e 'print sprintf("%.0f", 1000*64.60)."\n"';
64600

Test plan:
1) Create manual invoice for 64.60 (or 1.14, 1.36, ...)
2) Try to pay it using "Pay amount" or "Pay selected" buttons

Signed-off-by: Sally Healey <sally.healey@cheshiresharedservices.gov.uk>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Brendan Gallagher 2016-03-24 16:11:45 UTC
Pushed to Master - Should be in May 2016 release.  Thanks!
Comment 10 Julian Maurice 2016-04-06 14:32:53 UTC
Patches pushed to 3.22.x, will be in 3.22.6
Comment 11 Frédéric Demians 2016-04-27 15:40:17 UTC
Pushed to 3.20.x, will be in 3.20.11.
Comment 12 Lari Taskula 2016-08-17 12:38:25 UTC
This bug still appears in master at "Pay amount" and "Pay selected" for payment of 1.14, but not with 64.60.

To replicate:
1. Go to patron's fines tab, make sure there are no outstanding payments.
2. Via "Create manual invoice" tab, create one fine of 1.14
3. Go to Pay fines tab
4. Click "Pay amount" and confirm.
5. Observe error message "You must pay a value less than or equal to 1.14."
6. Go to Pay fines tab
7. Click "Pay selected" and confirm
8. Observe error message "You must pay a value less than or equal to 1.14."

However, via individual row's "Pay" button it seems to work.