Bug 20703: Add ability to void any credit
authorKyle M Hall <kyle@bywatetsolutions.com>
Thu, 3 May 2018 13:17:06 +0000 (09:17 -0400)
committerNick Clemens <nick@bywatersolutions.com>
Fri, 6 Jul 2018 10:36:07 +0000 (10:36 +0000)
At this time, only payments may be voided. There is no reason to have this limitation. There are situations where a librarian may need to void an accidental writeoff, or perhaps void an automatic credit that was created by Koha. For illustration, this is directly from a partner library:

"For example a lost book refund becomes a credit on account.  Presently the credit may be applied to a fine for a different item charged to patron. We perform a write off to clear the remaining credit, then add the fine back to the account and give the patron a refund for the lost/refunded amount. Our accounting system ask that we keep the Lost funds/refunds separate from all fines."

Test Plan:
1) Create a fine and write it off
2) Note there is no 'void' button for the writeoff
3) Apply this patch
4) Note the buttons now show
5) Test each button on a writeoff

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Koha/Account/Line.pm
koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt
members/boraccount.pl
t/db_dependent/Accounts.t

index b32f9a7..caba957 100644 (file)
@@ -60,11 +60,11 @@ sub void {
     my ($self) = @_;
 
     # Make sure it is a payment we are voiding
-    return unless $self->accounttype =~ /^Pay/;
+    return unless $self->amount < 0;
 
     my @account_offsets =
       Koha::Account::Offsets->search(
-        { credit_id => $self->id, type => 'Payment' } );
+        { credit_id => $self->id, amount => { '<' => 0 }  } );
 
     $self->_result->result_source->schema->txn_do(
         sub {
index dc1ff99..7ecaeb7 100644 (file)
         [% END %]
         <a href="accountline-details.pl?accountlines_id=[% account.accountlines_id %]" class="btn btn-default btn-xs"><i class="fa fa-list"></i> Details</a>
         [% IF ( reverse_col) %]
-          [% IF ( account.payment ) %]
-            <a href="boraccount.pl?action=reverse&amp;accountlines_id=[% account.accountlines_id %]&amp;borrowernumber=[% account.borrowernumber %]" class="btn btn-default btn-xs"><i class="fa fa-undo"></i> Reverse</a>
-            <a href="boraccount.pl?action=void&amp;accountlines_id=[% account.accountlines_id %]&amp;borrowernumber=[% account.borrowernumber %]" class="btn btn-default btn-xs"><i class="fa fa-ban"></i> Void</a>
-          [% ELSE %][% SET footerjs = 1 %]
+          [% IF ( account.payment || account.amount < 0 ) %]
+                [% IF account.payment %]
+                    <a href="boraccount.pl?action=reverse&amp;accountlines_id=[% account.accountlines_id %]&amp;borrowernumber=[% account.borrowernumber %]" class="btn btn-default btn-xs"><i class="fa fa-undo"></i> Reverse</a>
+                [% END %]
+                [% IF account.amount < 0 %]
+                    <a href="boraccount.pl?action=void&amp;accountlines_id=[% account.accountlines_id %]&amp;borrowernumber=[% account.borrowernumber %]" class="btn btn-default btn-xs"><i class="fa fa-ban"></i> Void</a>
+                [% END %]
+          [% ELSE %]
             &nbsp;
           [% END %]
         [% END %]
index b3d1431..f95a268 100755 (executable)
@@ -104,8 +104,10 @@ while ( my $line = $accts->next ) {
 
     $accountline->{amount} = sprintf '%.2f', $accountline->{amount};
     $accountline->{amountoutstanding} = sprintf '%.2f', $accountline->{amountoutstanding};
-    if ($accountline->{accounttype} =~ /^Pay/) {
-        $accountline->{payment} = 1;
+    if ($accountline->{amount} < 0) {
+        $accountline->{payment} = 1
+          if ( $accountline->{accounttype} =~ /^Pay/ );
+
         $reverse_col = 1;
     }
 
index f447d46..f6cae2b 100644 (file)
@@ -846,7 +846,7 @@ subtest "Koha::Account::non_issues_charges tests" => sub {
 
 subtest "Koha::Account::Line::void tests" => sub {
 
-    plan tests => 12;
+    plan tests => 15;
 
     # Create a borrower
     my $categorycode = $builder->build({ source => 'Category' })->{ categorycode };
@@ -886,8 +886,9 @@ subtest "Koha::Account::Line::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' );
 
-    $account_payment->void();
+    my $ret = $account_payment->void();
 
+    is( ref($ret), 'Koha::Account::Line', 'Void returns the account line' );
     is( $account->balance(), 30, "Account balance is again 30" );
 
     $account_payment->_result->discard_changes();
@@ -900,6 +901,14 @@ subtest "Koha::Account::Line::void tests" => sub {
 
     is( $line1->amountoutstanding+0, 10, 'First fee again has amount outstanding of 10' );
     is( $line2->amountoutstanding+0, 20, 'Second fee again has amount outstanding of 20' );
+
+    # Accountlines that are not credits should be un-voidable
+    my $line1_pre = $line1->unblessed();
+    $ret = $line1->void();
+    $line1->_result->discard_changes();
+    my $line1_post = $line1->unblessed();
+    is( $ret, undef, 'Attempted void on non-credit returns undef' );
+    is_deeply( $line1_pre, $line1_post, 'Non-credit account line cannot be voided' )
 };
 
 subtest "Koha::Account::Offset credit & debit tests" => sub {