[14.0][ADD] account_fiscal_product_rule

[chafique-delli]

No description provided.

[chafique-delli]

@sebastienbeau

[sebastienbeau]

In order to reduce the setup, it will be better to re-use the existing setup class of odoo.
https://github.com/odoo/odoo/blob/657fd6653705255f319756cba6406cefed9649d4/addons/account/tests/common.py#L11

You can do

from odoo.addons.account.tests.common import AccountTestInvoicingCommon

Then for the test you can use

self.product_a
self.partner_b (that have the fiscal position a) : https://github.com/odoo/odoo/blob/657fd6653705255f319756cba6406cefed9649d4/addons/account/tests/common.py#L148
and you can use the method : init_invoice (with the partner_b and the product a : https://github.com/odoo/odoo/blob/657fd6653705255f319756cba6406cefed9649d4/addons/account/tests/common.py#L366)

Then in your setup you only need to create

• a new tax (you can copy it, with a new rate copy(amount=30))
• a new receivable account (use the method copy_account : https://github.com/odoo/odoo/blob/657fd6653705255f319756cba6406cefed9649d4/addons/account/tests/common.py#L14)
• a fiscal product rule

For the test the aim is to test the impact of the fiscal_product_rule

So you should write 3 tests

def test_no_rule(self):
    invoice = self.init_invoice(partner=self.partner_b, products=self.product_a)
    # check the tax/account

def test_rule_on_categ(self):
    self.product.categ_id.fiscal_position_product_rule_ids = self.fiscal_product_rule
    invoice = self.init_invoice(partner=self.partner_b, products=self.product_a)
    # check the tax/account is the on define by the rule

def test_rule_on_product(self):
    self.product.fiscal_position_product_rule_ids = self.fiscal_product_rule
    invoice = self.init_invoice(partner=self.partner_b, products=self.product_a)
    # check the tax/account is the on define by the rule

With that solution you should reduce a lot the number of line of your test

[chafique-delli]

@sebastienbeau , modifications made on the tests following your comments.

[sebastienbeau]

you can rename rule instead of fiscal_product_rules

Suggested change

	fiscal_product_rules = (
	rule = (

Note we should have only one rule (see python constraint in product.py)

[sebastienbeau]

We should add a python constraint to be sure to not put 2 account.fiscal.position.rule related to the same fiscal position

something like

def check_no_duplicate_fiscal_position(self):
    for record in self:
        fps = []
        for rule in record.fiscal_position_product_rule_ids:
            if rule.fiscal_position_id in fps:
                raise ValidationError(...)
            else:
                fps.append(rule.fiscal_position_id)

To avoid duplicated code for this constraint. You can create a mixin (fiscal.position.product.rule.owner) and put the code for the field definition "fiscal_position_product_rule_ids" and the constrainte

[sebastienbeau]

• add the test that add 2 fiscal rule with the same fiscal position, it should raise an error (you can test only the product or the categ as the code is the same).

[sebastienbeau]

/!\ BE CAREFUL /!\
When doing a loop for and inside this loop you use return, in most of the time you do something really wrong !

Because if somebody call with method with several record it will return after processing the first element.

In that case it do not produce a bug because we always call map_tax with only one fiscal position. But adding a loop for mean that you expect several and make harder to understand the code (and also will avoid to raise an error if somewhere the code use an ensure_one)

So remove the loop and use self direct as there is only one element

[sebastienbeau]

You can write shorter/cleaner code

Something like

Suggested change

	if fiscal_product_rules:
	res = self.env["account.tax"]
	if (
	taxes[0].type_tax_use == "sale"
	and fiscal_product_rules[0].seller_tax_ids
	):
	res = fiscal_product_rules[0].seller_tax_ids[0]
	if (
	taxes[0].type_tax_use == "purchase"
	and fiscal_product_rules[0].supplier_tax_ids
	):
	res = fiscal_product_rules[0].supplier_tax_ids[0]
	if res:
	return res
	if rule and taxes:
	tax_use = taxes[0].type_tax_use
	if tax_use == "sale" and rule.seller_tax_ids:
	return rule.seller_tax_ids
	elif tax_use == "purchase" and rule.supplier_tax_ids:
	return rule.supplier_tax_ids

And please use if/elif and not only if

[sebastienbeau]

you can just do

prod = product or self._context.get("product")

avoid to set in the variable "product" the value passed by the context, as if we call super we will inject the product and this may produce error (as odoo do not pass it)

[sebastienbeau]

code is toooooo complex

just do this and the taxes will be the right one directly in the native code, we should avoid to call twice map taxes

Suggested change

	super()._onchange_product_id()
	for line in self:
	if not line.product_id or line.display_type in (
	"line_section",
	"line_note",
	):
	continue
	taxes = line._get_computed_taxes()
	if (
	taxes
	and line.move_id.fiscal_position_id.fiscal_position_product_rule_ids
	):
	taxes = line.move_id.fiscal_position_id.map_tax(
	taxes, line.product_id, line.partner_id
	)
	line.tax_ids = taxes
	line.product_uom_id = line._get_computed_uom()
	line.price_unit = line.with_context(
	product_id=line.product_id.id
	)._get_computed_price_unit()
	for record in self:
	super(
	AccountMoveLine, record.with_context(product=line.product_id)
	)._onchange_product_id()

[sebastienbeau]

do not make them optionnal, user will forget to fill it

[sebastienbeau]

We do not need a menu, we will fill it from the fiscal position

[sebastienbeau]

We do not need action/menu, we will fill it from the fiscal position

[jbaudoux]

As the rule is not dependent anymore from the (product) source tax, what about price included or not?
I think you need to ensure the new tax has the same price_include state

[github-actions]

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

[github-actions]

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.