[14.0][FIX] product_multi_company: recompute variant companies #590
Conversation
ilyasProgrammer
force-pushed
the
14.0-product_multi_company-fix-585
branch
from
437b94b
to
7900459
Compare
ilyasProgrammer
force-pushed
the
14.0-product_multi_company-fix-585
branch
from
7900459
to
aa7bd6e
Compare
|
This doesn't seem the correct approach, as it causes an overhead on all models. Why don't you redefine the field as computed in product variant, like other fields like |
Looks like it worked as well. |
| comodel_name="res.company", | ||
| relation="product_variant_companies_rel", | ||
| compute="_compute_company_ids", | ||
| store=True, |
There was a problem hiding this comment.
question: do you think a compute_sudo=True is warranted here?
There was a problem hiding this comment.
I think it needs to be computed with current user.
There was a problem hiding this comment.
From what I understand of the base module, company_ids contains all the companies (including the ones the current user cannot see) and it's company_id that needs to be computed with the current user.
In general, if it needs to be computed with the current user it shouldn't be stored, and the other way around as well, if it's stored, the result shouldn't change depending on the user that last triggered the computation.
There was a problem hiding this comment.
(Could this just be a related field? I don't know if it would work that way)
There was a problem hiding this comment.
It is related by default because of inheritance. Even if you make that relation explicit it doesnt work. As well as with store=False.
There was a problem hiding this comment.
It shouldn't depend on the current user for sure. The problem is that I'm giving you tips blindly, as I don't understand the reported issue and it doesn't seem related to this.
There was a problem hiding this comment.
Is there something I can improve in the issue itself to make it clearer? Is there anything in particular that you think is underexplained?
There was a problem hiding this comment.
The problem is that I can't reproduce the issue in this runboat:
and the error message about company employee rule indicates an issue with other module, not this one.
ilyasProgrammer
force-pushed
the
14.0-product_multi_company-fix-585
branch
2 times, most recently
from
59e8596
to
bb4fcee
Compare
| comodel_name="res.company", | ||
| relation="product_variant_companies_rel", | ||
| compute="_compute_company_ids", | ||
| store=True, |
|
With store=False error is still there |
|
Uhm, that's very weird. This should be an ORM fault, as there's nothing in the business side leading to the problem. OK, restore it and clean commit history, and let me check once again. |
ilyasProgrammer
force-pushed
the
14.0-product_multi_company-fix-585
branch
from
076599a
to
77d77d5
Compare
ilyasProgrammer
force-pushed
the
14.0-product_multi_company-fix-585
branch
from
77d77d5
to
d248a27
Compare
There was a problem hiding this comment.
It seems to work.
I think the inner problem is that in v13 it was falsely assumed that there's no need now to remove the auxiliary model:
https://github.com/OCA/multi-company/blob/12.0/base_multi_company/models/res_company_assignment.py
|
/ocabot merge minor |
|
This PR looks fantastic, let's merge it! |
|
Congratulations, your PR was merged at ec4609a. Thanks a lot for contributing to OCA. ❤️ |
FIX #585
Need to force recompute related product.product company_ids field. Otherwise it takes wrong values from the cache.