From 32b3b8ed9966ec0daf3badc3aa446b745bea593b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Mart=C3=ADnez?= Date: Mon, 20 Jan 2025 18:01:21 +0100 Subject: [PATCH 1/3] [FIX] account_reconcile_oca: Synchronize manual_partner_id with partner_id without onchange or subsequent changes with the _synchronize_to_moves() method. We do not define partner_id with the value of manual_partner_id to prevent _synchronize_to_moves() from making changes to the account.move.line leaving unintended values and/or data. Re-define the value of reconcile_data_info if the _synchronize_to_moves() method has changed anything on the lines. Related to https://github.com/OCA/account-reconcile/issues/779 TT52634 --- .../models/account_bank_statement_line.py | 86 +++++++++++++++++-- .../tests/test_bank_account_reconcile.py | 1 + 2 files changed, 79 insertions(+), 8 deletions(-) diff --git a/account_reconcile_oca/models/account_bank_statement_line.py b/account_reconcile_oca/models/account_bank_statement_line.py index a94c4f037..35da4d15f 100644 --- a/account_reconcile_oca/models/account_bank_statement_line.py +++ b/account_reconcile_oca/models/account_bank_statement_line.py @@ -9,7 +9,7 @@ from odoo import Command, _, api, fields, models from odoo.exceptions import UserError from odoo.fields import first -from odoo.tools import float_is_zero +from odoo.tools import float_compare, float_is_zero class AccountBankStatementLine(models.Model): @@ -340,6 +340,28 @@ def _check_line_changed(self, line): or self.analytic_distribution != line.get("analytic_distribution", False) ) + def _check_reconcile_data_changed(self): + self.ensure_one() + data = self.reconcile_data_info.get("data", []) + liquidity_lines, _suspense_lines, _other_lines = self._seek_for_lines() + move_amount_cur = sum(liquidity_lines.mapped("amount_currency")) + move_credit = sum(liquidity_lines.mapped("credit")) + move_debit = sum(liquidity_lines.mapped("debit")) + stmt_amount_curr = stmt_debit = stmt_credit = 0.0 + for line_data in data: + if line_data["kind"] != "liquidity": + continue + stmt_amount_curr += line_data["currency_amount"] + stmt_debit += line_data["debit"] + stmt_credit += line_data["credit"] + prec = self.currency_id.rounding + return ( + float_compare(move_amount_cur, move_amount_cur, precision_rounding=prec) + != 0 + or float_compare(move_credit, stmt_credit, precision_rounding=prec) != 0 + or float_compare(move_debit, stmt_debit, precision_rounding=prec) != 0 + ) + def _get_manual_delete_vals(self): return { "manual_reference": False, @@ -475,8 +497,6 @@ def _onchange_manual_reconcile_vals(self): line["kind"] if line["kind"] != "suspense" else "other" ) line.update(line_vals) - if line["kind"] == "liquidity": - self._update_move_partner() if self.manual_line_id and self.manual_line_id.id == line.get( "original_exchange_line_id" ): @@ -502,11 +522,6 @@ def _onchange_manual_reconcile_vals(self): ) self.can_reconcile = self.reconcile_data_info.get("can_reconcile", False) - def _update_move_partner(self): - if self.partner_id == self.manual_partner_id: - return - self.partner_id = self.manual_partner_id - @api.depends("reconcile_data", "is_reconciled") def _compute_reconcile_data_info(self): for record in self: @@ -953,6 +968,61 @@ def create(self, mvals): )(self._prepare_reconcile_line_data(data["data"])) return result + def _synchronize_to_moves_custom(self, changed_fields): + """Similar process to what _synchronize_to_moves() method would do but without + the relative to all the changed_fields, we just need to update partner_id. + We precisely do not do an onchange of self.partner_id = self.manual_partner_id + to avoid making all those unnecessary changes, but we need to apply this + change to the account.move and the lines without side effects. + A change of manual_partner_id that has been reconciled should NOT change the + values of the account.move lines. + """ + if self._context.get("skip_account_move_synchronization"): + return + + if not any(f_name in changed_fields for f_name in ("manual_partner_id",)): + return + + for st_line in self.with_context(skip_account_move_synchronization=True): + liquidity_lines, suspense_lines, _other_lines = st_line._seek_for_lines() + line_vals = {"partner_id": st_line.manual_partner_id.id} + line_ids_commands = [(1, liquidity_lines.id, line_vals)] + if suspense_lines: + line_ids_commands.append((1, suspense_lines.id, line_vals)) + st_line_vals = {"line_ids": line_ids_commands} + if st_line.move_id.partner_id != st_line.manual_partner_id: + st_line_vals["partner_id"] = st_line.manual_partner_id.id + st_line.move_id.write(st_line_vals) + st_line.write({"partner_id": st_line.manual_partner_id.id}) + + def _synchronize_to_moves(self, changed_fields): + """We take advantage of this method to call the custom method that does + specific things. Also, if something is changed, we will re-define + reconcile_data_info to make the data consistent (for example, if debit/credit + has changed by applying a different rate). + """ + super()._synchronize_to_moves(changed_fields=changed_fields) + self._synchronize_to_moves_custom(changed_fields) + if self._context.get("skip_account_move_synchronization"): + return + + if not any( + field_name in changed_fields + for field_name in ( + "payment_ref", + "amount", + "amount_currency", + "foreign_currency_id", + "currency_id", + "partner_id", + ) + ): + return + + for st_line in self.with_context(skip_account_move_synchronization=True): + if st_line._check_reconcile_data_changed(): + st_line.reconcile_data_info = st_line._default_reconcile_data() + def _prepare_reconcile_line_data(self, lines): new_lines = [] reverse_lines = {} diff --git a/account_reconcile_oca/tests/test_bank_account_reconcile.py b/account_reconcile_oca/tests/test_bank_account_reconcile.py index 4d2bdaf4a..e21bcf4a2 100644 --- a/account_reconcile_oca/tests/test_bank_account_reconcile.py +++ b/account_reconcile_oca/tests/test_bank_account_reconcile.py @@ -892,6 +892,7 @@ def test_widget_invoice_change_partner(self): self.assertFalse(f.partner_id) f.manual_reference = "account.move.line;%s" % liquidity_lines.id f.manual_partner_id = inv1.partner_id + f.save() self.assertEqual(f.partner_id, inv1.partner_id) bank_stmt_line.clean_reconcile() # As we have a set a partner, the cleaning should assign the invoice automatically From 2296473bb7b4d3203eb0b8f85dce2c2dffd81289 Mon Sep 17 00:00:00 2001 From: Florian da Costa Date: Tue, 21 Jan 2025 15:26:03 +0100 Subject: [PATCH 2/3] [FIX] Synchronise partner from statement line to accounting entries We need to look at the reconcile_data to find the partner as the manual_partner_id can contain the information of an auxiliary line --- .../models/account_bank_statement_line.py | 40 ++++++++++++++----- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/account_reconcile_oca/models/account_bank_statement_line.py b/account_reconcile_oca/models/account_bank_statement_line.py index 35da4d15f..4e8dc1987 100644 --- a/account_reconcile_oca/models/account_bank_statement_line.py +++ b/account_reconcile_oca/models/account_bank_statement_line.py @@ -980,20 +980,38 @@ def _synchronize_to_moves_custom(self, changed_fields): if self._context.get("skip_account_move_synchronization"): return - if not any(f_name in changed_fields for f_name in ("manual_partner_id",)): + # we actually check reconcile_data to find the partner of the liquidity lines + # because the written manual_partner_id is not always related to the liquidity + # line... + if not any(f_name in changed_fields for f_name in ("reconcile_data",)): return for st_line in self.with_context(skip_account_move_synchronization=True): - liquidity_lines, suspense_lines, _other_lines = st_line._seek_for_lines() - line_vals = {"partner_id": st_line.manual_partner_id.id} - line_ids_commands = [(1, liquidity_lines.id, line_vals)] - if suspense_lines: - line_ids_commands.append((1, suspense_lines.id, line_vals)) - st_line_vals = {"line_ids": line_ids_commands} - if st_line.move_id.partner_id != st_line.manual_partner_id: - st_line_vals["partner_id"] = st_line.manual_partner_id.id - st_line.move_id.write(st_line_vals) - st_line.write({"partner_id": st_line.manual_partner_id.id}) + data = st_line.reconcile_data_info.get("data", []) + partner_id = False + for line_data in data: + if line_data["kind"] == "liquidity": + partner_id = ( + line_data.get("partner_id") + and line_data.get("partner_id")[0] + or False + ) + break + if st_line.partner_id.id != partner_id: + ( + liquidity_lines, + suspense_lines, + _other_lines, + ) = st_line._seek_for_lines() + line_vals = {"partner_id": partner_id} + line_ids_commands = [(1, liquidity_lines.id, line_vals)] + if suspense_lines: + line_ids_commands.append((1, suspense_lines.id, line_vals)) + st_line_vals = {"line_ids": line_ids_commands} + if st_line.move_id.partner_id.id != partner_id: + st_line_vals["partner_id"] = partner_id + st_line.move_id.write(st_line_vals) + st_line.write({"partner_id": partner_id}) def _synchronize_to_moves(self, changed_fields): """We take advantage of this method to call the custom method that does From 976fc5993c688158843f3d45e7144b3b8ef1821e Mon Sep 17 00:00:00 2001 From: Florian da Costa Date: Wed, 5 Feb 2025 16:35:47 +0100 Subject: [PATCH 3/3] [FIX] partner update from reconciliation widget We cant to avoid to change amounts on accounting entries during the reconciliation process. One of the goal is to avoid a following case : The statement line is created in a foreign currency journal, later, the rate is updated in Odoo. During reconciliation process, the partner is set on liquidity line, the accounting entries are synchronized and the balance changes. --- .../models/account_bank_statement_line.py | 82 +++++++++---------- .../tests/test_bank_account_reconcile.py | 12 +++ 2 files changed, 51 insertions(+), 43 deletions(-) diff --git a/account_reconcile_oca/models/account_bank_statement_line.py b/account_reconcile_oca/models/account_bank_statement_line.py index 4e8dc1987..350714cdb 100644 --- a/account_reconcile_oca/models/account_bank_statement_line.py +++ b/account_reconcile_oca/models/account_bank_statement_line.py @@ -497,6 +497,8 @@ def _onchange_manual_reconcile_vals(self): line["kind"] if line["kind"] != "suspense" else "other" ) line.update(line_vals) + if line["kind"] == "liquidity": + self._update_move_partner() if self.manual_line_id and self.manual_line_id.id == line.get( "original_exchange_line_id" ): @@ -522,6 +524,11 @@ def _onchange_manual_reconcile_vals(self): ) self.can_reconcile = self.reconcile_data_info.get("can_reconcile", False) + def _update_move_partner(self): + if self.partner_id == self.manual_partner_id: + return + self.partner_id = self.manual_partner_id + @api.depends("reconcile_data", "is_reconciled") def _compute_reconcile_data_info(self): for record in self: @@ -968,61 +975,49 @@ def create(self, mvals): )(self._prepare_reconcile_line_data(data["data"])) return result - def _synchronize_to_moves_custom(self, changed_fields): - """Similar process to what _synchronize_to_moves() method would do but without - the relative to all the changed_fields, we just need to update partner_id. - We precisely do not do an onchange of self.partner_id = self.manual_partner_id - to avoid making all those unnecessary changes, but we need to apply this - change to the account.move and the lines without side effects. - A change of manual_partner_id that has been reconciled should NOT change the - values of the account.move lines. + def _synchronize_to_moves(self, changed_fields): + """We want to avoid to change stuff (mainly amounts ) in accounting entries + when some changes happen in the reconciliation widget. The only change + (among the fields triggering the synchronization) possible from the + reconciliation widget is the partner_id field. + + So, in case of change on partner_id field we do not call super but make + only the required change (relative to partner) on accounting entries. + + And if something else changes, we then re-define reconcile_data_info to + make the data consistent (for example, if debit/credit has changed by + applying a different rate or even if there was a correction on statement + line amount). """ if self._context.get("skip_account_move_synchronization"): return + if "partner_id" in changed_fields and not any( + field_name in changed_fields + for field_name in ( + "payment_ref", + "amount", + "amount_currency", + "foreign_currency_id", + "currency_id", + ) + ): + for st_line in self.with_context(skip_account_move_synchronization=True): - # we actually check reconcile_data to find the partner of the liquidity lines - # because the written manual_partner_id is not always related to the liquidity - # line... - if not any(f_name in changed_fields for f_name in ("reconcile_data",)): - return - - for st_line in self.with_context(skip_account_move_synchronization=True): - data = st_line.reconcile_data_info.get("data", []) - partner_id = False - for line_data in data: - if line_data["kind"] == "liquidity": - partner_id = ( - line_data.get("partner_id") - and line_data.get("partner_id")[0] - or False - ) - break - if st_line.partner_id.id != partner_id: ( liquidity_lines, suspense_lines, _other_lines, ) = st_line._seek_for_lines() - line_vals = {"partner_id": partner_id} + line_vals = {"partner_id": st_line.partner_id} line_ids_commands = [(1, liquidity_lines.id, line_vals)] if suspense_lines: line_ids_commands.append((1, suspense_lines.id, line_vals)) st_line_vals = {"line_ids": line_ids_commands} - if st_line.move_id.partner_id.id != partner_id: - st_line_vals["partner_id"] = partner_id + if st_line.move_id.partner_id != st_line.partner_id: + st_line_vals["partner_id"] = st_line.partner_id.id st_line.move_id.write(st_line_vals) - st_line.write({"partner_id": partner_id}) - - def _synchronize_to_moves(self, changed_fields): - """We take advantage of this method to call the custom method that does - specific things. Also, if something is changed, we will re-define - reconcile_data_info to make the data consistent (for example, if debit/credit - has changed by applying a different rate). - """ - super()._synchronize_to_moves(changed_fields=changed_fields) - self._synchronize_to_moves_custom(changed_fields) - if self._context.get("skip_account_move_synchronization"): - return + else: + super()._synchronize_to_moves(changed_fields=changed_fields) if not any( field_name in changed_fields @@ -1036,8 +1031,9 @@ def _synchronize_to_moves(self, changed_fields): ) ): return - - for st_line in self.with_context(skip_account_move_synchronization=True): + # reset reconcile_data_info if amounts are not consistent anymore with the + # amounts of the accounting entries + for st_line in self: if st_line._check_reconcile_data_changed(): st_line.reconcile_data_info = st_line._default_reconcile_data() diff --git a/account_reconcile_oca/tests/test_bank_account_reconcile.py b/account_reconcile_oca/tests/test_bank_account_reconcile.py index e21bcf4a2..a161c3e8f 100644 --- a/account_reconcile_oca/tests/test_bank_account_reconcile.py +++ b/account_reconcile_oca/tests/test_bank_account_reconcile.py @@ -1297,6 +1297,7 @@ def test_invoice_foreign_currency_late_change_of_rate(self): "rate": 1.25, } ) + liquidity_lines, suspense_lines, other_lines = bank_stmt_line._seek_for_lines() with Form( bank_stmt_line, view="account_reconcile_oca.bank_statement_line_form_reconcile_view", @@ -1310,6 +1311,17 @@ def test_invoice_foreign_currency_late_change_of_rate(self): line["amount"], 83.33, ) + # check that adding a partner does not recompute the amounts on accounting + # entries, but is still synchronized with accounting entries + f.manual_reference = "account.move.line;%s" % liquidity_lines.id + f.manual_partner_id = inv1.partner_id + self.assertEqual(f.partner_id, inv1.partner_id) + self.assertEqual(liquidity_lines.debit, 83.33) + f.save() + # check liquidity line did not recompute debit with the new rate with + # partner change + self.assertEqual(liquidity_lines.debit, 83.33) + self.assertEqual(liquidity_lines.partner_id, inv1.partner_id) f.manual_reference = "account.move.line;%s" % line["id"] # simulate click on statement line, check amount does not recompute f.manual_partner_id = inv1.partner_id