-
Notifications
You must be signed in to change notification settings - Fork 30.6k
[REF] stock: use custom company in stock tests #231842
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
[REF] stock: use custom company in stock tests #231842
Conversation
3ac3f55 to
1060fe9
Compare
0f6e54d to
d6263f3
Compare
055f8e5 to
84b262e
Compare
8a1668d to
d1e1881
Compare
d1e1881 to
f7ec7ad
Compare
svs-odoo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yop ! Here the first review !
I let you some comments but it's pretty minor, it seems already very good :)
addons/stock/tests/test_product.py
Outdated
| """ Checks we can't change the product's company if this product has | ||
| quant in another company. """ | ||
| company1 = self.env.ref('base.main_company') | ||
| company1 = self.company |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can remove this line and use directly self.company in the test (same for test_change_product_company_02)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Used self.company and renamed company2 to another_company 👍
| self.env.user.write({'group_ids': [Command.link(grp_multi_companies.id)]}) | ||
|
|
||
| company_2 = self.company | ||
| company_2 = self.company_2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you keep this variable to limite the change but I think you can ride off it.
It's useless and since you already refactor the tests here, it's the occasion to remove dead code and useless code.
Same comment for following tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed in both tests 👍
addons/mrp/tests/test_replenish.py
Outdated
| route_manufacture = self.warehouse_1.manufacture_pull_id.route_id.id | ||
| route_mto = self.warehouse_1.mto_pull_id.route_id.id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| route_manufacture = self.warehouse_1.manufacture_pull_id.route_id.id | |
| route_mto = self.warehouse_1.mto_pull_id.route_id.id | |
| route_manufacture_id = self.warehouse_1.manufacture_pull_id.route_id.id | |
| route_mto_id = self.warehouse_1.mto_pull_id.route_id.id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed, thanks 👍
| @classmethod | ||
| def setUpClass(cls): | ||
| super().setUpClass() | ||
| cls.company = cls.env['res.company'].create({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, a warehouse is automatically created when the company is created, so instead of creating a new warehouse for cls.warehouse_1, I think you should use the company's warehouse
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to use the company warehouse (with the original values) 👍
| self.start_tour('/odoo/purchase', "test_purchase_order_suggest_search_panel_ux", login='admin') | ||
| self.start_tour( | ||
| '/odoo/purchase', "test_purchase_order_suggest_search_panel_ux", login='admin', | ||
| cookies={"cids": f"{self.company.id}"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that using cookies to force the company is a "rubber tape" solution to run the tour with the right config.
The good solution would be to run it with the right user (res_users_purchase_user for example, so login pu instead of admin) but for now, this solution doesn't work because of an access right error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, should I keep my version for now, but add a comment to clarify?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| cls.uom_unit = cls.env.ref('uom.product_uom_unit') | ||
| cls.uom_pack_6 = cls.env.ref('uom.product_uom_pack_6') | ||
| cls.wh = cls.env['stock.warehouse'].search([('company_id', '=', cls.env.user.id)], limit=1) | ||
| cls.wh = cls.warehouse_1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove this variable and use warehouse_1 directly instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed 👍
| cls.warehouse = warehouse_form.save() | ||
|
|
||
| cls.uom_unit = cls.env.ref('uom.product_uom_unit') | ||
| cls.warehouse = cls.warehouse_1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think with this kind of changes, it doesn't make a lot of sense to keep variable like this to minimize the change.
I think it would be better to remove them and use directly the real variable (warehouse_1 here.) After all, it's a refactor PR :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed 👍
33cdcf0 to
d4298c5
Compare
b7aa854 to
c2dbfa4
Compare
… custom company in stock tests This is a part of stock tests refactoring. The final goal is to use only dedicated data in all stock tests, including a dedicated user to run the tests. This change introduces only a custom company.
c2dbfa4 to
85121a8
Compare

Description of the issue/feature this PR addresses:
Current behavior before PR:
Desired behavior after PR is merged:
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr