From 6bd3ab150479cc8d174f0abd698aaaa7bb8788c8 Mon Sep 17 00:00:00 2001 From: Laurent Smet Date: Mon, 6 Feb 2023 09:15:27 +0000 Subject: [PATCH] [FIX] account: revert commit f607572da64dd5f8b2ef8be3750696cbdbf2c6f7 This case of amls having a zero balance but an amount in foreign currency left is already managed by the new reconciliation in 16.0. Furthermore: -the test in POS is passing even with the revert -the reverted commit introduces a performance issue on amount_residual by forcing the orm to fetch the whole m2m debit/credit_matched_ids. -there is no test accounting-side depicting the issue (if there is really an issue fixed by the commit). X-original-commit: 63c9e9cfff8451af153cbf2f369021a3e6b20599 Part-of: odoo/odoo#112251 --- addons/account/models/account_move_line.py | 27 +++++--- .../tests/test_account_move_reconcile.py | 66 ++----------------- .../tests/test_pos_basic_config.py | 2 +- 3 files changed, 23 insertions(+), 72 deletions(-) diff --git a/addons/account/models/account_move_line.py b/addons/account/models/account_move_line.py index fbb9642f3b4..e70750e170f 100644 --- a/addons/account/models/account_move_line.py +++ b/addons/account/models/account_move_line.py @@ -746,7 +746,7 @@ class AccountMoveLine(models.Model): line.reconciled = ( comp_curr.is_zero(line.amount_residual) and foreign_curr.is_zero(line.amount_residual_currency) - and (line.matched_debit_ids or line.matched_credit_ids) + and line.move_id.state not in ('draft', 'cancel') ) @api.depends('move_id.move_type', 'tax_ids', 'tax_repartition_line_id', 'debit', 'credit', 'tax_tag_ids', 'is_refund') @@ -1713,6 +1713,17 @@ class AccountMoveLine(models.Model): recon_debit_amount = remaining_debit_amount recon_credit_amount = -remaining_credit_amount + # Check if there is something left to reconcile. Move to the next loop iteration if not. + skip_reconciliation = False + if recon_currency.is_zero(recon_debit_amount): + res['debit_vals'] = None + skip_reconciliation = True + if recon_currency.is_zero(recon_credit_amount): + res['credit_vals'] = None + skip_reconciliation = True + if skip_reconciliation: + return res + # ==== Match both lines together and compute amounts to reconcile ==== # Determine which line is fully matched by the other. @@ -1859,9 +1870,9 @@ class AccountMoveLine(models.Model): credit_vals['amount_residual'] = remaining_credit_amount credit_vals['amount_residual_currency'] = remaining_credit_amount_curr - if recon_currency.is_zero(recon_debit_amount) or debit_fully_matched: + if debit_fully_matched: res['debit_vals'] = None - if recon_currency.is_zero(recon_credit_amount) or credit_fully_matched: + if credit_fully_matched: res['credit_vals'] = None return res @@ -1871,9 +1882,8 @@ class AccountMoveLine(models.Model): Note: The order of records in self is important because the journal items will be reconciled using this order. :return: a tuple of 1) list of vals for partial reconciliation creation, 2) the list of vals for the exchange difference entries to be created ''' - debit_vals_list = iter([x for x in vals_list if x['balance'] > 0.0 or x['amount_currency'] > 0.0 and not x['reconciled']]) - credit_vals_list = iter([x for x in vals_list if x['balance'] < 0.0 or x['amount_currency'] < 0.0 and not x['reconciled']]) - void_vals_list = iter([x for x in vals_list if not x['balance'] and not x['amount_currency'] and not x['reconciled']]) + debit_vals_list = iter([x for x in vals_list if x['balance'] > 0.0 or x['amount_currency'] > 0.0]) + credit_vals_list = iter([x for x in vals_list if x['balance'] < 0.0 or x['amount_currency'] < 0.0]) debit_vals = None credit_vals = None @@ -1889,13 +1899,13 @@ class AccountMoveLine(models.Model): # Move to the next available debit line. if not debit_vals: - debit_vals = next(debit_vals_list, None) or next(void_vals_list, None) + debit_vals = next(debit_vals_list, None) if not debit_vals: break # Move to the next available credit line. if not credit_vals: - credit_vals = next(void_vals_list, None) or next(credit_vals_list, None) + credit_vals = next(credit_vals_list, None) if not credit_vals: break @@ -1927,7 +1937,6 @@ class AccountMoveLine(models.Model): 'company': line.company_id, 'currency': line.currency_id, 'date': line.date, - 'reconciled': line.reconciled, } for line in self ]) diff --git a/addons/account/tests/test_account_move_reconcile.py b/addons/account/tests/test_account_move_reconcile.py index 4403042f380..8b980da3a7f 100644 --- a/addons/account/tests/test_account_move_reconcile.py +++ b/addons/account/tests/test_account_move_reconcile.py @@ -349,7 +349,9 @@ class TestAccountMoveReconcile(AccountTestInvoicingCommon): self.assertEqual(line_2.full_reconcile_id, line_3.full_reconcile_id) def test_reconcile_lines_corner_case_1_zero_balance_different_foreign_currency(self): - """ Test the reconciliation of lines having a zero balance in different currencies. In that case, we enforce full reconciliation. """ + """ Test the reconciliation of lines having a zero balance in different currencies. + In that case, we don't reconcile anything. + """ currency_1 = self.currency_data['currency'] currency_2 = self.setup_multi_currency_data({ 'name': 'Bretonnian Ecu', @@ -363,67 +365,7 @@ class TestAccountMoveReconcile(AccountTestInvoicingCommon): res = (line_1 + line_2).reconcile() - self.assertRecordValues(res['partials'], [{ - 'amount': 0.0, - 'debit_amount_currency': 0.0, - 'credit_amount_currency': 0.0, - 'debit_move_id': line_2.id, - 'credit_move_id': line_1.id, - }]) - self.assertTrue(line_1.full_reconcile_id) - self.assertEqual(line_1.full_reconcile_id, line_2.full_reconcile_id) - self.assertTrue(res['partials'].exchange_move_id, "The partial reconciliation should have created an exchange move") - self.assertRecordValues(res['partials'].exchange_move_id, [{'date': fields.Date.from_string('2017-01-01')}]) - self.assertRecordValues(res['partials'].exchange_move_id.line_ids, [ - { - 'debit': 0.0, - 'credit': 0.0, - 'amount_currency': -0.02, - 'currency_id': currency_2.id, - 'account_id': line_2.account_id.id, - }, - { - 'debit': 0.0, - 'credit': 0.0, - 'amount_currency': 0.02, - 'currency_id': currency_2.id, - 'account_id': self.exch_expense_account.id, - }, - { - 'debit': 0.0, - 'credit': 0.0, - 'amount_currency': 0.01, - 'currency_id': currency_1.id, - 'account_id': line_1.account_id.id, - }, - { - 'debit': 0.0, - 'credit': 0.0, - 'amount_currency': -0.01, - 'currency_id': currency_1.id, - 'account_id': self.exch_income_account.id, - }, - ]) - self.assertRecordValues(res['exchange_partials'], [ - { - 'amount': 0.0, - 'debit_amount_currency': 0.02, - 'credit_amount_currency': 0.02, - 'debit_move_id': line_2.id, - 'credit_move_id': res['partials'].exchange_move_id.line_ids[0].id, - }, - { - 'amount': 0.0, - 'debit_amount_currency': 0.01, - 'credit_amount_currency': 0.01, - 'debit_move_id': res['partials'].exchange_move_id.line_ids[2].id, - 'credit_move_id': line_1.id, - }, - ]) - self.assertRecordValues(line_1 + line_2, [ - {'amount_residual': 0.0, 'amount_residual_currency': 0.0, 'reconciled': True}, - {'amount_residual': 0.0, 'amount_residual_currency': 0.0, 'reconciled': True}, - ]) + self.assertFalse(res['partials']) def test_reconcile_lines_corner_case_2_zero_amount_currency_same_foreign_currency(self): """ Test a corner case when both lines have something to reconcile in company currency but nothing diff --git a/addons/point_of_sale/tests/test_pos_basic_config.py b/addons/point_of_sale/tests/test_pos_basic_config.py index 02125bfcc17..fee1a5a0766 100644 --- a/addons/point_of_sale/tests/test_pos_basic_config.py +++ b/addons/point_of_sale/tests/test_pos_basic_config.py @@ -305,7 +305,7 @@ class TestPoSBasicConfig(TestPoSCommon): 'invoice': { 'line_ids': [ {'account_id': self.sales_account.id, 'partner_id': self.customer.id, 'debit': 0, 'credit': 0, 'reconciled': False}, - {'account_id': self.c1_receivable.id, 'partner_id': self.customer.id, 'debit': 0, 'credit': 0, 'reconciled': False}, + {'account_id': self.c1_receivable.id, 'partner_id': self.customer.id, 'debit': 0, 'credit': 0, 'reconciled': True}, ] }, 'payments': [