From d497f772bc69508d9ec2a1aad4a277076221d09d 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 --- 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 2cc81993ef1..476e0e49b9e 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 c0da07c4b94..0bdb242f8cc 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1056,7 +1056,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 00000000000..1f252b5655c --- /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 4e70bab2863..8a133197fe0 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 523300b0036..040135b9f95 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 ebe69db39c2..2aa7766f985 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 4a18526dac3..dffa5ebfcc3 100755 --- a/t/db_dependent/Koha/Account/Line.t +++ b/t/db_dependent/Koha/Account/Line.t @@ -744,7 +744,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" ); @@ -755,7 +755,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' ); @@ -763,7 +763,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.31.0