FIX acc_fisc_classif: find or create a classif if none

[bealdav]

• test

This PR is needed to install all modules of the branch

cc @gurneyalex @yankinmax

[OCA-git-bot]

Hi @legalsylvain,
some modules you are maintaining are being modified, check this out!

[legalsylvain]

Hi @bealdav. thanks a lot for fixing that point !

• just a point to fix, the case where there is no taxes and no classification in vals in create mode.
• otherwise, some minor remarks inline.

[bealdav]

Thanks a lot @legalsylvain for your review, less code, more test

[legalsylvain]

Hi @bealdav.

regarding create_mode, it was not exactly what I mean. I made a PR that include the changes. See commit messages and feel free to contact me if you have any doubt.

could you take a look and merge it ?

akretion#4

once done, the algorithm has to be fixed ! _find_or_create_classification doesn't return the correct classification. But this is a huge part.

See in a shell on demo data :

>>> bob = self.env["product.template"].create({"name": "TEST", "taxes_id": [], "supplier_taxes_id": []})
>>> bob.fiscal_classification_id.sale_tax_ids
account.tax(2,)
>>> bob.fiscal_classification_id.purchase_tax_ids
account.tax(1,)

[mourad-ehm]

LGTM

[legalsylvain]

@mourad-ehm : what do you think about the bug described here ? #435 (review)

[mourad-ehm]

I discussed with @bealdav about this bug. He will fix it.

[legalsylvain]

It looks like the current algorithm create new fiscal classification each time. Don't you think ?

2024-06-13 22:20:46,653 166314 INFO account_product_fiscal_classification_16_imp odoo.addons.account_product_fiscal_classification.models.product_template: Creating new Fiscal Classification 'Purchase Taxes: Demo Purchase Tax 7% (Company 2) + Demo Purchase Tax 30% (Company 2) - No Sale Taxes' for False 
'Purchase Taxes: Demo Purchase Tax 7% (Company 2) + Demo Purchase Tax 30% (Company 2) - No Sale Taxes'
>>> self.env["product.template"].create({"name": "TEST Coincoin", "supplier_taxes_id": [4, 5]}).fiscal_classification_id.name
2024-06-13 22:20:50,997 166314 INFO account_product_fiscal_classification_16_imp odoo.addons.account_product_fiscal_classification.models.product_template: Creating new Fiscal Classification 'Purchase Taxes: Demo Purchase Tax 7% (Company 2) + Demo Purchase Tax 30% (Company 2) - No Sale Taxes' for False 
'Purchase Taxes: Demo Purchase Tax 7% (Company 2) + Demo Purchase Tax 30% (Company 2) - No Sale Taxes'
>>> self.env["product.template"].create({"name": "TEST Coincoin", "supplier_taxes_id": [4, 5]}).fiscal_classification_id.name

[bealdav]

You're right !

# this code fails to find classification
>>> env["account.product.fiscal.classification"].search([('sale_tax_ids', 'in', []), ('purchase_tax_ids', 'in', [])])
account.product.fiscal.classification()
# this code achieve to find classifications
>>> env["account.product.fiscal.classification"].search([('sale_tax_ids', '=', False), ('purchase_tax_ids', '=', False)])
account.product.fiscal.classification(5, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44)
# syntax in incorrect
>>> env["account.product.fiscal.classification"].search([('sale_tax_ids', '=', []), ('purchase_tax_ids', '=', [])])
2024-06-14 10:12:07,924 337 WARNING db3 odoo.osv.expression: The domain term '('sale_tax_ids', '=', [])' should use the 'in' or 'not in' operator. 
2024-06-14 10:12:07,924 337 WARNING db3 odoo.osv.expression: The domain term '('purchase_tax_ids', '=', [])' should use the 'in' or 'not in' operator. 

Then the code to define domain should be

        domain = [("sale_tax_ids", "=", False)]
        if sale_tax_ids:
            domain = [("sale_tax_ids", "in", sale_tax_ids)]
        if purchase_tax_ids:
            domain.append(("purchase_tax_ids", "in", purchase_tax_ids))
        else:
            domain.append(("purchase_tax_ids", "=", False))

Are you ok ? @legalsylvain

I have done some additional tests in last commit to secure this kind of failures

[legalsylvain]

I don't think that line works :

domain = [("sale_tax_ids", "in", sale_tax_ids)]

In mean, if sale_tax_ids = [2] and sale_tax_ids = [2, 3] the domain is respected, but we should not select this classification. Don't you think ?

[bealdav]

Finalement je vois pas d'autres solution qui marche a 100%

        domain = [("sale_tax_ids", "=", False)]
        if sale_tax_ids:
            domain = [("sale_tax_ids", "=", sale_tax_ids)]
        if purchase_tax_ids:
            domain.append(("purchase_tax_ids", "=", purchase_tax_ids))
        else:
            domain.append(("purchase_tax_ids", "=", False))

On pourras rendre cela plus joli tard, mais si cela marche comme ça on pourrais merger ?

[legalsylvain]

As said privatly the algorithm to guess classification, based on taxes is wrong.

See tests : akretion#6

Thanks for your work, @bealdav !

[legalsylvain]

supersed by #441

[legalsylvain]

Hi @bealdav and @mourad-ehm. I fixed this PR, in #441, but I need review. Could you take a time to review it ?

thanks a lot.