From 35d1c7c41c93e9419ca39ad3d710c64202c309d3 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Wed, 17 Mar 2021 09:10:17 +0000 Subject: [PATCH] Bug 27971: Update void method to use double entry accounting This patch adds double-entry accounting to the Koha::Account::Line->void method. This results in the addition of a VOID debit type line that is offset against the original credit type line that is being voided. This allows us to accurately record when the void took place, at what branch and by whome the void was triggered. Test plan 1/ Apply the database update 2/ Add some debts to a borrower account 3/ Pay those debts 4/ Void the payment 5/ A new 'VOID' line should appear on the account for the full amount of the original payment. 6/ Payments should have all been reversed 7/ t/db_dependent/Koha/Account/Line.t should still pass 8/ Signoff Signed-off-by: Kyle M Hall Signed-off-by: Tomas Cohen Arazi Signed-off-by: Jonathan Druart --- Koha/Account/Line.pm | 88 ++++++++++++++++--- Koha/Item.pm | 2 +- .../data/mysql/atomicupdate/bug_27971.perl | 23 +++++ .../mysql/mandatory/account_debit_types.sql | 3 +- .../mysql/mandatory/account_offset_types.sql | 3 +- members/boraccount.pl | 8 +- t/db_dependent/Koha/Account/Line.t | 6 +- 7 files changed, 112 insertions(+), 21 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_27971.perl diff --git a/Koha/Account/Line.pm b/Koha/Account/Line.pm index dbba892790..b2388f835b 100644 --- a/Koha/Account/Line.pm +++ b/Koha/Account/Line.pm @@ -210,7 +210,10 @@ sub debits { =head3 void - $payment_accountline->void(); + $payment_accountline->void({ + interface => $interface, + [ staff_id => $staff_id, branch => $branchcode ] + }); Used to 'void' (or reverse) a payment/credit. It will roll back any offsets created by the application of this credit upon any debits and mark the credit @@ -219,17 +222,74 @@ as 'void' by updating it's status to "VOID". =cut sub void { - my ($self) = @_; + my ($self, $params) = @_; + + # Make sure it is a credit we are voiding + unless ( $self->is_credit ) { + Koha::Exceptions::Account::IsNotCredit->throw( + error => 'Account line ' . $self->id . 'is not a credit' ); + } - # Make sure it is a payment we are voiding - return unless $self->amount < 0; + # Make sure it is not already voided + if ( $self->status && $self->status eq 'VOID' ) { + Koha::Exceptions::Account->throw( + error => 'Account line ' . $self->id . 'is already void' ); + } + + # Check for mandatory parameters + my @mandatory = ( 'interface' ); + for my $param (@mandatory) { + unless ( defined( $params->{$param} ) ) { + Koha::Exceptions::MissingParameter->throw( + error => "The $param parameter is mandatory" ); + } + } + + # More mandatory parameters + if ( $params->{interface} eq 'intranet' ) { + my @optional = ( 'staff_id', 'branch' ); + for my $param (@optional) { + unless ( defined( $params->{$param} ) ) { + Koha::Exceptions::MissingParameter->throw( error => +"The $param parameter is mandatory when interface is set to 'intranet'" + ); + } + } + } + # Find any applied offsets for the credit so we may reverse them my @account_offsets = Koha::Account::Offsets->search( { credit_id => $self->id, amount => { '<' => 0 } } ); + my $void; $self->_result->result_source->schema->txn_do( sub { + + # A 'void' is a 'debit' + $void = Koha::Account::Line->new( + { + borrowernumber => $self->borrowernumber, + date => \'NOW()', + debit_type_code => 'VOID', + amount => $self->amount * -1, + amountoutstanding => $self->amount * -1, + manager_id => $params->{staff_id}, + interface => $params->{interface}, + branchcode => $params->{branch}, + } + )->store(); + + # Record the creation offset + Koha::Account::Offset->new( + { + debit_id => $void->id, + type => 'VOID', + amount => $self->amount * -1 + } + )->store(); + + # Reverse any applied payments foreach my $account_offset (@account_offsets) { my $fee_paid = Koha::Account::Lines->find( $account_offset->debit_id ); @@ -246,11 +306,18 @@ sub void { credit_id => $self->id, debit_id => $fee_paid->id, amount => $amount_paid, - type => 'Void Payment', + type => 'VOID', } )->store(); } + # Link void to payment + $self->set({ + amountoutstanding => $self->amount, + status => 'VOID' + })->store(); + $self->apply({ debits => [$void]}); + if ( C4::Context->preference("FinesLog") ) { logaction( "FINES", 'VOID', @@ -273,18 +340,11 @@ sub void { ) ); } - - $self->set( - { - status => 'VOID', - amountoutstanding => 0, - amount => 0, - } - ); - $self->store(); } ); + $void->discard_changes; + return $void; } =head3 cancel diff --git a/Koha/Item.pm b/Koha/Item.pm index ab498da607..2bad020287 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1088,7 +1088,7 @@ sub _set_found_trigger { if ( $refund ) { # Revert the forgive credit - $refund->void(); + $refund->void({ interface => 'trigger' }); $self->{_restored} = 1; } diff --git a/installer/data/mysql/atomicupdate/bug_27971.perl b/installer/data/mysql/atomicupdate/bug_27971.perl new file mode 100644 index 0000000000..1f252b5655 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_27971.perl @@ -0,0 +1,23 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + $dbh->do( + qq{ + INSERT IGNORE INTO account_debit_types ( + code, + description, + can_be_invoiced, + can_be_sold, + default_amount, + is_system + ) + VALUES + ('VOID', 'Credit has been voided', 0, 0, NULL, 1) + } + ); + + $dbh->do(q{ + INSERT IGNORE INTO account_offset_types ( type ) VALUES ('VOID'); + }); + + NewVersion( $DBversion, 27971, "Add VOID debit type code"); +} diff --git a/installer/data/mysql/mandatory/account_debit_types.sql b/installer/data/mysql/mandatory/account_debit_types.sql index 4e70bab286..8a133197fe 100644 --- a/installer/data/mysql/mandatory/account_debit_types.sql +++ b/installer/data/mysql/mandatory/account_debit_types.sql @@ -12,4 +12,5 @@ INSERT INTO account_debit_types ( code, description, can_be_invoiced, can_be_sol ('RENT_RENEW', 'Renewal of rental item', 0, 0, NULL, 1), ('RENT_DAILY_RENEW', 'Renewal of daily rental item', 0, 0, NULL, 1), ('RESERVE', 'Hold fee', 0, 0, NULL, 1), -('RESERVE_EXPIRED', 'Hold waiting too long', 0, 0, NULL, 1); +('RESERVE_EXPIRED', 'Hold waiting too long', 0, 0, NULL, 1), +('VOID', 'Credit has been voided', 0, 0, NULL, 1); diff --git a/installer/data/mysql/mandatory/account_offset_types.sql b/installer/data/mysql/mandatory/account_offset_types.sql index 523300b003..040135b9f9 100644 --- a/installer/data/mysql/mandatory/account_offset_types.sql +++ b/installer/data/mysql/mandatory/account_offset_types.sql @@ -23,4 +23,5 @@ INSERT INTO account_offset_types ( type ) VALUES ('PAYOUT'), ('DISCOUNT'), ('REFUND'), -('CANCELLATION'); +('CANCELLATION'), +('VOID'); diff --git a/members/boraccount.pl b/members/boraccount.pl index 0fec75ed59..c2befdabd0 100755 --- a/members/boraccount.pl +++ b/members/boraccount.pl @@ -72,7 +72,13 @@ my $registerid = $input->param('registerid'); if ( $action eq 'void' ) { my $payment_id = scalar $input->param('accountlines_id'); my $payment = Koha::Account::Lines->find( $payment_id ); - $payment->void(); + $payment->void( + { + branch => $library_id, + staff_id => $logged_in_user->id, + interface => 'intranet', + } + ); } if ( $action eq 'payout' ) { diff --git a/t/db_dependent/Koha/Account/Line.t b/t/db_dependent/Koha/Account/Line.t index dcc08aaae0..5a6677c77e 100755 --- a/t/db_dependent/Koha/Account/Line.t +++ b/t/db_dependent/Koha/Account/Line.t @@ -777,7 +777,7 @@ subtest "void() tests" => sub { is( $line1->amountoutstanding+0, 0, 'First fee has amount outstanding of 0' ); is( $line2->amountoutstanding+0, 0, 'Second fee has amount outstanding of 0' ); - my $ret = $account_payment->void(); + my $ret = $account_payment->void({ interface => 'test' }); is( ref($ret), 'Koha::Account::Line', 'Void returns the account line' ); is( $account->balance(), 30, "Account balance is again 30" ); @@ -788,7 +788,7 @@ subtest "void() tests" => sub { is( $account_payment->credit_type_code, 'PAYMENT', 'Voided payment credit_type_code is still PAYMENT' ); is( $account_payment->status, 'VOID', 'Voided payment status is VOID' ); - is( $account_payment->amount+0, 0, 'Voided payment amount is 0' ); + is( $account_payment->amount+0, -30, 'Voided payment amount is -30' ); is( $account_payment->amountoutstanding+0, 0, 'Voided payment amount outstanding is 0' ); is( $line1->amountoutstanding+0, 10, 'First fee again has amount outstanding of 10' ); @@ -796,7 +796,7 @@ subtest "void() tests" => sub { # Accountlines that are not credits should be un-voidable my $line1_pre = $line1->unblessed(); - $ret = $line1->void(); + $ret = $line1->void({ interface => 'test' }); $line1->_result->discard_changes(); my $line1_post = $line1->unblessed(); is( $ret, undef, 'Attempted void on non-credit returns undef' ); -- 2.39.5