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>
Followed test plan, works as expected.
Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

diff --git a/C4/Accounts.pm b/C4/Accounts.pm
index f65745e..45d270a 100644 (file)
--- a/C4/Accounts.pm
+++ b/C4/Accounts.pm
@@ -86,96 +86,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 (file)
--- 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;