Attachment 64297 Details for Bug 18805 – Bug 18805 - Add ability to use up account credits

[patch] Bug 18805 - Add ability to use up account credits

Bug-18805---Add-ability-to-use-up-account-credits.patch (text/plain), 8.80 KB, created by Kyle M Hall (khall) on 2017-06-14 18:17:05 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2017-06-14 18:17:05 UTC

Size: 8.80 KB

patch

obsolete

>From 536a274941e1fad683e70499c6d4a6aa3a401858 Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>Date: Wed, 14 Jun 2017 14:10:08 -0400
>Subject: [PATCH] Bug 18805 - Add ability to use up account credits
>
>Right now, in Koha, credits always have a static amount outstanding. So if a patron owes $10 in fines, and has a $10 credit, they technically owe nothing, but yet they still owe $10. This seems rather confusing.
>
>Instead, we should add the ability for credits to be used to pay down closed out fines and other fees.
>
>Test Plan:
>1) Apply this patch and dependencies
>2) Create a manual credit of some amount
>3) Create a manual invoice
>4) Note the manual invoice was automatically paid down using
>   the balance from the manual credit!
>---
> C4/Accounts.pm            |   2 +
> C4/Circulation.pm         |   2 +
> Koha/Account.pm           | 123 ++++++++++++++++++++++++++++++----------------
> t/db_dependent/Accounts.t |  38 +++++++++++++-
> 4 files changed, 123 insertions(+), 42 deletions(-)
>
>diff --git a/C4/Accounts.pm b/C4/Accounts.pm
>index b589da1..d2dcde5 100644
>--- a/C4/Accounts.pm
>+++ b/C4/Accounts.pm
>@@ -278,6 +278,8 @@ sub manualinvoice {
>         }));
>     }
> 
>+    Koha::Account->new({ patron_id => $borrowernumber })->normalize_balance();
>+
>     return 0;
> }
> 
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index ca41dd6..8752a87 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -2390,6 +2390,8 @@ sub _FixOverduesOnReturn {
>         $accountline->accounttype('F');
>     }
> 
>+    Koha::Account->new({ patron_id => $borrowernumber })->normalize_balance();
>+
>     return $accountline->store();
> }
> 
>diff --git a/Koha/Account.pm b/Koha/Account.pm
>index e0f9047..70889dd 100644
>--- a/Koha/Account.pm
>+++ b/Koha/Account.pm
>@@ -57,7 +57,8 @@ Koha::Account->new( { patron_id => $borrowernumber } )->pay(
>         library_id  => $branchcode,
>         lines        => $lines, # Arrayref of Koha::Account::Line objects to pay
>         account_type => $type,  # accounttype code
>-        offset_type => $offset_type,    # offset type code
>+        offset_type  => $offset_type,     # offset type code
>+        credit_id    => credit_id,        # pay from balance of existing credit
>     }
> );
> 
>@@ -76,6 +77,7 @@ sub pay {
>     my $payment_type = $params->{payment_type} || undef;
>     my $account_type = $params->{account_type};
>     my $offset_type  = $params->{offset_type} || $type eq 'writeoff' ? 'Writeoff' : 'Payment';
>+    my $credit_id    = $params->{credit_id};
> 
>     my $userenv = C4::Context->userenv;
> 
>@@ -209,20 +211,28 @@ sub pay {
> 
>     $description ||= $type eq 'writeoff' ? 'Writeoff' : q{};
> 
>-    my $payment = Koha::Account::Line->new(
>-        {
>-            borrowernumber    => $self->{patron_id},
>-            accountno         => $accountno,
>-            date              => dt_from_string(),
>-            amount            => 0 - $amount,
>-            description       => $description,
>-            accounttype       => $account_type,
>-            payment_type      => $payment_type,
>-            amountoutstanding => 0 - $balance_remaining,
>-            manager_id        => $manager_id,
>-            note              => $note,
>-        }
>-    )->store();
>+    my $payment;
>+    if ($credit_id) {
>+        $payment = Koha::Account::Lines->find($credit_id);
>+        $payment->amountoutstanding( $balance_remaining * -1 );
>+        $payment->store();
>+    }
>+    else {
>+        $payment = Koha::Account::Line->new(
>+            {
>+                borrowernumber    => $self->{patron_id},
>+                accountno         => $accountno,
>+                date              => dt_from_string(),
>+                amount            => 0 - $amount,
>+                description       => $description,
>+                accounttype       => $account_type,
>+                payment_type      => $payment_type,
>+                amountoutstanding => 0 - $balance_remaining,
>+                manager_id        => $manager_id,
>+                note              => $note,
>+            }
>+        )->store();
>+    }
> 
>     foreach my $o ( @account_offsets ) {
>         $o->credit_id( $payment->id() );
>@@ -231,33 +241,35 @@ sub pay {
> 
>     $library_id ||= $userenv ? $userenv->{'branch'} : undef;
> 
>-    UpdateStats(
>-        {
>-            branch         => $library_id,
>-            type           => $type,
>-            amount         => $amount,
>-            borrowernumber => $self->{patron_id},
>-            accountno      => $accountno,
>-        }
>-    );
>-
>-    if ( C4::Context->preference("FinesLog") ) {
>-        logaction(
>-            "FINES", 'CREATE',
>-            $self->{patron_id},
>-            Dumper(
>-                {
>-                    action            => "create_$type",
>-                    borrowernumber    => $self->{patron_id},
>-                    accountno         => $accountno,
>-                    amount            => 0 - $amount,
>-                    amountoutstanding => 0 - $balance_remaining,
>-                    accounttype       => $account_type,
>-                    accountlines_paid => \@fines_paid,
>-                    manager_id        => $manager_id,
>-                }
>-            )
>+    unless ( $credit_id ) {
>+        UpdateStats(
>+            {
>+                branch         => $library_id,
>+                type           => $type,
>+                amount         => $amount,
>+                borrowernumber => $self->{patron_id},
>+                accountno      => $accountno,
>+            }
>         );
>+
>+        if ( C4::Context->preference("FinesLog") ) {
>+            logaction(
>+                "FINES", 'CREATE',
>+                $self->{patron_id},
>+                Dumper(
>+                    {
>+                        action            => "create_$type",
>+                        borrowernumber    => $self->{patron_id},
>+                        accountno         => $accountno,
>+                        amount            => 0 - $amount,
>+                        amountoutstanding => 0 - $balance_remaining,
>+                        accounttype       => $account_type,
>+                        accountlines_paid => \@fines_paid,
>+                        manager_id        => $manager_id,
>+                    }
>+                )
>+            );
>+        }
>     }
> 
>     return $payment->id;
>@@ -287,6 +299,35 @@ sub balance {
>       : 0;
> }
> 
>+=head3 normalize_balance
>+
>+$account->normalize_balance();
>+
>+Find outstanding credits and use them to pay outstanding debits
>+
>+=cut
>+
>+sub normalize_balance {
>+    my ($self) = @_;
>+    my @credits = Koha::Account::Lines->search(
>+        {
>+            borrowernumber    => $self->{patron_id},
>+            amountoutstanding => { '<' => 0 },
>+        }
>+    );
>+
>+    foreach my $credit (@credits) {
>+        $self->pay(
>+            {
>+                credit_id => $credit->id,
>+                amount    => $credit->amountoutstanding * -1,
>+            }
>+        );
>+    }
>+
>+    return $self;
>+}
>+
> 1;
> 
> =head1 AUTHOR
>diff --git a/t/db_dependent/Accounts.t b/t/db_dependent/Accounts.t
>index 80124a7..d22b0c2 100644
>--- a/t/db_dependent/Accounts.t
>+++ b/t/db_dependent/Accounts.t
>@@ -18,7 +18,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 23;
>+use Test::More tests => 24;
> use Test::MockModule;
> use Test::Warn;
> 
>@@ -547,4 +547,40 @@ subtest "Koha::Account::Line::void tests" => sub {
>     is( $line2->amountoutstanding, '20.000000', 'Second fee again has amount outstanding of 20' );
> };
> 
>+subtest "Koha::Account::normalize_balance tests" => sub {
>+
>+    plan tests => 6;
>+
>+    # Create a borrower
>+    my $categorycode = $builder->build({ source => 'Category' })->{ categorycode };
>+    my $branchcode   = $builder->build({ source => 'Branch' })->{ branchcode };
>+
>+    my $borrower = Koha::Patron->new( {
>+        cardnumber => 'kyliehall',
>+        surname => 'Hall',
>+        firstname => 'Kylie',
>+    } );
>+    $borrower->categorycode( $categorycode );
>+    $borrower->branchcode( $branchcode );
>+    $borrower->store;
>+
>+    my $account = Koha::Account->new({ patron_id => $borrower->id });
>+
>+    my $line1 = Koha::Account::Line->new({ borrowernumber => $borrower->borrowernumber, amount => -10, amountoutstanding => -10 })->store();
>+    my $line2 = Koha::Account::Line->new({ borrowernumber => $borrower->borrowernumber, amount => 10, amountoutstanding => 10 })->store();
>+
>+    is( $account->balance(), "0.000000", "Account balance is 0" );
>+    is( $line1->amountoutstanding, "-10", 'Credit has amount outstanding of -10' );
>+    is( $line2->amountoutstanding, "10", 'Fee has amount outstanding of 10' );
>+
>+    $account->normalize_balance();
>+
>+    is( $account->balance(), "0.000000", "Account balance is 0" );
>+
>+    $line1->_result->discard_changes();
>+    $line2->_result->discard_changes();
>+    is( $line1->amountoutstanding, '0.000000', 'First fee has amount outstanding of 0' );
>+    is( $line2->amountoutstanding, '0.000000', 'Second fee has amount outstanding of 0' );
>+};
>+
> 1;
>-- 
>2.10.2

Actions: View | Diff | Splinter Review

Attachments on bug 18805: 64297 | 72025 | 80415 | 81609 | 82686 | 82711 | 82712 | 82758 | 82816