BankTransaction: save_single_bank_transaction API-Änderung
authorJan Büren <jan@kivitendo.de>
Fri, 1 Mar 2019 12:32:36 +0000 (13:32 +0100)
committerJan Büren <jan@kivitendo.de>
Fri, 1 Mar 2019 15:10:41 +0000 (16:10 +0100)
S.a. POD und devel-Liste
Testfälle angepasst

SL/Controller/BankTransaction.pm
t/bank/bank_transactions.t

index bb9d071..2ec9563 100644 (file)
@@ -558,23 +558,18 @@ sub save_single_bank_transaction {
 
   my $bank_transaction = $data{bank_transaction};
 
-  # see pod
-  if (@{ $bank_transaction->linked_invoices } || $bank_transaction->invoice_amount != 0) {
-        return {
-          %data,
-          result  => 'error',
-          message => $::locale->text("Bank transaction with id #1 has already been linked to one or more record and/or some amount is already assigned.", $bank_transaction->id),
-        };
-      }
   my (@warnings);
 
   my $worker = sub {
     my $bt_id                 = $data{bank_transaction_id};
     my $sign                  = $bank_transaction->amount < 0 ? -1 : 1;
     my $amount_of_transaction = $sign * $bank_transaction->amount;
+    my $assigned_amount       = $sign * $bank_transaction->invoice_amount;
+    my $not_assigned_amount   = $amount_of_transaction - $assigned_amount;
     my $payment_received      = $bank_transaction->amount > 0;
     my $payment_sent          = $bank_transaction->amount < 0;
 
+    croak("No amount left to assign") if ($not_assigned_amount <= 0);
 
     foreach my $invoice_id (@{ $params{invoice_ids} }) {
       my $invoice = SL::DB::Manager::Invoice->find_by(id => $invoice_id) || SL::DB::Manager::PurchaseInvoice->find_by(id => $invoice_id);
@@ -634,82 +629,41 @@ sub save_single_bank_transaction {
       } else {
         $payment_type = 'without_skonto';
       };
-
-
-      # pay invoice or go to the next bank transaction if the amount is not sufficiently high
-      if ($invoice->open_amount <= $amount_of_transaction && $n_invoices < $max_invoices) {
-        my $open_amount = ($payment_type eq 'with_skonto_pt'?$invoice->amount_less_skonto:$invoice->open_amount);
-        # first calculate new bank transaction amount ...
-        if ($invoice->is_sales) {
-          $amount_of_transaction -= $sign * $open_amount;
-          $bank_transaction->invoice_amount($bank_transaction->invoice_amount + $open_amount);
-        } else {
-          $amount_of_transaction += $sign * $open_amount;
-          $bank_transaction->invoice_amount($bank_transaction->invoice_amount - $open_amount);
-        }
-        # ... and then pay the invoice
-        my @acc_ids = $invoice->pay_invoice(chart_id => $bank_transaction->local_bank_account->chart_id,
-                              trans_id     => $invoice->id,
-                              amount       => $open_amount,
-                              payment_type => $payment_type,
-                              source       => $source,
-                              memo         => $memo,
-                              transdate    => $bank_transaction->transdate->to_kivitendo);
-        # ... and record the origin via BankTransactionAccTrans
-        if (scalar(@acc_ids) != 2) {
-          return {
-            %data,
-            result  => 'error',
-            message => $::locale->text("Unable to book transactions for bank purpose #1", $bank_transaction->purpose),
-          };
-        }
-        foreach my $acc_trans_id (@acc_ids) {
-            my $id_type = $invoice->is_sales ? 'ar' : 'ap';
-            my  %props_acc = (
-              acc_trans_id        => $acc_trans_id,
-              bank_transaction_id => $bank_transaction->id,
-              $id_type            => $invoice->id,
-            );
-            SL::DB::BankTransactionAccTrans->new(%props_acc)->save;
-        }
-
-
-      } else {
-      # use the whole amount of the bank transaction for the invoice, overpay the invoice if necessary
-
-        # $invoice->open_amount     is negative for credit_notes
-        # $bank_transaction->amount is negative for outgoing transactions
-        # so $amount_of_transaction is negative but needs positive
-        # $invoice->open_amount may be negative for ap_transaction but may be positiv for negative ap_transaction
-        # if $invoice->open_amount is negative $bank_transaction->amount is positve
-        # if $invoice->open_amount is positive $bank_transaction->amount is negative
-        # but amount of transaction is for both positive
-
-        $amount_of_transaction *= -1 if ($invoice->amount < 0);
-
-        # if we have a skonto case - the last invoice needs skonto
-        $amount_of_transaction = $invoice->amount_less_skonto if ($payment_type eq 'with_skonto_pt');
-
-
-        my $overpaid_amount = $amount_of_transaction - $invoice->open_amount;
-        $invoice->pay_invoice(chart_id     => $bank_transaction->local_bank_account->chart_id,
-                              trans_id     => $invoice->id,
-                              amount       => $amount_of_transaction,
-                              payment_type => $payment_type,
-                              source       => $source,
-                              memo         => $memo,
-                              transdate    => $bank_transaction->transdate->to_kivitendo);
-        $bank_transaction->invoice_amount($bank_transaction->amount);
-        $amount_of_transaction = 0;
-
-        if ($overpaid_amount >= 0.01) {
-          push @warnings, {
-            %data,
-            result  => 'warning',
-            message => $::locale->text('Invoice #1 was overpaid by #2.', $invoice->invnumber, $::form->format_amount(\%::myconfig, $overpaid_amount, 2)),
-          };
-        }
-      }
+    # pay invoice
+    # TODO rewrite this: really booked amount should be a return value of Payment.pm
+    # also this controller shouldnt care about how to calc skonto. we simply delegate the
+    # payment_type to the helper and get the corresponding bank_transaction values back
+
+    my $open_amount = ($payment_type eq 'with_skonto_pt' ? $invoice->amount_less_skonto : $invoice->open_amount);
+    my $amount_for_booking = abs(($open_amount < $not_assigned_amount) ? $open_amount : $not_assigned_amount);
+    $amount_for_booking *= $sign;
+    $bank_transaction->invoice_amount($bank_transaction->invoice_amount + $amount_for_booking);
+
+    # ... and then pay the invoice
+    my @acc_ids = $invoice->pay_invoice(chart_id => $bank_transaction->local_bank_account->chart_id,
+                          trans_id     => $invoice->id,
+                          amount       => ($open_amount < $not_assigned_amount) ? $open_amount : $not_assigned_amount,
+                          payment_type => $payment_type,
+                          source       => $source,
+                          memo         => $memo,
+                          transdate    => $bank_transaction->transdate->to_kivitendo);
+    # ... and record the origin via BankTransactionAccTrans
+    if (scalar(@acc_ids) != 2) {
+      return {
+        %data,
+        result  => 'error',
+        message => $::locale->text("Unable to book transactions for bank purpose #1", $bank_transaction->purpose),
+      };
+    }
+    foreach my $acc_trans_id (@acc_ids) {
+        my $id_type = $invoice->is_sales ? 'ar' : 'ap';
+        my  %props_acc = (
+          acc_trans_id        => $acc_trans_id,
+          bank_transaction_id => $bank_transaction->id,
+          $id_type            => $invoice->id,
+        );
+        SL::DB::BankTransactionAccTrans->new(%props_acc)->save;
+    }
       # Record a record link from the bank transaction to the invoice
       my %props = (
         from_table => 'bank_transactions',
@@ -965,12 +919,15 @@ tries to post its amount to a certain number of invoices (parameter
 C<invoice_ids>, an array ref of database IDs to purchase or sales
 invoice objects).
 
+This method handles already partly assigned bank transactions.
+
 This method cannot handle already partly assigned bank transactions, i.e.
 a bank transaction that has a invoice_amount <> 0 but not the fully
 transaction amount (invoice_amount == amount).
 
 If the amount of the bank transaction is higher than the sum of
-the assigned invoices (1 .. n) the last invoice will be overpayed.
+the assigned invoices (1 .. n) the bank transaction will only be
+partly assigned.
 
 The whole function is wrapped in a database transaction. If an
 exception occurs the bank transaction is not posted at all. The same
index 9cbf12f..1652c70 100644 (file)
@@ -1,4 +1,4 @@
-use Test::More tests => 208;
+use Test::More tests => 211;
 
 use strict;
 
@@ -11,6 +11,7 @@ use Test::Exception;
 use List::Util qw(sum);
 
 use SL::DB::AccTransaction;
+use SL::DB::BankTransactionAccTrans;
 use SL::DB::Buchungsgruppe;
 use SL::DB::Currency;
 use SL::DB::Customer;
@@ -34,6 +35,7 @@ my ($ar_transaction, $ap_transaction);
 
 sub clear_up {
 
+  SL::DB::Manager::BankTransactionAccTrans->delete_all(all => 1);
   SL::DB::Manager::BankTransaction->delete_all(all => 1);
   SL::DB::Manager::InvoiceItem->delete_all(all => 1);
   SL::DB::Manager::InvoiceItem->delete_all(all => 1);
@@ -442,18 +444,19 @@ sub test_overpayment {
   $ar_transaction->load;
   $bt->load;
 
-  is($ar_transaction->paid                     , '135.00000' , "$testname: 'salesinv overpaid' was overpaid");
-  is($bt->invoice_amount                       , '135.00000' , "$testname: bt invoice amount was assigned overpaid amount");
+  is($ar_transaction->paid                     , '119.00000' , "$testname: 'salesinv overpaid' was not overpaid");
+  is($bt->invoice_amount                       , '119.00000' , "$testname: bt invoice amount was not fully assigned with the overpaid amount");
 { local $TODO = 'this currently fails because closed ignores over-payments, see commit d90966c7';
   is($ar_transaction->closed                   , 0           , "$testname: 'salesinv overpaid' is open (via 'closed' method')");
 }
-  is($ar_transaction->open_amount == 0 ? 1 : 0 , 0           , "$testname: 'salesinv overpaid is open (via amount-paid)");
+  is($ar_transaction->open_amount == 0 ? 1 : 0 , 1           , "$testname: 'salesinv overpaid is closed (via amount-paid)");
 
 };
 
 sub test_overpayment_with_partialpayment {
 
-  # two payments on different days, 10 and 119. If there is only one invoice we want it be overpaid.
+  # two payments on different days, 10 and 119. If there is only one invoice we
+  # don't want it to be overpaid.
   my $testname = 'test_overpayment_with_partialpayment';
 
   $ar_transaction = test_ar_transaction(invnumber => 'salesinv overpaid partial');
@@ -473,18 +476,19 @@ sub test_overpayment_with_partialpayment {
   };
   save_btcontroller_to_string();
 
+  $bt_1->load;
+  is($bt_1->invoice_amount ,  '10.00000' , "$testname: bt_1 invoice amount was fully assigned");
   $::form->{invoice_ids} = {
     $bt_2->id => [ $ar_transaction->id ]
   };
   save_btcontroller_to_string();
 
   $ar_transaction->load;
-  $bt_1->load;
   $bt_2->load;
 
-  is($ar_transaction->paid , '129.00000' , "$testname: 'salesinv overpaid partial' was overpaid");
-  is($bt_1->invoice_amount ,  '10.00000' , "$testname: bt_1 invoice amount was assigned overpaid amount");
-  is($bt_2->invoice_amount , '119.00000' , "$testname: bt_2 invoice amount was assigned overpaid amount");
+  is($bt_1->invoice_amount ,  '10.00000' , "$testname: bt_1 invoice amount was fully assigned");
+  is($ar_transaction->paid , '119.00000' , "$testname: 'salesinv overpaid partial' was not overpaid");
+  is($bt_2->invoice_amount , '109.00000' , "$testname: bt_2 invoice amount was partly assigned");
 
 };
 
@@ -585,6 +589,7 @@ sub test_neg_ap_transaction {
                                               bank_chart_id => $bank->id,
                                               transdate     => DateTime->today->add(days => 10),
                                                                );
+
   my ($agreement, $rule_matches) = $bt->get_agreement_with_invoice($invoice);
   is($agreement, 15, "points for negative ap transaction ok");
 
@@ -601,6 +606,7 @@ sub test_neg_ap_transaction {
   is($invoice->netamount, '-20.00000', "$testname: netamount ok");
   is($invoice->paid     , '-23.80000', "$testname: paid ok");
   is($bt->invoice_amount, '23.80000', "$testname: bt invoice amount for ap was assigned");
+  is($bt->amount,         '23.80000', "$testname: bt  amount for ap was assigned");
 
   return $invoice;
 };