[Bug 27079] New: UpdateFine adds refunds for fines paid off before return
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Bug ID: 27079 Summary: UpdateFine adds refunds for fines paid off before return Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Fines and fees Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org The issue seems to be that the amount passed in to UpdateFine fails the comparison to $accountline->amount - one is $1.80 and the other is $1.80000 and comparison fails: 576 if ( $accountline ) { 577 if ( $accountline->amount != $amount ) { To recreate: 1 - Set a circ rule to have a .15 per day fine 2 - Issue an item of this type to a borrower 3 - Make it 12 days overdue either at checkout or in the db: UPDATE issues SET date_due = DATE_SUB(CURDATE(),INTERVAL 12 DAY); 4 - Run the fines job or just put a fine in place: INSERT INTO accountlines (issue_id,borrowernumber,itemnumber,amount,debit_type_code,status,amountoutstanding) SELECT issue_id,5,itemnumber,1.80,'OVERDUE','UNRETURNED',1.80 FROM issues WHERE borrowernumber=5; 5 - Pay the fine for the patron 6 - Check in the item 7 - Note that the patron has an 'Overpayment refund' of $0.00 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org, | |kyle@bywatersolutions.com, | |martin.renvoize@ptfs-europe | |.com, tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=17138 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |major -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 --- Comment #1 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Looking at this at the moment... Our dealing with money and floating point are generally a mess. :( -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 113981 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113981&action=edit Bug 27079: Floating point comparison correction We cannot compare calculated values directly as those calculations may have introduced floating point differences. We have the Koha::Number::Price->round function to consistently round the floating point number to a decimal with 2 points precission for comparison. Test plan 1/ Set a circ rule to have a .15 per day fine 2/ Issue an item of this type to a borrower (making it 12 days overdue by setting the due date to 12 days in the past) 3/ Run the fines job: 4/ Pay the fine for the patron 5/ Check in the item 6/ Note that the patron does not have an 'Overpayment refund' of $0.00 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.com -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Working on a unit test now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |17138 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17138 [Bug 17138] UpdateFine() modyfies existing fine records even when there is no need -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=25127 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 113983 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113983&action=edit Bug 27079: Unit tests Add a unit test to check for floating point errors in UpdateFine relating to catching matching CalcFine amounts compared to DB amount values. Test plan 1/ Run the unit test and confirm it fails 2/ Apply the second patch in the series 3/ Run the unit test and confirm it passes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113981|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 113984 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113984&action=edit Bug 27079: Floating point comparison correction We cannot compare calculated values directly as those calculations may have introduced floating point differences. We have the Koha::Number::Price->round function to consistently round the floating point number to a decimal with 2 points precission for comparison. Test plan 1/ Set a circ rule to have a .15 per day fine 2/ Issue an item of this type to a borrower (making it 12 days overdue by setting the due date to 12 days in the past) 3/ Run the fines job: 4/ Pay the fine for the patron 5/ Check in the item 6/ Note that the patron does not have an 'Overpayment refund' of $0.00 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Should not we compare the 2 rounded values (to be on the safe side)? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- The value coming from the database is in effect already rounded.. al be it to 6 decimal places instead of 2.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- My feeling is.. get this one in as is and work on a much nicer solution to catch cases in a more general way in bug 17138 for the 21.05 cycle. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_20_11_target -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Martin Renvoize from comment #7)
The value coming from the database is in effect already rounded.. al be it to 6 decimal places instead of 2..
Yes, that's why I added "to be on the safe side", in case it is modified later (regression proof). And also for consistency: when you read the line it would read better if you understand directly that you are comparing the same thing :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 113997 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113997&action=edit Bug 27079: (QA follow-up) Round both sides for clarity -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113983|0 |1 is obsolete| | --- Comment #11 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 113999 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113999&action=edit Bug 27079: Unit tests Add a unit test to check for floating point errors in UpdateFine relating to catching matching CalcFine amounts compared to DB amount values. Test plan 1/ Run the unit test and confirm it fails 2/ Apply the second patch in the series 3/ Run the unit test and confirm it passes Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113984|0 |1 is obsolete| | --- Comment #12 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 114000 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114000&action=edit Bug 27079: Floating point comparison correction We cannot compare calculated values directly as those calculations may have introduced floating point differences. We have the Koha::Number::Price->round function to consistently round the floating point number to a decimal with 2 points precission for comparison. Test plan 1/ Set a circ rule to have a .15 per day fine 2/ Issue an item of this type to a borrower (making it 12 days overdue by setting the due date to 12 days in the past) 3/ Run the fines job: 4/ Pay the fine for the patron 5/ Check in the item 6/ Note that the patron does not have an 'Overpayment refund' of $0.00 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113997|0 |1 is obsolete| | --- Comment #13 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 114001 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114001&action=edit Bug 27079: (QA follow-up) Round both sides for clarity Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |josef.moravec@gmail.com Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113999|0 |1 is obsolete| | --- Comment #14 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 114018 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114018&action=edit Bug 27079: Unit tests Add a unit test to check for floating point errors in UpdateFine relating to catching matching CalcFine amounts compared to DB amount values. Test plan 1/ Run the unit test and confirm it fails 2/ Apply the second patch in the series 3/ Run the unit test and confirm it passes Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114000|0 |1 is obsolete| | --- Comment #15 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 114019 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114019&action=edit Bug 27079: Floating point comparison correction We cannot compare calculated values directly as those calculations may have introduced floating point differences. We have the Koha::Number::Price->round function to consistently round the floating point number to a decimal with 2 points precission for comparison. Test plan 1/ Set a circ rule to have a .15 per day fine and enable finesMode syspref. 2/ Issue an item of this type to a borrower (making it 12 days overdue by setting the due date to 12 days in the past) 3/ Run the fines job: 4/ Pay the fine for the patron 5/ Check in the item 6/ Note that the patron does not have an 'Overpayment refund' of $0.00 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114001|0 |1 is obsolete| | --- Comment #16 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 114020 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114020&action=edit Bug 27079: (QA follow-up) Round both sides for clarity Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net Status|Signed Off |Passed QA --- Comment #17 from Victor Grousset/tuxayo <victor@tuxayo.net> --- test plan ok test code looks ok code ok QA script ok -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114019|0 |1 is obsolete| | Attachment #114020|0 |1 is obsolete| | --- Comment #18 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 114029 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114029&action=edit Bug 27079: Floating point comparison correction We cannot compare calculated values directly as those calculations may have introduced floating point differences. We have the Koha::Number::Price->round function to consistently round the floating point number to a decimal with 2 points precission for comparison. Test plan 1/ Set a circ rule to have a .15 per day fine and enable finesMode syspref. 2/ Issue an item of this type to a borrower (making it 12 days overdue by setting the due date to 12 days in the past) 3/ Run the fines job: 4/ Pay the fine for the patron 5/ Check in the item 6/ Note that the patron does not have an 'Overpayment refund' of $0.00 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Bug 27079: (QA follow-up) Round both sides for clarity Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 --- Comment #19 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 20.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_20_11_target | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable Version(s)|20.11.00 |20.11.00, 20.05.07 released in| | CC| |andrew@bywatersolutions.com --- Comment #20 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Pushed to 20.05.x for 20.05.07 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27079 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|20.11.00, 20.05.07 |20.11.00, 20.05.07, released in| |19.11.13 Status|Pushed to oldstable |Pushed to oldoldstable --- Comment #21 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Backported to 19.11.x branch for 19.11.13 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org