[Bug 25127] New: Fines with an amountoutstanding of 0 can be created due to maxFine but cannot be forgiven
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25127 Bug ID: 25127 Summary: Fines with an amountoutstanding of 0 can be created due to maxFine but cannot be forgiven Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major 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 To recreate: 1 - Set maxFine to 7.50 2 - Manually create some overdues for a patron in the amounts of: 6.99, .1, .1, .1, .1, .1, .01 (create a manual debits then 'UPDATE accountlines SET debit_type_code = "OVERDUE";' 3 - Set fines for an item type to .1 4 - Checkout an item of that type to patron and backdate so it is overdue 5 - Run fines.pl 6 - Check the DB, they have a new fine of "0" 7 - Checkin the item, checking 'forgive fines' option 8 - ISE "Debit amount passed is not positive" -- 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=25127 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |25123 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25123 [Bug 25123] MaxFines does not count the current updating fine -- 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=25127 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 102826 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102826&action=edit Bug 25127: Unit tests -- 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=25127 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com | |, | |jonathan.druart@bugs.koha-c | |ommunity.org, | |katrin.fischer@bsz-bw.de, | |kyle@bywatersolutions.com, | |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25127 --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I'm confused.. are your libraries literally doing step 2? By skipping UpdateFine for the test data you're ending up with 'fines' without an associated issue.. which shouldn't be possible. If I correct the test to either add the issue_id in or use UpdateFine (which results in the same thing) then I cannot replicate the problem? -- 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=25127 --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 103235 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103235&action=edit Bug 25127: Correction to tests -- 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=25127 --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Aha.. I think that update to the tests clarifies it -- 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=25127 --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Man I hate rounding maths using Floats :( 7.2 (maxfine) - 7.2 (total_amount_other) = 8.88178419700125e-16 ( 😢 ) -- 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=25127 --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 103249 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103249&action=edit Bug 25127: Crude float fix -- 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=25127 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=25127 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Not entirely happy with my fix, but it's the best I've come up with to date and does appear to work for all existing test cases. -- 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=25127 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- You usually need to "$integer + 0.0" to compare it with floats. It's what we do in tests. -- 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=25127 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Long term, we should really switch away from `decimal(28,6)` to `int` or `smallint` at the database level and then set a rounding routine for anywhere we end up with parts of a penny/cent/etc. (Probably we want to stick to 'round to half even' - https://en.wikipedia.org/wiki/Rounding#Round_half_to_even too) -- 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=25127 --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 103253 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103253&action=edit Bug 25127: Compare with 2 decimals -- 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=25127 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Jonathan Druart from comment #10)
Created attachment 103253 [details] [review] Bug 25127: Compare with 2 decimals
This is what I meant. (alternative patch for "Crude float fix") -- 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=25127 --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Using sprintf isn't a bad solution.. it limits us to 2 currencies using only 2 decimal places though.. I'd be interested to know if there are any Koha users out there actually using 3 or more decimals. Also.. using sprintf does the half round by even thing I was talking about in my earlier comment. -- 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=25127 --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Martin Renvoize from comment #12)
Using sprintf isn't a bad solution.. it limits us to 2 currencies using only 2 decimal places though.. I'd be interested to know if there are any Koha users out there actually using 3 or more decimals.
Also.. using sprintf does the half round by even thing I was talking about in my earlier comment.
I think your *100/100 trick will only work for 2 decimals as well. Koha cannot work with more than 2 decimals right now. -- 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=25127 --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Martin Renvoize from comment #12)
Using sprintf isn't a bad solution.. it limits us to 2 currencies using only 2 decimal places though.. I'd be interested to know if there are any Koha users out there actually using 3 or more decimals.
I remember looking it up - it appears there are some currencies where more decimals are valid (column E): https://en.wikipedia.org/wiki/ISO_4217 We might have to treat this as configurable. -- 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=25127 --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Katrin Fischer from comment #14)
We might have to treat this as configurable.
We are supporting 2 decimals, it's hardcoded all over our codebase I think. -- 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=25127 --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 103276 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103276&action=edit Bug 25127: Use Koha::Number::Price->round for comparison -- 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=25127 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |14825 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14825 [Bug 14825] Accounts Rewrite Omnibus -- 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=25127 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103249|0 |1 is obsolete| | -- 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=25127 --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Final alternative.. use the existing Koha::Number::Price->round method for the comparison -- 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=25127 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_20_05_candidate, | |RM_priority -- 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=25127 --- Comment #18 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Yes, that's definitely the way to go! -- 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=25127 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bgkriegel@gmail.com --- Comment #19 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Hi, what is the test plan? patch 3 and 4 are exclusive, seems to me that 4 is preferred. On top of 25123 and patches 1, 2 and 4, the test pass. Is anything more to test? -- 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=25127 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103253|0 |1 is obsolete| | -- 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=25127 --- Comment #20 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I've now obsoleted patch three as discussed. Test plan wise, the Unit tests effectively emulate the plan in Nicks first comment. It's never a bad idea to give it a go manually as a double check that the unit tests are doing what they're supposed to. -- 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=25127 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102826|0 |1 is obsolete| | --- Comment #21 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 103325 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103325&action=edit Bug 25127: Unit tests -- 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=25127 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103235|0 |1 is obsolete| | --- Comment #22 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 103326 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103326&action=edit Bug 25127: Correction to tests -- 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=25127 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103276|0 |1 is obsolete| | --- Comment #23 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 103327 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103327&action=edit Bug 25127: Use Koha::Number::Price->round for comparison -- 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=25127 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 watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25127 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=25127 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103325|0 |1 is obsolete| | --- Comment #24 from David Nind <david@davidnind.com> --- Created attachment 103430 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103430&action=edit Bug 25127: Unit tests Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25127 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103326|0 |1 is obsolete| | --- Comment #25 from David Nind <david@davidnind.com> --- Created attachment 103431 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103431&action=edit Bug 25127: Correction to tests Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25127 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103327|0 |1 is obsolete| | --- Comment #26 from David Nind <david@davidnind.com> --- Created attachment 103432 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103432&action=edit Bug 25127: Use Koha::Number::Price->round for comparison Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25127 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #27 from David Nind <david@davidnind.com> --- (In reply to Martin Renvoize from comment #20)
I've now obsoleted patch three as discussed.
Test plan wise, the Unit tests effectively emulate the plan in Nicks first comment. It's never a bad idea to give it a go manually as a double check that the unit tests are doing what they're supposed to.
I applied the patch and ran prove t/db_dependent/Overdues.t Hope that is sufficient for testing! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25127 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25127 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103430|0 |1 is obsolete| | --- Comment #28 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 104078 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104078&action=edit Bug 25127: Unit tests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25127 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103431|0 |1 is obsolete| | --- Comment #29 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 104079 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104079&action=edit Bug 25127: Correction to tests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25127 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103432|0 |1 is obsolete| | --- Comment #30 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 104080 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104080&action=edit Bug 25127: Use Koha::Number::Price->round for comparison Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25127 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.05.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=25127 --- Comment #31 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25127 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_20_05_candidate, | |RM_priority | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25127 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|20.05.00 |20.05.00, 19.11.06 released in| | CC| |joy@bywatersolutions.com Status|Pushed to master |Pushed to stable --- Comment #32 from Joy Nelson <joy@bywatersolutions.com> --- backported to 19.11.x for 19.11.06 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25127 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #33 from Lucas Gass <lucas@bywatersolutions.com> --- missing dependencies for 19.05.x, no backport -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25127 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=27079 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25127 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |40409 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40409 [Bug 40409] t/db_dependent/Overdues.t generates warnings -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org