X-Git-Url: http://wagnertech.de/git?a=blobdiff_plain;f=SL%2FController%2FBankTransaction.pm;h=2b338318f9f54d5a18e525c696a4cb57aa7cf63e;hb=e83fcf3aa3394efdf2293ea60d5510fcfbf41b52;hp=54716879fb6e7965fb0ac40559303d9e5b8c71c4;hpb=2f3b969e3f315ad23225d9661c5dd8bc9ffd95ef;p=kivitendo-erp.git diff --git a/SL/Controller/BankTransaction.pm b/SL/Controller/BankTransaction.pm index 54716879f..2b338318f 100644 --- a/SL/Controller/BankTransaction.pm +++ b/SL/Controller/BankTransaction.pm @@ -20,13 +20,14 @@ use SL::DB::RecordLink; use SL::JSON; use SL::DB::Chart; use SL::DB::AccTransaction; +use SL::DB::BankTransactionAccTrans; use SL::DB::Tax; use SL::DB::BankAccount; use SL::DB::RecordTemplate; use SL::DB::SepaExportItem; use SL::DBUtils qw(like); -use SL::Presenter; +use List::UtilsBy qw(partition_by); use List::MoreUtils qw(any); use List::Util qw(max); @@ -98,10 +99,10 @@ sub action_list { where => [ amount => {ne => \'invoice_amount'}, local_bank_account_id => $::form->{filter}{bank_account}, + cleared => 0, @where ], ); - $::lxdebug->log_time('asdf'); # credit notes have a negative amount, treat differently my $all_open_ar_invoices = SL::DB::Manager::Invoice ->get_all(where => [ or => [ amount => { gt => \'paid' }, and => [ type => 'credit_note', @@ -115,33 +116,31 @@ sub action_list { 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']); - $::lxdebug->log_time('asdf 2'); 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 }; my %sepa_exports; + my %sepa_export_items_by_id = partition_by { $_->ar_id || $_->ap_id } @$all_open_sepa_export_items; + # first collect sepa export items to open invoices foreach my $open_invoice (@all_open_invoices){ $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 && $_->ap_id == $open_invoice->id) || ($_->ar_id && $_->ar_id == $open_invoice->id)) { - my $factor = ($_->ar_id == $open_invoice->id ? 1 : -1); - #$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->{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; - } + foreach (@{ $sepa_export_items_by_id{ $open_invoice->id } || [] }) { + my $factor = ($_->ar_id == $open_invoice->id ? 1 : -1); + $open_invoice->{realamount} = $::form->format_amount(\%::myconfig,$open_invoice->amount*$factor,2); + + $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; } } - $::lxdebug->log_time('asdf 4'); # try to match each bank_transaction with each of the possible open invoices # by awarding points my @proposals; @@ -157,6 +156,7 @@ sub action_list { $bt->{remote_name} .= $bt->{remote_name_1} if $bt->{remote_name_1}; if ( $bt->is_batch_transaction ) { + my $found=0; foreach ( keys %sepa_exports) { if ( abs(($sepa_exports{$_}->{amount} * 1) - ($bt->amount * 1)) < 0.01 ) { ## jupp @@ -164,9 +164,11 @@ sub action_list { $bt->{sepa_export_ok} = 1; $sepa_exports{$_}->{proposed}=1; push(@proposals, $bt); - next; + $found=1; + last; } } + next if $found; # batch transaction has no remotename !! } else { next unless $bt->{remote_name}; # bank has no name, usually fees, use create invoice to assign @@ -182,7 +184,9 @@ sub action_list { # score is stored in $bt->{agreement} foreach my $open_invoice (@all_open_invoices) { - ($open_invoice->{agreement}, $open_invoice->{rule_matches}) = $bt->get_agreement_with_invoice($open_invoice); + ($open_invoice->{agreement}, $open_invoice->{rule_matches}) = $bt->get_agreement_with_invoice($open_invoice, + sepa_export_items => $all_open_sepa_export_items, + ); $open_invoice->{realamount} = $::form->format_amount(\%::myconfig, $open_invoice->amount * ($open_invoice->{is_ar} ? 1 : -1), 2); } @@ -202,7 +206,6 @@ sub action_list { push(@{$bt->{rule_matches}}, $_->{rule_matches}); }; }; - $::lxdebug->log_time('asdf 5'); } # finished one bt # finished all bt @@ -232,7 +235,7 @@ sub action_list { # for testing with t/bank/banktransaction.t : if ( $::form->{dont_render_for_test} ) { - return $bank_transactions; + return ( $bank_transactions , \@proposals ); } $::request->layout->add_javascripts("kivi.BankTransaction.js"); @@ -261,12 +264,6 @@ sub action_create_invoice { $self->transaction(SL::DB::Manager::BankTransaction->find_by(id => $::form->{bt_id})); - # This was dead code: We compared vendor.account_name with bank_transaction.iban. - # This did never match (Kontonummer != IBAN). It's kivis 09/02 (2013) day - # If refactored/improved, also consider that vendor.iban should be normalized - # user may like to input strings like: 'AT 3333 3333 2222 1111' -> can be checked strictly - # at Vendor code because we need the correct data for all sepa exports. - my $vendor_of_transaction = SL::DB::Manager::Vendor->find_by(iban => $self->transaction->{remote_account_number}); my $use_vendor_filter = $self->transaction->{remote_account_number} && $vendor_of_transaction; @@ -295,9 +292,9 @@ sub action_create_invoice { 'bank_transactions/create_invoice', { layout => 0 }, title => t8('Create invoice'), - TEMPLATES_GL => $use_vendor_filter ? undef : $templates_gl, + TEMPLATES_GL => $use_vendor_filter && @{ $templates_ap } ? undef : $templates_gl, TEMPLATES_AP => $templates_ap, - vendor_name => $use_vendor_filter ? $vendor_of_transaction->name : undef, + vendor_name => $use_vendor_filter && @{ $templates_ap } ? $vendor_of_transaction->name : undef, ); } @@ -332,15 +329,13 @@ sub action_filter_templates { $self->{transaction} = SL::DB::Manager::BankTransaction->find_by(id => $::form->{bt_id}); - my @filter; - push @filter, ('vendor.name' => { ilike => '%' . $::form->{vendor} . '%' }) if $::form->{vendor}; - push @filter, ('template_name' => { ilike => '%' . $::form->{template} . '%' }) if $::form->{template}; - push @filter, ('reference' => { ilike => '%' . $::form->{reference} . '%' }) if $::form->{reference}; + my (@filter, @filter_ap); - my $templates_ap = SL::DB::Manager::RecordTemplate->get_all( - where => [ template_type => 'ap_transaction', (and => \@filter) x !!@filter ], - with_objects => [ qw(employee vendor) ], - ); + # filter => gl and ap | filter_ap = ap (i.e. vendorname) + push @filter, ('template_name' => { ilike => '%' . $::form->{template} . '%' }) if $::form->{template}; + push @filter, ('reference' => { ilike => '%' . $::form->{reference} . '%' }) if $::form->{reference}; + push @filter_ap, ('vendor.name' => { ilike => '%' . $::form->{vendor} . '%' }) if $::form->{vendor}; + push @filter_ap, @filter; my $templates_gl = SL::DB::Manager::RecordTemplate->get_all( query => [ template_type => 'gl_transaction', chart_id => SL::DB::Manager::BankAccount->find_by(id => $self->transaction->local_bank_account_id)->chart_id, @@ -349,6 +344,10 @@ sub action_filter_templates { with_objects => [ qw(employee record_template_items) ], ); + my $templates_ap = SL::DB::Manager::RecordTemplate->get_all( + where => [ template_type => 'ap_transaction', (and => \@filter_ap) x !!@filter_ap ], + with_objects => [ qw(employee vendor) ], + ); $::form->{filter} //= {}; $self->callback($self->url_for( @@ -505,17 +504,22 @@ sub save_invoices { $count += scalar( @{$invoice_ids} ); } } + my $max_count = $count; foreach (@{ $self->problems }) { $count-- if $_->{result} eq 'error'; } - return $count; + return ($count, $max_count); } sub action_save_invoices { my ($self) = @_; - my $count = $self->save_invoices(); + my ($success_count, $max_count) = $self->save_invoices(); - flash('ok', t8('#1 invoice(s) saved.', $count)); + if ($success_count == $max_count) { + flash('ok', t8('#1 invoice(s) saved.', $success_count)); + } else { + flash('error', t8('At least #1 invoice(s) not saved', $max_count - $success_count)); + } $self->action_list(); } @@ -552,13 +556,16 @@ sub save_single_bank_transaction { }; } + my $bank_transaction = $data{bank_transaction}; + my (@warnings); my $worker = sub { my $bt_id = $data{bank_transaction_id}; - my $bank_transaction = $data{bank_transaction}; 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; @@ -606,15 +613,6 @@ sub save_single_bank_transaction { $n_invoices++ ; - # Check if bank_transaction already has a link to the invoice, may only be linked once per invoice - # This might be caused by the user reloading a page and resending the form - if (_existing_record_link($bank_transaction, $invoice)) { - return { - %data, - result => 'error', - message => $::locale->text("Bank transaction with id #1 has already been linked to #2.", $bank_transaction->id, $invoice->displayable_name), - }; - } if (!$amount_of_transaction && $invoice->open_amount) { return { @@ -630,73 +628,49 @@ 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 - $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); - } else { - # use the whole amount of the bank transaction for the invoice, overpay the invoice if necessary - - # this catches credit_notes and negative sales invoices - if ( $invoice->is_sales && $invoice->amount < 0 ) { - # $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 - $amount_of_transaction *= -1; - - } elsif (!$invoice->is_sales && $invoice->invoice_type =~ m/ap_transaction|purchase_invoice/) { - # $invoice->open_amount may be negative for ap_transaction but may be positiv for negativ 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->open_amount == - $amount_of_transaction; - } - - 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 = ( + my %props = ( from_table => 'bank_transactions', from_id => $bt_id, to_table => $invoice->is_sales ? 'ar' : 'ap', to_id => $invoice->id, ); - - SL::DB::RecordLink->new(@props)->save; + SL::DB::RecordLink->new(%props)->save; # "close" a sepa_export_item if it exists # code duplicated in action_save_proposals! @@ -732,6 +706,9 @@ sub save_single_bank_transaction { # Rollback Fehler nicht weiterreichen # die if $error; + # aber einen rollback von hand + $::lxdebug->message(LXDebug->DEBUG2(),"finish worker with ". ($error ? $error->{result} : '-')); + $data{bank_transaction}->db->dbh->rollback if $error && $error->{result} eq 'error'; }); return grep { $_ } ($error, @warnings); @@ -792,7 +769,8 @@ sub prepare_report { align => 'right' }, invoice_amount => { sub => sub { $_[0]->invoice_amount_as_number }, align => 'right' }, - invoices => { sub => sub { $_[0]->linked_invoices } }, + invoices => { sub => sub { my @invnumbers; for my $obj (@{ $_[0]->linked_invoices }) { + next unless $obj; push @invnumbers, $obj->invnumber } return \@invnumbers } }, currency => { sub => sub { $_[0]->currency->name } }, purpose => { }, local_account_number => { sub => sub { $_[0]->local_bank_account->account_number } }, @@ -827,20 +805,6 @@ sub prepare_report { ); } -sub _existing_record_link { - my ($bt, $invoice) = @_; - - # check whether a record link from banktransaction $bt already exists to - # invoice $invoice, returns 1 if that is the case - - die unless $bt->isa("SL::DB::BankTransaction") && ( $invoice->isa("SL::DB::Invoice") || $invoice->isa("SL::DB::PurchaseInvoice") ); - - my $linked_record_to_table = $invoice->is_sales ? 'Invoice' : 'PurchaseInvoice'; - my $linked_records = $bt->linked_records( direction => 'to', to => $linked_record_to_table, query => [ id => $invoice->id ] ); - - return @$linked_records ? 1 : 0; -}; - sub init_problems { [] } sub init_models { @@ -895,9 +859,11 @@ sub load_gl_record_template_url { controller => 'gl.pl', action => 'load_record_template', id => $template->id, - 'form_defaults.amount_1' => abs($self->transaction->amount), # always positive + 'form_defaults.amount_1' => abs($self->transaction->not_assigned_amount), # always positive 'form_defaults.transdate' => $self->transaction->transdate_as_date, 'form_defaults.callback' => $self->callback, + 'form_defaults.bt_id' => $self->transaction->id, + 'form_defaults.bt_chart_id' => $self->transaction->local_bank_account->chart->id, ); } @@ -952,6 +918,16 @@ 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 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 is true if the code detects an error during the execution, e.g. a bank