Bug 11644 - Cannot update fund if total is equal to amount unallocated
Summary: Cannot update fund if total is equal to amount unallocated
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Kyle M Hall (khall)
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-30 17:37 UTC by Kyle M Hall (khall)
Modified: 2015-06-04 23:30 UTC (History)
5 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:
Circulation function:


Attachments
Bug 11644 - Cannot update fund if total is equal to amount unallocated (1.43 KB, patch)
2014-01-30 17:44 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 11644 - Cannot update fund if total is equal to amount unallocated (1.49 KB, patch)
2014-01-31 16:03 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 11644 - Cannot update fund if total is equal to amount unallocated (1.54 KB, patch)
2014-02-17 12:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11644: (follow-up) Add a FIXME for the precision value (1.23 KB, patch)
2014-02-17 12:48 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall (khall) 2014-01-30 17:37:44 UTC
When attempting to update a fund, if the amount unalloccated for the fund is equal to the total, you can be prevented from saving. This is due to imprecise floating point number comparison in check_parent_total.pl
Comment 1 Kyle M Hall (khall) 2014-01-30 17:44:23 UTC Comment hidden (obsolete)
Comment 2 Sean McGarvey 2014-01-31 15:49:15 UTC
Tested on pasco county production server, our acquisitions team says fix worked perfectly!
Comment 3 Kyle M Hall (khall) 2014-01-31 16:03:33 UTC Comment hidden (obsolete)
Comment 4 Katrin Fischer 2014-02-02 12:54:46 UTC
I am failing to reproduce the problem, here is what I did:
- My budget had 200,00 unallocated
- I created a new fund with 200,00, it saved correctly
- I tried editing the newly created fund, it also saved correctly

Quite sure I am missing something - any hints?
Comment 5 Kyle M Hall (khall) 2014-02-03 20:38:52 UTC
(In reply to Katrin Fischer from comment #4)
> I am failing to reproduce the problem, here is what I did:
> - My budget had 200,00 unallocated
> - I created a new fund with 200,00, it saved correctly
> - I tried editing the newly created fund, it also saved correctly
> 
> Quite sure I am missing something - any hints?

I wish I had some hints to give you! I had to debug this on a production system. I tried to come up with the best way to reproduce the issue I could. It may even be that you'd be unable to reproduce the error on a different system, as this may have something to do with Perl versions. Can you at least verify the patch does not cause any regressions?
Comment 6 Katrin Fischer 2014-02-04 07:50:07 UTC
Ok, I can do regression testing, it might take a bit - busy week. If someone else beats me to it that's ok of course :)
Comment 7 Jonathan Druart 2014-02-04 11:13:41 UTC
What is the error and what was the amount used?
Comment 8 Jonathan Druart 2014-02-04 11:37:08 UTC
I found bug 11675 on testing this patch.
Comment 9 Barton Chittenden 2014-02-14 21:27:39 UTC
To re-create the bug:

1) Create a budget of 732214.84

2) Create Fund 1 of amount 651162.37

3) Create Fund 2 of amount 81052.46

   Note that the sum of these amounts is 732214.83: One cent less 
   than the budget amount of 732214.84.

4) Edit Fund 2, change the amount to 81052.47

This will generate the following warning:

Form not submitted because of the following problem(s)
------------------------------------------------------------------------------------

- Fund amount exceeds period allocation
Comment 10 Barton Chittenden 2014-02-14 21:29:54 UTC
To re-create the bug:

1) Create a budget of 732214.84

2) Create Fund 1 of amount 651162.37

3) Create Fund 2 of amount 81052.46

   Note that the sum of these amounts is 732214.83: One cent less 
   than the budget amount of 732214.84.

4) Edit Fund 2, change the amount to 81052.47

This will generate the following warning:

Form not submitted because of the following problem(s)
------------------------------------------------------------------------------------

- Fund amount exceeds period allocation
Comment 11 Jonathan Druart 2014-02-17 12:48:33 UTC
Created attachment 25348 [details] [review]
Bug 11644 - Cannot update fund if total is equal to amount unallocated

When attempting to update a fund, if the amount unalloccated for the
fund is equal to the total, you can be prevented from saving. This is
due to imprecise floating point number comparison in
check_parent_total.pl

Test Plan:
1) Create a fund where the amount unallocated is equal to the amount
   unallocated for the budget period
2) Edit the fund, attempt to change the name of the fund
3) Note you recieve an error and cannot save
4) Apply this patch
5) Repeat step 2
6) Note you can now update the fund

Signed-off-by: Sean McGarvey <smcgarvey@pascocountyfl.net>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 12 Jonathan Druart 2014-02-17 12:48:39 UTC
Created attachment 25349 [details] [review]
Bug 11644: (follow-up) Add a FIXME for the precision value

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 13 Jonathan Druart 2014-02-17 12:49:42 UTC
(In reply to Barton Chittenden from comment #10)
> To re-create the bug:

Thank you for that!
I am able to reproduce the issue and confirm the patch fixes it.
Comment 14 Galen Charlton 2014-02-19 20:09:38 UTC
Pushed to master.  Thanks, Kyle!

However, I have declined to push Jonathan's follow-up, and instead have pushed one of my own with the following FIXME (which to be clear, is NOT directed at Kyle's patch in particular):

+# FIXME - we really need a better way to do this consistently
+# and across the board, be it bigints, Math::FixedPoint, a
+# modernized version of Math::Curency that isn't tied to the system
+# locale, or something else.

The problem with Jonathan's suggestion of using a system preference to specify the number of decimal places for a currency is that such a thing is not actually a global attribute -- it's more properly an attribute of the currency.  Hard-coding the number of decimal places to 2, while not ideal, won't break things for folks who use JPY.
Comment 15 Fridolin Somers 2014-05-19 15:16:15 UTC
Pushed to 3.14.x, will be in 3.14.07
Comment 16 Kyle M Hall (khall) 2014-06-06 13:52:54 UTC
Pushed to 3.12.x will be in 3.12.13