-
Notifications
You must be signed in to change notification settings - Fork 6
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
[16.0][IMP] sale_import_base: add crm_team_id and methods to inherit sale.channel.importer #87
Conversation
Tests are red because this PR depends now on the module sale_channel from the OCA repo sale-channel. What is the best solution to solve this kind of problem when you have the same module on 2 different repo ? Is it possible to just remove sale_channel from this repo? |
[("default_code", "=", line_data["product_code"])] | ||
[ | ||
("default_code", "=", line_data["product_code"]), | ||
("product_tmpl_id.company_id", "=", company_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.
I think it is not ok, because we could have products shared between companies, in that case, the company_id is not set and we should find the product.
I believe we should at least check if company is the same as the channel OR is empty
Is it possible to just remove sale_channel from this repo? I think it should work indeed. But anyway, oca modules and current one are equals, are they not ? |
if not product: | ||
product = self.env["product.product"].search( | ||
[ | ||
("default_code", "=", line_data["product_code"]), | ||
("product_tmpl_id.company_id", "=", False), | ||
] | ||
) |
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.
@florian-dacosta I added this separated search to avoid raising error in case of having a product with the same code and both with the company_id and without.
Do you think it's OK like this?
@florian-dacosta I made the PR to put sale_import_base on OCA : OCA/sale-channel#21 Should we continue the conversation there? |
PR extracted from the original #83 to start pushing sale_import_base to OCA already with these modifications :
@bealdav @florian-dacosta @sebastienbeau