Attachment 53786 Details for Bug 15896 – Bug 15896 - Use Koha::Account::pay internally for makepayment

[patch] Bug 15896 - Use Koha::Account::pay internally for makepayment

Bug-15896---Use-KohaAccountpay-internally-for-make.patch (text/plain), 8.66 KB, created by Josef Moravec on 2016-07-28 15:59:45 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Josef Moravec

Created: 2016-07-28 15:59:45 UTC

Size: 8.66 KB

patch

obsolete

>From 29bd179294e064bbf95195ed303f469908e9d7c1 Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>Date: Wed, 24 Feb 2016 12:36:20 +0000
>Subject: [PATCH] Bug 15896 - Use Koha::Account::pay internally for makepayment
>
>This is the second patch in a series to unify all payment functions into
>a single mathod
>
>Test Plan:
>1) Apply this patch
>2) prove t/db_dependent/Accounts.t
>3) Test fine payment via the "Pay" button
>
>Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
>---
> C4/Accounts.pm  | 90 ++-------------------------------------------------------
> Koha/Account.pm | 79 ++++++++++++++++++++++++++++++++++++++++++++------
> 2 files changed, 72 insertions(+), 97 deletions(-)
>
>diff --git a/C4/Accounts.pm b/C4/Accounts.pm
>index 0a59235..3d30b01 100644
>--- a/C4/Accounts.pm
>+++ b/C4/Accounts.pm
>@@ -114,96 +114,10 @@ was made.
> # FIXME - I'm not at all sure about the above, because I don't
> # understand what the acct* tables in the Koha database are for.
> sub makepayment {
>-
>-    #here we update both the accountoffsets and the account lines
>-    #updated to check, if they are paying off a lost item, we return the item
>-    # from their card, and put a note on the item record
>     my ( $accountlines_id, $borrowernumber, $accountno, $amount, $user, $branch, $payment_note ) = @_;
>-    my $dbh = C4::Context->dbh;
>-    my $manager_id = 0;
>-    $manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv; 
>-
>-    # begin transaction
>-    my $nextaccntno = getnextacctno($borrowernumber);
>-    my $newamtos    = 0;
>-    my $sth         = $dbh->prepare("SELECT * FROM accountlines WHERE accountlines_id=?");
>-    $sth->execute( $accountlines_id );
>-    my $data = $sth->fetchrow_hashref;
>-
>-    my $payment;
>-    if ( $data->{'accounttype'} eq "Pay" ){
>-        my $udp = 		
>-            $dbh->prepare(
>-                "UPDATE accountlines
>-                    SET amountoutstanding = 0
>-                    WHERE accountlines_id = ?
>-                "
>-            );
>-        $udp->execute($accountlines_id);
>-    }else{
>-        my $udp = 		
>-            $dbh->prepare(
>-                "UPDATE accountlines
>-                    SET amountoutstanding = 0
>-                    WHERE accountlines_id = ?
>-                "
>-            );
>-        $udp->execute($accountlines_id);
>-
>-         # create new line
>-        my $payment = 0 - $amount;
>-        $payment_note //= "";
>-        
>-        my $ins = 
>-            $dbh->prepare( 
>-                "INSERT 
>-                    INTO accountlines (borrowernumber, accountno, date, amount, itemnumber, description, accounttype, amountoutstanding, manager_id, note)
>-                    VALUES ( ?, ?, now(), ?, ?, '', 'Pay', 0, ?, ?)"
>-            );
>-        $ins->execute($borrowernumber, $nextaccntno, $payment, $data->{'itemnumber'}, $manager_id, $payment_note);
>-    }
>-
>-    if ( C4::Context->preference("FinesLog") ) {
>-        logaction("FINES", 'MODIFY', $borrowernumber, Dumper({
>-            action                => 'fee_payment',
>-            borrowernumber        => $borrowernumber,
>-            old_amountoutstanding => $data->{'amountoutstanding'},
>-            new_amountoutstanding => 0,
>-            amount_paid           => $data->{'amountoutstanding'},
>-            accountlines_id       => $data->{'accountlines_id'},
>-            accountno             => $data->{'accountno'},
>-            manager_id            => $manager_id,
>-        }));
> 
>-
>-        logaction("FINES", 'CREATE',$borrowernumber,Dumper({
>-            action            => 'create_payment',
>-            borrowernumber    => $borrowernumber,
>-            accountno         => $nextaccntno,
>-            amount            => $payment,
>-            amountoutstanding => 0,,
>-            accounttype       => 'Pay',
>-            accountlines_paid => [$data->{'accountlines_id'}],
>-            manager_id        => $manager_id,
>-        }));
>-    }
>-
>-    UpdateStats({
>-        branch => $branch,
>-        type   => 'payment',
>-        amount => $amount,
>-        borrowernumber => $borrowernumber,
>-        accountno => $accountno
>-    });
>-
>-    #check to see what accounttype
>-    if ( $data->{'accounttype'} eq 'Rep' || $data->{'accounttype'} eq 'L' ) {
>-        C4::Circulation::ReturnLostItem( $borrowernumber, $data->{'itemnumber'} );
>-    }
>-    my $sthr = $dbh->prepare("SELECT max(accountlines_id) AS lastinsertid FROM accountlines");
>-    $sthr->execute();
>-    my $datalastinsertid = $sthr->fetchrow_hashref;
>-    return $datalastinsertid->{'lastinsertid'};
>+    return Koha::Account->new( { patron_id => $borrowernumber } )
>+      ->pay( { accountlines_id => $accountlines_id, amount => $amount, library_id => $branch, note => $payment_note } );
> }
> 
> =head2 getnextacctno
>diff --git a/Koha/Account.pm b/Koha/Account.pm
>index b7341b1..c1ea1c1 100644
>--- a/Koha/Account.pm
>+++ b/Koha/Account.pm
>@@ -47,14 +47,26 @@ sub new {
> 
> This method allows payments to be made against feees
> 
>+Koha::Account->new( { patron_id => $borrowernumber } )->pay(
>+    {
>+        amount     => $amount,
>+        sip        => $sipmode,
>+        note       => $note,
>+        id         => $accountlines_id,
>+        library_id => $branchcode,
>+    }
>+);
>+
> =cut
> 
> sub pay {
>     my ( $self, $params ) = @_;
> 
>-    my $amount = $params->{amount};
>-    my $sip    = $params->{sip};
>-    my $note   = $params->{note} || q{};
>+    my $amount          = $params->{amount};
>+    my $sip             = $params->{sip};
>+    my $note            = $params->{note} || q{};
>+    my $accountlines_id = $params->{accountlines_id};
>+    my $library_id      = $params->{library_id};
> 
>     my $userenv = C4::Context->userenv;
> 
>@@ -71,15 +83,61 @@ sub pay {
> 
>     my $manager_id = $userenv ? $userenv->{number} : 0;
> 
>-    my @outstanding_fines = Koha::Account::Lines->search(
>+    my @fines_paid; # List of account lines paid on with this payment
>+
>+    my $balance_remaining = $amount; # Set it now so we can adjust the amount if necessary
>+    $balance_remaining ||= 0;
>+
>+    # We were passed a specific line to pay
>+    if ( $accountlines_id ) {
>+        my $fine = Koha::Account::Lines->find( $accountlines_id );
>+
>+        # If accountline id is passed but no amount, we pay that line in full
>+        $amount = $fine->amountoutstanding unless defined($amount);
>+
>+        my $old_amountoutstanding = $fine->amountoutstanding;
>+        my $new_amountoutstanding = $old_amountoutstanding - $amount;
>+        $fine->amountoutstanding( $new_amountoutstanding )->store();
>+        $balance_remaining = $balance_remaining - $amount;
>+
>+        if ( $fine->accounttype eq 'Rep' || $fine->accounttype eq 'L' )
>+        {
>+            C4::Circulation::ReturnLostItem( $self->{patron_id}, $fine->itemnumber );
>+        }
>+
>+        if ( C4::Context->preference("FinesLog") ) {
>+            logaction(
>+                "FINES", 'MODIFY',
>+                $self->{patron_id},
>+                Dumper(
>+                    {
>+                        action                => 'fee_payment',
>+                        borrowernumber        => $fine->borrowernumber,
>+                        old_amountoutstanding => $old_amountoutstanding,
>+                        new_amountoutstanding => 0,
>+                        amount_paid           => $old_amountoutstanding,
>+                        accountlines_id       => $fine->id,
>+                        accountno             => $fine->accountno,
>+                        manager_id            => $manager_id,
>+                        note                  => $note,
>+                    }
>+                )
>+            );
>+            push( @fines_paid, $fine->id );
>+        }
>+    }
>+
>+    # Were not passed a specific line to pay, or the payment was for more
>+    # than the what was owed on the given line. In that case pay down other
>+    # lines with remaining balance.
>+    my @outstanding_fines;
>+    @outstanding_fines = Koha::Account::Lines->search(
>         {
>             borrowernumber    => $self->{patron_id},
>             amountoutstanding => { '>' => 0 },
>         }
>-    );
>+    ) if $balance_remaining > 0;
> 
>-    my $balance_remaining = $amount;
>-    my @fines_paid;
>     foreach my $fine (@outstanding_fines) {
>         my $amount_to_pay =
>             $fine->amountoutstanding > $balance_remaining
>@@ -131,10 +189,11 @@ sub pay {
>         }
>     )->store();
> 
>-    my $branch = $userenv ? $userenv->{'branch'} : undef;
>+    $library_id ||= $userenv ? $userenv->{'branch'} : undef;
>+
>     UpdateStats(
>         {
>-            branch         => $branch,
>+            branch         => $library_id,
>             type           => 'payment',
>             amount         => $amount,
>             borrowernumber => $self->{patron_id},
>@@ -160,6 +219,8 @@ sub pay {
>             )
>         );
>     }
>+
>+    return $payment->id;
> }
> 
> 1;
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 15896: 48332 | 51920 | 52139 | 53314 | 53782 | 53783 | 53784 | 53785 | 53786 | 55924 | 55925 | 57175 | 57176 | 57177 | 57178 | 57182 | 57183 | 57184 | 57185 | 57186 | 57187 | 57188 | 57189 | 57190 | 57191 | 57192 | 57193