https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20325 --- Comment #1 from Barton Chittenden <barton@bywatersolutions.com> --- Here's data from a production system that I think would be adversely affected if cleanup_database.pl --fees 1 was run Query: SELECT a.*, credit.amount AS 'credit amount', credit.amountoutstanding 'credit outstanding', credit.description 'credit description', credit.accounttype AS 'credit type', debit.amount AS 'debit amount', debit.amountoutstanding 'debit outstanding', debit.description 'debit description', debit.accounttype AS 'debit type' FROM account_offsets a INNER JOIN accountlines credit on a.credit_id = credit.accountlines_id INNER JOIN accountlines debit on a.debit_id = debit.accountlines_id WHERE credit.amountoutstanding = 0 and debit.amountoutstanding > 0 ORDER BY a.id desc limit 1 *************************** 1. row *************************** id: 4214002 credit_id: 1708799 debit_id: 1708796 type: Payment amount: 0.000000 created_on: 2018-03-02 11:51:05 credit amount: -5.000000 credit outstanding: 0.000000 credit description: credit type: Pay debit amount: 0.500000 debit outstanding: 0.500000 debit description: I survived the eruption of Mount St. Helens, 1980 / 02/28/2018 23:59 debit type: F In this case, credit outstanding, i.e. accountlines.amountoutstanding where accountlines_id = 1708799 is 0, so that row would be deleted by purge_zero_balance_fees, even though it's still linked to by account_offsets, and debit outstanding is not 0. -- 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=20325 Kyle M Hall <kyle@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=20325 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | CC| |kyle@bywatersolutions.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=20325 --- Comment #4 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Kyle, this checks out well for me and I am signing off to move this along. Just one question: Would it be safe to remove this warning now from the docs of cleanup_database? WARNING: Fees and payments may not be deleted together. This will not affect the account balance but may be confusing to staff. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20325 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #73350|0 |1 is obsolete| | --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 73516 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73516&action=edit Bug 20325: C4::Accounts::purge_zero_balance_fees does not check account_offsets purge_zero_balance_fees is used in cleanup_database.pl to determine which fees can be cleaned up. It uses a simple SQL query to determine which rows in accountlines need to be removed: 463 my $sth = $dbh->prepare( 464 q{ 465 DELETE FROM accountlines 466 WHERE date < date_sub(curdate(), INTERVAL ? DAY) 467 AND ( amountoutstanding = 0 or amountoutstanding IS NULL ); 468 } The function comes with the following warning: 451 B<Warning:> Because fines and payments are not linked in accountlines, it is 452 possible for a fine to be deleted without the accompanying payment, 453 or vise versa. This won't affect the account balance, but might be 454 confusing to staff. This was a reasonable solution prior to the addition of account_offsets in 17.11. The problem now is that rows in accountlines which are linked as credits in accountlines will *always* have amountoutstanding marked as 0. These are linked to debits via account_offsets. purge_zero_balance_fees will delete credits and leave rows in account_offsets which link to deleted credits. Sites using the --fees option cleanup_database.pl which upgrade to 17.11 may have all of their credits removed without warning. Test Plan: 1) Apply this patch 2) prove t/db_dependent/Accounts.t Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20325 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #73516|0 |1 is obsolete| | --- Comment #6 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 73599 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73599&action=edit Bug 20325: C4::Accounts::purge_zero_balance_fees does not check account_offsets purge_zero_balance_fees is used in cleanup_database.pl to determine which fees can be cleaned up. It uses a simple SQL query to determine which rows in accountlines need to be removed: 463 my $sth = $dbh->prepare( 464 q{ 465 DELETE FROM accountlines 466 WHERE date < date_sub(curdate(), INTERVAL ? DAY) 467 AND ( amountoutstanding = 0 or amountoutstanding IS NULL ); 468 } The function comes with the following warning: 451 B<Warning:> Because fines and payments are not linked in accountlines, it is 452 possible for a fine to be deleted without the accompanying payment, 453 or vise versa. This won't affect the account balance, but might be 454 confusing to staff. This was a reasonable solution prior to the addition of account_offsets in 17.11. The problem now is that rows in accountlines which are linked as credits in accountlines will *always* have amountoutstanding marked as 0. These are linked to debits via account_offsets. purge_zero_balance_fees will delete credits and leave rows in account_offsets which link to deleted credits. Sites using the --fees option cleanup_database.pl which upgrade to 17.11 may have all of their credits removed without warning. Test Plan: 1) Apply this patch 2) prove t/db_dependent/Accounts.t Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> 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=20325 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #73599|0 |1 is obsolete| | --- Comment #7 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 73603 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73603&action=edit Bug 20325: C4::Accounts::purge_zero_balance_fees does not check account_offsets purge_zero_balance_fees is used in cleanup_database.pl to determine which fees can be cleaned up. It uses a simple SQL query to determine which rows in accountlines need to be removed: 463 my $sth = $dbh->prepare( 464 q{ 465 DELETE FROM accountlines 466 WHERE date < date_sub(curdate(), INTERVAL ? DAY) 467 AND ( amountoutstanding = 0 or amountoutstanding IS NULL ); 468 } The function comes with the following warning: 451 B<Warning:> Because fines and payments are not linked in accountlines, it is 452 possible for a fine to be deleted without the accompanying payment, 453 or vise versa. This won't affect the account balance, but might be 454 confusing to staff. This was a reasonable solution prior to the addition of account_offsets in 17.11. The problem now is that rows in accountlines which are linked as credits in accountlines will *always* have amountoutstanding marked as 0. These are linked to debits via account_offsets. purge_zero_balance_fees will delete credits and leave rows in account_offsets which link to deleted credits. Sites using the --fees option cleanup_database.pl which upgrade to 17.11 may have all of their credits removed without warning. Test Plan: 1) Apply this patch 2) prove t/db_dependent/Accounts.t Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> 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=20325 --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 73655 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73655&action=edit Bug 20325: Move tests to a subtest -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20325 --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #8)

(In reply to Katrin Fischer from comment #4)

...

Hi Kyle, this checks out well for me and I am signing off to move this along. Just one question:

Would it be safe to remove this warning now from the docs of cleanup_database?

WARNING: Fees and payments may not be deleted together. This will not affect the account balance but may be confusing to staff.

Kyle, ping

Absolutely! I'll submit a followup right now! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20325 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 18.05, 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=20325 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #15 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Can you confirm this depends on Bug 14826 ? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20325 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |14826 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14826 [Bug 14826] Store account offsets -- You are receiving this mail because: You are watching all bug changes.