Attachment 49453 Details for Bug 15895 – Bug 15895 - Add Koha::Account module, use Koha::Account::pay internally for recordpayment

[patch] Bug 15895 - Add Koha::Account module, use Koha::Account::pay internally for recordpayment

Bug-15895---Add-KohaAccount-module-use-KohaAccount.patch (text/plain), 13.68 KB, created by Owen Leonard on 2016-03-23 16:14:28 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Owen Leonard

Created: 2016-03-23 16:14:28 UTC

Size: 13.68 KB

patch

obsolete

>From 50a12da289a408701b46b4645b9032f3ff526c87 Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>Date: Fri, 19 Feb 2016 19:57:12 +0000
>Subject: [PATCH] Bug 15895 - Add Koha::Account module, use Koha::Account::pay
> internally for recordpayment
>Content-Type: text/plain; charset="utf-8"
>
>This is the first patch in a series to unify all payment functions into
>a single method.
>
>Test Plan:
>1) Apply this patch
>2) prove t/db_dependent/Accounts.t
>3) Test fine payment via the "Pay amount" button
>
>Signed-off-by: Owen Leonard <oleonard@myacpl.org>
>
>Paying a fine using the "Pay amount" button works correctly.
>---
> C4/Accounts.pm            |  92 +------------------------
> Koha/Account.pm           | 171 ++++++++++++++++++++++++++++++++++++++++++++++
> t/db_dependent/Accounts.t |  27 ++++----
> 3 files changed, 187 insertions(+), 103 deletions(-)
> create mode 100644 Koha/Account.pm
>
>diff --git a/C4/Accounts.pm b/C4/Accounts.pm
>index 4f5f8d4..0631855 100644
>--- a/C4/Accounts.pm
>+++ b/C4/Accounts.pm
>@@ -25,6 +25,7 @@ use C4::Stats;
> use C4::Members;
> use C4::Circulation qw(ReturnLostItem);
> use C4::Log qw(logaction);
>+use Koha::Account;
> 
> use Data::Dumper qw(Dumper);
> 
>@@ -91,96 +92,9 @@ sub recordpayment {
> 
>     #here we update the account lines
>     my ( $borrowernumber, $data, $sip_paytype, $payment_note ) = @_;
>-    my $dbh        = C4::Context->dbh;
>-    my $newamtos   = 0;
>-    my $accdata    = "";
>-    my $branch     = C4::Context->userenv->{'branch'};
>-    my $amountleft = $data;
>-    my $manager_id = 0;
>-    $manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv;
>-
>-    $payment_note //= "";
>-
>-    # begin transaction
>-    my $nextaccntno = getnextacctno($borrowernumber);
>-
>-    # get lines with outstanding amounts to offset
>-    my $sth = $dbh->prepare(
>-        "SELECT * FROM accountlines
>-  WHERE (borrowernumber = ?) AND (amountoutstanding<>0)
>-  ORDER BY date"
>-    );
>-    $sth->execute($borrowernumber);
>-
>-    # offset transactions
>-    my @ids;
>-    while ( ( $accdata = $sth->fetchrow_hashref ) and ( $amountleft > 0 ) ) {
>-        if ( $accdata->{'amountoutstanding'} < $amountleft ) {
>-            $newamtos = 0;
>-            $amountleft -= $accdata->{'amountoutstanding'};
>-        }
>-        else {
>-            $newamtos   = $accdata->{'amountoutstanding'} - $amountleft;
>-            $amountleft = 0;
>-        }
>-        my $thisacct = $accdata->{accountlines_id};
>-        my $usth     = $dbh->prepare(
>-            "UPDATE accountlines SET amountoutstanding= ?
>-     WHERE (accountlines_id = ?)"
>-        );
>-        $usth->execute( $newamtos, $thisacct );
>-
>-        if ( C4::Context->preference("FinesLog") ) {
>-            $accdata->{'amountoutstanding_new'} = $newamtos;
>-            logaction("FINES", 'MODIFY', $borrowernumber, Dumper({
>-                action                => 'fee_payment',
>-                borrowernumber        => $accdata->{'borrowernumber'},
>-                old_amountoutstanding => $accdata->{'amountoutstanding'},
>-                new_amountoutstanding => $newamtos,
>-                amount_paid           => $accdata->{'amountoutstanding'} - $newamtos,
>-                accountlines_id       => $accdata->{'accountlines_id'},
>-                accountno             => $accdata->{'accountno'},
>-                manager_id            => $manager_id,
>-                note                  => $payment_note,
>-            }));
>-            push( @ids, $accdata->{'accountlines_id'} );
>-        }
>-    }
>-
>-    # create new line
>-    my $usth = $dbh->prepare(
>-        "INSERT INTO accountlines
>-  (borrowernumber, accountno,date,amount,description,accounttype,amountoutstanding,manager_id, note)
>-  VALUES (?,?,now(),?,'',?,?,?,?)"
>-    );
>-
>-    my $paytype = "Pay";
>-    $paytype .= $sip_paytype if defined $sip_paytype;
>-    $usth->execute( $borrowernumber, $nextaccntno, 0 - $data, $paytype, 0 - $amountleft, $manager_id, $payment_note );
>-    $usth->finish;
>-
>-    UpdateStats({
>-                branch => $branch,
>-                type =>'payment',
>-                amount => $data,
>-                borrowernumber => $borrowernumber,
>-                accountno => $nextaccntno }
>-    );
>-
>-    if ( C4::Context->preference("FinesLog") ) {
>-        $accdata->{'amountoutstanding_new'} = $newamtos;
>-        logaction("FINES", 'CREATE',$borrowernumber,Dumper({
>-            action            => 'create_payment',
>-            borrowernumber    => $borrowernumber,
>-            accountno         => $nextaccntno,
>-            amount            => $data * -1,
>-            amountoutstanding => $amountleft * -1,
>-            accounttype       => 'Pay',
>-            accountlines_paid => \@ids,
>-            manager_id        => $manager_id,
>-        }));
>-    }
> 
>+    return Koha::Account->new( { patron_id => $borrowernumber } )
>+      ->pay( { amount => $data, sip => $sip_paytype, note => $payment_note } );
> }
> 
> =head2 makepayment
>diff --git a/Koha/Account.pm b/Koha/Account.pm
>new file mode 100644
>index 0000000..b7341b1
>--- /dev/null
>+++ b/Koha/Account.pm
>@@ -0,0 +1,171 @@
>+package Koha::Account;
>+
>+# Copyright 2016 ByWater Solutions
>+#
>+# This file is part of Koha.
>+#
>+# Koha is free software; you can redistribute it and/or modify it
>+# under the terms of the GNU General Public License as published by
>+# the Free Software Foundation; either version 3 of the License, or
>+# (at your option) any later version.
>+#
>+# Koha is distributed in the hope that it will be useful, but
>+# WITHOUT ANY WARRANTY; without even the implied warranty of
>+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>+# GNU General Public License for more details.
>+#
>+# You should have received a copy of the GNU General Public License
>+# along with Koha; if not, see <http://www.gnu.org/licenses>.
>+
>+use Modern::Perl;
>+
>+use Carp;
>+use Data::Dumper;
>+
>+use C4::Log qw( logaction );
>+use C4::Stats qw( UpdateStats );
>+
>+use Koha::Account::Line;
>+use Koha::Account::Lines;
>+use Koha::DateUtils qw( dt_from_string );
>+
>+=head1 NAME
>+
>+Koha::Accounts - Module for managing payments and fees for patrons
>+
>+=cut
>+
>+sub new {
>+    my ( $class, $params ) = @_;
>+
>+    Carp::croak("No patron id passed in!") unless $params->{patron_id};
>+
>+    return bless( $params, $class );
>+}
>+
>+=head2 pay
>+
>+This method allows payments to be made against feees
>+
>+=cut
>+
>+sub pay {
>+    my ( $self, $params ) = @_;
>+
>+    my $amount = $params->{amount};
>+    my $sip    = $params->{sip};
>+    my $note   = $params->{note} || q{};
>+
>+    my $userenv = C4::Context->userenv;
>+
>+    # We should remove accountno, it is no longer needed
>+    my $last = Koha::Account::Lines->search(
>+        {
>+            borrowernumber => $self->{patron_id}
>+        },
>+        {
>+            order_by => 'accountno'
>+        }
>+    )->next();
>+    my $accountno = $last ? $last->accountno + 1 : 1;
>+
>+    my $manager_id = $userenv ? $userenv->{number} : 0;
>+
>+    my @outstanding_fines = Koha::Account::Lines->search(
>+        {
>+            borrowernumber    => $self->{patron_id},
>+            amountoutstanding => { '>' => 0 },
>+        }
>+    );
>+
>+    my $balance_remaining = $amount;
>+    my @fines_paid;
>+    foreach my $fine (@outstanding_fines) {
>+        my $amount_to_pay =
>+            $fine->amountoutstanding > $balance_remaining
>+          ? $balance_remaining
>+          : $fine->amountoutstanding;
>+
>+        my $old_amountoutstanding = $fine->amountoutstanding;
>+        $fine->amountoutstanding( $old_amountoutstanding - $amount_to_pay );
>+        $fine->store();
>+
>+        if ( C4::Context->preference("FinesLog") ) {
>+            logaction(
>+                "FINES", 'MODIFY',
>+                $self->{patron_id},
>+                Dumper(
>+                    {
>+                        action                => 'fee_payment',
>+                        borrowernumber        => $fine->borrowernumber,
>+                        old_amountoutstanding => $old_amountoutstanding,
>+                        new_amountoutstanding => $fine->amountoutstanding,
>+                        amount_paid           => $amount_to_pay,
>+                        accountlines_id       => $fine->id,
>+                        accountno             => $fine->accountno,
>+                        manager_id            => $manager_id,
>+                        note                  => $note,
>+                    }
>+                )
>+            );
>+            push( @fines_paid, $fine->id );
>+        }
>+
>+        $balance_remaining = $balance_remaining - $amount_to_pay;
>+        last unless $balance_remaining > 0;
>+    }
>+
>+    my $account_type = defined($sip) ? "Pay$sip" : 'Pay';
>+
>+    my $payment = Koha::Account::Line->new(
>+        {
>+            borrowernumber    => $self->{patron_id},
>+            accountno         => $accountno,
>+            date              => dt_from_string(),
>+            amount            => 0 - $amount,
>+            description       => q{},
>+            accounttype       => $account_type,
>+            amountoutstanding => 0 - $balance_remaining,
>+            manager_id        => $manager_id,
>+            note              => $note,
>+        }
>+    )->store();
>+
>+    my $branch = $userenv ? $userenv->{'branch'} : undef;
>+    UpdateStats(
>+        {
>+            branch         => $branch,
>+            type           => 'payment',
>+            amount         => $amount,
>+            borrowernumber => $self->{patron_id},
>+            accountno      => $accountno,
>+        }
>+    );
>+
>+    if ( C4::Context->preference("FinesLog") ) {
>+        logaction(
>+            "FINES", 'CREATE',
>+            $self->{patron_id},
>+            Dumper(
>+                {
>+                    action            => 'create_payment',
>+                    borrowernumber    => $self->{patron_id},
>+                    accountno         => $accountno,
>+                    amount            => 0 - $amount,
>+                    amountoutstanding => 0 - $balance_remaining,
>+                    accounttype       => 'Pay',
>+                    accountlines_paid => \@fines_paid,
>+                    manager_id        => $manager_id,
>+                }
>+            )
>+        );
>+    }
>+}
>+
>+1;
>+
>+=head1 AUTHOR
>+
>+Kyle M Hall <kyle.m.hall@gmail.com>
>+
>+=cut
>diff --git a/t/db_dependent/Accounts.t b/t/db_dependent/Accounts.t
>index ee92a48..c9b9774 100644
>--- a/t/db_dependent/Accounts.t
>+++ b/t/db_dependent/Accounts.t
>@@ -24,6 +24,9 @@ use Test::Warn;
> 
> use t::lib::TestBuilder;
> 
>+use Koha::Account::Lines;
>+use Koha::Account::Line;
>+
> BEGIN {
>     use_ok('C4::Accounts');
>     use_ok('Koha::Object');
>@@ -153,19 +156,15 @@ subtest "recordpayment() tests" => sub {
>     $borrower->branchcode( $branchcode );
>     $borrower->store;
> 
>-    my $sth = $dbh->prepare(
>-        "INSERT INTO accountlines (
>-            borrowernumber,
>-            amountoutstanding )
>-        VALUES ( ?, ? )"
>-    );
>-    $sth->execute($borrower->borrowernumber, '100');
>-    $sth->execute($borrower->borrowernumber, '200');
>+    my $line1 = Koha::Account::Line->new({ borrowernumber => $borrower->borrowernumber, amountoutstanding => 100 })->store();
>+    my $line2 = Koha::Account::Line->new({ borrowernumber => $borrower->borrowernumber, amountoutstanding => 200 })->store();
>+    $line1->_result->discard_changes;
>+    $line2->_result->discard_changes;
> 
>     $sth = $dbh->prepare("SELECT count(*) FROM accountlines");
>     $sth->execute;
>     my $count = $sth->fetchrow_array;
>-    is ($count, 2, 'There is 2 lines as expected');
>+    is($count, 2, 'There is 2 lines as expected');
> 
>     # Testing recordpayment -------------------------
>     # There is $100 in the account
>@@ -175,7 +174,7 @@ subtest "recordpayment() tests" => sub {
>     for my $line ( @$amountoutstanding ) {
>         $amountleft += $line;
>     }
>-    ok($amountleft == 300, 'The account has 300$ as expected' );
>+    is($amountleft, 300, 'The account has 300$ as expected' );
> 
>     # We make a $20 payment
>     my $borrowernumber = $borrower->borrowernumber;
>@@ -190,7 +189,7 @@ subtest "recordpayment() tests" => sub {
>     for my $line ( @$amountoutstanding ) {
>         $amountleft += $line;
>     }
>-    ok($amountleft == 280, 'The account has $280 as expected' );
>+    is($amountleft, 280, 'The account has $280 as expected' );
>     # Is the payment note well registered
>     $sth = $dbh->prepare("SELECT note FROM accountlines WHERE borrowernumber=? ORDER BY accountlines_id DESC LIMIT 1");
>     $sth->execute($borrower->borrowernumber);
>@@ -208,7 +207,7 @@ subtest "recordpayment() tests" => sub {
>     for my $line ( @$amountoutstanding ) {
>         $amountleft += $line;
>     }
>-    ok($amountleft == 310, 'The account has $310 as expected' );
>+    is($amountleft, 310, 'The account has $310 as expected' );
>     # Is the payment note well registered
>     $sth = $dbh->prepare("SELECT note FROM accountlines WHERE borrowernumber=? ORDER BY accountlines_id DESC LIMIT 1");
>     $sth->execute($borrower->borrowernumber);
>@@ -226,7 +225,7 @@ subtest "recordpayment() tests" => sub {
>     for my $line ( @$amountoutstanding ) {
>         $amountleft += $line;
>     }
>-    ok($amountleft == 160, 'The account has $160 as expected' );
>+    is($amountleft, 160, 'The account has $160 as expected' );
>     # Is the payment note well registered
>     $sth = $dbh->prepare("SELECT note FROM accountlines WHERE borrowernumber=? ORDER BY accountlines_id DESC LIMIT 1");
>     $sth->execute($borrower->borrowernumber);
>@@ -244,7 +243,7 @@ subtest "recordpayment() tests" => sub {
>     for my $line ( @$amountoutstanding ) {
>         $amountleft += $line;
>     }
>-    ok($amountleft == -40, 'The account has -$40 as expected, (credit situation)' );
>+    is($amountleft, -40, 'The account has -$40 as expected, (credit situation)' );
>     # Is the payment note well registered
>     $sth = $dbh->prepare("SELECT note FROM accountlines WHERE borrowernumber=? ORDER BY accountlines_id DESC LIMIT 1");
>     $sth->execute($borrower->borrowernumber);
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 15895: 48331 | 49453 | 52138 | 53883 | 54543 | 55870 | 55871