Bugzilla – Attachment 111256 Details for
Bug 24603
Allow to cancel charges in patron accounting
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 24603: (follow-up) Update to double entry accounting
Bug-24603-follow-up-Update-to-double-entry-account.patch (text/plain), 19.00 KB, created by
David Nind
on 2020-10-05 19:31:22 UTC
(
hide
)
Description:
Bug 24603: (follow-up) Update to double entry accounting
Filename:
MIME Type:
Creator:
David Nind
Created:
2020-10-05 19:31:22 UTC
Size:
19.00 KB
patch
obsolete
>From f7461873b194e7457a29812c93bb47982be09cf4 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Fri, 10 Jul 2020 14:49:35 +0100 >Subject: [PATCH] Bug 24603: (follow-up) Update to double entry accounting > >This patch updates the logic to create a cancellation accountline and >apply it to the charge line so we correction record the transaction in >terms of double entry accounting standards. > >Test plan: >1. Go to a patron's accounting section >2. Create a manual invoice >3. In Transactions tab, you should see a 'Cancel charge' button. Click > on it. It should now be marked as cancelled >4. A cancellation line should be associated with the original charge. >5. Create another manual invoice >6. Pay it (partially or fully) >7. Notice that the 'Cancel charge' button is not available >8. Void the payment >9. 'Cancel charge' button is available again. Click on it and verify > that it still works >10. prove t/db_dependent/Koha/Account/Lines.t > >Signed-off-by: David Nind <david@davidnind.com> >--- > Koha/Account/Line.pm | 99 +++++++++++----- > installer/data/mysql/account_credit_types.sql | 3 +- > installer/data/mysql/account_offset_types.sql | 3 +- > installer/data/mysql/atomicupdate/bug_24603.perl | 20 ++++ > .../prog/en/modules/members/boraccount.tt | 4 +- > members/cancel-charge.pl | 9 +- > t/db_dependent/Koha/Account/Line.t | 131 +++++++++++++++------ > 7 files changed, 202 insertions(+), 67 deletions(-) > create mode 100644 installer/data/mysql/atomicupdate/bug_24603.perl > >diff --git a/Koha/Account/Line.pm b/Koha/Account/Line.pm >index 475ec1a7b6..2f87c081f5 100644 >--- a/Koha/Account/Line.pm >+++ b/Koha/Account/Line.pm >@@ -291,46 +291,90 @@ sub void { > > Cancel a charge. It will mark the debit as 'cancelled' by updating its > status to 'CANCELLED'. >+ > Charges that have been fully or partially paid cannot be cancelled. > >-Return self in case of success, undef otherwise >+Returns the cancellation accountline. > > =cut > > sub cancel { >- my ($self) = @_; >+ my ( $self, $params ) = @_; > >- # Make sure it is a charge we are cancelling >- return unless $self->is_debit; >+ # Make sure it is a charge we are reducing >+ unless ( $self->is_debit ) { >+ Koha::Exceptions::Account::IsNotDebit->throw( >+ error => 'Account line ' . $self->id . 'is not a debit' ); >+ } >+ if ( $self->debit_type_code eq 'PAYOUT' ) { >+ Koha::Exceptions::Account::IsNotDebit->throw( >+ error => 'Account line ' . $self->id . 'is a payout' ); >+ } > > # Make sure it is not already cancelled >- return if $self->status && $self->status eq 'CANCELLED'; >+ if ( $self->status && $self->status eq 'CANCELLED' ) { >+ Koha::Exceptions::Account->throw( >+ error => 'Account line ' . $self->id . 'is already cancelled' ); >+ } > > # Make sure it has not be paid yet >- return if $self->amount != $self->amountoutstanding; >- >- if ( C4::Context->preference("FinesLog") ) { >- logaction('FINES', 'CANCEL', $self->borrowernumber, Dumper({ >- action => 'cancel_charge', >- borrowernumber => $self->borrowernumber, >- amount => $self->amount, >- amountoutstanding => $self->amountoutstanding, >- description => $self->description, >- debit_type_code => $self->debit_type_code, >- note => $self->note, >- itemnumber => $self->itemnumber, >- manager_id => $self->manager_id, >- })); >+ if ( $self->amount != $self->amountoutstanding ) { >+ Koha::Exceptions::Account->throw( >+ error => 'Account line ' . $self->id . 'is already offset' ); > } > >- $self->set({ >- status => 'CANCELLED', >- amountoutstanding => 0, >- amount => 0, >- }); >- $self->store(); >+ # Check for mandatory parameters >+ my @mandatory = ( 'staff_id', 'branch' ); >+ for my $param (@mandatory) { >+ unless ( defined( $params->{$param} ) ) { >+ Koha::Exceptions::MissingParameter->throw( >+ error => "The $param parameter is mandatory" ); >+ } >+ } > >- return $self; >+ my $cancellation; >+ $self->_result->result_source->schema->txn_do( >+ sub { >+ >+ # A 'cancellation' is a 'credit' >+ $cancellation = Koha::Account::Line->new( >+ { >+ date => \'NOW()', >+ amount => 0 - $self->amount, >+ credit_type_code => 'CANCELLATION', >+ status => 'ADDED', >+ amountoutstanding => 0 - $self->amount, >+ manager_id => $params->{staff_id}, >+ borrowernumber => $self->borrowernumber, >+ interface => 'intranet', >+ branchcode => $params->{branch}, >+ } >+ )->store(); >+ >+ my $cancellation_offset = Koha::Account::Offset->new( >+ { >+ credit_id => $cancellation->accountlines_id, >+ type => 'CANCELLATION', >+ amount => $self->amount >+ } >+ )->store(); >+ >+ # Link cancellation to charge >+ $cancellation->apply( >+ { >+ debits => [$self], >+ offset_type => 'CANCELLATION' >+ } >+ ); >+ $cancellation->status('APPLIED')->store(); >+ >+ # Update status of original debit >+ $self->status('CANCELLED')->store; >+ } >+ ); >+ >+ $cancellation->discard_changes; >+ return $cancellation; > } > > =head3 reduce >@@ -453,7 +497,8 @@ sub reduce { > } > else { > >- # Zero amount offset used to link original 'debit' to reduction 'credit' >+ # Zero amount offset used to link original 'debit' to >+ # reduction 'credit' > my $link_reduction_offset = Koha::Account::Offset->new( > { > credit_id => $reduction->accountlines_id, >diff --git a/installer/data/mysql/account_credit_types.sql b/installer/data/mysql/account_credit_types.sql >index 178b98b1c6..2c34471e90 100644 >--- a/installer/data/mysql/account_credit_types.sql >+++ b/installer/data/mysql/account_credit_types.sql >@@ -6,6 +6,7 @@ INSERT INTO account_credit_types ( code, description, can_be_added_manually, is_ > ('DISCOUNT', 'A discount applied to a patrons fine', 0, 1), > ('REFUND', 'A refund applied to a patrons fine', 0, 1), > ('LOST_FOUND', 'Lost item fee refund', 0, 1), >-('PURCHASE', 'Purchase', 0, 1); >+('PURCHASE', 'Purchase', 0, 1), >+('CANCELLATION', 'Cancellation', 0, 1); > > INSERT INTO authorised_values (category,authorised_value,lib) VALUES ('PAYMENT_TYPE','CASH','Cash'); >diff --git a/installer/data/mysql/account_offset_types.sql b/installer/data/mysql/account_offset_types.sql >index c27a68996f..86ebdafb88 100644 >--- a/installer/data/mysql/account_offset_types.sql >+++ b/installer/data/mysql/account_offset_types.sql >@@ -21,4 +21,5 @@ INSERT INTO account_offset_types ( type ) VALUES > ('Credit Applied'), > ('PAYOUT'), > ('DISCOUNT'), >-('REFUND'); >+('REFUND'), >+('CANCELLATION'); >diff --git a/installer/data/mysql/atomicupdate/bug_24603.perl b/installer/data/mysql/atomicupdate/bug_24603.perl >new file mode 100644 >index 0000000000..a9c6bfe3b8 >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/bug_24603.perl >@@ -0,0 +1,20 @@ >+$DBversion = 'XXX'; # will be replaced by the RM >+if ( CheckVersion($DBversion) ) { >+ >+ $dbh->do( >+ qq{ >+ INSERT IGNORE INTO account_credit_types (code, description, can_be_added_manually, is_system) >+ VALUES >+ ('CANCELLATION', 'A cancellation applied to a patron charge', 0, 1) >+ } >+ ); >+ >+ $dbh->do( >+ qq{ >+ INSERT IGNORE INTO account_offset_types ( type ) VALUES ('CANCELLATION'); >+ } >+ ); >+ >+ # Always end with this (adjust the bug info) >+ NewVersion( $DBversion, 24603, "Add CANCELLATION credit_type_code" ); >+} >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt >index cc6d0f0cbd..0fb385447e 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt >@@ -87,12 +87,12 @@ > [% END %] > <a href="accountline-details.pl?accountlines_id=[% account.accountlines_id | uri %]" class="btn btn-default btn-xs"><i class="fa fa-list"></i> Details</a> > [% IF account.is_debit && account.amountoutstanding > 0 %] >- <a class="btn btn-default btn-xs" href="/cgi-bin/koha/members/paycollect.pl?borrowernumber=[% account.borrowernumber | html %]&pay_individual=1&debit_type_code=[% account.debit_type_code | html %]&amount=[% account.amount | html %]&amountoutstanding=[% account.amountoutstanding | html %]&description=[% account.description | html %]&itemnumber=[% account.itemnumber | html %]&accountlines_id=[% account.accountlines_id | html %]">Pay</a> >+ <a class="btn btn-default btn-xs" href="/cgi-bin/koha/members/paycollect.pl?borrowernumber=[% account.borrowernumber | html %]&pay_individual=1&debit_type_code=[% account.debit_type_code | html %]&amount=[% account.amount | html %]&amountoutstanding=[% account.amountoutstanding | html %]&description=[% account.description | html %]&itemnumber=[% account.itemnumber | html %]&accountlines_id=[% account.accountlines_id | html %]"><i class="fa fa-money"></i> Pay</a> > [% END %] > [% IF account.is_credit && account.status != 'VOID' %] > <a href="boraccount.pl?action=void&accountlines_id=[% account.accountlines_id | uri %]&borrowernumber=[% account.borrowernumber | uri %]" class="btn btn-default btn-xs void"><i class="fa fa-ban"></i> Void payment</a> > [% END %] >- [% IF account.is_debit && account.amount == account.amountoutstanding && account.status != 'CANCELLED' %] >+ [% IF account.is_debit && account.amount == account.amountoutstanding && account.status != 'CANCELLED' && !(account.debit_type_code == 'PAYOUT') %] > <form method="post" action="/cgi-bin/koha/members/cancel-charge.pl"> > <input type="hidden" name="csrf_token" value="[% csrf_token | html %]"> > <input type="hidden" name="borrowernumber" value="[% patron.borrowernumber | html %]"> >diff --git a/members/cancel-charge.pl b/members/cancel-charge.pl >index 4aa0b00517..91184036e9 100755 >--- a/members/cancel-charge.pl >+++ b/members/cancel-charge.pl >@@ -42,7 +42,12 @@ unless ($csrf_token_is_valid) { > my $borrowernumber = $cgi->param('borrowernumber'); > my $accountlines_id = $cgi->param('accountlines_id'); > >-my $line = Koha::Account::Lines->find($accountlines_id); >-$line->cancel(); >+my $charge = Koha::Account::Lines->find($accountlines_id); >+$charge->cancel( >+ { >+ branch => C4::Context->userenv->{'branch'}, >+ staff_id => $user >+ } >+); > > print $cgi->redirect('/cgi-bin/koha/members/boraccount.pl?borrowernumber=' . $borrowernumber); >diff --git a/t/db_dependent/Koha/Account/Line.t b/t/db_dependent/Koha/Account/Line.t >index f757ae0f92..8d23c531d8 100755 >--- a/t/db_dependent/Koha/Account/Line.t >+++ b/t/db_dependent/Koha/Account/Line.t >@@ -901,7 +901,7 @@ subtest "payout() tests" => sub { > > subtest "reduce() tests" => sub { > >- plan tests => 27; >+ plan tests => 29; > > $schema->storage->txn_begin; > >@@ -961,7 +961,7 @@ subtest "reduce() tests" => sub { > > my $reduce_params = { > interface => 'commandline', >- reduction_type => 'REFUND', >+ reduction_type => 'DISCOUNT', > amount => 5, > staff_id => $staff->borrowernumber, > branch => $branchcode >@@ -1009,7 +1009,7 @@ subtest "reduce() tests" => sub { > '->reduce() cannot reduce more than original amount'; > > # Partial Reduction >- # (Refund 5 on debt of 20) >+ # (Discount 5 on debt of 20) > my $reduction = $debit1->reduce($reduce_params); > > is( ref($reduction), 'Koha::Account::Line', >@@ -1019,6 +1019,7 @@ subtest "reduce() tests" => sub { > 0, "Reduce amountoutstanding is 0" ); > is( $debit1->amountoutstanding() * 1, > 15, "Debit amountoutstanding reduced by 5 to 15" ); >+ is( $debit1->status(), 'DISCOUNTED', "Debit status updated to DISCOUNTED"); > is( $account->balance() * 1, -5, "Account balance is -5" ); > is( $reduction->status(), 'APPLIED', "Reduction status is 'APPLIED'" ); > >@@ -1028,17 +1029,19 @@ subtest "reduce() tests" => sub { > my $THE_offset = $offsets->next; > is( $THE_offset->amount * 1, > -5, 'Correct amount was applied against debit' ); >- is( $THE_offset->type, 'REFUND', "Offset type set to 'REFUND'" ); >+ is( $THE_offset->type, 'DISCOUNT', "Offset type set to 'DISCOUNT'" ); > > # Zero offset created when zero outstanding > # (Refund another 5 on paid debt of 20) > $credit1->apply( { debits => [$debit1] } ); > is( $debit1->amountoutstanding + 0, > 0, 'Debit1 amountoutstanding reduced to 0' ); >+ $reduce_params->{reduction_type} = 'REFUND'; > $reduction = $debit1->reduce($reduce_params); > is( $reduction->amount() * 1, -5, "Reduce amount is -5" ); > is( $reduction->amountoutstanding() * 1, > -5, "Reduce amountoutstanding is -5" ); >+ is( $debit1->status(), 'REFUNDED', "Debit status updated to REFUNDED"); > > $offsets = Koha::Account::Offsets->search( > { credit_id => $reduction->id, debit_id => $debit1->id } ); >@@ -1077,28 +1080,47 @@ subtest "reduce() tests" => sub { > }; > > subtest "cancel() tests" => sub { >- plan tests => 9; >+ plan tests => 16; > > $schema->storage->txn_begin; > > # Create a borrower >- my $categorycode = $builder->build({ source => 'Category' })->{ categorycode }; >- my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode }; >+ my $categorycode = >+ $builder->build( { source => 'Category' } )->{categorycode}; >+ my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; > >- my $borrower = Koha::Patron->new( { >- cardnumber => 'dariahall', >- surname => 'Hall', >- firstname => 'Daria', >- } ); >- $borrower->categorycode( $categorycode ); >- $borrower->branchcode( $branchcode ); >+ my $borrower = Koha::Patron->new( >+ { >+ cardnumber => 'dariahall', >+ surname => 'Hall', >+ firstname => 'Daria', >+ } >+ ); >+ $borrower->categorycode($categorycode); >+ $borrower->branchcode($branchcode); > $borrower->store; > >- t::lib::Mocks::mock_userenv({ branchcode => $branchcode, borrowernumber => $borrower->borrowernumber }); >+ my $staff = Koha::Patron->new( >+ { >+ cardnumber => 'bobby', >+ surname => 'Bloggs', >+ firstname => 'Bobby', >+ } >+ ); >+ $staff->categorycode($categorycode); >+ $staff->branchcode($branchcode); >+ $staff->store; >+ >+ t::lib::Mocks::mock_userenv( >+ { >+ branchcode => $branchcode, >+ borrowernumber => $borrower->borrowernumber >+ } >+ ); > >- my $account = Koha::Account->new({ patron_id => $borrower->id }); >+ my $account = Koha::Account->new( { patron_id => $borrower->id } ); > >- my $line1 = Koha::Account::Line->new( >+ my $debit1 = Koha::Account::Line->new( > { > borrowernumber => $borrower->borrowernumber, > amount => 10, >@@ -1107,7 +1129,7 @@ subtest "cancel() tests" => sub { > debit_type_code => 'OVERDUE', > } > )->store(); >- my $line2 = Koha::Account::Line->new( >+ my $debit2 = Koha::Account::Line->new( > { > borrowernumber => $borrower->borrowernumber, > amount => 20, >@@ -1117,27 +1139,68 @@ subtest "cancel() tests" => sub { > } > )->store(); > >- my $id = $account->pay({ >- lines => [$line2], >- amount => 5, >- }); >+ my $ret = $account->pay( >+ { >+ lines => [$debit2], >+ amount => 5, >+ } >+ ); >+ my $credit = Koha::Account::Lines->find({ accountlines_id => $ret->{payment_id} }); > > is( $account->balance(), 25, "Account balance is 25" ); >- is( $line1->amountoutstanding+0, 10, 'First fee has amount outstanding of 10' ); >- is( $line2->amountoutstanding+0, 15, 'Second fee has amount outstanding of 15' ); >+ is( $debit1->amountoutstanding + 0, >+ 10, 'First fee has amount outstanding of 10' ); >+ is( $debit2->amountoutstanding + 0, >+ 15, 'Second fee has amount outstanding of 15' ); >+ throws_ok { >+ $credit->cancel( >+ { staff_id => $staff->borrowernumber, branch => $branchcode } ); >+ } >+ 'Koha::Exceptions::Account::IsNotDebit', >+ '->cancel() can only be used with debits'; > >- my $ret = $line1->cancel(); >- is( ref($ret), 'Koha::Account::Line', 'Cancel returns the account line' ); >- is( $account->balance(), 15, "Account balance is 15" ); >- is( $line1->amount+0, 0, 'First fee has amount of 0' ); >- is( $line1->amountoutstanding+0, 0, 'First fee has amount outstanding of 0' ); >+ throws_ok { >+ $debit1->reduce( { staff_id => $staff->borrowernumber } ); >+ } >+ 'Koha::Exceptions::MissingParameter', >+ "->cancel() requires the `branch` parameter is passed"; >+ throws_ok { >+ $debit1->reduce( { branch => $branchcode } ); >+ } >+ 'Koha::Exceptions::MissingParameter', >+ "->cancel() requires the `staff_id` parameter is passed"; >+ >+ throws_ok { >+ $debit2->cancel( >+ { staff_id => $staff->borrowernumber, branch => $branchcode } ); >+ } >+ 'Koha::Exceptions::Account', >+ '->cancel() can only be used with debits that have not been offset'; > >- $ret = $line2->cancel(); >- is ($ret, undef, 'cancel returns undef when line cannot be cancelled'); >+ my $cancellation = $debit1->cancel( >+ { staff_id => $staff->borrowernumber, branch => $branchcode } ); >+ is( ref($cancellation), 'Koha::Account::Line', >+ 'Cancel returns an account line' ); >+ is( >+ $cancellation->amount() * 1, >+ $debit1->amount * -1, >+ "Cancellation amount is " . $debit1->amount >+ ); >+ is( $cancellation->amountoutstanding() * 1, >+ 0, "Cancellation amountoutstanding is 0" ); >+ is( $debit1->amountoutstanding() * 1, >+ 0, "Debit amountoutstanding reduced to 0" ); >+ is( $debit1->status(), 'CANCELLED', "Debit status updated to CANCELLED" ); >+ is( $account->balance() * 1, 15, "Account balance is 15" ); > >- my $account_payment = Koha::Account::Lines->find($id); >- $ret = $account_payment->cancel(); >- is ($ret, undef, 'payment cannot be cancelled'); >+ my $offsets = Koha::Account::Offsets->search( >+ { credit_id => $cancellation->id, debit_id => $debit1->id } ); >+ is( $offsets->count, 1, 'Only one offset is generated' ); >+ my $THE_offset = $offsets->next; >+ is( $THE_offset->amount * 1, >+ -10, 'Correct amount was applied against debit' ); >+ is( $THE_offset->type, 'CANCELLATION', >+ "Offset type set to 'CANCELLATION'" ); > > $schema->storage->txn_rollback; > }; >-- >2.11.0
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=111256">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 24603
:
98534
|
98551
|
102005
|
106772
|
111217
|
111218
|
111255
|
111256
|
111794
|
111795
|
111796
|
111797
|
111914
|
113048
|
113049
|
113050
|
113051
|
113052
|
113113
|
113114