[Bug 21759] New: _FixAccountForLostAndReturned does not always create an account offset
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21759 Bug ID: 21759 Summary: _FixAccountForLostAndReturned does not always create an account offset Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: kyle@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org If a lost item is stilled owed in full, the fee is reduced to 0 but no corresponding account offset is generated. All changes to accountlines should generate a corresponding account offset. -- 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=21759 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.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=21759 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21759 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |13098 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13098 [Bug 13098] Item lost multiple times by the same patron will create only be charged once -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21759 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=21727 CC| |martin.renvoize@ptfs-europe | |.com --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Just a guess: this could be implemented using Koha::Account::Line->adjust, provided we allowed negative increments and a suitable update type/offset type is defined. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21759 --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I indeed left the amount in the adjust method open for just this sort of thing. A new offset type seems sensible 😃. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21759 --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Though actually, thinking more about this I'm not sure I understand why we're editing the amount directly here at all.. surely it's just a case of adding a credit to offset the amount outstanding to zero as per other payments/waivers.. I don't know the history, perhaps someone could enlighten me? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21759 --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Martin Renvoize from comment #3)
Though actually, thinking more about this I'm not sure I understand why we're editing the amount directly here at all.. surely it's just a case of adding a credit to offset the amount outstanding to zero as per other payments/waivers.. I don't know the history, perhaps someone could enlighten me?
I'm suspecting about adjust as well -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21759 Bug 21759 depends on bug 13098, which changed state. Bug 13098 Summary: Item lost multiple times by the same patron will create only be charged once https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13098 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21759 --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Kyle, Martin and I agreed this will be implemented by making this add and apply a credit instead of manually setting amount to 0. The use of the changed accounttype is spread in many places and should remain untouched. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21759 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21759 --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 83029 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83029&action=edit Bug 21759: Regression tests This patch tests for a new behaviour in the _FixAccountForLostAndFound method. The method will now add the amountoutstanding value for the lost item fee to the CR credit to be generated. This means that: - If there's some remaining debt, the same amount will be added to the CR credit and used to cancel that debt. The final amountoutstanding will be the same as before, but an offset will be generated as required. - If the line was written off, the behaviour remains unchanged, so no offset. - If the line was payed and/or written off in full only the payments are refund, preserving the current behaviour. Only changes to the 'remaining debt' use cases on this tests are expected. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/Circulation.t => FAIL: Tests fail because the behaviour is not correct. Note: some tests order changes are introduced to avoid calling discard_changes twice -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21759 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 83030 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83030&action=edit Bug 21759: Avoid manually setting amountoutstanding in _FixAccountForLostAndReturned This patch changes the behaviour in the _FixAccountForLostAndFound method. The method will now add the amountoutstanding value for the lost item fee to the CR credit to be generated. This means that: - If there's some remaining debt, the same amount will be added to the CR credit and used to cancel that debt. The final amountoutstanding will be the same as before, but an offset will be generated as required. - If the line was written off, the behaviour remains unchanged, so no offset. - If the line was payed and/or written off in full only the payments are refund, preserving the current behaviour. To test: - Apply the regression tests patch - Run: $ kshell k$ prove t/db_dependent/Circulation.t => FAIL: Tests fail because the behaviour is not correct - Apply this patch - Run: k$ prove t/db_dependent/Circulation.t => SUCCESS: Tests now pass! - Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21759 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|_FixAccountForLostAndReturn |Avoid manually setting |ed does not always create |amountoutstanding in |an account offset |_FixAccountForLostAndReturn | |ed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21759 Kyle M Hall <kyle@bywatersolutions.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=21759 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83029|0 |1 is obsolete| | Attachment #83030|0 |1 is obsolete| | --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 83038 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83038&action=edit Bug 21759: Regression tests This patch tests for a new behaviour in the _FixAccountForLostAndFound method. The method will now add the amountoutstanding value for the lost item fee to the CR credit to be generated. This means that: - If there's some remaining debt, the same amount will be added to the CR credit and used to cancel that debt. The final amountoutstanding will be the same as before, but an offset will be generated as required. - If the line was written off, the behaviour remains unchanged, so no offset. - If the line was payed and/or written off in full only the payments are refund, preserving the current behaviour. Only changes to the 'remaining debt' use cases on this tests are expected. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/Circulation.t => FAIL: Tests fail because the behaviour is not correct. Note: some tests order changes are introduced to avoid calling discard_changes twice 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=21759 --- Comment #9 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 83039 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83039&action=edit Bug 21759: Avoid manually setting amountoutstanding in _FixAccountForLostAndReturned This patch changes the behaviour in the _FixAccountForLostAndFound method. The method will now add the amountoutstanding value for the lost item fee to the CR credit to be generated. This means that: - If there's some remaining debt, the same amount will be added to the CR credit and used to cancel that debt. The final amountoutstanding will be the same as before, but an offset will be generated as required. - If the line was written off, the behaviour remains unchanged, so no offset. - If the line was payed and/or written off in full only the payments are refund, preserving the current behaviour. To test: - Apply the regression tests patch - Run: $ kshell k$ prove t/db_dependent/Circulation.t => FAIL: Tests fail because the behaviour is not correct - Apply this patch - Run: k$ prove t/db_dependent/Circulation.t => SUCCESS: Tests now pass! - Sign off :-D 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=21759 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83038|0 |1 is obsolete| | --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 83040 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83040&action=edit Bug 21759: Regression tests This patch tests for a new behaviour in the _FixAccountForLostAndFound method. The method will now add the amountoutstanding value for the lost item fee to the CR credit to be generated. This means that: - If there's some remaining debt, the same amount will be added to the CR credit and used to cancel that debt. The final amountoutstanding will be the same as before, but an offset will be generated as required. - If the line was written off, the behaviour remains unchanged, so no offset. - If the line was payed and/or written off in full only the payments are refund, preserving the current behaviour. Only changes to the 'remaining debt' use cases on this tests are expected. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/Circulation.t => FAIL: Tests fail because the behaviour is not correct. Note: some tests order changes are introduced to avoid calling discard_changes twice 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=21759 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83039|0 |1 is obsolete| | --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 83041 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83041&action=edit Bug 21759: Avoid manually setting amountoutstanding in _FixAccountForLostAndReturned This patch changes the behaviour in the _FixAccountForLostAndFound method. The method will now add the amountoutstanding value for the lost item fee to the CR credit to be generated. This means that: - If there's some remaining debt, the same amount will be added to the CR credit and used to cancel that debt. The final amountoutstanding will be the same as before, but an offset will be generated as required. - If the line was written off, the behaviour remains unchanged, so no offset. - If the line was payed and/or written off in full only the payments are refund, preserving the current behaviour. To test: - Apply the regression tests patch - Run: $ kshell k$ prove t/db_dependent/Circulation.t => FAIL: Tests fail because the behaviour is not correct - Apply this patch - Run: k$ prove t/db_dependent/Circulation.t => SUCCESS: Tests now pass! - Sign off :-D 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=21759 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83040|0 |1 is obsolete| | --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 83042 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83042&action=edit Bug 21759: Regression tests This patch tests for a new behaviour in the _FixAccountForLostAndFound method. The method will now add the amountoutstanding value for the lost item fee to the CR credit to be generated. This means that: - If there's some remaining debt, the same amount will be added to the CR credit and used to cancel that debt. The final amountoutstanding will be the same as before, but an offset will be generated as required. - If the line was written off, the behaviour remains unchanged, so no offset. - If the line was payed and/or written off in full only the payments are refund, preserving the current behaviour. Only changes to the 'remaining debt' use cases on this tests are expected. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/Circulation.t => FAIL: Tests fail because the behaviour is not correct. Note: some tests order changes are introduced to avoid calling discard_changes twice Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> 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=21759 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83041|0 |1 is obsolete| | --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 83043 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83043&action=edit Bug 21759: Avoid manually setting amountoutstanding in _FixAccountForLostAndReturned This patch changes the behaviour in the _FixAccountForLostAndFound method. The method will now add the amountoutstanding value for the lost item fee to the CR credit to be generated. This means that: - If there's some remaining debt, the same amount will be added to the CR credit and used to cancel that debt. The final amountoutstanding will be the same as before, but an offset will be generated as required. - If the line was written off, the behaviour remains unchanged, so no offset. - If the line was payed and/or written off in full only the payments are refund, preserving the current behaviour. To test: - Apply the regression tests patch - Run: $ kshell k$ prove t/db_dependent/Circulation.t => FAIL: Tests fail because the behaviour is not correct - Apply this patch - Run: k$ prove t/db_dependent/Circulation.t => SUCCESS: Tests now pass! - Sign off :-D Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> 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=21759 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Passing QA.. this works as expected and is a marked improvement for auditing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21759 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |nick@bywatersolutions.com --- Comment #15 from Nick Clemens <nick@bywatersolutions.com> --- Awesome work all! Pushed to master for 19.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21759 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Pushed to 18.11.x for 18.11.01 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21759 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This patch results in a release notes| |proper offset always being | |recorded for auditing | |purposes when a user is | |refunded after returning a | |previously lost item. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21759 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #17 from Lucas Gass <lucas@bywatersolutions.com> --- depends on 13098, won't backport to 18.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21759 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to Stable |RESOLVED CC| |fridolin.somers@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org