From 1704f0aa6f46cca855d88ac7091d5d60c2976f4e Mon Sep 17 00:00:00 2001 From: Brice bib Bartoletti Date: Tue, 21 Feb 2023 14:47:31 +0000 Subject: [PATCH] [IMP] account: make multivat great again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The aim of this commit is: - to bring back a feature that was wiped out during the refactoring of the chart of account instantiation in commit 512574861691f425ec6a17f20fe4b586bb88a299 - add a test that will solidify the feature - Improve the instantiation by installing the required l10n module directly instead of asking the user to install it manually Previous to this commit: The button instantiating the foreign taxes leads to a traceback. After this commit: The button instantiate the foreign taxes correctly Note: To prevent xml_id collision on account, we prefix the xml_ids with their module's name closes odoo/odoo#118056 closes odoo/odoo#125950 Community-pr: https://github.com/odoo/odoo/pull/118056/ Task-id: 3196556 X-original-commit: b66ebb78ec750a4657a6de6726deb9eea7e0c7d1 Signed-off-by: Brice Bartoletti (bib) Signed-off-by: William André (wan) --- addons/account/models/chart_template.py | 163 +++++++++++++- addons/account/models/ir_module.py | 4 +- addons/account/models/partner.py | 29 +-- addons/account/tests/__init__.py | 1 + addons/account/tests/common.py | 2 +- addons/account/tests/test_chart_template.py | 2 +- addons/account/tests/test_multivat.py | 225 ++++++++++++++++++++ addons/account/views/partner_view.xml | 14 +- 8 files changed, 408 insertions(+), 32 deletions(-) create mode 100644 addons/account/tests/test_multivat.py diff --git a/addons/account/models/chart_template.py b/addons/account/models/chart_template.py index 19219f485db..303afba42bc 100644 --- a/addons/account/models/chart_template.py +++ b/addons/account/models/chart_template.py @@ -80,7 +80,7 @@ class AccountChartTemplate(models.AbstractModel): def _get_chart_template_mapping(self, get_all=False): """Get basic information about available CoA and their modules. - :return: a mapping between the template code and a dictionnary constaining the + :return: a mapping between the template code and a dictionary containing the name, country id, country name, module dependencies and parent template :rtype: dict[str, dict] """ @@ -363,7 +363,7 @@ class AccountChartTemplate(models.AbstractModel): """Load all the data linked to the template into the database. The data can contain translation values (i.e. `name@fr_FR` to translate the name in French) - An xml_id tht doesn't contain a `.` will be treated as being linked to `account` and prefixed + An xml_id that doesn't contain a `.` will be treated as being linked to `account` and prefixed with the company's id (i.e. `cash` is interpreted as `account.1_cash` if the company's id is 1) :param data: Basically all the final data of records to create/update for the chart @@ -383,7 +383,11 @@ class AccountChartTemplate(models.AbstractModel): field.type == 'many2one' or (field.type in ('integer', 'many2one_reference') and not value.isdigit()) ): - values[fname] = self.ref(value).id if value not in ('', 'False', 'None') else False + try: + values[fname] = self.ref(value).id if value not in ('', 'False', 'None') else False + except ValueError as e: + _logger.warning("Failed when trying to recover %s for field=%s", value, field) + raise e elif field.type in ('one2many', 'many2many') and isinstance(value[0], (list, tuple)): for i, (command, _id, *last_part) in enumerate(value): if last_part: @@ -523,7 +527,7 @@ class AccountChartTemplate(models.AbstractModel): def _get_chart_template_data(self, template_code): template_data = defaultdict(lambda: defaultdict(dict)) - template_data['res.company'] + template_data['res.company'] # ensure it's the first property when iterating for code in [None] + self._get_parent_template(template_code): for model, funcs in sorted( self._template_register[code].items(), @@ -612,6 +616,157 @@ class AccountChartTemplate(models.AbstractModel): for company_attr_name, account in zip(accounts_data.keys(), accounts): company[company_attr_name] = account + @api.model + def _instantiate_foreign_taxes(self, country, company): + """Create and configure foreign taxes from the provided country. + + Instantiate the taxes as they would be for the foreign localization only replacing the accounts used by the most + probable account we can retrieve from the company's localization. + This method is intended as a shortcut for instantiation, accelerating it, not as an out-of-the-box solution 100% + correct solution. + """ + # Implementation: + # - Check if there is any tax for this country and stop the process if yes + # - Retrieve the tax group and tax template data + # - Try to create accounts at most probable location in the CoA + # - Assign those accounts to the data + # - Creates tax group and taxes with their ir.model.data + + taxes_in_country = self.env['account.tax'].search([ + ('country_id', '=', country.id), + ('company_id', '=', company.id) + ]) + if taxes_in_country: + return + + def create_foreign_tax_account(existing_account, additional_label): + new_code = self.env['account.account']._search_new_account_code( + existing_account.company_id, + len(existing_account.code), + existing_account.code[:-2] + ) + return self.env['account.account'].create({ + 'name': f"{existing_account.name} - {additional_label}", + 'code': new_code, + 'account_type': existing_account.account_type, + 'company_id': existing_account.company_id.id, + }) + + existing_accounts = {'': None, None: None} # keeps tracks of the created account by foreign xml_id + default_company_taxes = company.account_sale_tax_id + company.account_purchase_tax_id + chart_template_code = self._guess_chart_template(country=country) + tax_group_data = self._get_chart_template_data(chart_template_code)['account.tax.group'] + tax_data = self._get_chart_template_data(chart_template_code)['account.tax'] + + # Populate foreign accounts mapping + # Try to create tax group accounts if not mapped + field_and_names = ( + ('tax_payable_account_id', _("Foreign tax account payable (%s)", country.code)), + ('tax_receivable_account_id', _("Foreign tax account receivable (%s)", country.code)), + ('advance_tax_payment_account_id', _("Foreign tax account advance payment (%s)", country.code)), + ) + for field, account_name in field_and_names: + for tax_group in tax_group_data.values(): + account_template_xml_id = tax_group.get(field) + if account_template_xml_id in existing_accounts: + continue + local_tax_group = self.env["account.tax.group"].search([ + ('country_id', '=', company.account_fiscal_country_id.id), + (field, '!=', False), + ], limit=1) + if local_tax_group: + existing_accounts[account_template_xml_id] = create_foreign_tax_account(local_tax_group[field], account_name).id + + # Try to create repartition lines account if not mapped + for tax_template in tax_data.values(): + for _command, _id, rep_line in tax_template.get('repartition_line_ids', []): + if 'account_id' in rep_line and rep_line['repartition_type'] == 'tax': + type_tax_use, foreign_tax_rep_line = tax_template['type_tax_use'], rep_line + account_template_xml_id = foreign_tax_rep_line['account_id'] + if account_template_xml_id in existing_accounts: + continue + + sign_comparator = '<' if float(foreign_tax_rep_line.get('factor_percent', 100)) < 0 else '>' + minimal_domain = [ + ('company_id', '=', company.id), + ('account_id', '!=', False), + ('factor_percent', sign_comparator, 0), + ] + additional_domain = [ + ('tax_id.type_tax_use', '=', type_tax_use), + ('tax_id.country_id', '=', company.account_fiscal_country_id.id), + ('tax_id', 'in', default_company_taxes.ids), + ] + + # Trying to find an account being less restrictive on each iteration until the minimum acceptable is + # reached. If nothing is found, don't fill it to avoid setting a wrong account + similar_repartition_line = None + while not similar_repartition_line and additional_domain: + search_domain = minimal_domain + additional_domain + similar_repartition_line = self.env['account.tax.repartition.line'].search(search_domain, limit=1) + additional_domain.pop() + + if similar_repartition_line: + local_tax_account = similar_repartition_line.account_id + similar_account_id = create_foreign_tax_account(local_tax_account, _("Foreign tax account (%s)", country.code)) + existing_accounts[account_template_xml_id] = similar_account_id.id + + # Try to create cash basis account if not mapped + local_cash_basis_tax = self.env["account.tax"].search([ + ('country_id', '=', company.account_fiscal_country_id.id), + ('cash_basis_transition_account_id', '!=', False) + ], limit=1) + for tax_template in tax_data.values(): + account_xml_id = tax_template.get('cash_basis_transition_account_id') + if account_xml_id in existing_accounts: + continue + + if local_cash_basis_tax: + existing_accounts[account_xml_id] = create_foreign_tax_account( + local_cash_basis_tax.cash_basis_transition_account_id, + _("Cash basis transition account") + ).id + continue + + account_id = [rep_line['account_id'] for _command, _id, rep_line in tax_template['repartition_line_ids'] if rep_line.get('account_id')] + if account_id: + local_account = self.env['account.account'].browse(account_id[0]) + existing_accounts[account_xml_id] = create_foreign_tax_account(local_account, _("Cash basis transition account")).id + continue + + # Assign the account based on the map + for field, account_name in field_and_names: + for tax_group in tax_group_data.values(): + tax_group[field] = existing_accounts.get(account_template_xml_id) + + for tax_template in tax_data.values(): + # This is required because the country isn't provided directly by the template + tax_template['country_id'] = country.id + + if tax_template.get('tax_group_id'): + tax_template['tax_group_id'] = f"{chart_template_code}_{tax_template['tax_group_id']}" + + for _command, _id, rep_line in tax_template.get('repartition_line_ids', []): + rep_line['account_id'] = existing_accounts.get(rep_line.get('account_id')) + + account_xml_id = tax_template.get('cash_basis_transition_account_id') + tax_template['cash_basis_transition_account_id'] = existing_accounts[account_xml_id] + + data = { + 'account.tax.group': tax_group_data, + 'account.tax': tax_data, + } + # prefix the xml_id with the chart template code to avoid collision + # because since 16.2 xml_ids are regrouped under module account + data = { + model: { + f"{chart_template_code}_{xml_id}": template + for xml_id, template in templates.items() + } + for model, templates in data.items() + } + self._load_data(data) + # -------------------------------------------------------------------------------- # Root template functions # -------------------------------------------------------------------------------- diff --git a/addons/account/models/ir_module.py b/addons/account/models/ir_module.py index b7552605161..995a5c1eb5d 100644 --- a/addons/account/models/ir_module.py +++ b/addons/account/models/ir_module.py @@ -1,7 +1,7 @@ from importlib import import_module from inspect import getmembers, ismodule, isclass, isfunction, getsourcelines -from odoo import models, fields +from odoo import api, models, fields from odoo.tools import get_flag @@ -25,6 +25,7 @@ class IrModule(models.Model): account_templates = fields.Binary(compute='_compute_account_templates', exportable=False) + @api.depends('state') def _compute_account_templates(self): chart_category = self.env.ref('base.module_category_accounting_localizations_account_charts') ChartTemplate = self.env['account.chart.template'] @@ -45,6 +46,7 @@ class IrModule(models.Model): 'parent': fct(ChartTemplate).get('parent'), 'country': fct(ChartTemplate).get('country', ''), 'visible': fct(ChartTemplate).get('visible', True), + 'installed': module.state == "installed", 'module': module.name, } for _name, mdl in sorted(getmembers(python_module, template_module), key=(lambda mdl: module_order[mdl[0]])) diff --git a/addons/account/models/partner.py b/addons/account/models/partner.py index 50b5d9034cf..3ad17456dc7 100644 --- a/addons/account/models/partner.py +++ b/addons/account/models/partner.py @@ -58,17 +58,18 @@ class AccountFiscalPosition(models.Model): @api.depends('foreign_vat', 'country_id') def _compute_foreign_vat_header_mode(self): - for record in self: - if not record.foreign_vat or not record.country_id: - record.foreign_vat_header_mode = None - continue - - if self.env['account.tax'].search([('country_id', '=', record.country_id.id)], limit=1): - record.foreign_vat_header_mode = None - elif self.env['account.chart.template']._guess_chart_template(record.country_id) != 'generic_coa': - record.foreign_vat_header_mode = 'templates_found' + for fiscal_position in self: + if ( + not fiscal_position.foreign_vat + or not fiscal_position.country_id + or self.env['account.tax'].search([('country_id', '=', fiscal_position.country_id.id)], limit=1) + ): + fiscal_position.foreign_vat_header_mode = False else: - record.foreign_vat_header_mode = 'no_template' + template_code = self.env['account.chart.template']._guess_chart_template(fiscal_position.country_id) + template = self.env['account.chart.template']._get_chart_template_mapping()[template_code] + # 'no_template' kept for compatibility in stable. To remove in master + fiscal_position.foreign_vat_header_mode = 'templates_found' if template['installed'] else 'no_template' @api.constrains('zip_from', 'zip_to') def _check_zip(self): @@ -250,8 +251,12 @@ class AccountFiscalPosition(models.Model): def action_create_foreign_taxes(self): self.ensure_one() - self.env['account.tax.template']._try_instantiating_foreign_taxes(self.country_id, self.company_id) - + template_code = self.env['account.chart.template']._guess_chart_template(self.country_id) + template = self.env['account.chart.template']._get_chart_template_mapping()[template_code] + if not template['installed']: + localization_module = self.env['ir.module.module'].search([('name', '=', template['module'])]) + localization_module.sudo().button_immediate_install() + self.env["account.chart.template"]._instantiate_foreign_taxes(self.country_id, self.company_id) class AccountFiscalPositionTax(models.Model): _name = 'account.fiscal.position.tax' diff --git a/addons/account/tests/__init__.py b/addons/account/tests/__init__.py index 982d2354d8e..7478c5ab6c5 100644 --- a/addons/account/tests/__init__.py +++ b/addons/account/tests/__init__.py @@ -40,3 +40,4 @@ from . import test_tour from . import test_early_payment_discount from . import test_ir_actions_report from . import test_download_xsds +from . import test_multivat diff --git a/addons/account/tests/common.py b/addons/account/tests/common.py index 1ddb8f77f52..eea8fe0ad0a 100644 --- a/addons/account/tests/common.py +++ b/addons/account/tests/common.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. from odoo import fields, Command -from odoo.tests.common import TransactionCase, HttpCase, Form +from odoo.tests.common import TransactionCase, HttpCase, tagged, Form import json import time diff --git a/addons/account/tests/test_chart_template.py b/addons/account/tests/test_chart_template.py index 198178e7192..c63e19d115a 100644 --- a/addons/account/tests/test_chart_template.py +++ b/addons/account/tests/test_chart_template.py @@ -7,7 +7,7 @@ from odoo.tests import tagged from odoo.tests.common import TransactionCase -def _get_chart_template_mapping(self): +def _get_chart_template_mapping(self, get_all=False): return {'test': { 'name': 'test', 'country_id': None, diff --git a/addons/account/tests/test_multivat.py b/addons/account/tests/test_multivat.py new file mode 100644 index 00000000000..98169e68f99 --- /dev/null +++ b/addons/account/tests/test_multivat.py @@ -0,0 +1,225 @@ + +from unittest.mock import patch + +from odoo import Command +from odoo.addons.account.models.chart_template import AccountChartTemplate +from odoo.addons.account.tests.common import instantiate_accountman +from odoo.tests import tagged +from odoo.tests.common import TransactionCase + + +def _get_chart_template_mapping(self, get_all=False): + return { + 'local': { + 'name': 'local', + 'country_id': self.env.ref("base.be").id, + 'country_code': 'be', + 'modules': ['account'], + 'parent': None, + 'installed': True, + }, + 'foreign': { + 'name': 'foreign', + 'country_id': self.env.ref("base.fr").id, + 'country_code': 'fr', + 'modules': ['account'], + 'parent': None, + 'installed': True, + }, + } + + +def data_method_provider(chart_template_name, country_code): + country = f"base.{country_code}" + # this is used to simulated differences between xml_ids + external_id_prefix = '' if chart_template_name == 'local' else f"{chart_template_name}_" + + def test_data_getter(self, template_code): + return { + 'template_data': { + 'code_digits': 6, + 'currency_id': 'base.EUR', + 'property_account_income_categ_id': f'{external_id_prefix}test_account_income_template', + 'property_account_expense_categ_id': f'{external_id_prefix}test_account_expense_template', + }, + 'res.company': { + self.env.company.id: { + 'bank_account_code_prefix': '1000', + 'cash_account_code_prefix': '2000', + 'transfer_account_code_prefix': '3000', + }, + }, + 'account.account': { + f'{external_id_prefix}test_account_tax_recoverable_template': { + 'name': f'{external_id_prefix}tax recoverable', + 'code': '411000', + 'account_type': 'asset_current', + }, + f'{external_id_prefix}test_account_tax_receivable_template': { + 'name': f'{external_id_prefix}tax recoverable', + 'code': '411200', + 'account_type': 'asset_current', + }, + f'{external_id_prefix}test_account_advance_payment_tax_template': { + 'name': f'{external_id_prefix}advance tax payment', + 'code': '411900', + 'account_type': 'asset_current', + }, + f'{external_id_prefix}test_account_tax_payable_template': { + 'name': f'{external_id_prefix}tax recoverable', + 'code': '451200', + 'account_type': 'liability_current', + }, + f'{external_id_prefix}test_account_cash_basis_transition_account_id': { + 'name': f'{external_id_prefix}cash basis transition account', + 'code': '451500', + 'account_type': 'liability_current', + }, + f'{external_id_prefix}test_account_income_template': { + 'name': f'{external_id_prefix}income', + 'code': '600000', + 'account_type': 'income', + }, + f'{external_id_prefix}test_account_expense_template': { + 'name': f'{external_id_prefix}expense', + 'code': '700000', + 'account_type': 'expense', + }, + }, + 'account.journal': self._get_account_journal(template_code), + 'account.tax.group': { + 'tax_group_taxes': { + 'name': f"{external_id_prefix}Taxes", + 'sequence': 0, + 'country_id': country, + 'tax_payable_account_id': f'{external_id_prefix}test_account_tax_payable_template', + 'tax_receivable_account_id': f'{external_id_prefix}test_account_tax_receivable_template', + 'advance_tax_payment_account_id': f'{external_id_prefix}test_account_advance_payment_tax_template', + }, + }, + 'account.tax': { + xmlid: _tax_vals(name, amount, external_id_prefix) + for name, xmlid, amount in ( + (f'{external_id_prefix}Tax 1', 'test_tax_1_template', 15), + (f'{external_id_prefix}Tax 2', 'test_tax_2_template', 0), + ) + }, + } + return test_data_getter + + +def _tax_vals(name, amount, external_id_prefix): + return { + 'name': name, + 'amount': amount, + 'type_tax_use': 'purchase', + 'tax_group_id': 'tax_group_taxes', + 'cash_basis_transition_account_id': f'{external_id_prefix}test_account_cash_basis_transition_account_id', + 'repartition_line_ids': [ + Command.create({'document_type': 'invoice', 'factor_percent': 100, 'repartition_type': 'base'}), + Command.create({'document_type': 'invoice', 'factor_percent': 100, 'repartition_type': 'tax', + 'account_id': f'{external_id_prefix}test_account_tax_recoverable_template'}), + Command.create({'document_type': 'refund', 'factor_percent': 100, 'repartition_type': 'base'}), + Command.create({'document_type': 'refund', 'factor_percent': 100, 'repartition_type': 'tax', + 'account_id': f'{external_id_prefix}test_account_tax_recoverable_template'}), + ] + } + + +@tagged('post_install', '-at_install') +@patch.object(AccountChartTemplate, '_get_chart_template_mapping', _get_chart_template_mapping) +class TestMultiVAT(TransactionCase): + + @classmethod + @patch.object(AccountChartTemplate, '_get_chart_template_mapping', _get_chart_template_mapping) + def setUpClass(cls): + """ + Setups a company with a custom chart template, containing a tax and a fiscal position. + We need to add xml_ids to the templates because they are loaded from their xml_ids + """ + super().setUpClass() + instantiate_accountman(cls) + + cls.company_1 = cls.env['res.company'].create({ + 'name': 'TestCompany1', + 'country_id': cls.env.ref('base.be').id, + }) + + cls.user.write({ + 'company_ids': [Command.set(cls.company_1.ids)], + 'company_id': cls.company_1.id, + }) + + test_get_data = data_method_provider("local", "be") + with patch.object(AccountChartTemplate, '_get_chart_template_data', side_effect=test_get_data, autospec=True): + cls.env['account.chart.template'].try_loading('local', company=cls.company_1, install_demo=False) + + foreign_country = cls.env.ref("base.fr") + cls.foreign_vat_fpos = cls.env["account.fiscal.position"].create({ + "name": "FR foreign VAT", + "auto_apply": True, + "country_id": foreign_country.id, + "foreign_vat": "FR23334175221", + }) + + test_get_data = data_method_provider("foreign", "fr") + with patch.object(AccountChartTemplate, '_get_chart_template_data', side_effect=test_get_data, autospec=True): + cls.foreign_vat_fpos.action_create_foreign_taxes() + + def test_tax_and_tax_group_should_be_reachable_using_standard_api(self): + # Ensure local and foreign tax is reachable using the custom ref api + for xml_id in ( + # tax group + 'tax_group_taxes', + 'foreign_tax_group_taxes', + # tax + 'test_tax_1_template', + 'test_tax_2_template', + 'foreign_test_tax_1_template', + 'foreign_test_tax_2_template', + ): + with self.subTest(xml_id=xml_id): + record = self.env["account.chart.template"].ref(xml_id, raise_if_not_found=False) + self.assertTrue(record, "We should be able to retrieve the record") + + def test_tax_group_data(self): + # Ensure the correct country is set on tax group + for xml_id, country_code in (('tax_group_taxes', 'BE'), ('foreign_tax_group_taxes', 'FR')): + tax_group = self.env["account.chart.template"].ref(xml_id) + with self.subTest(xml_id=xml_id, country_code=country_code): + self.assertEqual(tax_group.country_id.code, country_code) + + local_tax_group = self.env["account.chart.template"].ref('tax_group_taxes') + foreign_tax_group = self.env["account.chart.template"].ref('foreign_tax_group_taxes') + for field in ('tax_payable_account_id', 'tax_receivable_account_id', 'advance_tax_payment_account_id'): + with self.subTest(field=field): + self.assertTrue(foreign_tax_group[field], "This account should have been set") + self.assertNotEqual(foreign_tax_group[field], local_tax_group[field], + "A copy of the local tax group account should have been created and set") + + def test_tax_data_should_be_consistent(self): + # Ensure the correct country is set + for xml_id, country_code in ( + # tax + ('test_tax_1_template', 'BE'), + ('test_tax_2_template', 'BE'), + ('foreign_test_tax_1_template', 'FR'), + ('foreign_test_tax_2_template', 'FR'), + ): + model = self.env["account.chart.template"].ref(xml_id) + with self.subTest(xml_id=xml_id, country_code=country_code): + self.assertEqual(model.country_id.code, country_code) + + tax = self.env["account.chart.template"].ref('foreign_test_tax_1_template') + self.assertEqual(tax.country_id.code, 'FR') + self.assertEqual(tax.cash_basis_transition_account_id.code, '451501') + _base_line, tax_line = tax.invoice_repartition_line_ids + self.assertEqual(tax_line.account_id.code, '411001', + "The foreign tax account should be a new account with a code close to the local tax account code") + + tax = self.env["account.chart.template"].ref('foreign_test_tax_2_template') + self.assertEqual(tax.country_id.code, 'FR') + self.assertEqual(tax.cash_basis_transition_account_id.code, '451501') + _base_line, tax_line = tax.invoice_repartition_line_ids + self.assertEqual(tax_line.account_id.code, '411001', + "The previously created tax account should be reused for similar tax") diff --git a/addons/account/views/partner_view.xml b/addons/account/views/partner_view.xml index ebd337f1ea7..f23cdd55e1c 100644 --- a/addons/account/views/partner_view.xml +++ b/addons/account/views/partner_view.xml @@ -10,8 +10,7 @@ class="alert alert-info mb-0" role="alert" attrs="{'invisible': [('foreign_vat_header_mode', '=', False)]}"> - -
+
Click
- - - No tax template found for this country. Please install the corresponding localization module. -