From b4de10fdc5c572304e411023771c9e92ad25e89c Mon Sep 17 00:00:00 2001 From: william-andre Date: Thu, 14 Dec 2023 15:16:39 +0100 Subject: [PATCH] [FIX] account,*: remove manual use of check_move_validity TLDR: the context key `check_move_validity` should never be used explicitly. `check_move_validity` is a context key used to disable some important integrity check in accounting: `debit=credit`, which is one of the most fundamental rule. It is possible to disable this in some cases, when creating and updating lines one by one, since the constraint cannot be satisfied between each line if the operations are not atomic. A mechanism has been created for this, with `_check_balanced`. It is a context manager using `_disable_recursion`, which will taint all the contexts while inside of the context manager. Putting any code changing lines one by one inside of that context manager will then allow having a transient invalid state, but still always check the validity at the end, when exiting the context. That context key has been kept only in `point_of_sale` because a special wizard is used there to be able to validate a session with an unbalanced move, but a wizard is then in charge of balancing it. The check is then done at that time. closes odoo/odoo#146824 Signed-off-by: Brice Bartoletti (bib) --- addons/account/demo/account_demo.py | 2 +- addons/account/models/account_account.py | 114 ++++++++++-------- addons/account/models/account_move_line.py | 1 - addons/account/models/company.py | 2 +- addons/hr_expense/models/hr_expense_sheet.py | 1 - .../models/account_payment.py | 2 +- addons/point_of_sale/models/pos_order.py | 93 +++++++------- addons/point_of_sale/models/pos_payment.py | 2 +- 8 files changed, 113 insertions(+), 104 deletions(-) diff --git a/addons/account/demo/account_demo.py b/addons/account/demo/account_demo.py index 572956d5e70..7d1a7d05707 100644 --- a/addons/account/demo/account_demo.py +++ b/addons/account/demo/account_demo.py @@ -47,7 +47,7 @@ class AccountChartTemplate(models.AbstractModel): + self.ref('demo_move_auto_reconcile_7') + self.ref('demo_move_auto_reconcile_8') + self.ref('demo_move_auto_reconcile_9') - ).with_context(check_move_validity=False) + ) # the invoice_extract acts like a placeholder for the OCR to be ran and doesn't contain # any lines yet diff --git a/addons/account/models/account_account.py b/addons/account/models/account_account.py index 02839685861..6cb67e2fe44 100644 --- a/addons/account/models/account_account.py +++ b/addons/account/models/account_account.py @@ -1,5 +1,7 @@ # -*- coding: utf-8 -*- -from odoo import api, fields, models, _, tools +from contextlib import nullcontext + +from odoo import api, fields, models, _, tools, Command from odoo.osv import expression from odoo.exceptions import UserError, ValidationError from odoo.tools.float_utils import float_is_zero @@ -478,44 +480,52 @@ class AccountAccount(models.Model): opening_move = self.company_id.account_opening_move_id if opening_move.state == 'draft': - # check whether we should create a new move line or modify an existing one - account_op_lines = self.env['account.move.line'].search([('account_id', '=', self.id), - ('move_id','=', opening_move.id), - (field,'!=', False), - (field,'!=', 0.0)]) # 0.0 condition important for import + with self.env['account.move']._check_balanced({'records': opening_move}): + # check whether we should create a new move line or modify an existing one + account_op_lines = self.env['account.move.line'].search([ + ('account_id', '=', self.id), + ('move_id', '=', opening_move.id), + (field, '!=', False), + (field, '!=', 0.0), # 0.0 condition important for import + ]) - if account_op_lines: - op_aml_debit = sum(account_op_lines.mapped('debit')) - op_aml_credit = sum(account_op_lines.mapped('credit')) + if account_op_lines: + op_aml_debit = sum(account_op_lines.mapped('debit')) + op_aml_credit = sum(account_op_lines.mapped('credit')) - # There might be more than one line on this account if the opening entry was manually edited - # If so, we need to merge all those lines into one before modifying its balance - opening_move_line = account_op_lines[0] - if len(account_op_lines) > 1: - merge_write_cmd = [(1, opening_move_line.id, {'debit': op_aml_debit, 'credit': op_aml_credit, 'partner_id': None ,'name': _("Opening balance")})] - unlink_write_cmd = [(2, line.id) for line in account_op_lines[1:]] - opening_move.write({'line_ids': merge_write_cmd + unlink_write_cmd}) + # There might be more than one line on this account if the opening entry was manually edited + # If so, we need to merge all those lines into one before modifying its balance + opening_move_line = account_op_lines[0] + if len(account_op_lines) > 1: + merge_write_cmd = [Command.update(opening_move_line.id, { + 'debit': op_aml_debit, + 'credit': op_aml_credit, + 'partner_id': False, + 'name': _("Opening balance"), + })] + unlink_write_cmd = [Command.unlink(line.id) for line in account_op_lines[1:]] + opening_move.write({'line_ids': merge_write_cmd + unlink_write_cmd}) - if amount: - # modify the line - opening_move_line.with_context(check_move_validity=False)[field] = amount - else: - # delete the line (no need to keep a line with value = 0) - opening_move_line.with_context(check_move_validity=False).unlink() + if amount: + # modify the line + opening_move_line[field] = amount + else: + # delete the line (no need to keep a line with value = 0) + opening_move_line.unlink() - elif amount: - # create a new line, as none existed before - self.env['account.move.line'].with_context(check_move_validity=False).create({ - 'name': _('Opening balance'), - field: amount, - 'move_id': opening_move.id, - 'account_id': self.id, - }) + elif amount: + # create a new line, as none existed before + self.env['account.move.line'].create({ + 'name': _('Opening balance'), + field: amount, + 'move_id': opening_move.id, + 'account_id': self.id, + }) - # Then, we automatically balance the opening move, to make sure it stays valid - if not 'import_file' in self.env.context: - # When importing a file, avoid recomputing the opening move for each account and do it at the end, for better performances - self.company_id._auto_balance_opening_move() + # Then, we automatically balance the opening move, to make sure it stays valid + if not 'import_file' in self.env.context: + # When importing a file, avoid recomputing the opening move for each account and do it at the end, for better performances + self.company_id._auto_balance_opening_move() @api.model def default_get(self, default_fields): @@ -656,20 +666,28 @@ class AccountAccount(models.Model): with opening debit/credit. In that case, the auto-balance is postpone until the whole file has been imported. """ - rslt = super(AccountAccount, self).load(fields, data) - - if 'import_file' in self.env.context and 'opening_balance' in fields: - companies = self.search([('id', 'in', rslt['ids'])]).mapped('company_id') - for company in companies: - if company.account_opening_move_id.filtered(lambda m: m.state == "posted"): - raise UserError( - _('You cannot import the "openning_balance" if the opening move (%s) is already posted. \ - If you are absolutely sure you want to modify the opening balance of your accounts, reset the move to draft.', - company.account_opening_move_id.name)) - company._auto_balance_opening_move() - # the current_balance of the account only includes posted moves and - # would always amount to 0 after the import if we didn't post the opening move - company.account_opening_move_id.action_post() + importing = 'import_file' in self.env.context and 'opening_balance' in fields + if importing: + container = {'records': self.env['account.move']} + manager = self.env['account.move']._check_balanced(container) + else: + manager = nullcontext + with manager: + rslt = super(AccountAccount, self).load(fields, data) + if importing: + companies = self.search([('id', 'in', rslt['ids'])]).mapped('company_id') + container['records'] = companies.account_opening_move_id + for company in companies: + if company.account_opening_move_id.filtered(lambda m: m.state == "posted"): + raise UserError(_( + 'You cannot import the "openning_balance" if the opening move (%s) is already posted. ' + 'If you are absolutely sure you want to modify the opening balance of your accounts, reset the move to draft.', + company.account_opening_move_id.name, + )) + company._auto_balance_opening_move() + # the current_balance of the account only includes posted moves and + # would always amount to 0 after the import if we didn't post the opening move + companies.account_opening_move_id.action_post() return rslt def _toggle_reconcile_to_true(self): diff --git a/addons/account/models/account_move_line.py b/addons/account/models/account_move_line.py index 9c96ee36f88..0ec425a8e9b 100644 --- a/addons/account/models/account_move_line.py +++ b/addons/account/models/account_move_line.py @@ -2514,7 +2514,6 @@ class AccountMoveLine(models.Model): skip_invoice_sync=True, skip_invoice_line_sync=True, skip_account_move_synchronization=True, - check_move_validity=False, )\ .create(full_reconcile_values_list) diff --git a/addons/account/models/company.py b/addons/account/models/company.py index a1510d22571..97e9c9a01c5 100644 --- a/addons/account/models/company.py +++ b/addons/account/models/company.py @@ -494,7 +494,7 @@ class ResCompany(models.Model): balancing_move_line = self.account_opening_move_id.line_ids.filtered(lambda x: x.account_id == balancing_account) # There could be multiple lines if we imported the balance from unaffected earnings account too if len(balancing_move_line) > 1: - self.with_context(check_move_validity=False).account_opening_move_id.line_ids -= balancing_move_line[1:] + self.account_opening_move_id.line_ids -= balancing_move_line[1:] balancing_move_line = balancing_move_line[0] debit_diff, credit_diff = self.get_opening_move_differences(self.account_opening_move_id.line_ids) diff --git a/addons/hr_expense/models/hr_expense_sheet.py b/addons/hr_expense/models/hr_expense_sheet.py index 69bdcb0735a..a8eae8a011f 100644 --- a/addons/hr_expense/models/hr_expense_sheet.py +++ b/addons/hr_expense/models/hr_expense_sheet.py @@ -668,7 +668,6 @@ class HrExpenseSheet(models.Model): 'skip_invoice_sync': True, 'skip_invoice_line_sync': True, 'skip_account_move_synchronization': True, - 'check_move_validity': False, } own_account_sheets = self.filtered(lambda sheet: sheet.payment_mode == 'own_account') company_account_sheets = self - own_account_sheets diff --git a/addons/l10n_ar_withholding/models/account_payment.py b/addons/l10n_ar_withholding/models/account_payment.py index dccdfa553c0..63ee5a8d30d 100644 --- a/addons/l10n_ar_withholding/models/account_payment.py +++ b/addons/l10n_ar_withholding/models/account_payment.py @@ -17,7 +17,7 @@ class AccountPayment(models.Model): return for pay in self.with_context( - skip_account_move_synchronization=True, check_move_validity=False, skip_invoice_sync=True, dynamic_unlink=True): + skip_account_move_synchronization=True, skip_invoice_sync=True, dynamic_unlink=True): pay.line_ids.filtered(lambda x: x.account_id == pay.company_id.l10n_ar_tax_base_account_id or x.tax_line_id.l10n_ar_withholding_payment_type).unlink() res = super()._synchronize_to_moves(changed_fields) return res diff --git a/addons/point_of_sale/models/pos_order.py b/addons/point_of_sale/models/pos_order.py index 8f757c52333..a02a9d30c63 100644 --- a/addons/point_of_sale/models/pos_order.py +++ b/addons/point_of_sale/models/pos_order.py @@ -567,61 +567,54 @@ class PosOrder(models.Model): new_move.message_post(body=message) if self.config_id.cash_rounding: - rounding_applied = float_round(self.amount_paid - self.amount_total, - precision_rounding=new_move.currency_id.rounding) - rounding_line = new_move.line_ids.filtered(lambda line: line.display_type == 'rounding') - if rounding_line and rounding_line.debit > 0: - rounding_line_difference = rounding_line.debit + rounding_applied - elif rounding_line and rounding_line.credit > 0: - rounding_line_difference = -rounding_line.credit + rounding_applied - else: - rounding_line_difference = rounding_applied - if rounding_applied: - if rounding_applied > 0.0: - account_id = new_move.invoice_cash_rounding_id.loss_account_id.id + with self.env['account.move']._check_balanced({'records': new_move}): + rounding_applied = float_round(self.amount_paid - self.amount_total, + precision_rounding=new_move.currency_id.rounding) + rounding_line = new_move.line_ids.filtered(lambda line: line.display_type == 'rounding') + if rounding_line and rounding_line.debit > 0: + rounding_line_difference = rounding_line.debit + rounding_applied + elif rounding_line and rounding_line.credit > 0: + rounding_line_difference = -rounding_line.credit + rounding_applied else: - account_id = new_move.invoice_cash_rounding_id.profit_account_id.id - if rounding_line: - if rounding_line_difference: - rounding_line.with_context(skip_invoice_sync=True, check_move_validity=False).write({ - 'debit': rounding_applied < 0.0 and -rounding_applied or 0.0, - 'credit': rounding_applied > 0.0 and rounding_applied or 0.0, - 'account_id': account_id, - 'price_unit': rounding_applied, - }) + rounding_line_difference = rounding_applied + if rounding_applied: + if rounding_applied > 0.0: + account_id = new_move.invoice_cash_rounding_id.loss_account_id.id + else: + account_id = new_move.invoice_cash_rounding_id.profit_account_id.id + if rounding_line: + if rounding_line_difference: + rounding_line.with_context(skip_invoice_sync=True).write({ + 'debit': rounding_applied < 0.0 and -rounding_applied or 0.0, + 'credit': rounding_applied > 0.0 and rounding_applied or 0.0, + 'account_id': account_id, + 'price_unit': rounding_applied, + }) + else: + self.env['account.move.line'].with_context(skip_invoice_sync=True).create({ + 'balance': -rounding_applied, + 'quantity': 1.0, + 'partner_id': new_move.partner_id.id, + 'move_id': new_move.id, + 'currency_id': new_move.currency_id.id, + 'company_id': new_move.company_id.id, + 'company_currency_id': new_move.company_id.currency_id.id, + 'display_type': 'rounding', + 'sequence': 9999, + 'name': self.config_id.rounding_method.name, + 'account_id': account_id, + }) else: - self.env['account.move.line'].with_context(skip_invoice_sync=True, check_move_validity=False).create({ - 'balance': -rounding_applied, - 'quantity': 1.0, - 'partner_id': new_move.partner_id.id, - 'move_id': new_move.id, - 'currency_id': new_move.currency_id.id, - 'company_id': new_move.company_id.id, - 'company_currency_id': new_move.company_id.currency_id.id, - 'display_type': 'rounding', - 'sequence': 9999, - 'name': self.config_id.rounding_method.name, - 'account_id': account_id, - }) - else: - if rounding_line: - rounding_line.with_context(skip_invoice_sync=True, check_move_validity=False).unlink() - if rounding_line_difference: - existing_terms_line = new_move.line_ids.filtered( - lambda line: line.account_id.account_type in ('asset_receivable', 'liability_payable')) - if existing_terms_line.debit > 0: + if rounding_line: + rounding_line.with_context(skip_invoice_sync=True).unlink() + if rounding_line_difference: + existing_terms_line = new_move.line_ids.filtered( + lambda line: line.account_id.account_type in ('asset_receivable', 'liability_payable')) existing_terms_line_new_val = float_round( - existing_terms_line.debit + rounding_line_difference, + existing_terms_line.balance + rounding_line_difference, precision_rounding=new_move.currency_id.rounding) - else: - existing_terms_line_new_val = float_round( - -existing_terms_line.credit + rounding_line_difference, - precision_rounding=new_move.currency_id.rounding) - existing_terms_line.with_context(skip_invoice_sync=True).write({ - 'debit': existing_terms_line_new_val > 0.0 and existing_terms_line_new_val or 0.0, - 'credit': existing_terms_line_new_val < 0.0 and -existing_terms_line_new_val or 0.0, - }) + existing_terms_line.with_context(skip_invoice_sync=True).balance = existing_terms_line_new_val return new_move def action_pos_order_paid(self): diff --git a/addons/point_of_sale/models/pos_payment.py b/addons/point_of_sale/models/pos_payment.py index 08a4e7a685a..f6ba767ce6f 100644 --- a/addons/point_of_sale/models/pos_payment.py +++ b/addons/point_of_sale/models/pos_payment.py @@ -91,6 +91,6 @@ class PosPayment(models.Model): 'account_id': pos_session.company_id.account_default_pos_receivable_account_id.id, 'move_id': payment_move.id, }, amounts['amount'], amounts['amount_converted']) - self.env['account.move.line'].with_context(check_move_validity=False).create([credit_line_vals, debit_line_vals]) + self.env['account.move.line'].create([credit_line_vals, debit_line_vals]) payment_move._post() return result