Skip to content
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] mail: do not crash if a partner has more than one mail.channel.… #31259

Closed
wants to merge 1 commit into
base: saas-12.1
from

Conversation

Projects
None yet
6 participants
@len-odoo
Copy link
Contributor

len-odoo commented Feb 20, 2019

…partner

It is assumed that there is a unique mail.channel.partner per (channel, partner)
pair, yet there is nothing enforcing this restriction.
As a result field access on the channel partner crashes on field access.

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

@len-odoo len-odoo requested a review from alexkuhn Feb 20, 2019

@robodoo robodoo added the seen 🙂 label Feb 20, 2019

@C3POdoo C3POdoo added the OE label Feb 20, 2019

@robodoo robodoo added the CI 🤖 label Feb 20, 2019

@alexkuhn
Copy link
Contributor

alexkuhn left a comment

Fix is okay, though this is quite inconvenient that there is no constraint enforcing single pair partner_id/channel_id in this table. For instance, the search could pick the wrong record...

@Yenthe666

This comment has been minimized.

Copy link
Contributor

Yenthe666 commented Feb 20, 2019

Fix is okay, though this is quite inconvenient that there is no constraint enforcing single pair partner_id/channel_id in this table. For instance, the search could pick the wrong record...

+1. This can give results back to end users which might make no sense and can be wrong. Perhaps for now a limit=1 and a constraint in master?

@len-odoo

This comment has been minimized.

Copy link
Contributor Author

len-odoo commented Feb 20, 2019

You're more likely to know how multiple channel partners are created, I just know it happens a lot on our prod.
If you have any idea of what should really be fixed go ahead. But it will also require a way to clean up existing data for databases that are already in production.
(For now it gives notifications that result in traceback and thus you can't never get rid of.)

@Yenthe666

This comment has been minimized.

Copy link
Contributor

Yenthe666 commented Feb 20, 2019

Hmm good point @len-odoo
How about some warning when the user creates a new channel partner for a user where there is already one existing? If the user still continues he has been warned and it is at own risk then.

@len-odoo

This comment has been minimized.

Copy link
Contributor Author

len-odoo commented Feb 22, 2019

robodoo r+

@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Feb 22, 2019

Staging failed: ci/runbot (view more at http://runbot.odoo.com/runbot/build/458910)

@len-odoo

This comment has been minimized.

Copy link
Contributor Author

len-odoo commented Feb 22, 2019

robodoo retry

@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Feb 22, 2019

Staging failed: ci/runbot (view more at http://runbot.odoo.com/runbot/build/459028)

@robodoo robodoo added the error 🙅 label Feb 22, 2019

@len-odoo len-odoo force-pushed the odoo-dev:saas-12.1-nopw-sgtn-len branch Feb 22, 2019

@KangOl

This comment has been minimized.

Copy link
Contributor

KangOl commented Feb 22, 2019

robodoo retry

@KangOl

This comment has been minimized.

Copy link
Contributor

KangOl commented Feb 22, 2019

@robodoo robodoo added the r+ 👌 label Feb 22, 2019

@robodoo robodoo added CI 🤖 and removed merging 👷 r+ 👌 labels Feb 22, 2019

@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Feb 22, 2019

Staging failed: ci/runbot on 763c30155cce296455f2b506f6788cd63c2ca9fd (view more at http://runbot.odoo.com/runbot/build/459667)

@robodoo robodoo added error 🙅 and removed CI 🤖 labels Feb 22, 2019

@KangOl

This comment has been minimized.

Copy link
Contributor

KangOl commented Feb 22, 2019

@robodoo retry

@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Feb 22, 2019

Staging failed: ci/runbot on 422d95a2c6e28bad93426c234b5ad5fbfdc3959f (view more at http://runbot.odoo.com/runbot/build/459792)

@len-odoo

This comment has been minimized.

Copy link
Contributor Author

len-odoo commented Feb 26, 2019

robodoo retry

@robodoo robodoo added CI 🤖 r+ 👌 and removed error 🙅 labels Feb 26, 2019

robodoo pushed a commit that referenced this pull request Feb 26, 2019

[FIX] mail: do not crash if a partner has more than one mail.channel.…
…partner

It is assumed that there is a unique mail.channel.partner per (channel, partner)
pair, yet there is nothing enforcing this restriction.
As a result field access on the channel partner crashes on field access.

closes #31259
@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Feb 26, 2019

Merged, thanks!

@robodoo robodoo closed this Feb 26, 2019

@len-odoo len-odoo deleted the odoo-dev:saas-12.1-nopw-sgtn-len branch Feb 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.