[FIX] analytic: plan's applicability don't work without companies #162152
Conversation
gawa-odoo
force-pushed
the
17.0-analytic-applicabilities_companies-gawa
branch
from
1e916d0
to
86cb417
Compare
gawa-odoo
requested review from
a team and
RicGR98
and removed request for
a team
| @@ -76,8 +76,7 @@ def test_validate_company_plans(self): | |||
| 'name': 'Mandatory Plan', | |||
| 'default_applicability': 'optional', | |||
| }]) | |||
| mandatory_plan.with_company(company_2).write({'default_applicability': 'mandatory'}) | |||
There was a problem hiding this comment.
The mandatory_plan created above is no more mandatory without this line. Is this normal ?
There was a problem hiding this comment.
Well it wasn't quite mandatory, it was only mandatory for self.env.company
And either this definition, or the applicability defined below was completely useless as they both did the same thing (on the only thing that was tested).
To be able to test other situations, I removed the default_applicability.
Doing so, I now changed the name of the variable, to reflect the difference
There was a problem hiding this comment.
Okaaaay, got it ! Thanks :)
gawa-odoo
force-pushed
the
17.0-analytic-applicabilities_companies-gawa
branch
from
86cb417
to
a068b6d
Compare
gawa-odoo
force-pushed
the
17.0-analytic-applicabilities_companies-gawa
branch
from
a068b6d
to
44692ba
Compare
| score = applicability_without_company._get_score(business_domain='invoice', product=self.product_a.id) | ||
| self.assertEqual(score, 2, "company_id shouldn't be taken into account") | ||
| score = applicability_with_company._get_score(business_domain='invoice', product=self.product_a.id) | ||
| self.assertEqual(score, 1, "company_id shouldn't be taken into account either") |
There was a problem hiding this comment.
I find the message a bit misleading here (especially the "either"). Not sure those two (this one and the one above) make the test any more explicit. Wouldn't it be just better, to leave those two assertEqual without error message ? The "product takes precedence over company" above already states the expected behavior, if I get the test well 🤔
There was a problem hiding this comment.
Not exactly, we're not really testing the same thing.
But still, I reworded the message to make clearer what I wanted to test in these 2 asserts as indeed, it wasn't explicit
gawa-odoo
force-pushed
the
17.0-analytic-applicabilities_companies-gawa
branch
2 times, most recently
from
c77a581
to
aaf18a4
Compare
If you create an applicability and remove the company field, they are never used. An applicability like this should be valid for all companies. We put a 0.5 value for the company field so an applicability so it has a lesser priority than other fields. Same idea as the distribution models. opw-3847415
gawa-odoo
force-pushed
the
17.0-analytic-applicabilities_companies-gawa
branch
from
aaf18a4
to
6221341
Compare
| @@ -76,8 +76,7 @@ def test_validate_company_plans(self): | |||
| 'name': 'Mandatory Plan', | |||
| 'default_applicability': 'optional', | |||
| }]) | |||
| mandatory_plan.with_company(company_2).write({'default_applicability': 'mandatory'}) | |||
There was a problem hiding this comment.
Okaaaay, got it ! Thanks :)
If you create an applicability and remove the company field, they are never used. An applicability like this should be valid for all companies. We put a 0.5 value for the company field so an applicability so it has a lesser priority than other fields. Same idea as the distribution models. opw-3847415 closes #162152 Signed-off-by: Olivier Colson (oco) <oco@odoo.com>
If you create an applicability and remove the company field, they are never used. An applicability like this should be valid for all companies. We put a 0.5 value for the company field so an applicability so it has a lesser priority than other fields. Same idea as the distribution models. opw-3847415 closes #162152 Signed-off-by: Olivier Colson (oco) <oco@odoo.com>
If you create an applicability and remove the company field, they are never used. An applicability like this should be valid for all companies. We put a 0.5 value for the company field so an applicability so it has a lesser priority than other fields. Same idea as the distribution models. opw-3847415 closes odoo#162152 Signed-off-by: Olivier Colson (oco) <oco@odoo.com>
If you create an applicability and remove the company field, they are never used.
An applicability like this should be valid for all companies.
We put a 0.5 value for the company field so an applicability so it has a lesser priority than other fields.
Same idea as the distribution models.
opw-3847415
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr