Merge branch 'bank-transaction-improvements'
authorMoritz Bunkus <m.bunkus@linet-services.de>
Tue, 16 Aug 2016 09:39:41 +0000 (11:39 +0200)
committerMoritz Bunkus <m.bunkus@linet-services.de>
Tue, 16 Aug 2016 09:39:41 +0000 (11:39 +0200)
SL/Controller/BankTransaction.pm
SL/DB/Helper/Payment.pm
locale/de/all
templates/webpages/bank_transactions/_problems.html [new file with mode: 0644]
templates/webpages/bank_transactions/list.html

index aba5fdb..8f4bffd 100644 (file)
@@ -25,11 +25,13 @@ use SL::DB::Draft;
 use SL::DB::BankAccount;
 use SL::DBUtils qw(like);
 use SL::Presenter;
+
+use List::MoreUtils qw(any);
 use List::Util qw(max);
 
 use Rose::Object::MakeMethods::Generic
 (
'scalar --get_set_init' => [ qw(models) ],
 'scalar --get_set_init' => [ qw(models problems) ],
 );
 
 __PACKAGE__->run_before('check_auth');
@@ -394,53 +396,117 @@ sub action_save_invoices {
   #           '44' => [ '50', '51', 52' ]
   #         };
 
-  my $skonto_hash  = delete $::form->{invoice_skontos} || {}; # array containing the payment type, could be empty
+  $::form->{invoice_skontos} ||= {}; # hash of arrays containing the payment types, could be empty
 
   # a bank_transaction may be assigned to several invoices, i.e. a customer
   # might pay several open invoices with one transaction
 
-  while ( my ($bt_id, $invoice_ids) = each(%$invoice_hash) ) {
-    my $bank_transaction      = SL::DB::Manager::BankTransaction->find_by(id => $bt_id);
+  $self->problems([]);
+
+  while ( my ($bank_transaction_id, $invoice_ids) = each(%$invoice_hash) ) {
+    push @{ $self->problems }, $self->save_single_bank_transaction(
+      bank_transaction_id => $bank_transaction_id,
+      invoice_ids         => $invoice_ids,
+    );
+  }
+
+  $self->action_list();
+}
+
+sub save_single_bank_transaction {
+  my ($self, %params) = @_;
+
+  my %data = (
+    %params,
+    bank_transaction => SL::DB::Manager::BankTransaction->find_by(id => $params{bank_transaction_id}),
+    invoices         => [],
+  );
+
+  if (!$data{bank_transaction}) {
+    return {
+      %data,
+      result => 'error',
+      message => $::locale->text('The ID #1 is not a valid database ID.', $data{bank_transaction_id}),
+    };
+  }
+
+  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 $payment_received      = $bank_transaction->amount > 0;
+    my $payment_sent          = $bank_transaction->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);
+      if (!$invoice) {
+        return {
+          %data,
+          result  => 'error',
+          message => $::locale->text("The ID #1 is not a valid database ID.", $invoice_id),
+        };
+      }
 
-    my @invoices;
-    foreach my $invoice_id (@{ $invoice_ids }) {
-      push @invoices, (SL::DB::Manager::Invoice->find_by(id => $invoice_id) || SL::DB::Manager::PurchaseInvoice->find_by(id => $invoice_id));
+      push @{ $data{invoices} }, $invoice;
     }
-    @invoices = sort { return 1 if ( $a->is_sales and $a->amount > 0);
-                       return 1 if (!$a->is_sales and $a->amount < 0);
-                       return -1;
-                     } @invoices if $bank_transaction->amount > 0;
-    @invoices = sort { return -1 if ( $a->is_sales and $a->amount > 0);
-                       return -1 if (!$a->is_sales and $a->amount < 0);
-                       return 1;
-                     } @invoices if $bank_transaction->amount < 0;
-
-    my $max_invoices = scalar(@invoices);
+
+    if (   $payment_received
+        && any {    ( $_->is_sales && ($_->amount < 0))
+                 || (!$_->is_sales && ($_->amount > 0))
+               } @{ $data{invoices} }) {
+      return {
+        %data,
+        result  => 'error',
+        message => $::locale->text("Received payments can only be posted for sales invoices and purchase credit notes."),
+      };
+    }
+
+    if (   $payment_sent
+        && any {    ( $_->is_sales && ($_->amount > 0))
+                 || (!$_->is_sales && ($_->amount < 0))
+               } @{ $data{invoices} }) {
+      return {
+        %data,
+        result  => 'error',
+        message => $::locale->text("Sent payments can only be posted for purchase invoices and sales credit notes."),
+      };
+    }
+
+    my $max_invoices = scalar(@{ $data{invoices} });
     my $n_invoices   = 0;
 
-    foreach my $invoice (@invoices) {
+    foreach my $invoice (@{ $data{invoices} }) {
 
       $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
-      die t8("Bank transaction with id #1 has already been linked to #2.", $bank_transaction->id, $invoice->displayable_name)
-        if _existing_record_link($bank_transaction, $invoice);
+      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 {
+          %data,
+          result  => 'error',
+          message => $::locale->text("A payment can only be posted for multiple invoices if the amount to post is equal to or bigger than the sum of the open amounts of the affected invoices."),
+        };
+      }
 
       my $payment_type;
-      if ( defined $skonto_hash->{"$bt_id"} ) {
-        $payment_type = shift(@{ $skonto_hash->{"$bt_id"} });
+      if ( defined $::form->{invoice_skontos}->{"$bt_id"} ) {
+        $payment_type = shift(@{ $::form->{invoice_skontos}->{"$bt_id"} });
       } else {
         $payment_type = 'without_skonto';
       };
-      if ($amount_of_transaction == 0) {
-        flash('warning',  $::locale->text('There are invoices which could not be paid by bank transaction #1 (Account number: #2, bank code: #3)!',
-                                          $bank_transaction->purpose,
-                                          $bank_transaction->remote_account_number,
-                                          $bank_transaction->remote_bank_code));
-        last;
-      }
+
       # 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) {
         # first calculate new bank transaction amount ...
@@ -458,6 +524,7 @@ sub action_save_invoices {
                               payment_type => $payment_type,
                               transdate    => $bank_transaction->transdate->to_kivitendo);
       } else { # use the whole amount of the bank transaction for the invoice, overpay the invoice if necessary
+        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,
@@ -465,6 +532,12 @@ sub action_save_invoices {
                               transdate    => $bank_transaction->transdate->to_kivitendo);
         $bank_transaction->invoice_amount($bank_transaction->amount);
         $amount_of_transaction = 0;
+
+        push @warnings, {
+          %data,
+          result  => 'warning',
+          message => $::locale->text('Invoice #1 was overpaid by #2.', $invoice->invnumber, $::form->format_amount(\%::myconfig, $overpaid_amount, 2)),
+        };
       }
 
       # Record a record link from the bank transaction to the invoice
@@ -490,9 +563,29 @@ sub action_save_invoices {
 
     }
     $bank_transaction->save;
-  }
 
-  $self->action_list();
+    # 'undef' means 'no error' here.
+    return undef;
+  };
+
+  my $error;
+  my $rez = $data{bank_transaction}->db->with_transaction(sub {
+    eval {
+      $error = $worker->();
+      1;
+
+    } or do {
+      $error = {
+        %data,
+        result  => 'error',
+        message => $@,
+      };
+    };
+
+    die if $error;
+  });
+
+  return grep { $_ } ($error, @warnings);
 }
 
 sub action_save_proposals {
@@ -651,6 +744,7 @@ sub _existing_record_link {
   return @$linked_records ? 1 : 0;
 };
 
+sub init_problems { [] }
 
 sub init_models {
   my ($self) = @_;
@@ -681,3 +775,68 @@ sub init_models {
 }
 
 1;
+__END__
+
+=pod
+
+=encoding utf8
+
+=head1 NAME
+
+SL::Controller::BankTransaction - Posting payments to invoices from
+bank transactions imported earlier
+
+=head1 FUNCTIONS
+
+=over 4
+
+=item C<save_single_bank_transaction %params>
+
+Takes a bank transaction ID (as parameter C<bank_transaction_id> and
+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).
+
+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
+transaction that's already been posted earlier. In both cases the
+database transaction will be rolled back.
+
+If warnings but not errors occur the database transaction is still
+committed.
+
+The return value is an error object or C<undef> if the function
+succeeded. The calling function will collect all warnings and errors
+and display them in a nicely formatted table if any occurred.
+
+An error object is a hash reference containing the following members:
+
+=over 2
+
+=item * C<result> — can be either C<warning> or C<error>. Warnings are
+displayed slightly different than errors.
+
+=item * C<message> — a human-readable message included in the list of
+errors meant as the description of why the problem happened
+
+=item * C<bank_transaction_id>, C<invoice_ids> — the same parameters
+that the function was called with
+
+=item * C<bank_transaction> — the database object
+(C<SL::DB::BankTransaction>) corresponding to C<bank_transaction_id>
+
+=item * C<invoices> — an array ref of the database objects (either
+C<SL::DB::Invoice> or C<SL::DB::PurchaseInvoice>) corresponding to
+C<invoice_ids>
+
+=back
+
+=back
+
+=head1 AUTHOR
+
+Niclas Zimmermann E<lt>niclas@kivitendo-premium.deE<gt>,
+Geoffrey Richardson E<lt>information@richardson-bueren.deE<gt>
+
+=cut
index 5b3a138..b7bf38a 100644 (file)
@@ -119,7 +119,7 @@ sub pay_invoice {
   my $fx_gain_loss_amount = 0; # for fx_gain and fx_loss
 
   my $db = $self->db;
-  $db->do_transaction(sub {
+  $db->with_transaction(sub {
     my $new_acc_trans;
 
     # all three payment type create 1 AR/AP booking (the paid part)
@@ -278,36 +278,38 @@ sub pay_invoice {
     # than adding them to the transaction relation array.
     $self->forget_related('transactions');
 
-  my $datev_check = 0;
-  if ( $is_sales )  {
-    if ( (  $self->invoice && $::instance_conf->get_datev_check_on_sales_invoice  ) ||
-         ( !$self->invoice && $::instance_conf->get_datev_check_on_ar_transaction )) {
-      $datev_check = 1;
-    };
-  } else {
-    if ( (  $self->invoice && $::instance_conf->get_datev_check_on_purchase_invoice ) ||
-         ( !$self->invoice && $::instance_conf->get_datev_check_on_ap_transaction   )) {
-      $datev_check = 1;
-    };
-  };
+    my $datev_check = 0;
+    if ( $is_sales )  {
+      if ( (  $self->invoice && $::instance_conf->get_datev_check_on_sales_invoice  ) ||
+           ( !$self->invoice && $::instance_conf->get_datev_check_on_ar_transaction )) {
+        $datev_check = 1;
+      }
+    } else {
+      if ( (  $self->invoice && $::instance_conf->get_datev_check_on_purchase_invoice ) ||
+           ( !$self->invoice && $::instance_conf->get_datev_check_on_ap_transaction   )) {
+        $datev_check = 1;
+      }
+    }
 
-  if ( $datev_check ) {
+    if ( $datev_check ) {
 
-    my $datev = SL::DATEV->new(
-      exporttype => DATEV_ET_BUCHUNGEN,
-      format     => DATEV_FORMAT_KNE,
-      dbh        => $db->dbh,
-      trans_id   => $self->{id},
-    );
+      my $datev = SL::DATEV->new(
+        exporttype => DATEV_ET_BUCHUNGEN,
+        format     => DATEV_FORMAT_KNE,
+        dbh        => $db->dbh,
+        trans_id   => $self->{id},
+      );
 
-    $datev->clean_temporary_directories;
-    $datev->export;
+      $datev->clean_temporary_directories;
+      $datev->export;
 
-    if ($datev->errors) {
-      # this exception should be caught by do_transaction, which handles the rollback
-      die join "\n", $::locale->text('DATEV check returned errors:'), $datev->errors;
+      if ($datev->errors) {
+        # this exception should be caught by with_transaction, which handles the rollback
+        die join "\n", $::locale->text('DATEV check returned errors:'), $datev->errors;
+      }
     }
-  };
+
+    1;
 
   }) || die t8('error while paying invoice #1 : ', $self->invnumber) . $db->error . "\n";
 
index ccf3340..9c09cb8 100755 (executable)
@@ -54,6 +54,7 @@ $self->{texts} = {
   'A directory with the name for the new print templates exists already.' => 'Ein Verzeichnis mit dem selben Namen wie die neuen Druckvorlagen existiert bereits.',
   'A lot of the usability of kivitendo has been enhanced with javascript. Although it is currently possible to use every aspect of kivitendo without javascript, we strongly recommend it. In a future version this may change and javascript may be necessary to access advanced features.' => 'Die Bedienung von kivitendo wurde an vielen Stellen mit Javascript verbessert. Obwohl es derzeit möglich ist, jeden Aspekt von kivitendo auch ohne Javascript zu benutzen, empfehlen wir es. In einer zukünftigen Version wird Javascript eventuell notwendig sein um weitergehende Features zu benutzen.',
   'A lower-case character is required.' => 'Ein Kleinbuchstabe ist vorgeschrieben.',
+  'A payment can only be posted for multiple invoices if the amount to post is equal to or bigger than the sum of the open amounts of the affected invoices.' => 'Eine Zahlung kann nur dann für mehrere Rechnungen verbucht werden, wenn die Zahlung gleich oder größer als die Summe der offenen Beträge der betroffenen Rechnungen ist.',
   'A special character is required (valid characters: #1).' => 'Ein Sonderzeichen ist vorgeschrieben (gültige Zeichen: #1).',
   'A transaction description is required.' => 'Die Vorgangsbezeichnung muss eingegeben werden.',
   'A unit with this name does already exist.' => 'Eine Einheit mit diesem Namen existiert bereits.',
@@ -369,6 +370,8 @@ $self->{texts} = {
   'Bank transaction with id #1 has already been linked to #2.' => 'Bankbuchung mit id #1 wurde schon mit #2 verlinkt.',
   'Bank transactions'           => 'Bankbewegungen',
   'Bank transactions MT940'     => 'Kontoauszug verbuchen',
+  'Bank transactions that either only have warnings or no message at all have been posted.' => 'Banktransaktionen, die entweder nur Warnungen oder gar keine Nachricht haben, wurden hingegen verbucht.',
+  'Bank transactions with errors have not been posted.' => 'Banktransaktionen mit Fehlern wurden nicht verbucht.',
   'Bank transfer amount'        => 'Überweisungssumme',
   'Bank transfer payment list for export #1' => 'Überweisungszahlungsliste für SEPA-Export #1',
   'Bank transfer via SEPA'      => 'Überweisung via SEPA',
@@ -1500,6 +1503,7 @@ $self->{texts} = {
   'Invnumber'                   => 'Rechnungsnummer',
   'Invnumber missing!'          => 'Rechnungsnummer fehlt!',
   'Invoice'                     => 'Rechnung',
+  'Invoice #1 was overpaid by #2.' => 'Rechnung #1 wurde um #2 überzahlt.',
   'Invoice (one letter abbreviation)' => 'R',
   'Invoice Date'                => 'Rechnungsdatum',
   'Invoice Date missing!'       => 'Rechnungsdatum fehlt!',
@@ -2238,6 +2242,7 @@ $self->{texts} = {
   'Receivables'                 => 'Forderungen',
   'Receivables account'         => 'Forderungskonto',
   'Receivables account (account number)' => 'Forderungskonto (Kontonummer)',
+  'Received payments can only be posted for sales invoices and purchase credit notes.' => 'Erhaltene Zahlungen können nur mit Verkaufsrechnungen und Einkaufsgutschriften verbucht werden.',
   'Recipients'                  => 'Empfänger',
   'Reconcile'                   => 'Abgleichen',
   'Reconciliation'              => 'Kontenabgleich',
@@ -2473,6 +2478,7 @@ $self->{texts} = {
   'Sending E-mail: '            => 'E-Mail versenden: ',
   'Sent emails can be optionally stored in the database with or without their attachments.' => 'Gesendete E-Mails können optional mit oder ohne ihre Anhänge in der Datenbank gespeichert werden.',
   'Sent on'                     => 'Verschickt am',
+  'Sent payments can only be posted for purchase invoices and sales credit notes.' => 'Gesendete Zahlungen können nur mit Einkaufsrechnungen und Verkaufsgutschriften verbucht werden.',
   'Sep'                         => 'Sep',
   'Separator'                   => 'Trennzeichen',
   'Separator chararacter'       => 'Feldtrennzeichen',
@@ -2730,6 +2736,7 @@ $self->{texts} = {
   'The GL transaction #1 has been deleted.' => 'Die Dialogbuchung #1 wurde gelöscht.',
   'The IBAN \'#1\' is not valid as IBANs in #2 must be exactly #3 characters long.' => 'Die IBAN \'#1\' ist ungültig, da IBANs in #2 genau #3 Zeichen lang sein müssen.',
   'The IBAN is missing.'        => 'Die IBAN fehlt.',
+  'The ID #1 is not a valid database ID.' => 'Die ID #1 ist keine gültige Datenbank-ID.',
   'The LDAP server "#1:#2" is unreachable. Please check config/kivitendo.conf.' => 'Der LDAP-Server "#1:#2" ist nicht erreichbar. Bitte &uuml;berpr&uuml;fen Sie die Angaben in config/kivitendo.conf.',
   'The MT940 import needs an import profile called MT940' => 'Der MT940 Import benötigt ein Importprofil mit dem Namen "MT940"',
   'The PDF has been created'    => 'Die PDF-Datei wurde erstellt.',
@@ -3032,7 +3039,6 @@ $self->{texts} = {
   'There are entries in tax where taxkey is NULL.' => 'In der Datenbank sind Steuern ohne Steuerschlüssel vorhanden (in der Tabelle tax Spalte taxkey).',
   'There are invalid taxnumbers in use.' => 'Es werden ungültige Steuerautomatik-Konten benutzt.',
   'There are invalid transactions in your database.' => 'Sie haben ungültige Buchungen in Ihrer Datenbank.',
-  'There are invoices which could not be paid by bank transaction #1 (Account number: #2, bank code: #3)!' => 'Einige Rechnungen konnten nicht durch die Bankbewegung #1 (Kontonummer: #2, Bankleitzahl: #3) bezahlt werden!',
   'There are no documents in the WebDAV directory at the moment.' => 'Es befinden sich im WebDAV-Verzeichnis momentan keine Dokumente.',
   'There are no entries in the background job history.' => 'Es gibt keine Einträge im Hintergrund-Job-Verlauf.',
   'There are no entries that match the filter.' => 'Es gibt keine Einträge, auf die der Filter zutrifft.',
@@ -3327,6 +3333,7 @@ $self->{texts} = {
   'Warn before saving orders with duplicate parts (new controller only)' => 'Beim Speichern warnen, wenn doppelte Artikel in einem Auftrag sind',
   'Warning'                     => 'Warnung',
   'Warning! Loading a draft will discard unsaved data!' => 'Achtung! Beim Laden eines Entwurfs werden ungespeicherte Daten verworfen!',
+  'Warnings and errors'         => 'Warnungen und Fehler',
   'WebDAV'                      => 'WebDAV',
   'WebDAV link'                 => 'WebDAV-Link',
   'WebDAV save documents'       => 'Belege in WebDAV-Ablage speichern',
diff --git a/templates/webpages/bank_transactions/_problems.html b/templates/webpages/bank_transactions/_problems.html
new file mode 100644 (file)
index 0000000..9a10fca
--- /dev/null
@@ -0,0 +1,46 @@
+[%- USE LxERP -%][%- USE T8 -%][%- USE HTML -%][%- USE P -%]
+<h3>
+ [% LxERP.t8("Warnings and errors") %]
+</h3>
+
+<p>
+ [% LxERP.t8("Bank transactions with errors have not been posted.") %]
+ [% LxERP.t8("Bank transactions that either only have warnings or no message at all have been posted.") %]
+</p>
+
+<table>
+ <thead>
+  <tr class="listheading">
+   <th>[% LxERP.t8("Type") %]</th>
+   <th>[% LxERP.t8("Invoices") %]</th>
+   <th>[% LxERP.t8("Transdate") %]</th>
+   <th>[% LxERP.t8("Amount") %]</th>
+   <th>[% LxERP.t8("Remote name") %]</th>
+   <th>[% LxERP.t8("Purpose") %]</th>
+   <th>[% LxERP.t8("Remote account number") %]</th>
+   <th>[% LxERP.t8("Remote bank code") %]</th>
+   <th>[% LxERP.t8("Message") %]</th>
+  </tr>
+ </thead>
+
+ <tbody>
+  [% FOREACH problem = SELF.problems %]
+   <tr class="listrow[% IF problem.result == 'error' %]_error[% END %]">
+    <td>[% IF problem.result == 'error' %][% LxERP.t8("Error") %][% ELSE %][% LxERP.t8("Warning") %][% END %]</td>
+    <td>
+     [% FOREACH invoice = problem.invoices %]
+      [% P.invoice(invoice) %]
+      [% UNLESS loop.last %]<br>[% END %]
+     [% END %]
+    </td>
+    <td>[% HTML.escape(problem.bank_transaction.transdate.to_kivitendo) %]</td>
+    <td>[% HTML.escape(LxERP.format_amount(problem.bank_transaction.amount, 2)) %]</td>
+    <td>[% HTML.escape(problem.bank_transaction.remote_name) %]</td>
+    <td>[% HTML.escape(problem.bank_transaction.purpose) %]</td>
+    <td>[% HTML.escape(problem.bank_transaction.remote_account_number) %]</td>
+    <td>[% HTML.escape(problem.bank_transaction.remote_bank_code) %]</td>
+    <td>[% HTML.escape(problem.message) %]</td>
+   </tr>
+  [% END %]
+ </tbody>
+</table>
index af6ef22..c810c8f 100644 (file)
@@ -4,6 +4,10 @@
 
 [%- INCLUDE 'common/flash.html' %]
 
+[% IF SELF.problems.size %]
+ [% INCLUDE 'bank_transactions/_problems.html' %]
+[% END %]
+
 <p>[% HTML.escape(bank_account.name) %] [% HTML.escape(bank_account.iban) %], [% 'Bank code' | $T8 %] [% HTML.escape(bank_account.bank_code) %], [% 'Bank' | $T8 %] [% HTML.escape(bank_account.bank) %]</p>
 <p>
 [% IF FORM.filter.fromdate %] [% 'From' | $T8 %] [% FORM.filter.fromdate %] [% END %]