From 503fabbf4b2c77b2aab2dc1940f1eec0842cd490 Mon Sep 17 00:00:00 2001 From: "Martin Helmling martin.helmling@octosoft.eu" Date: Thu, 10 Aug 2017 14:23:50 +0200 Subject: [PATCH] =?utf8?q?BankTransaction:=20=C3=9Cberarbeitung=20von=20"K?= =?utf8?q?ontoauszug=20verbuchen"=20,=20SEPA-Export=20wieder=20integriert?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Die Punktebewertung findet wieder ausschließlich in "get_agreement_with_bank_transactions" statt, auch die SEPA-Sammelüberweisung. Diese bekommt dor extra Punkte, da ggf. für bestimmte Rechnungen negative Punkte entstehen. Auch gibt es dort keine Remote Banknummer etc. Die Testdatei t/bank/bank_transactions.t wurde um zwei Tests erweitert, 1. ein Test der das Verbuchen ohne SEPA-Export macht, 2. ein Test mit SEPA-Export fixt #277 --- SL/Controller/BankTransaction.pm | 72 ++++++----------------------- SL/DB/BankTransaction.pm | 21 ++++++--- SL/Dev/Payment.pm | 24 ++++++++++ t/bank/bank_transactions.t | 78 +++++++++++++++++++++++++++++++- 4 files changed, 130 insertions(+), 65 deletions(-) diff --git a/SL/Controller/BankTransaction.pm b/SL/Controller/BankTransaction.pm index 038267391..e47c1cd64 100644 --- a/SL/Controller/BankTransaction.pm +++ b/SL/Controller/BankTransaction.pm @@ -101,7 +101,6 @@ sub action_list { @where ], ); - $main::lxdebug->message(LXDebug->DEBUG2(),"count bt=".scalar(@{$bank_transactions}." bank_account=".$bank_account->id." chart=".$bank_account->chart_id)); # credit notes have a negative amount, treat differently my $all_open_ar_invoices = SL::DB::Manager::Invoice ->get_all(where => [ or => [ amount => { gt => \'paid' }, @@ -115,43 +114,31 @@ sub action_list { my $all_open_ap_invoices = SL::DB::Manager::PurchaseInvoice->get_all(where => [amount => { ne => \'paid' }], with_objects => ['vendor' ,'payment_terms']); my $all_open_sepa_export_items = SL::DB::Manager::SepaExportItem->get_all(where => [chart_id => $bank_account->chart_id , 'sepa_export.executed' => 0, 'sepa_export.closed' => 0 ], with_objects => ['sepa_export']); - $main::lxdebug->message(LXDebug->DEBUG2(),"count sepaexport=".scalar(@{$all_open_sepa_export_items})); my @all_open_invoices; # filter out invoices with less than 1 cent outstanding push @all_open_invoices, map { $_->{is_ar}=1 ; $_ } grep { abs($_->amount - $_->paid) >= 0.01 } @{ $all_open_ar_invoices }; push @all_open_invoices, map { $_->{is_ar}=0 ; $_ } grep { abs($_->amount - $_->paid) >= 0.01 } @{ $all_open_ap_invoices }; - $main::lxdebug->message(LXDebug->DEBUG2(),"bank_account=".$::form->{filter}{bank_account}." invoices: ".scalar(@{ $all_open_ar_invoices }). - " + ".scalar(@{ $all_open_ap_invoices })." non fully paid=".scalar(@all_open_invoices)." transactions=".scalar(@{ $bank_transactions })); - my @all_sepa_invoices; - my @all_non_sepa_invoices; my %sepa_exports; # first collect sepa export items to open invoices foreach my $open_invoice (@all_open_invoices){ - # my @items = grep { $_->ap_id == $open_invoice->id || $_->ar_id == $open_invoice->id } @{$all_open_sepa_export_items}; $open_invoice->{realamount} = $::form->format_amount(\%::myconfig,$open_invoice->amount,2); $open_invoice->{skonto_type} = 'without_skonto'; foreach ( @{$all_open_sepa_export_items}) { if ( $_->ap_id == $open_invoice->id || $_->ar_id == $open_invoice->id ) { my $factor = ($_->ar_id == $open_invoice->id?1:-1); - $main::lxdebug->message(LXDebug->DEBUG2(),"exitem=".$_->id." for invoice ".$open_invoice->id." factor=".$factor); + #$main::lxdebug->message(LXDebug->DEBUG2(),"sepa_exitem=".$_->id." for invoice ".$open_invoice->id." factor=".$factor); $open_invoice->{realamount} = $::form->format_amount(\%::myconfig,$open_invoice->amount*$factor,2); - $open_invoice->{sepa_export_item} = $_ ; + push @{$open_invoice->{sepa_export_item}} , $_ ; $open_invoice->{skonto_type} = $_->payment_type; $sepa_exports{$_->sepa_export_id} ||= { count => 0, is_ar => 0, amount => 0, proposed => 0, invoices => [], item => $_ }; $sepa_exports{$_->sepa_export_id}->{count}++ ; $sepa_exports{$_->sepa_export_id}->{is_ar}++ if $_->ar_id == $open_invoice->id; $sepa_exports{$_->sepa_export_id}->{amount} += $_->amount * $factor; push @{ $sepa_exports{$_->sepa_export_id}->{invoices} }, $open_invoice; - #$main::lxdebug->message(LXDebug->DEBUG2(),"amount for export id ".$_->sepa_export_id." = ". - # $sepa_exports{$_->sepa_export_id}->{amount}." count = ". - # $sepa_exports{$_->sepa_export_id}->{count}." is_ar = ". - # $sepa_exports{$_->sepa_export_id}->{is_ar} ); - push @all_sepa_invoices , $open_invoice; } } - push @all_non_sepa_invoices , $open_invoice if ! $open_invoice->{sepa_export_item}; } # try to match each bank_transaction with each of the possible open invoices @@ -162,55 +149,26 @@ sub action_list { ## 5 Stellen hinter dem Komma auf 2 Stellen reduzieren $bt->amount($bt->amount*1); $bt->invoice_amount($bt->invoice_amount*1); - $main::lxdebug->message(LXDebug->DEBUG2(),"BT ".$bt->id." amount=".$bt->amount." invoice_amount=".$bt->invoice_amount." remote=". $bt->{remote_name}); $bt->{proposals} = []; $bt->{rule_matches} = []; $bt->{remote_name} .= $bt->{remote_name_1} if $bt->{remote_name_1}; - if ( $self->is_collective_transaction($bt) ) { + if ( $bt->is_collective_transaction ) { foreach ( keys %sepa_exports) { - #$main::lxdebug->message(LXDebug->DEBUG2(),"Exp ID=".$_." compare sum amount ".($sepa_exports{$_}->{amount} *1) ." == ".($bt->amount * 1)); - if ( $bt->transaction_code eq '191' && abs(($sepa_exports{$_}->{amount} * 1) - ($bt->amount * 1)) < 0.01 ) { + if ( abs(($sepa_exports{$_}->{amount} * 1) - ($bt->amount * 1)) < 0.01 ) { ## jupp @{$bt->{proposals}} = @{$sepa_exports{$_}->{invoices}}; - $bt->{agreement} = 20; - push(@{$bt->{rule_matches}},'sepa_export_item(20)'); + $bt->{sepa_export_ok} = 1; $sepa_exports{$_}->{proposed}=1; - #$main::lxdebug->message(LXDebug->DEBUG2(),"has ".scalar($bt->{proposals})." invoices"); push(@proposals, $bt); next; } } - } - next unless $bt->{remote_name}; # bank has no name, usually fees, use create invoice to assign - - foreach ( @{$all_open_sepa_export_items}) { - last if scalar (@all_sepa_invoices) == 0; - foreach my $open_invoice (@all_sepa_invoices){ - $open_invoice->{agreement} = 0; - $open_invoice->{rule_matches} =''; - if ( $_->ap_id == $open_invoice->id || $_->ar_id == $open_invoice->id ) { - #$main::lxdebug->message(LXDebug->DEBUG2(),"exitem2=".$_->id." for invoice ".$open_invoice->id); - my $factor = ( $_->ar_id == $open_invoice->id?1:-1); - $_->amount($_->amount*1); - #$main::lxdebug->message(LXDebug->DEBUG2(),"remote account '".$bt->{remote_account_number}."' bt_amount=".$bt->amount." factor=".$factor); - #$main::lxdebug->message(LXDebug->DEBUG2(),"compare with '".$_->vc_iban."' amount=".$_->amount); - if ( $bt->{remote_account_number} eq $_->vc_iban && abs(abs($_->amount) - abs($bt->amount)) < 0.01 ) { - my $iban; - $iban = $open_invoice->customer->iban if $open_invoice->is_sales; - $iban = $open_invoice->vendor->iban if ! $open_invoice->is_sales; - if($bt->{remote_account_number} eq $iban && abs(abs($open_invoice->amount) - abs($bt->amount)) < 0.01 ) { - ($open_invoice->{agreement}, $open_invoice->{rule_matches}) = $bt->get_agreement_with_invoice($open_invoice); - $open_invoice->{agreement} += 5; - $open_invoice->{rule_matches} .= 'sepa_export_item(5) '; - $main::lxdebug->message(LXDebug->DEBUG2(),"sepa invoice_id=".$open_invoice->id." agreement=".$open_invoice->{agreement}." rules matches=".$open_invoice->{rule_matches}); - $open_invoice->{realamount} = $::form->format_amount(\%::myconfig,$open_invoice->amount*$factor,2); - } - } - } - } + # colletive transaction has no remotename !! + } else { + next unless $bt->{remote_name}; # bank has no name, usually fees, use create invoice to assign } # try to match the current $bt to each of the open_invoices, saving the @@ -222,7 +180,7 @@ sub action_list { # the arrays $bt->{proposals} and $bt->{rule_matches}, and the agreement # score is stored in $bt->{agreement} - foreach my $open_invoice (@all_non_sepa_invoices, @all_sepa_invoices) { + foreach my $open_invoice (@all_open_invoices) { ($open_invoice->{agreement}, $open_invoice->{rule_matches}) = $bt->get_agreement_with_invoice($open_invoice); $open_invoice->{realamount} = $::form->format_amount(\%::myconfig, $open_invoice->amount * ($open_invoice->{is_ar} ? 1 : -1), 2); @@ -259,12 +217,17 @@ sub action_list { : abs(@{ $_->{proposals} }[0]->amount + $_->amount) < 0.01) } @{ $bank_transactions }; - push ( @proposals, @otherproposals); + push @proposals, @otherproposals; # sort bank transaction proposals by quality (score) of proposal $bank_transactions = [ sort { $a->{agreement} <=> $b->{agreement} } @{ $bank_transactions } ] if $::form->{sort_by} eq 'proposal' and $::form->{sort_dir} == 1; $bank_transactions = [ sort { $b->{agreement} <=> $a->{agreement} } @{ $bank_transactions } ] if $::form->{sort_by} eq 'proposal' and $::form->{sort_dir} == 0; + # for testing with t/bank/banktransaction.t : + if ( $::form->{dont_render_for_test} ) { + return $bank_transactions; + } + $::request->layout->add_javascripts("kivi.BankTransaction.js"); $self->render('bank_transactions/list', title => t8('Bank transactions MT940'), @@ -545,11 +508,6 @@ sub action_save_proposals { } -sub is_collective_transaction { - my ($self, $bt) = @_; - return $bt->transaction_code eq "191"; -} - sub save_single_bank_transaction { my ($self, %params) = @_; diff --git a/SL/DB/BankTransaction.pm b/SL/DB/BankTransaction.pm index 6ad0348c7..8c26cd40a 100644 --- a/SL/DB/BankTransaction.pm +++ b/SL/DB/BankTransaction.pm @@ -47,6 +47,11 @@ sub linked_invoices { return [ @linked_invoices ]; } +sub is_collective_transaction { + $_[0]->transaction_code eq "191"; +} + + sub get_agreement_with_invoice { my ($self, $invoice) = @_; @@ -73,10 +78,16 @@ sub get_agreement_with_invoice { skonto_exact_amount => 5, wrong_sign => -1, sepa_export_item => 5, + collective_sepa_transaction => 20, ); my ($agreement,$rule_matches); + if ( $self->is_collective_transaction && $self->{sepa_export_ok}) { + $agreement += $points{collective_sepa_transaction}; + $rule_matches .= 'collective_sepa_transaction(' . $points{'collective_sepa_transaction'} . ') '; + } + # compare banking arrangements my ($iban, $bank_code, $account_number); $bank_code = $invoice->customer->bank_code if $invoice->is_sales; @@ -210,15 +221,11 @@ sub get_agreement_with_invoice { }; # if there is exactly one non-executed sepa_export_item for the invoice - if ( my $seis = $invoice->find_sepa_export_items({ executed => 0 }) ) { - if (scalar @$seis == 1) { + if ( my $seis = $invoice->{sepa_export_item} ) { + if (scalar @$seis == 1) { my $sei = $seis->[0]; - # test for amount and id matching only, sepa transfer date and bank - # transaction date needn't match - my $arap = $invoice->is_sales ? 'ar' : 'ap'; - - if (abs($self->amount) == ($sei->amount) && $invoice->id == $sei->arap_id) { + if ( abs(abs($self->amount) - abs($sei->amount)) < 0.01 ) { $agreement += $points{sepa_export_item}; $rule_matches .= 'sepa_export_item(' . $points{'sepa_export_item'} . ') '; } diff --git a/SL/Dev/Payment.pm b/SL/Dev/Payment.pm index e1acac681..2d1846e3f 100644 --- a/SL/Dev/Payment.pm +++ b/SL/Dev/Payment.pm @@ -39,6 +39,30 @@ sub create_bank_account { $bank_account->save; } +sub create_sepa_export { + my (%params) = @_; + my $sepa_export = SL::DB::SepaExport->new( + closed => 0, + employee_id => $params{employee_id} // SL::DB::Manager::Employee->current->id, + executed => 0, + vc => 'customer', + ); + $sepa_export->assign_attributes(%params) if %params; + $sepa_export->save; +} + +sub create_sepa_export_item { + my (%params) = @_; + my $sepa_exportitem = SL::DB::SepaExportItem->new( + chart_id => delete $params{chart_id} // $::instance_conf->get_ar_paid_accno_id, + payment_type => 'without_skonto', + our_bic => 'BANK1234', + our_iban => 'DE12500105170648489890', + ); + $sepa_exportitem->assign_attributes(%params) if %params; + $sepa_exportitem->save; +} + sub create_bank_transaction { my (%params) = @_; diff --git a/t/bank/bank_transactions.t b/t/bank/bank_transactions.t index fd9757518..d2376a22a 100644 --- a/t/bank/bank_transactions.t +++ b/t/bank/bank_transactions.t @@ -1,4 +1,4 @@ -use Test::More tests => 105; +use Test::More tests => 130; use strict; @@ -41,6 +41,8 @@ sub clear_up { SL::DB::Manager::Part->delete_all(all => 1); SL::DB::Manager::Customer->delete_all(all => 1); SL::DB::Manager::Vendor->delete_all(all => 1); + SL::DB::Manager::SepaExportItem->delete_all(all => 1); + SL::DB::Manager::SepaExport->delete_all(all => 1); SL::DB::Manager::BankAccount->delete_all(all => 1); SL::DB::Manager::PaymentTerm->delete_all(all => 1); SL::DB::Manager::Currency->delete_all(where => [ name => 'CUR' ]); @@ -79,6 +81,9 @@ test_ap_payment_transaction(); test_ap_payment_part_transaction(); test_neg_sales_invoice(); +test_bt_rule1(); +test_sepa_export(); + # remove all created data at end of test clear_up(); @@ -656,4 +661,75 @@ sub test_neg_sales_invoice { is($bt->invoice_amount , '-345.10000', "$testname: bt invoice_amount ok"); } +sub test_bt_rule1 { + + my $testname = 'test_bt_rule1'; + + $ar_transaction = test_ar_transaction(invnumber => 'bt_rule1'); + + my $bt = SL::Dev::Payment::create_bank_transaction(record => $ar_transaction) or die "Couldn't create bank_transaction"; + + $ar_transaction->load; + $bt->load; + is($ar_transaction->paid , '0.00000' , "$testname: not paid"); + is($bt->invoice_amount , '0.00000' , "$testname: bt invoice amount was not assigned"); + + my $bt_controller = SL::Controller::BankTransaction->new; + $::form->{dont_render_for_test} = 1; + $::form->{filter}{bank_account} = $bank_account->id; + my $bt_transactions = $bt_controller->action_list; + + is(scalar(@$bt_transactions) , 1 , "$testname: one bank_transaction"); + is($bt_transactions->[0]->{agreement}, 20 , "$testname: agreement == 20"); + my $match = join ( ' ',@{$bt_transactions->[0]->{rule_matches}}); + #print "rule_matches='".$match."'\n"; + is($match, + "remote_account_number(3) exact_amount(4) own_invnumber_in_purpose(5) depositor_matches(2) remote_name(2) payment_within_30_days(1) datebonus0(3) ", + "$testname: rule_matches ok"); + $bt->invoice_amount($bt->amount); + $bt->save; + is($bt->invoice_amount , '119.00000' , "$testname: bt invoice amount now set"); +}; + +sub test_sepa_export { + + my $testname = 'test_sepa_export'; + + $ar_transaction = test_ar_transaction(invnumber => 'sepa1'); + + my $bt = SL::Dev::Payment::create_bank_transaction(record => $ar_transaction) or die "Couldn't create bank_transaction"; + my $se = SL::Dev::Payment::create_sepa_export(); + my $sei = SL::Dev::Payment::create_sepa_export_item( + chart_id => $bank->id, + ar_id => $ar_transaction->id, + sepa_export_id => $se->id, + vc_iban => $customer->iban, + vc_bic => $customer->bic, + vc_mandator_id => $customer->mandator_id, + vc_depositor => $customer->depositor, + amount => $ar_transaction->amount, + ); + + $ar_transaction->load; + $bt->load; + $sei->load; + is($ar_transaction->paid , '0.00000' , "$testname: sepa1 not paid"); + is($bt->invoice_amount , '0.00000' , "$testname: bt invoice amount was not assigned"); + is($bt->amount , '119.00000' , "$testname: bt amount ok"); + is($sei->amount , '119.00000' , "$testname: sepa export amount ok"); + + my $bt_controller = SL::Controller::BankTransaction->new; + $::form->{dont_render_for_test} = 1; + $::form->{filter}{bank_account} = $bank_account->id; + my $bt_transactions = $bt_controller->action_list; + + is(scalar(@$bt_transactions) , 1 , "$testname: one bank_transaction"); + is($bt_transactions->[0]->{agreement}, 25 , "$testname: agreement == 25"); + my $match = join ( ' ',@{$bt_transactions->[0]->{rule_matches}}); + is($match, + "remote_account_number(3) exact_amount(4) own_invnumber_in_purpose(5) depositor_matches(2) remote_name(2) payment_within_30_days(1) datebonus0(3) sepa_export_item(5) ", + "$testname: rule_matches ok"); +}; + + 1; -- 2.20.1