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>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/C4/Accounts.pm b/C4/Accounts.pm
index b84627b..1adf8d4 100644 (file)
--- a/C4/Accounts.pm
+++ b/C4/Accounts.pm
@@ -448,9 +448,18 @@ sub purge_zero_balance_fees {
     my $dbh = C4::Context->dbh;
     my $sth = $dbh->prepare(
         q{
-            DELETE FROM accountlines
-            WHERE date < date_sub(curdate(), INTERVAL ? DAY)
-              AND ( amountoutstanding = 0 or amountoutstanding IS NULL );
+            DELETE a1 FROM accountlines a1
+
+            LEFT JOIN account_offsets credit_offset ON ( a1.accountlines_id = credit_offset.credit_id )
+            LEFT JOIN accountlines a2 ON ( credit_offset.debit_id = a2.accountlines_id )
+
+            LEFT JOIN account_offsets debit_offset ON ( a1.accountlines_id = debit_offset.debit_id )
+            LEFT JOIN accountlines a3 ON ( debit_offset.credit_id = a3.accountlines_id )
+
+            WHERE a1.date < date_sub(curdate(), INTERVAL ? DAY)
+              AND ( a1.amountoutstanding = 0 OR a1.amountoutstanding IS NULL )
+              AND ( a2.amountoutstanding = 0 OR a2.amountoutstanding IS NULL )
+              AND ( a3.amountoutstanding = 0 OR a3.amountoutstanding IS NULL )
         }
     );
     $sth->execute($days) or die $dbh->errstr;

diff --git a/t/db_dependent/Accounts.t b/t/db_dependent/Accounts.t
index affaacd..426a4a4 100644 (file)
--- a/t/db_dependent/Accounts.t
+++ b/t/db_dependent/Accounts.t
@@ -18,7 +18,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 24;
+use Test::More tests => 30;
 use Test::MockModule;
 use Test::Warn;
 
@@ -134,6 +134,37 @@ for my $data  (@test_data) {
 }
 
 $dbh->do(q|DELETE FROM accountlines|);
+my $debit = Koha::Account::Line->new({ borrowernumber => $borrower->id, date => '1900-01-01', amountoutstanding => 0 })->store();
+my $credit = Koha::Account::Line->new({ borrowernumber => $borrower->id, date => '1900-01-01', amountoutstanding => -5 })->store();
+my $offset = Koha::Account::Offset->new({ credit_id => $credit->id, debit_id => $debit->id, type => 'Payment' })->store();
+purge_zero_balance_fees( 1 );
+my $debit_2 = Koha::Account::Lines->find( $debit->id );
+my $credit_2 = Koha::Account::Lines->find( $credit->id );
+ok( $debit_2, 'Debit was correctly not deleted when credit has balance' );
+ok( $credit_2, 'Credit was correctly not deleted when credit has balance' );
+
+$dbh->do(q|DELETE FROM accountlines|);
+$debit = Koha::Account::Line->new({ borrowernumber => $borrower->id, date => '1900-01-01', amountoutstanding => 5 })->store();
+$credit = Koha::Account::Line->new({ borrowernumber => $borrower->id, date => '1900-01-01', amountoutstanding => 0 })->store();
+$offset = Koha::Account::Offset->new({ credit_id => $credit->id, debit_id => $debit->id, type => 'Payment' })->store();
+purge_zero_balance_fees( 1 );
+$debit_2 = $credit_2 = undef;
+$debit_2 = Koha::Account::Lines->find( $debit->id );
+$credit_2 = Koha::Account::Lines->find( $credit->id );
+ok( $debit_2, 'Debit was correctly not deleted when debit has balance' );
+ok( $credit_2, 'Credit was correctly not deleted when debit has balance' );
+
+$dbh->do(q|DELETE FROM accountlines|);
+$debit = Koha::Account::Line->new({ borrowernumber => $borrower->id, date => '1900-01-01', amountoutstanding => 0 })->store();
+$credit = Koha::Account::Line->new({ borrowernumber => $borrower->id, date => '1900-01-01', amountoutstanding => 0 })->store();
+$offset = Koha::Account::Offset->new({ credit_id => $credit->id, debit_id => $debit->id, type => 'Payment' })->store();
+purge_zero_balance_fees( 1 );
+$debit_2 = Koha::Account::Lines->find( $debit->id );
+$credit_2 = Koha::Account::Lines->find( $credit->id );
+ok( !$debit_2, 'Debit was correctly deleted' );
+ok( !$credit_2, 'Credit was correctly deleted' );
+
+$dbh->do(q|DELETE FROM accountlines|);
 
 subtest "Koha::Account::pay tests" => sub {