diff --git a/modules/account/doc/configuration.rst b/modules/account/doc/configuration.rst deleted file mode 100644 index 21e1cc12ff..0000000000 --- a/modules/account/doc/configuration.rst +++ /dev/null @@ -1,30 +0,0 @@ -************* -Configuration -************* - -The *Account Module* uses values from settings in the ``[account]`` section -of the :ref:`trytond:topics-configuration`. - -.. _config-account.reconciliation_chunk: - -``reconciliation_chunk`` -======================== - -The ``reconciliation_chunk`` defines the size of each block of sequential -`Account Move Lines ` that are searched for -`Reconciliation ` proposals. -Larger chunk sizes allow more lines to be considered together, and sometimes -better matches to be found. - -.. warning:: - - The number of combinations of lines considered, and consequently the search - time, increases exponentially along with the chunk size. - So you should keep this setting to a relatively low value. - -.. seealso:: - - The `Reconcile Accounts ` wizard for details - of how this setting is used. - -The default value is: ``10`` diff --git a/modules/account/doc/design/move.inc.rst b/modules/account/doc/design/move.inc.rst index e0887b2c8a..cf520568ff 100644 --- a/modules/account/doc/design/move.inc.rst +++ b/modules/account/doc/design/move.inc.rst @@ -166,11 +166,6 @@ The wizard goes through each account and `Party ` in turn, and attempts to find the best possible reconciliations from the unreconciled account move lines that should be considered. -It does this by splitting up the account move lines into sequential -`chunks `. -It then tries all possible combinations of the account move lines within each -chunk, in an attempt to find the best set of balanced lines. - .. seealso:: The reconcile accounts wizard can be started from the main menu item: diff --git a/modules/account/doc/index.rst b/modules/account/doc/index.rst index 94d3c623a0..9be51dc67b 100644 --- a/modules/account/doc/index.rst +++ b/modules/account/doc/index.rst @@ -12,7 +12,6 @@ income statement. setup usage/index - configuration design/index reference releases diff --git a/modules/account/move.py b/modules/account/move.py index cff30196b0..0be64f670b 100644 --- a/modules/account/move.py +++ b/modules/account/move.py @@ -2,7 +2,7 @@ # this repository contains the full copyright notices and license terms. from collections import defaultdict from decimal import Decimal -from itertools import chain, combinations, groupby, islice +from itertools import chain, groupby, islice from dateutil.relativedelta import relativedelta from sql import Literal, Null, Window @@ -11,7 +11,6 @@ from sql.conditionals import Case, Coalesce from sql.functions import Abs, CharLength, Round from sql.operators import Exists -from trytond.config import config from trytond.i18n import gettext from trytond.model import ( Check, DeactivableMixin, Index, ModelSQL, ModelView, dualmethod, fields) @@ -1806,6 +1805,54 @@ class Line(DescriptionOriginMixin, MoveLineMixin, ModelSQL, ModelView): move.lines = lines return move + @classmethod + def find_best_reconciliation(cls, lines, currency, amount=0): + """Return the list of lines to reconcile for the amount + with the smallest remaining. + + For performance reason it is an approximation that searches for the + smallest remaining by removing the last line that decrease it.""" + assert len({l.account for l in lines}) <= 1 + + def get_balance(line): + if line.second_currency == currency: + return line.amount_second_currency + elif line.currency == currency: + return line.debit - line.credit + else: + return 0 + + lines = sorted(lines, key=cls._reconciliation_sort_key) + debit, credit = [], [] + remaining = -amount + for line in list(lines): + balance = get_balance(line) + if balance > 0: + debit.append(line) + elif balance < 0: + credit.append(line) + else: + lines.remove(line) + remaining += balance + + best_lines, best_remaining = list(lines), remaining + if remaining: + while lines: + try: + line = (debit if remaining > 0 else credit).pop() + except IndexError: + break + lines.remove(line) + remaining -= get_balance(line) + if lines and abs(remaining) < abs(best_remaining): + best_lines, best_remaining = list(lines), remaining + if not remaining: + break + return best_lines, best_remaining + + def _reconciliation_sort_key(self): + return self.maturity_date or self.date + class LineReceivablePayableContext(ModelView): "Receivable/Payable Line Context" @@ -2357,6 +2404,9 @@ class Reconcile(Wizard): def _default_lines(self): 'Return the larger list of lines which can be reconciled' + pool = Pool() + Line = pool.get('account.move.line') + if self.model and self.model.__name__ == 'account.move.line': requested = { l for l in self.records @@ -2364,45 +2414,15 @@ class Reconcile(Wizard): and l.party == self.show.party and (l.second_currency or l.currency) == self.show.currency} else: - requested = None + requested = [] currency = self.show.currency - def balance(line): - if line.second_currency == currency: - return line.amount_second_currency - elif line.currency == currency: - return line.debit - line.credit - else: - return 0 - - all_lines = self._all_lines() - amount = sum(map(balance, all_lines)) - if currency.is_zero(amount): - return all_lines - - chunk = config.getint('account', 'reconciliation_chunk', default=10) - # Combination is exponential so it must be limited to small number - default = [] - for lines in grouped_slice( - sorted(all_lines, key=self._line_sort_key), chunk): - lines = list(lines) - best = None - for n in range(len(lines), 1, -1): - for comb_lines in combinations(lines, n): - if requested and not requested.intersection(comb_lines): - continue - amount = sum(balance(l) for l in comb_lines) - if currency.is_zero(amount): - best = comb_lines - break - if best: - break - if best: - default.extend(best) - if not default and requested: + lines, remaining = Line.find_best_reconciliation( + self._all_lines(), currency) + if remaining: return requested else: - return default + return lines def transition_reconcile(self): pool = Pool() diff --git a/modules/account/tests/test_module.py b/modules/account/tests/test_module.py index 0373751c41..fefeda546f 100644 --- a/modules/account/tests/test_module.py +++ b/modules/account/tests/test_module.py @@ -184,6 +184,7 @@ class AccountTestCase( PartyCompanyCheckEraseMixin, CompanyTestMixin, ModuleTestCase): 'Test Account module' module = 'account' + maxDiff = None @with_transaction() def test_account_chart(self): @@ -770,6 +771,71 @@ class AccountTestCase( ] move.save() + @with_transaction() + def test_find_best_reconciliation(self): + "Test find best reconciliation" + pool = Pool() + Line = pool.get('account.move.line') + + currency = create_currency('USD') + + line1 = Line( + debit=Decimal(10), credit=0, currency=currency, + second_currency=None, account=None, + maturity_date=datetime.date(2024, 1, 1)) + line2 = Line( + debit=0, credit=Decimal(10), currency=currency, + second_currency=None, account=None, + maturity_date=datetime.date(2024, 1, 2)) + line3 = Line( + debit=0, credit=Decimal(5), currency=currency, + second_currency=None, account=None, + maturity_date=datetime.date(2024, 1, 3)) + + for (lines, amount, best, remaining) in [ + ([line1], 0, [line1], Decimal('10')), + ([line1], Decimal(10), [line1], Decimal(0)), + ([line1], Decimal(3), [line1], Decimal(7)), + ([line1, line2], 0, [line1, line2], Decimal(0)), + ([line1, line2], Decimal(10), [line1], Decimal(0)), + ([line1, line2], Decimal(-10), [line2], Decimal(0)), + ([line1, line2], Decimal(20), [line1], Decimal(-10)), + ([line1, line2], Decimal(-3), [line1, line2], Decimal(3)), + ([line1, line2], Decimal(-9), [line2], Decimal(-1)), + ([line1, line3], 0, [line1, line3], Decimal(5)), + ([line1, line2, line3], 0, [line1, line2], Decimal(0)), + ]: + with self.subTest(lines=lines, amount=amount): + self.assertEqual( + Line.find_best_reconciliation(lines, currency, amount), + (best, remaining)) + + @with_transaction() + def test_find_best_reconciliation_currency(self): + "Test find best reconciliation with different currencies" + pool = Pool() + Line = pool.get('account.move.line') + + currency1 = create_currency('USD') + currency2 = create_currency('EUR') + + line1 = Line( + debit=Decimal(10), credit=0, currency=currency1, + second_currency=None, account=None, + maturity_date=datetime.date(2024, 1, 1)) + line2 = Line( + debit=0, credit=Decimal(20), currency=currency2, + second_currency=currency1, amount_second_currency=Decimal('-10'), + account=None, maturity_date=datetime.date(2024, 1, 1)) + line3 = Line( + debit=0, credit=Decimal(5), currency=None, + second_currency=None, account=None, + maturity_date=datetime.date(2024, 1, 3)) + + self.assertEqual( + Line.find_best_reconciliation([line1, line2, line3], currency1), + ([line1, line2], Decimal(0))) + @with_transaction() def test_tax_compute(self): 'Test tax compute/reverse_compute' diff --git a/modules/account_invoice/invoice.py b/modules/account_invoice/invoice.py index fc03a3533c..6639beccc4 100644 --- a/modules/account_invoice/invoice.py +++ b/modules/account_invoice/invoice.py @@ -1,9 +1,9 @@ # This file is part of Tryton. The COPYRIGHT file at the top level of # this repository contains the full copyright notices and license terms. import datetime as dt -from collections import defaultdict, namedtuple +from collections import defaultdict from decimal import Decimal -from itertools import chain, combinations, groupby +from itertools import chain, groupby from genshi.template.text import TextTemplate from sql import Null @@ -1541,38 +1541,20 @@ class Invoice(Workflow, ModelSQL, ModelView, TaxableMixin, InvoiceReportMixin): ''' Return list of lines and the remainder to make reconciliation. ''' - Result = namedtuple('Result', ['lines', 'remainder']) + pool = Pool() + Line = pool.get('account.move.line') + + assert currency in {self.currency, self.company.currency} if party is None: party = self.party - - assert currency in [self.currency, self.company.currency] - - def balance(line): - if currency == line.second_currency: - return line.amount_second_currency - elif currency == self.company.currency: - return line.debit - line.credit - else: - return 0 - lines = [ l for l in self.payment_lines + self.lines_to_pay if not l.reconciliation and (not self.account.party_required or l.party == party)] - remainder = sum(map(balance, lines)) - amount - best = Result(lines, remainder) - if remainder: - for n in range(len(lines) - 1, 0, -1): - for comb_lines in combinations(lines, n): - remainder = sum(map(balance, comb_lines)) - amount - result = Result(list(comb_lines), remainder) - if currency.is_zero(remainder): - return result - if abs(remainder) < abs(best.remainder): - best = result - return best + return Line.find_best_reconciliation( + lines, currency, amount=amount) def pay_invoice( self, amount, payment_method, date, description=None,