From 2df6107cd752c05903aee87ca03e38d0fc59bf1f Mon Sep 17 00:00:00 2001 From: oco-odoo Date: Mon, 27 Sep 2021 13:14:22 +0000 Subject: [PATCH] [IMP] account: reconciliation models: don't suggest too many matches in case of partial mathing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Create a statement line for partner A, amounting to 90€ - Make 5 invoices for partner A, of 10, 50, 100, 500 and 100 € - create a reconciliation model (make sure it's the only one active for testing), with >>> "invoice matching" selected >>> "payment tolerance" disabled >>> "partner should be set" enabled >>> "same currency" enabled >>> "auto-validate" disabled Try to reconcile your statement. The reconciliation model associates your statement line to the 5 invoices, showing a partial match of 30 for the line of 100€, as only 30€ remain after matching 10 and 50. The following lines (500 and 100) are useless in the reconciliation and confusing for the user. They shouldn't be there. After this commit, no useless line will be proposed anymore. In our example, only lines 10, 50 and 100 will be proposed. Task 2652915 Part-of: odoo/odoo#77801 --- .../account/models/account_reconcile_model.py | 33 ++++++++++++++----- .../test_reconciliation_matching_rules.py | 32 +++++++++++++++++- 2 files changed, 56 insertions(+), 9 deletions(-) diff --git a/addons/account/models/account_reconcile_model.py b/addons/account/models/account_reconcile_model.py index 2198f4987cb..10d9341e3e9 100644 --- a/addons/account/models/account_reconcile_model.py +++ b/addons/account/models/account_reconcile_model.py @@ -802,26 +802,43 @@ class AccountReconcileModel(models.Model): new_treated_aml_ids = set() candidates, priorities = self._filter_candidates(candidates, aml_ids_to_exclude, reconciled_amls_ids) - # Special case: the amounts are the same, submit the line directly. st_line_currency = st_line.foreign_currency_id or st_line.currency_id candidate_currencies = set(candidate['aml_currency_id'] or st_line.company_id.currency_id.id for candidate in candidates) + kept_candidates = candidates if candidate_currencies == {st_line_currency.id}: + kept_candidates = [] + sum_kept_candidates = 0 for candidate in candidates: - residual_amount = candidate['aml_currency_id'] and candidate['aml_amount_residual_currency'] or candidate['aml_amount_residual'] - if st_line_currency.is_zero(residual_amount + st_line.amount_residual): - candidates, priorities = self._filter_candidates([candidate], aml_ids_to_exclude, reconciled_amls_ids) + candidate_residual = candidate['aml_amount_residual_currency'] if candidate['aml_currency_id'] else candidate['aml_amount_residual'] + + if st_line_currency.compare_amounts(candidate_residual, -st_line.amount_residual) == 0: + # Special case: the amounts are the same, submit the line directly. + kept_candidates = [candidate] break + elif st_line_currency.compare_amounts(abs(sum_kept_candidates), abs(st_line.amount_residual)) < 0: + # Candidates' and statement line's balances have the same sign, thanks to _get_invoice_matching_query. + # We hence can compare their absolute value without any issue. + # Here, we still have room for other candidates ; so we add the current one to the list we keep. + # Then, we continue iterating, even if there is no room anymore, just in case one of the following candidates + # is an exact match, which would then be preferred on the current candidates. + kept_candidates.append(candidate) + sum_kept_candidates += candidate_residual + + # It is possible kept_candidates now contain less different priorities; update them + kept_candidates_by_priority = self._sort_reconciliation_candidates_by_priority(kept_candidates, aml_ids_to_exclude, reconciled_amls_ids) + priorities = set(kept_candidates_by_priority.keys()) + # We check the amount criteria of the reconciliation model, and select the - # candidates if they pass the verification. - matched_candidates_values = self._process_matched_candidates_data(st_line, candidates) + # kept_candidates if they pass the verification. + matched_candidates_values = self._process_matched_candidates_data(st_line, kept_candidates) status = self._check_rule_propositions(matched_candidates_values) if 'rejected' in status: rslt = None else: rslt = { 'model': self, - 'aml_ids': [candidate['aml_id'] for candidate in candidates], + 'aml_ids': [candidate['aml_id'] for candidate in kept_candidates], } new_treated_aml_ids = set(rslt['aml_ids']) @@ -843,7 +860,7 @@ class AccountReconcileModel(models.Model): if 'allow_auto_reconcile' in status: # Process auto-reconciliation. We only do that for the first two priorities, if they are not matched elsewhere. - aml_ids = [candidate['aml_id'] for candidate in candidates] + aml_ids = [candidate['aml_id'] for candidate in kept_candidates] lines_vals_list = [{'id': aml_id} for aml_id in aml_ids] if lines_vals_list and priorities & {1, 3} and self.auto_reconcile: diff --git a/addons/account/tests/test_reconciliation_matching_rules.py b/addons/account/tests/test_reconciliation_matching_rules.py index 217aea2ac9a..64b4a0d2697 100644 --- a/addons/account/tests/test_reconciliation_matching_rules.py +++ b/addons/account/tests/test_reconciliation_matching_rules.py @@ -893,7 +893,7 @@ class TestReconciliationMatchingRules(AccountTestInvoicingCommon): }) self._check_statement_matching(self.rule_1, { self.bank_line_1.id: {'aml_ids': [payment_bnk_line.id], 'model': self.rule_1, 'partner': self.bank_line_1.partner_id}, - self.bank_line_2.id: {'aml_ids': []}, + self.bank_line_2.id: {'aml_ids': [self.invoice_line_1.id, self.invoice_line_2.id, self.invoice_line_3.id], 'model': self.rule_1, 'partner': self.bank_line_2.partner_id}, }, statements=self.bank_st) def test_match_different_currencies(self): @@ -1238,3 +1238,33 @@ class TestReconciliationMatchingRules(AccountTestInvoicingCommon): self._check_statement_matching(self.rule_1, { self.bank_line_1.id: {'aml_ids': (pmt_line_1 + pmt_line_2).ids, 'model': self.rule_1, 'partner': payment_partner}, }, statements=self.bank_line_1.statement_id) + + def test_no_amount_check_keep_first(self): + """ In case the reconciliation model doesn't check the total amount of the candidates, + we still don't want to suggest more than are necessary to match the statement. + For example, if a statement line amounts to 250 and is to be matched with three invoices + of 100, 200 and 300 (retrieved in this order), only 100 and 200 should be proposed. + """ + self.rule_1.allow_payment_tolerance = False + self.bank_line_2.amount = 250 + self.bank_line_1.partner_id = None + + self._check_statement_matching(self.rule_1, { + self.bank_line_1.id: {'aml_ids': []}, + self.bank_line_2.id: {'aml_ids': [self.invoice_line_1.id, self.invoice_line_2.id], 'model': self.rule_1, 'partner': self.bank_line_2.partner_id}, + }, statements=self.bank_st) + + def test_no_amount_check_exact_match(self): + """ If a reconciliation model finds enough candidates for a full reconciliation, + it should still check the following candidates, in case one of them exactly + matches the amount of the statement line. If such a candidate exist, all the + other ones are disregarded. + """ + self.rule_1.allow_payment_tolerance = False + self.bank_line_2.amount = 300 + self.bank_line_1.partner_id = None + + self._check_statement_matching(self.rule_1, { + self.bank_line_1.id: {'aml_ids': []}, + self.bank_line_2.id: {'aml_ids': [self.invoice_line_3.id], 'model': self.rule_1, 'partner': self.bank_line_2.partner_id}, + }, statements=self.bank_st)