[REV] account: Remove tax restrictions.

Problem --------- Due to commit 8d77045b46, restrictions were added to taxes that were used in transactions. However, this behavior worsens user experience; users that would like to modify a tax would have to duplicate it, update the duplicated version and modify the tax usage where necessary. For such reason, this process is being reverted (we keep the tracking of the modification). The tracking of repartition lines now also includes the sequence of the lines since it is not restricted anymore. Furthermore, `_message_log_repartition_lines` has been updated to log messages when new repartition lines are added or repartition lines are removed. It also has been update to track changes of lines by comparing their position in the tax (rel. sequence) and not their IDs. Doing so allows to remove unnecessary logging when, for example, users remove a repartition line and immediately add a new one similar to the one deleted. It also allows to not log reordering event of two similar lines. original-commit: 8d77045b46 task-3450002 closes odoo/odoo#155055 Signed-off-by: John Laterre (jol) <jol@odoo.com>
2024-03-01 13:48:18 +00:00
parent 0462ab901f
commit f98dc05a03
4 changed files with 233 additions and 105 deletions
@@ -9249,6 +9249,13 @@ msgstr ""
 msgid "Never"
 msgstr ""

+#. module: account
+#. odoo-python
+#: code:addons/account/models/account_tax.py:0
+#, python-format
+msgid "New"
+msgstr ""
+
 #. module: account
 #: model:ir.model.fields,field_description:account.field_account_setup_bank_manual_config__new_journal_name
 msgid "New Journal Name"
@@ -11379,6 +11386,14 @@ msgstr ""
 msgid "Remake the sequence of Journal Entries."
 msgstr ""

+#. module: account
+#. odoo-python
+#: code:addons/account/models/account_tax.py:0
+#: code:addons/account/models/account_tax.py:0
+#, python-format
+msgid "Removed"
+msgstr ""
+
 #. module: account
 #: model:account.account,name:account.1_expense_rent
 msgid "Rent"
@@ -95,10 +95,10 @@ class AccountTax(models.Model):
          help="This dummy field lets us use another search method on the field 'name'."
               "This allows more freedom on how to search the 'name' compared to 'filter_domain'."
               "See '_search_name' and '_parse_name_search' for why this is not possible with 'filter_domain'.")
-    type_tax_use = fields.Selection(TYPE_TAX_USE, string='Tax Type', required=True, default="sale",
+    type_tax_use = fields.Selection(TYPE_TAX_USE, string='Tax Type', required=True, default="sale", tracking=True,
        help="Determines where the tax is selectable. Note: 'None' means a tax can't be used by itself, however it can still be used in a group. 'adjustment' is used to perform tax adjustment.")
    tax_scope = fields.Selection([('service', 'Services'), ('consu', 'Goods')], string="Tax Scope", help="Restrict the use of taxes to a type of product.")
-    amount_type = fields.Selection(default='percent', string="Tax Computation", required=True,
+    amount_type = fields.Selection(default='percent', string="Tax Computation", required=True, tracking=True,
        selection=[('group', 'Group of Taxes'), ('fixed', 'Fixed'), ('percent', 'Percentage of Price'), ('division', 'Percentage of Price Tax Included')],
        help="""
    - Group of Taxes: The tax is a set of sub taxes.
@@ -121,7 +121,7 @@ class AccountTax(models.Model):
    amount = fields.Float(required=True, digits=(16, 4), default=0.0, tracking=True)
    description = fields.Char(string='Description', translate=True)
    invoice_label = fields.Char(string='Label on Invoices', translate=True)
-    price_include = fields.Boolean(string='Included in Price', default=False,
+    price_include = fields.Boolean(string='Included in Price', default=False, tracking=True,
        help="Check this if the price you use on the product and invoices includes this tax.")
    include_base_amount = fields.Boolean(string='Affect Base of Subsequent Taxes', default=False, tracking=True,
        help="If set, taxes with a higher sequence than this one will be affected by it, provided they accept it.")
@@ -213,12 +213,6 @@ class AccountTax(models.Model):
            if record.tax_group_id.country_id and record.tax_group_id.country_id != record.country_id:
                raise ValidationError(_("The tax group must have the same country_id as the tax using it."))

-    @api.constrains('amount_type', 'type_tax_use', 'price_include', 'include_base_amount')
-    def _constrains_fields_after_tax_is_used(self):
-        for tax in self:
-            if tax.is_used:
-                raise ValidationError(_("This tax has been used in transactions. For that reason, it is forbidden to modify this field."))
-
    @api.depends('company_id')
    def _compute_country_id(self):
        for tax in self:
@@ -292,42 +286,55 @@ class AccountTax(models.Model):
        for tax in self:
            tax.is_used = tax.id in used_taxes

-    @api.depends('repartition_line_ids.account_id', 'repartition_line_ids.factor_percent', 'repartition_line_ids.use_in_tax_closing', 'repartition_line_ids.tag_ids')
+    @api.depends('repartition_line_ids.account_id', 'repartition_line_ids.sequence', 'repartition_line_ids.factor_percent', 'repartition_line_ids.use_in_tax_closing', 'repartition_line_ids.tag_ids')
    def _compute_repartition_lines_str(self):
        for tax in self:
            repartition_lines_str = tax.repartition_lines_str or ""
            if tax.is_used:
-                for repartition_line in tax.repartition_line_ids:
-                    repartition_line_info = {
-                        _('id'): repartition_line.id,
+                repartition_line_info = {}
+                invoice_sequence = 0
+                refund_sequence = 0
+                for repartition_line in tax.repartition_line_ids.sorted(key=lambda r: (r.document_type, r.sequence)):
+                    # Clean sequence numbers to avoid unnecessary logging when complex
+                    # operations are executed such as:
+                    #   1. Create a invoice repartition line with a factor of 50%
+                    #   2. Delete the invoice line above
+                    #   3. Update the last refund repartition line factor to 50%
+                    sequence = (invoice_sequence := invoice_sequence + 1) if repartition_line.document_type == 'invoice' else (refund_sequence := refund_sequence + 1)
+                    repartition_line_info[(repartition_line.document_type, sequence)] = {
                        _('Factor Percent'): repartition_line.factor_percent,
-                        _('Account'): repartition_line.account_id.name or _('None'),
+                        _('Account'): repartition_line.account_id.display_name or _('None'),
                        _('Tax Grids'): repartition_line.tag_ids.mapped('name') or _('None'),
                        _('Use in tax closing'): _('True') if repartition_line.use_in_tax_closing else _('False'),
                    }
-                    repartition_lines_str += str(repartition_line_info) + '//'
-                repartition_lines_str = repartition_lines_str.strip('//')
+                repartition_lines_str = str(repartition_line_info)
            tax.repartition_lines_str = repartition_lines_str

-    def _message_log_repartition_lines(self, old_value_str, new_value_str):
+    def _message_log_repartition_lines(self, old_values_str, new_values_str):
        self.ensure_one()
        if not self.is_used:
            return

-        old_values = old_value_str.split('//')
-        new_values = new_value_str.split('//')
+        old_line_values_dict = ast.literal_eval(old_values_str)
+        new_line_values_dict = ast.literal_eval(new_values_str)

-        kwargs = {}
-        for old_value, new_value in zip(old_values, new_values):
-            if old_value != new_value:
-                old_value = ast.literal_eval(old_value)
-                new_value = ast.literal_eval(new_value)
-                diff_keys = [key for key in old_value if old_value[key] != new_value[key]]
-                repartition_line = self.env['account.tax.repartition.line'].search([('id', '=', new_value['id'])])
+        # Categorize the lines that were added/removed/modified
+        modified_lines = [
+            (line, old_line_values_dict[line], new_line_values_dict[line])
+            for line in old_line_values_dict.keys() & new_line_values_dict.keys()
+        ]
+        added_and_deleted_lines = [
+            (line, _('Removed'), old_line_values_dict[line]) if line in old_line_values_dict else (line, _('New'), new_line_values_dict[line])
+            for line in old_line_values_dict.keys() ^ new_line_values_dict.keys()
+        ]
+
+        for (document_type, sequence), old_value, new_value in modified_lines:
+            diff_keys = [key for key in old_value if old_value[key] != new_value[key]]
+            if diff_keys:
                body = Markup("<b>{type}</b> {rep} {seq}:<ul class='mb-0 ps-4'>{changes}</ul>").format(
-                    type=repartition_line.document_type.capitalize(),
+                    type=document_type.capitalize(),
                    rep=_('repartition line'),
-                    seq=repartition_line.sequence + 1,
+                    seq=sequence,
                    changes=Markup().join(
                        [Markup("""
                            <li>
@@ -335,12 +342,31 @@ class AccountTax(models.Model):
                                <i class='o-mail-Message-trackingSeparator fa fa-long-arrow-right mx-1 text-600'/>
                                <span class='o-mail-Message-trackingNew me-1 fw-bold text-info'>{new}</span>
                                <span class='o-mail-Message-trackingField ms-1 fst-italic text-muted'>({diff})</span>
-                            </li>""").format(old=old_value[diff_key], new=new_value[diff_key], diff=diff_key)
+                            </li>
+                        """).format(old=old_value[diff_key], new=new_value[diff_key], diff=diff_key)
                        for diff_key in diff_keys]
                    )
                )
-                kwargs['body'] = body
-                super()._message_log(**kwargs)
+                super()._message_log(body=body)
+
+        for (document_type, sequence), operation, value in added_and_deleted_lines:
+            body = Markup("<b>{op} {type}</b> {rep} {seq}:<ul class='mb-0 ps-4'>{changes}</ul>").format(
+                op=operation,
+                type=document_type.capitalize(),
+                rep=_('repartition line'),
+                seq=sequence,
+                changes=Markup().join(
+                    [Markup("""
+                        <li>
+                            <span class='o-mail-Message-trackingNew me-1 fw-bold text-info'>{value}</span>
+                            <span class='o-mail-Message-trackingField ms-1 fst-italic text-muted'>({diff})</span>
+                        </li>
+                    """).format(value=value[key], diff=key)
+                    for key in value]
+                )
+            )
+            super()._message_log(body=body)
+        return

    def _message_log(self, **kwargs):
        # OVERRIDE _message_log
@@ -1549,20 +1575,6 @@ class AccountTaxRepartitionLine(models.Model):

    tag_ids_domain = fields.Binary(string="tag domain", help="Dynamic domain used for the tag that can be set on tax", compute="_compute_tag_ids_domain")

-    @api.model_create_multi
-    def create(self, vals):
-        tax_ids = {tax_id for line in vals if (tax_id := line.get('tax_id'))}
-        taxes = self.env['account.tax'].browse(tax_ids)
-        for tax in taxes.filtered('is_used'):
-            raise ValidationError(_("The tax named %s has already been used, you cannot add nor delete its tax repartition lines.", tax.name))
-        return super().create(vals)
-
-    @api.ondelete(at_uninstall=False)
-    def _check_tax_use(self):
-        for repartition_line in self:
-            if repartition_line.tax_id.is_used:
-                raise ValidationError(_("The tax named %s has already been used, you cannot add nor delete its tax repartition lines.", repartition_line.tax_id.name))
-
    @api.depends('company_id.multi_vat_foreign_country_ids', 'company_id.account_fiscal_country_id')
    def _compute_tag_ids_domain(self):
        for rep_line in self:
@@ -3,12 +3,45 @@
 from odoo import Command
 from odoo.addons.account.tests.common import AccountTestInvoicingCommon
 from odoo.tests import tagged
-from odoo.exceptions import UserError, ValidationError
+from odoo.exceptions import UserError


@tagged('post_install', '-at_install')
 class TestAccountTax(AccountTestInvoicingCommon):

+    def set_up_and_use_tax(self):
+
+        self.env['account.move'].create({
+            'move_type': 'out_invoice',
+            'date': '2023-01-01',
+            'invoice_line_ids': [
+                Command.create({
+                    'name': 'invoice_line',
+                    'quantity': 1.0,
+                    'price_unit': 100.0,
+                    'tax_ids': [Command.set(self.company_data['default_tax_sale'].ids)],
+                }),
+            ],
+        })
+
+        # Create two lines after creating the move so that those lines are not used in the move
+        self.company_data['default_tax_sale'].write({
+            'invoice_repartition_line_ids': [
+                Command.create({'repartition_type': 'tax'}),
+            ],
+            'refund_repartition_line_ids': [
+                Command.create({'repartition_type': 'tax'}),
+            ],
+        })
+
+        self.flush_tracking()
+        self.assertTrue(self.company_data['default_tax_sale'].is_used)
+
+    def flush_tracking(self):
+        """ Force the creation of tracking values. """
+        self.env.flush_all()
+        self.cr.flush()
+
    def test_changing_tax_company(self):
        ''' Ensure you can't change the company of an account.tax if there are some journal entries '''

@@ -31,6 +64,128 @@ class TestAccountTax(AccountTestInvoicingCommon):
        with self.assertRaises(UserError), self.cr.savepoint():
            self.company_data['default_tax_sale'].company_id = self.company_data_2['company']

+    def test_logging_of_tax_update_when_tax_is_used(self):
+        """ Modifications of a used tax should be logged. """
+
+        self.set_up_and_use_tax()
+
+        self.company_data['default_tax_sale'].write({
+            'name': self.company_data['default_tax_sale'].name + ' MODIFIED',
+            'amount': 21,
+            'amount_type': 'fixed',
+            'type_tax_use': 'purchase',
+            'price_include': True,
+            'include_base_amount': True,
+            'is_base_affected': False,
+        })
+        self.flush_tracking()
+        self.assertEqual(len(self.company_data['default_tax_sale'].message_ids), 1,
+                         "Only 1 message should have been created when updating all the values.")
+        # There are 7 tracked values in account.tax and we update each of them, each on should be included in the message
+        self.assertEqual(len(self.company_data['default_tax_sale'].message_ids.tracking_value_ids), 7,
+                         "The number of updated value should be 7.")
+
+    def test_logging_of_repartition_lines_addition_when_tax_is_used(self):
+        """ Adding repartition lines in a used tax should be logged. """
+
+        self.set_up_and_use_tax()
+
+        self.company_data['default_tax_sale'].write({
+            'invoice_repartition_line_ids': [
+                Command.create({'repartition_type': 'tax'}),
+            ],
+            'refund_repartition_line_ids': [
+                Command.create({'repartition_type': 'tax'}),
+            ],
+        })
+        self.flush_tracking()
+
+        previews = self.company_data['default_tax_sale'].message_ids.mapped('preview')
+        self.assertIn(
+            "New Invoice repartition line 4: 100.0 (Factor Percent) None (Account) None (Tax Grids) False (Use in tax closing)",
+            previews
+        )
+        self.assertIn(
+            "New Refund repartition line 4: 100.0 (Factor Percent) None (Account) None (Tax Grids) False (Use in tax closing)",
+            previews
+        )
+
+    def test_logging_of_repartition_lines_update_when_tax_is_used(self):
+        """ Updating repartition lines in a used tax should be logged. """
+
+        self.set_up_and_use_tax()
+
+        last_invoice_rep_line = self.company_data['default_tax_sale'].invoice_repartition_line_ids.sorted(key=lambda r: r.sequence)[-1]
+        last_refund_rep_line = self.company_data['default_tax_sale'].refund_repartition_line_ids.sorted(key=lambda r: r.sequence)[-1]
+
+        self.company_data['default_tax_sale'].write({
+            "invoice_repartition_line_ids": [
+                Command.update(last_invoice_rep_line.id, {
+                    'factor_percent': -50,
+                    'tag_ids': [Command.create({'name': 'TaxTag12345'})]
+                }),
+            ],
+            "refund_repartition_line_ids": [
+                Command.update(last_refund_rep_line.id, {
+                    'factor_percent': -50,
+                    'account_id': self.company_data['default_account_tax_purchase'].id,
+                }),
+            ],
+        })
+        self.flush_tracking()
+
+        previews = self.company_data['default_tax_sale'].message_ids.mapped('preview')
+        self.assertIn("Invoice repartition line 3: 100.0 -50.0 (Factor Percent) None ['TaxTag12345'] (Tax Grids)", previews)
+        self.assertIn("Refund repartition line 3: 100.0 -50.0 (Factor Percent) None 131000 Tax Paid (Account) False True (Use in tax closing)", previews)
+
+    def test_logging_of_repartition_lines_reordering_when_tax_is_used(self):
+        """ Reordering repartition lines in a used tax should be logged. """
+
+        self.set_up_and_use_tax()
+
+        before_last_invoice_rep_line, last_invoice_rep_line = self.company_data['default_tax_sale'].invoice_repartition_line_ids.sorted(key=lambda r: r.sequence)[-2:]
+
+        self.company_data['default_tax_sale'].write({
+            "invoice_repartition_line_ids": [
+                Command.update(before_last_invoice_rep_line.id, {
+                    'sequence': last_invoice_rep_line.sequence + 1,
+                }),
+            ],
+        })
+        self.flush_tracking()
+
+        previews = self.company_data['default_tax_sale'].message_ids.mapped('preview')
+        self.assertIn("Invoice repartition line 2: 251000 Tax Received None (Account) True False (Use in tax closing)", previews)
+        self.assertIn("Invoice repartition line 3: None 251000 Tax Received (Account) False True (Use in tax closing)", previews)
+
+    def test_logging_of_repartition_lines_removal_when_tax_is_used(self):
+        """ Deleting repartition lines in a used tax should be logged. """
+
+        self.set_up_and_use_tax()
+
+        last_invoice_rep_line = self.company_data['default_tax_sale'].invoice_repartition_line_ids.sorted(key=lambda r: r.sequence)[-1]
+        last_refund_rep_line = self.company_data['default_tax_sale'].refund_repartition_line_ids.sorted(key=lambda r: r.sequence)[-1]
+
+        self.company_data['default_tax_sale'].write({
+            "invoice_repartition_line_ids": [
+                Command.delete(last_invoice_rep_line.id),
+            ],
+            "refund_repartition_line_ids": [
+                Command.delete(last_refund_rep_line.id),
+            ],
+        })
+        self.flush_tracking()
+
+        previews = self.company_data['default_tax_sale'].message_ids.mapped('preview')
+        self.assertIn(
+            "Removed Invoice repartition line 3: 100.0 (Factor Percent) None (Account) None (Tax Grids) False (Use in tax closing)",
+            previews
+        )
+        self.assertIn(
+            "Removed Refund repartition line 3: 100.0 (Factor Percent) None (Account) None (Tax Grids) False (Use in tax closing)",
+            previews
+        )
+
    def test_tax_is_used_when_in_transactions(self):
        ''' Ensures that a tax is set to used when it is part of some transactions '''

@@ -71,57 +226,3 @@ class TestAccountTax(AccountTestInvoicingCommon):
        })
        tax_reconciliation.invalidate_model(fnames=['is_used'])
        self.assertTrue(tax_reconciliation.is_used)
-
-    def test_tax_is_used_restrictions(self):
-        ''' You should not be able to modify some values of a used tax '''
-
-        tax = self.env['account.tax'].create({
-            'name': 'test_is_used',
-            'amount': '100',
-        })
-
-        # A newly created tax should not be used
-        self.assertFalse(tax.is_used)
-
-        self.env['account.move'].create({
-            'move_type': 'out_invoice',
-            'date': '2023-01-01',
-            'invoice_line_ids': [
-                Command.create({
-                    'name': 'invoice_line',
-                    'quantity': 1.0,
-                    'price_unit': 100.0,
-                    'tax_ids': [Command.set(tax.ids)],
-                }),
-            ],
-        })
-        tax.invalidate_model(fnames=['is_used'])
-
-        #   amount_type
-        with self.assertRaises(ValidationError), self.cr.savepoint():
-            tax.amount_type = 'fixed'
-
-        #   type_tax_use
-        with self.assertRaises(ValidationError), self.cr.savepoint():
-            tax.type_tax_use = 'purchase'
-
-        #   price_include
-        with self.assertRaises(ValidationError), self.cr.savepoint():
-            tax.price_include = True
-
-        #   include_base_amount
-        with self.assertRaises(ValidationError), self.cr.savepoint():
-            tax.include_base_amount = True
-
-        #   add repartition lines
-        with self.assertRaises(ValidationError), self.cr.savepoint():
-            self.env['account.tax.repartition.line'].create([
-                {'tax_id': tax.id, 'document_type': 'invoice', 'repartition_type': 'tax', 'factor': -100},
-                {'tax_id': tax.id, 'document_type': 'refund', 'repartition_type': 'tax', 'factor': -100},
-            ])
-
-        #   remove repartition lines
-        with self.assertRaises(ValidationError), self.cr.savepoint():
-            invoice_repartition_lines = tax.invoice_repartition_line_ids
-            for rep_line in invoice_repartition_lines:
-                rep_line.unlink()
@@ -37,7 +37,7 @@
            <field name="model">account.tax.repartition.line</field>
            <field name="arch" type="xml">
                <tree editable="bottom" create="1" delete="1">
-                    <field name="sequence" widget="handle" column_invisible="parent.is_used"/>
+                    <field name="sequence" widget="handle"/>
                    <field name="factor_percent" invisible="repartition_type == 'base'"/>
                    <field name="repartition_type"/>
                    <field name="account_id" invisible="repartition_type == 'base'" options="{'no_create': True}"/>
@@ -140,12 +140,12 @@
                        <group>
                            <field name="name"/>
                            <field name="description"/>
-                            <field name="amount_type" readonly="is_used"/>
+                            <field name="amount_type"/>
                            <field name="active" widget="boolean_toggle"/>
                        </group>
                        <group>
                            <field name="is_used" invisible="1"/>
-                            <field name="type_tax_use" readonly="is_used"/>
+                            <field name="type_tax_use"/>
                            <field name="tax_scope"/>
                            <label for="amount" invisible="amount_type not in ('fixed', 'percent', 'division')"/>
                            <div invisible="amount_type not in ('fixed', 'percent', 'division')">
@@ -184,8 +184,8 @@
                                    <field name="country_id" required="True"/>
                                </group>
                                <group name="advanced_booleans">
-                                    <field name="price_include" invisible="amount_type == 'group'" readonly="is_used"/>
-                                    <field name="include_base_amount" invisible="amount_type == 'group'" readonly="is_used"/>
+                                    <field name="price_include" invisible="amount_type == 'group'"/>
+                                    <field name="include_base_amount" invisible="amount_type == 'group'"/>
                                    <field name="is_base_affected"
                                           invisible="amount_type == 'group' or price_include"
                                           groups="base.group_no_one"/>