Attachment 73516 Details for Bug 20325 – Bug 20325: C4::Accounts::purge_zero_balance_fees does not check account_offsets

[patch] Bug 20325: C4::Accounts::purge_zero_balance_fees does not check account_offsets

Bug-20325-C4Accountspurgezerobalancefees-does-not-.patch (text/plain), 5.47 KB, created by Katrin Fischer on 2018-04-01 10:37:49 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Katrin Fischer

Created: 2018-04-01 10:37:49 UTC

Size: 5.47 KB

patch

obsolete

>From 9aa731c8dab001fcdcc03effaa3f66936a356db3 Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatetsolutions.com>
>Date: Tue, 27 Mar 2018 15:16:16 -0400
>Subject: [PATCH] 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>
>---
> C4/Accounts.pm            | 15 ++++++++++++---
> t/db_dependent/Accounts.t | 33 ++++++++++++++++++++++++++++++++-
> 2 files changed, 44 insertions(+), 4 deletions(-)
>
>diff --git a/C4/Accounts.pm b/C4/Accounts.pm
>index b84627b..4a2b188 100644
>--- 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
>--- 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 {
> 
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 20325: 73350 | 73516 | 73599 | 73603 | 73655 | 73656 | 73678