-
Notifications
You must be signed in to change notification settings - Fork 30.4k
[FIX] crm: Lead 2 opportunities error when no salespersons #88108
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
[FIX] crm: Lead 2 opportunities error when no salespersons #88108
Conversation
872f971 to
33526d7
Compare
kitan191
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.
Technically, LGTM
|
@tde-banana-odoo Can you review this ? Thanks ! |
|
FYI: some discussions are ongoing on task 2824913 |
|
@tde-banana-odoo Is there any news regarding this issue ? Have you had the time to discuss it with others in the team ? |
1 similar comment
|
@tde-banana-odoo Is there any news regarding this issue ? Have you had the time to discuss it with others in the team ? |
tde-banana-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.
Validation !
addons/crm/models/crm_lead.py
Outdated
| proposal = lead.user_id.company_id & self.env.companies | ||
| elif lead.team_id: | ||
| proposal = lead.team_id.company_id | ||
| elif lead.partner_id and lead.partner_id.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.
I don't think we want to force a new company based on the customer's one ... shouldn't we keep it void instead ? Generally this means less issues.
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.
- This review contradicts your first one concerning splitting the
ifcondition. We cannot apply both reviews, it would break the fix. - I would say that it's best to avoid having a void company in most cases as there are validation upon saving the record. Plus, the way the new condition is placed enforced to avoid having unnecessary warning message upon saving.
addons/crm/models/crm_lead.py
Outdated
| proposal = False | ||
| # no user and no team -> void company and let assignment do its job | ||
| if not lead.team_id and not lead.user_id: | ||
| if not lead.team_id and not lead.user_id and not (lead.partner_id and proposal == lead.partner_id.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.
About code: would make a new entry in the "if" list to clearly explain the case, aka something like
# no user and no team -> void company and let assignment do its job
if not lead.team_id and not lead.user_id:
proposal = False
# incompatible with partner's company
if proposal and lead.partner_id.company_id != proposal:
proposal= False
|
@tde-banana-odoo Is the changes ok for you now ? |
|
Hello, I still did not have time to test in details. I am unsure this fix is the right one, and for tricky process like that I would prefer taking time to review it carefully. Will try to have a look soon :) |
33526d7 to
beab457
Compare
Currently using a customer with a company set on a lead without company crashes, as it keep a void company on the lead. This is not compatible with the company set on the partner itself. OPW-2805181 Task-2888330
40d97a5 to
0807e80
Compare
|
Hello, I added some tests and rebased. @tfr-odoo could you have a look and see if it is ok for you ? Cheers ! |
In a multi-company environment, when we transform a lead (which has no salesperson) into an opportunity, we get an error if the customer has a company set. To reproduce the issue 1) Create a new company 2) Create a Lead for a customer with the Company set 3) Remove the Sales Team 4) Set the company on the Lead 5) Convert to an opportunity and you will see an error showing up Solution A previous commit (1fad826) adapted the `_compute_company_id` computation in which a case was forgotten (the case described here-above). The fix was to force the `company_id` to the one of the `partner_id` if it has one. OPW-2805181 Task-2888330
0807e80 to
644c6e5
Compare
For odoo.com it won't make any difference since our partner have no company and check_company is False on crm.lead. I found a bit sad to reach this level of complexity to ensure the company consistency on a object like lead where is not really needed but that's my humble opinion Nevertheless this fix seems consistent |
|
@tfr-odoo I agree this should probably be simplified, company check is a bit too aggressive for lead. But in stable we have to ensure it runs smoothly :) . |
|
@robodoo r+ rebase-ff |
|
Merge method set to rebase and fast-forward |
|
Staging failed: ci/runbot (view more at https://runbot.odoo.com/runbot/build/16681036) |
|
@robodoo retry |
Currently using a customer with a company set on a lead without company crashes, as it keep a void company on the lead. This is not compatible with the company set on the partner itself. OPW-2805181 Task-2888330 Part-of: odoo#88108 X-original-commit: 51be6ca
In a multi-company environment, when we transform a lead (which has no salesperson) into an opportunity, we get an error if the customer has a company set. To reproduce the issue 1) Create a new company 2) Create a Lead for a customer with the Company set 3) Remove the Sales Team 4) Set the company on the Lead 5) Convert to an opportunity and you will see an error showing up Solution A previous commit (1fad826) adapted the `_compute_company_id` computation in which a case was forgotten (the case described here-above). The fix was to force the `company_id` to the one of the `partner_id` if it has one. OPW-2805181 Task-2888330 closes odoo#88108 Signed-off-by: Thibault Delavallee (tde) <[email protected]> X-original-commit: fc3bb66
In a multi-company environment, when we transform a lead (which has no salesperson) into an opportunity, we get an error if the customer has a company set. To reproduce the issue 1) Create a new company 2) Create a Lead for a customer with the Company set 3) Remove the Sales Team 4) Set the company on the Lead 5) Convert to an opportunity and you will see an error showing up Solution A previous commit (1fad826) adapted the `_compute_company_id` computation in which a case was forgotten (the case described here-above). The fix was to force the `company_id` to the one of the `partner_id` if it has one. OPW-2805181 Task-2888330 closes #88108 closes #94483 X-original-commit: fc3bb66 Signed-off-by: Thibault Delavallee (tde) <[email protected]>
In a multi-company environment, when we transform a lead (which has no salesperson) into an opportunity, we get an error if the customer has a company set. To reproduce the issue 1) Create a new company 2) Create a Lead for a customer with the Company set 3) Remove the Sales Team 4) Set the company on the Lead 5) Convert to an opportunity and you will see an error showing up Solution A previous commit (1fad826) adapted the `_compute_company_id` computation in which a case was forgotten (the case described here-above). The fix was to force the `company_id` to the one of the `partner_id` if it has one. OPW-2805181 Task-2888330 closes #88108 closes #94493 X-original-commit: fc3bb66 Signed-off-by: Thibault Delavallee (tde) <[email protected]>
In a multi-company environment, when we transform a lead (which has no salesperson) into an opportunity, we get an error if the customer has a company set. To reproduce the issue 1) Create a new company 2) Create a Lead for a customer with the Company set 3) Remove the Sales Team 4) Set the company on the Lead 5) Convert to an opportunity and you will see an error showing up Solution A previous commit (1fad826) adapted the `_compute_company_id` computation in which a case was forgotten (the case described here-above). The fix was to force the `company_id` to the one of the `partner_id` if it has one. OPW-2805181 Task-2888330 closes #88108 closes #94473 X-original-commit: fc3bb66 Signed-off-by: Thibault Delavallee (tde) <[email protected]>
In a multi-company environment, when we transform a lead (which has no salesperson) into an opportunity, we get an error if the customer has a company set. To reproduce the issue 1) Create a new company 2) Create a Lead for a customer with the Company set 3) Remove the Sales Team 4) Set the company on the Lead 5) Convert to an opportunity and you will see an error showing up Solution A previous commit (1fad826) adapted the `_compute_company_id` computation in which a case was forgotten (the case described here-above). The fix was to force the `company_id` to the one of the `partner_id` if it has one. OPW-2805181 Task-2888330 closes #88108 closes #94462 X-original-commit: fc3bb66 Signed-off-by: Thibault Delavallee (tde) <[email protected]>
In a multi-company environment, when we transform a lead (which has no salesperson) into an opportunity, we get an error if the customer has a company set. To reproduce the issue 1) Create a new company 2) Create a Lead for a customer with the Company set 3) Remove the Sales Team 4) Set the company on the Lead 5) Convert to an opportunity and you will see an error showing up Solution A previous commit (1fad826) adapted the `_compute_company_id` computation in which a case was forgotten (the case described here-above). The fix was to force the `company_id` to the one of the `partner_id` if it has one. OPW-2805181 Task-2888330 closes #88108 closes #94473 X-original-commit: fc3bb66 Signed-off-by: Thibault Delavallee (tde) <[email protected]>
In a multi-company environment, when we transform a lead (which has no salesperson) into an opportunity, we get an error if the customer has a company set. To reproduce the issue 1) Create a new company 2) Create a Lead for a customer with the Company set 3) Remove the Sales Team 4) Set the company on the Lead 5) Convert to an opportunity and you will see an error showing up Solution A previous commit (1fad826) adapted the `_compute_company_id` computation in which a case was forgotten (the case described here-above). The fix was to force the `company_id` to the one of the `partner_id` if it has one. OPW-2805181 Task-2888330 closes #88108 closes #94462 X-original-commit: fc3bb66 Signed-off-by: Thibault Delavallee (tde) <[email protected]>
In a multi-company environment, when we transform a lead (which has no salesperson) into an opportunity, we get an error if the customer has a company set. To reproduce the issue 1) Create a new company 2) Create a Lead for a customer with the Company set 3) Remove the Sales Team 4) Set the company on the Lead 5) Convert to an opportunity and you will see an error showing up Solution A previous commit (1fad826) adapted the `_compute_company_id` computation in which a case was forgotten (the case described here-above). The fix was to force the `company_id` to the one of the `partner_id` if it has one. OPW-2805181 Task-2888330 closes #88108 closes #94473 X-original-commit: fc3bb66 Signed-off-by: Thibault Delavallee (tde) <[email protected]>
1 similar comment
Currently using a customer with a company set on a lead without company crashes, as it keep a void company on the lead. This is not compatible with the company set on the partner itself. OPW-2805181 Task-2888330 Part-of: odoo#88108 X-original-commit: 51be6ca
In a multi-company environment, when we transform a lead (which has no salesperson) into an opportunity, we get an error if the customer has a company set. To reproduce the issue 1) Create a new company 2) Create a Lead for a customer with the Company set 3) Remove the Sales Team 4) Set the company on the Lead 5) Convert to an opportunity and you will see an error showing up Solution A previous commit (1fad826) adapted the `_compute_company_id` computation in which a case was forgotten (the case described here-above). The fix was to force the `company_id` to the one of the `partner_id` if it has one. OPW-2805181 Task-2888330 closes odoo#88108 Signed-off-by: Thibault Delavallee (tde) <[email protected]> X-original-commit: fc3bb66
Currently using a customer with a company set on a lead without company crashes, as it keep a void company on the lead. This is not compatible with the company set on the partner itself. OPW-2805181 Task-2888330 Part-of: odoo#88108 X-original-commit: 51be6ca
In a multi-company environment, when we transform a lead (which has no salesperson) into an opportunity, we get an error if the customer has a company set. To reproduce the issue 1) Create a new company 2) Create a Lead for a customer with the Company set 3) Remove the Sales Team 4) Set the company on the Lead 5) Convert to an opportunity and you will see an error showing up Solution A previous commit (1fad826) adapted the `_compute_company_id` computation in which a case was forgotten (the case described here-above). The fix was to force the `company_id` to the one of the `partner_id` if it has one. OPW-2805181 Task-2888330 closes odoo#88108 Signed-off-by: Thibault Delavallee (tde) <[email protected]> X-original-commit: fc3bb66
In a multi-company environment, when we transform a lead (which has no salesperson) into an opportunity, we get an error if the customer has a company set. To reproduce the issue 1) Create a new company 2) Create a Lead for a customer with the Company set 3) Remove the Sales Team 4) Set the company on the Lead 5) Convert to an opportunity and you will see an error showing up Solution A previous commit (1fad826) adapted the `_compute_company_id` computation in which a case was forgotten (the case described here-above). The fix was to force the `company_id` to the one of the `partner_id` if it has one. OPW-2805181 Task-2888330 closes #88108 closes #94473 X-original-commit: fc3bb66 Signed-off-by: Thibault Delavallee (tde) <[email protected]>
In a multi-company environment, when we transform a lead (which has no salesperson) into an opportunity, we get an error if the customer has a company set. To reproduce the issue 1) Create a new company 2) Create a Lead for a customer with the Company set 3) Remove the Sales Team 4) Set the company on the Lead 5) Convert to an opportunity and you will see an error showing up Solution A previous commit (1fad826) adapted the `_compute_company_id` computation in which a case was forgotten (the case described here-above). The fix was to force the `company_id` to the one of the `partner_id` if it has one. OPW-2805181 Task-2888330 closes #88108 closes #94462 X-original-commit: fc3bb66 Signed-off-by: Thibault Delavallee (tde) <[email protected]>

In a multi-company environment, when we transform a lead (which has
no salesperson) into an opportunity, we get an error.
To reproduce the issue:
Solution:
A previous commit (1fad826) adapted the
_compute_company_idcomputation in which a case was forgotten (the case described here-above). The fix was to force
the
company_idto the one of thepartner_idif it has one.opw-2805181