From: Jan Büren Date: Fri, 1 Mar 2019 12:32:36 +0000 (+0100) Subject: BankTransaction: save_single_bank_transaction API-Änderung X-Git-Tag: release-3.5.4~122 X-Git-Url: http://wagnertech.de/git?a=commitdiff_plain;h=665741c4bc6d05d6092a76383c1a444a3507921e;p=kivitendo-erp.git BankTransaction: save_single_bank_transaction API-Änderung S.a. POD und devel-Liste Testfälle angepasst --- diff --git a/SL/Controller/BankTransaction.pm b/SL/Controller/BankTransaction.pm index bb9d07140..2ec9563c2 100644 --- a/SL/Controller/BankTransaction.pm +++ b/SL/Controller/BankTransaction.pm @@ -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, 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 diff --git a/t/bank/bank_transactions.t b/t/bank/bank_transactions.t index 9cbf12f92..1652c708d 100644 --- a/t/bank/bank_transactions.t +++ b/t/bank/bank_transactions.t @@ -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; };