[Bug 30254] New: New overdue fine applied to incorrectly when using "Refund lost item charge and charge new overdue fine" option in circ rules
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 Bug ID: 30254 Summary: New overdue fine applied to incorrectly when using "Refund lost item charge and charge new overdue fine" option in circ rules Change sponsored?: --- Product: Koha Version: 21.05 Hardware: All OS: All Status: NEW Severity: critical Priority: P5 - low Component: Fines and fees Assignee: koha-bugs@lists.koha-community.org Reporter: andrew@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org Depends on: 23091 I've only confirmed this on 21.05. I haven't been able to replicate on master. When using the lost item fee refund feature to charge a new overdue fee when a lost item is found, Koha can end up creating an overdue fine on a patron who never actually had an overdue item. To recreate: - set Lost item fee refund on return policy to "Refund lost item charge and charge new overdue fine" - have an itemtype / patron combo that charges an overdue fine - check item out to patron and then right back in again - confirm patron doesn't have a fine because the item was not late - set the item to Missing - wait until after the item's due date from your previous checkout - check the item out to a new patron - confirm your original patron now has a fine When the item is checked out and Koha clears its missing status, it looks back for the most recent checkout in old_issues and generates an overdue fine as if that item had been checked out the whole time. Even though this item wasn't lost by a patron and the first patron never had an overdue fine on it. Things get more confusing if the first patron is anonymizing their reading history, as then the fine ends up on the anonymous borrower. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23091 [Bug 23091] Add options to charge new or restore forgiven overdues when a lost item is returned -- 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=30254 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|21.05 |master --- Comment #1 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Confirmed in master. To recreate: 1 - set Lost item fee refund on return policy to "Refund lost item charge and charge new overdue fine", turn on FinesMode, make sure your circ rules charge fines 2 - have an itemtype / patron combo that charges an overdue fine 3 - check item out (with a due date in the future) and then right back in again 4 - confirm patron doesn't have a fine because the item was not late 5 - set the item to Lost 6 - in the database, edit the date_due of your checkout to a date in the past 7 - check the item in, it is marked found 8 - confirm your patron now has a fine -- 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=30254 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |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=30254 Sara Brown <sbrown@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |sbrown@bywatersolutions.com --- Comment #2 from Sara Brown <sbrown@bywatersolutions.com> --- This remains an issue in 21.11. -- 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=30254 hebah@bywatersolutions.com changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hebah@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 --- Comment #3 from Sara Brown <sbrown@bywatersolutions.com> --- Still applicable as of 22.05.05. -- 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=30254 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=30254 --- Comment #4 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 142839 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142839&action=edit Bug 30254: 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=30254 --- Comment #5 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 142840 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142840&action=edit Bug 30254: Don't charge overdue fines unless some fine exists We need to determine if a book was lost by a patron, the clues we have are previous charges. If we don't find any, we shouldn't charge a new fine To test: 1 - set Lost item fee refund on return policy to "Refund lost item charge and charge new overdue fine", turn on FinesMode, make sure your circ rules charge fines 2 - have an itemtype / patron combo that charges an overdue fine 3 - check item out (with a due date in the future) and then right back in again 4 - confirm patron doesn't have a fine because the item was not late 5 - set the item to Lost 6 - in the database, edit the date_due of your checkout to a date in the past 7 - check the item in, it is marked found 8 - confirm your patron now has a fine 9 - Apply patch 10 - Repeat with a new item and patron 11 - Confirm no charges -- 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=30254 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | -- 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=30254 Martin Renvoize <martin.renvoize@ptfs-europe.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=30254 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142839|0 |1 is obsolete| | Attachment #142840|0 |1 is obsolete| | --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 142851 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142851&action=edit Bug 30254: Unit tests Signed-off-by: Martin Renvoize <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=30254 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 142852 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142852&action=edit Bug 30254: Don't charge overdue fines unless some fine exists We need to determine if a book was lost by a patron, the clues we have are previous charges. If we don't find any, we shouldn't charge a new fine To test: 1 - set Lost item fee refund on return policy to "Refund lost item charge and charge new overdue fine", turn on FinesMode, make sure your circ rules charge fines 2 - have an itemtype / patron combo that charges an overdue fine 3 - check item out (with a due date in the future) and then right back in again 4 - confirm patron doesn't have a fine because the item was not late 5 - set the item to Lost 6 - in the database, edit the date_due of your checkout to a date in the past 7 - check the item in, it is marked found 8 - confirm your patron now has a fine 9 - Apply patch 10 - Repeat with a new item and patron 11 - Confirm no charges Signed-off-by: Martin Renvoize <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=30254 --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 142853 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142853&action=edit Bug 30254: (QA follow-up) Remove warn from tests Signed-off-by: Martin Renvoize <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=30254 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Thanks for beating me to this one Nick, much appreciated.. Simple fix, signing off. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |katrin.fischer@bsz-bw.de |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|katrin.fischer@bsz-bw.de | --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I am sorry, looks good, passes QA tests and unit tests, but couldn't finish testing here (ran out of time). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I think this may have fallen off Katrin's radar... bump -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Happy for someone else to take this on - please don't wait on me! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Martin Renvoize from comment #9)
Thanks for beating me to this one Nick, much appreciated..
Simple fix, signing off.
Looking here now. If this is a simple fix, I may not understand Koha :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 --- Comment #14 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- A first glance tells me: The unit test does not cover all changes made in the second patch. There is a huge amount of changed lines and only one very trivial test that does not convince me at all. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- This is a bit arbitrary, but looking at sub _set_found_trigger and the associated tests in subtest '_set_found_trigger() tests', I think that we should improve the tests here. It looks like that we do not even check the account balances while we are refunding and charging.. Changing status for need of feedback/further attention. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #16 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Marcel de Rooy from comment #14)
A first glance tells me: The unit test does not cover all changes made in the second patch. There is a huge amount of changed lines and only one very trivial test that does not convince me at all.
git diff -w HEAD~3.. Koha/Item.pm It changes three lines to prevent sending a 'lost_charge' message when there is no charge to replace The test fails before, and passes after and other behaviour is covered by _set_found_trigger tests. I agree, expanding coverage is good, but should be on another bug -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com QA Contact| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 Kyle M Hall <kyle@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=30254 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142851|0 |1 is obsolete| | Attachment #142852|0 |1 is obsolete| | Attachment #142853|0 |1 is obsolete| | --- Comment #17 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 145549 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145549&action=edit Bug 30254: Unit tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #145549|0 |1 is obsolete| | --- Comment #18 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 145550 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145550&action=edit Bug 30254: Unit tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 --- Comment #19 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 145551 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145551&action=edit Bug 30254: Don't charge overdue fines unless some fine exists We need to determine if a book was lost by a patron, the clues we have are previous charges. If we don't find any, we shouldn't charge a new fine To test: 1 - set Lost item fee refund on return policy to "Refund lost item charge and charge new overdue fine", turn on FinesMode, make sure your circ rules charge fines 2 - have an itemtype / patron combo that charges an overdue fine 3 - check item out (with a due date in the future) and then right back in again 4 - confirm patron doesn't have a fine because the item was not late 5 - set the item to Lost 6 - in the database, edit the date_due of your checkout to a date in the past 7 - check the item in, it is marked found 8 - confirm your patron now has a fine 9 - Apply patch 10 - Repeat with a new item and patron 11 - Confirm no charges Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 --- Comment #20 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 145552 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145552&action=edit Bug 30254: (QA follow-up) Remove warn from tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |23.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=30254 --- Comment #21 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.05. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 Jacob O'Mara <jacob.omara@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.05.00 |23.05.00,22.11.03 released in| | Status|Pushed to master |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 --- Comment #22 from Jacob O'Mara <jacob.omara@ptfs-europe.com> --- Nice work, thanks everyone! Pushed to 22.11.x for the next release. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #23 from Lucas Gass <lucas@bywatersolutions.com> --- Tests won't apply to 22.05.x, if needed please rebase. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 --- Comment #24 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 147185 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147185&action=edit Bug 30254: [22.05.x] Don't charge overdue fines unless some fine exists We need to determine if a book was lost by a patron, the clues we have are previous charges. If we don't find any, we shouldn't charge a new fine To test: 1 - set Lost item fee refund on return policy to "Refund lost item charge and charge new overdue fine", turn on FinesMode, make sure your circ rules charge fines 2 - have an itemtype / patron combo that charges an overdue fine 3 - check item out (with a due date in the future) and then right back in again 4 - confirm patron doesn't have a fine because the item was not late 5 - set the item to Lost 6 - in the database, edit the date_due of your checkout to a date in the past 7 - check the item in, it is marked found 8 - confirm your patron now has a fine 9 - Apply patch 10 - Repeat with a new item and patron 11 - Confirm no charges Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Bug 30254: Unit tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Bug 30254: (QA follow-up) Remove warn from tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> https://bugs.koha-community.org/show_bug.cgi?id=32054 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable Version(s)|23.05.00,22.11.03 |23.05.00,22.11.03, 22.05.10 released in| | --- Comment #25 from Lucas Gass <lucas@bywatersolutions.com> --- Backported to 22.05.x for upcoming 22.05.10 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |arthur.suzuki@biblibre.com --- Comment #26 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Conflicts when trying to apply to 21.11.x from 22.05.x. Can you provide a backport patch if needed? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |38477 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38477 [Bug 38477] Regression: new overdue fine applied incorrectly when using "Refund Lost Item Charge and Charge New Overdue Fine" option in circ rules -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30254 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Version(s)|23.05.00,22.11.03, 22.05.10 |23.05.00,22.11.03,22.05.10 released in| | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org