[IMP] account: Logging and tracking of some fields.

Problem
---------
All fields in the tax definition are currently modifiable even after the
tax has been used. This raises some issues in tax reports and make
investigations difficult when things go wrong in tax report.

Objective
---------
Once an account tax has been used:
 - Some fields in the account tax should not be modifiable anymore to
 avoid tax report issues;
 - Other fields should be logged when modified to ease debugging and
 investigations.

Solution
---------
1. Add a logger to the tax form to track the modification of some fields
2. Track whether the task is being used in transactions or not. This is
done thanks to a compute field. In order to be flexible, the compute
function uses a hook function that other modules can override to easily
add transactions to computation of the field.
3. Make the fields that should not be modified anymore readonly when the
tax is being used.
4. In order to track the repartition line values, a tracked computed
field that stores the relevant value in a string is used.

Task-3450002

Part-of: odoo/odoo#130403
This commit is contained in:
Antoine Boonen
2023-10-20 14:59:13 +00:00
parent 38956115b4
commit 8d77045b46
12 changed files with 343 additions and 12 deletions
+130 -6
View File
@@ -3,10 +3,13 @@ from odoo import api, fields, models, _, Command
from odoo.osv import expression
from odoo.tools.float_utils import float_round
from odoo.exceptions import UserError, ValidationError
from odoo.tools.misc import formatLang
from odoo.tools.misc import clean_context, formatLang
from odoo.tools import frozendict, groupby
from collections import defaultdict
from collections import Counter, defaultdict
from markupsafe import Markup
import ast
import math
import re
@@ -80,13 +83,14 @@ class AccountTaxGroup(models.Model):
class AccountTax(models.Model):
_name = 'account.tax'
_inherit = ['mail.thread']
_description = 'Tax'
_order = 'sequence,id'
_check_company_auto = True
_rec_names_search = ['name', 'description', 'invoice_label']
_check_company_domain = models.check_company_domain_parent_of
name = fields.Char(string='Tax Name', required=True, translate=True)
name = fields.Char(string='Tax Name', required=True, translate=True, tracking=True)
name_searchable = fields.Char(store=False, search='_search_name',
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'."
@@ -114,16 +118,17 @@ class AccountTax(models.Model):
string='Children Taxes')
sequence = fields.Integer(required=True, default=1,
help="The sequence field is used to define order in which the tax lines are applied.")
amount = fields.Float(required=True, digits=(16, 4), default=0.0)
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,
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,
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.")
is_base_affected = fields.Boolean(
string="Base Affected by Previous Taxes",
default=True,
tracking=True,
help="If set, taxes with a lower sequence might affect this one, provided they try to do it.")
analytic = fields.Boolean(string="Include in Analytic Cost", help="If set, the amount computed by this tax will be assigned to the same analytic account as the invoice line (if any)")
tax_group_id = fields.Many2one(
@@ -175,6 +180,8 @@ class AccountTax(models.Model):
help="The country for which this tax is applicable.",
)
country_code = fields.Char(related='country_id.code', readonly=True)
is_used = fields.Boolean(string="Tax used", compute='_compute_is_used')
repartition_lines_str = fields.Char(string="Repartition Lines", tracking=True, compute='_compute_repartition_lines_str')
@api.constrains('company_id', 'name', 'type_tax_use', 'tax_scope')
def _constrains_name(self):
@@ -201,6 +208,12 @@ 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.account_fiscal_country_id')
def _compute_country_id(self):
for tax in self:
@@ -225,6 +238,93 @@ class AccountTax(models.Model):
('country_id', '=', False),
], limit=1)
def _hook_compute_is_used(self):
'''
To be overriden to add taxed transactions in the computation of `is_used`
Should return a Counter containing a dictionary {record: int} where
the record is an account.tax object. The int should be greater than 0
if the tax is used in a transaction.
'''
return Counter()
def _compute_is_used(self):
taxes_in_transactions_ctr = (
Counter(dict(self.env['account.move.line']._read_group([], groupby=['tax_ids'], aggregates=['__count']))) +
Counter(dict(self.env['account.reconcile.model.line']._read_group([], groupby=['tax_ids'], aggregates=['__count']))) +
self._hook_compute_is_used()
)
for tax in self:
tax.is_used = bool(taxes_in_transactions_ctr[tax])
@api.depends('repartition_line_ids.account_id', '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,
_('Factor Percent'): repartition_line.factor_percent,
_('Account'): repartition_line.account_id.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('//')
tax.repartition_lines_str = repartition_lines_str
def _message_log_repartition_lines(self, old_value_str, new_value_str):
self.ensure_one()
if not self.is_used:
return
old_values = old_value_str.split('//')
new_values = new_value_str.split('//')
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'])])
body = Markup("<b>{type}</b> {rep} {seq}:<ul class='mb-0 ps-4'>{changes}</ul>").format(
type=repartition_line.document_type.capitalize(),
rep=_('repartition line'),
seq=repartition_line.sequence + 1,
changes=Markup().join(
[Markup("""
<li>
<span class='o-mail-Message-trackingOld me-1 px-1 text-muted fw-bold'>{old}</span>
<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)
for diff_key in diff_keys]
)
)
kwargs['body'] = body
super()._message_log(**kwargs)
def _message_log(self, **kwargs):
# OVERRIDE _message_log
# We only log the modification of the tracked fields if the tax is
# currently used in transactions. We remove the `repartition_lines_str`
# from tracked value to avoid having it logged twice (once in the raw
# string format and one in the nice formatted way thanks to
# `_message_log_repartition_lines`)
self.ensure_one()
if self.is_used:
repartition_line_str_field_id = self.env['ir.model.fields']._get('account.tax', 'repartition_lines_str').id
for tracked_value_id in kwargs['tracking_value_ids']:
if tracked_value_id[2]['field_id'] == repartition_line_str_field_id:
kwargs['tracking_value_ids'].remove(tracked_value_id)
self._message_log_repartition_lines(tracked_value_id[2]['old_value_char'], tracked_value_id[2]['new_value_char'])
return super()._message_log(**kwargs)
@api.depends('company_id')
def _compute_invoice_repartition_line_ids(self):
for tax in self:
@@ -359,7 +459,14 @@ class AccountTax(models.Model):
@api.model_create_multi
def create(self, vals_list):
return super().create([self._sanitize_vals(vals) for vals in vals_list])
context = clean_context(self.env.context)
context.update({
'mail_create_nosubscribe': True, # At create or message_post, do not subscribe the current user to the record thread
'mail_auto_subscribe_no_notify': True, # Do no notify users set as followers of the mail thread
'mail_create_nolog': True, # At create, do not log the automatic ‘<Document> created’ message
})
taxes = super(AccountTax, self.with_context(context)).create([self._sanitize_vals(vals) for vals in vals_list])
return taxes
def write(self, vals):
return super().write(self._sanitize_vals(vals))
@@ -1382,6 +1489,23 @@ 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 = list(set([line.get('tax_id') for line in vals])) # Sorted
taxes = self.env['account.tax'].search_fetch([('id', 'in', tax_ids)], ['name'], order='id ASC')
tax_dict = dict(zip(tax_ids, taxes))
for line in vals:
tax = tax_dict.get(line.get('tax_id'))
if tax and tax.is_used:
raise ValidationError(_("The tax named {} has already been used, you cannot add nor delete its tax repartition lines.").format(tax.name))
return super().create(vals)
def unlink(self):
for repartition_line in self:
if repartition_line.tax_id.is_used:
raise ValidationError(_("The tax named {} has already been used, you cannot add nor delete its tax repartition lines.").format(repartition_line.tax_id.name))
return super().unlink()
@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:
+98 -1
View File
@@ -1,7 +1,9 @@
# -*- coding: utf-8 -*-
from odoo import Command
from odoo.addons.account.tests.common import AccountTestInvoicingCommon
from odoo.tests import tagged
from odoo.exceptions import UserError
from odoo.exceptions import UserError, ValidationError
@tagged('post_install', '-at_install')
@@ -28,3 +30,98 @@ 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_tax_is_used_when_in_transactions(self):
''' Ensures that a tax is set to used when it is part of some transactions '''
# Account.move is one type of transaction
tax_invoice = self.env['account.tax'].create({
'name': 'test_is_used_invoice',
'amount': '100',
})
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_invoice.ids)],
}),
],
})
tax_invoice.invalidate_model(fnames=['is_used'])
self.assertTrue(tax_invoice.is_used)
# Account.reconcile is another of transaction
tax_reconciliation = self.env['account.tax'].create({
'name': 'test_is_used_reconcilition',
'amount': '100',
})
self.env['account.reconcile.model'].create({
'name': "test_tax_is_used",
'rule_type': 'writeoff_suggestion',
'auto_reconcile': False,
'line_ids': [Command.create({
'account_id': self.company_data['default_account_revenue'].id,
'tax_ids': [Command.set(tax_reconciliation.ids)],
})],
})
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()
+10 -5
View File
@@ -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"/>
<field name="sequence" widget="handle" column_invisible="parent.is_used"/>
<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,11 +140,12 @@
<group>
<field name="name"/>
<field name="description"/>
<field name="amount_type"/>
<field name="amount_type" readonly="is_used"/>
<field name="active" widget="boolean_toggle"/>
</group>
<group>
<field name="type_tax_use"/>
<field name="is_used" invisible="1"/>
<field name="type_tax_use" readonly="is_used"/>
<field name="tax_scope"/>
<label for="amount" invisible="amount_type not in ('fixed', 'percent', 'division')"/>
<div invisible="amount_type not in ('fixed', 'percent', 'division')">
@@ -183,8 +184,8 @@
<field name="country_id" required="True"/>
</group>
<group name="advanced_booleans">
<field name="price_include" invisible="amount_type == 'group'" />
<field name="include_base_amount" invisible="amount_type == 'group'" />
<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="is_base_affected"
invisible="amount_type == 'group' or price_include"
groups="base.group_no_one"/>
@@ -196,6 +197,10 @@
</page>
</notebook>
</sheet>
<div class="oe_chatter">
<field name="message_follower_ids" widget="mail_followers"/>
<field name="message_ids" widget="mail_thread"/>
</div>
</form>
</field>
</record>
+1
View File
@@ -5,6 +5,7 @@ from . import hr_employee
from . import account_move
from . import account_move_line
from . import account_payment
from . import account_tax
from . import hr_department
from . import hr_expense
from . import product_template
+15
View File
@@ -0,0 +1,15 @@
# -*- coding: utf-8 -*-
from collections import Counter
from odoo import models
class AccountTax(models.Model):
_inherit = "account.tax"
def _hook_compute_is_used(self):
# OVERRIDE in order to count the usage of taxes in expenses
taxes_in_transactions_ctr = Counter(dict(self.env['hr.expense']._read_group([], groupby=['tax_ids'], aggregates=['__count'])))
return super()._hook_compute_is_used() + taxes_in_transactions_ctr
+1
View File
@@ -6,3 +6,4 @@ from . import test_expenses
from . import test_expenses_access_rights
from . import test_expenses_mail_import
from . import test_expenses_multi_company
from . import test_expenses_tax
@@ -0,0 +1,28 @@
# -*- coding: utf-8 -*-
from odoo import Command
from odoo.addons.hr_expense.tests.common import TestExpenseCommon
from odoo.tests import tagged
@tagged('post_install', '-at_install')
class TestExpensesTax(TestExpenseCommon):
def test_tax_is_used_when_in_transactions(self):
''' Ensures that a tax is set to used when it is part of some transactions '''
# Account.move is one type of transaction
tax_expense = self.env['account.tax'].create({
'name': 'test_is_used_expenses',
'amount': '100',
'include_base_amount': True,
})
self.env['hr.expense'].create({
'name': 'Test Tax Used',
'employee_id': self.expense_employee.id,
'product_id': self.product_a.id,
'unit_amount': 350.00,
'tax_ids': [Command.set(tax_expense.ids)]
})
tax_expense.invalidate_model(fnames=['is_used'])
self.assertTrue(tax_expense.is_used)
@@ -1,5 +1,6 @@
# -*- coding: utf-8 -*-
from collections import Counter
from odoo import _, api, models
from odoo.exceptions import UserError
from odoo.tools import split_every
@@ -26,3 +27,10 @@ class AccountTax(models.Model):
))
lines_chunk.invalidate_recordset(['tax_ids'])
return super(AccountTax, self).write(vals)
def _hook_compute_is_used(self):
# OVERRIDE in order to count the usage of taxes in pos order lines
taxes_in_transactions_ctr = Counter(dict(self.env['pos.order.line']._read_group([], groupby=['tax_ids'], aggregates=['__count'])))
return super()._hook_compute_is_used() + taxes_in_transactions_ctr
@@ -541,3 +541,13 @@ class TestPoSProductsWithTax(TestPoSCommon):
{'account_id': self.sale_account.id, 'balance': 0},
{'account_id': self.cash_pm1.receivable_account_id.id, 'balance': 1},
])
def test_tax_is_used_when_in_transactions(self):
''' Ensures that a tax is set to used when it is part of some transactions '''
# Call another test that uses product_1
tax_pos = self.product1.taxes_id
self.assertFalse(tax_pos.is_used)
self.test_orders_no_invoiced()
tax_pos.invalidate_model(fnames=['is_used'])
self.assertTrue(tax_pos.is_used)
+1
View File
@@ -4,6 +4,7 @@
from . import account_invoice
from . import analytic_account
from . import analytic_applicability
from . import account_tax
from . import purchase
from . import product
from . import res_company
+15
View File
@@ -0,0 +1,15 @@
# -*- coding: utf-8 -*-
from collections import Counter
from odoo import models
class AccountTax(models.Model):
_inherit = "account.tax"
def _hook_compute_is_used(self):
# OVERRIDE in order to count the usage of taxes in purchase order lines
taxes_in_transactions_ctr = Counter(dict(self.env['purchase.order.line']._read_group([], groupby=['taxes_id'], aggregates=['__count'])))
return super()._hook_compute_is_used() + taxes_in_transactions_ctr
@@ -1,4 +1,6 @@
# -*- coding: utf-8 -*-
from odoo import Command
from odoo.addons.account.tests.test_invoice_tax_totals import TestTaxTotals
from odoo.tests import tagged
@@ -34,3 +36,27 @@ class PurchaseTestTaxTotals(TestTaxTotals):
'partner_id': self.partner_a.id,
'order_line': lines_vals,
})
def test_tax_is_used_when_in_transactions(self):
''' Ensures that a tax is set to used when it is part of some transactions '''
# Account.move is one type of transaction
tax_purchase = self.env['account.tax'].create({
'name': 'test_is_used_purchase',
'amount': '100',
})
self.env['purchase.order'].create({
'partner_id': self.partner_a.id,
'order_line': [
Command.create({
'name': 'order_line',
'product_id': self.po_product.id,
'product_qty': 1.0,
'price_unit': 100.0,
'taxes_id': [Command.set(tax_purchase.ids)],
}),
],
})
tax_purchase.invalidate_model(fnames=['is_used'])
self.assertTrue(tax_purchase.is_used)