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

[IMP] website_sale: avoid misconfiguration in multi-website/multi-company #31929

Closed

Conversation

rdeodoo
Copy link
Contributor

@rdeodoo rdeodoo commented Mar 18, 2019

Before this commit, a pricelist could be easily misconfigured when
multi-company and multi-website were both activated.
Eg: website 2 is for company 1, create a pricelist and set website 2 and
company 2, it would make no sense and code would not behave as expected.

Now, we prevent this type of misconfiguration by ensuring a pricelist can't
have a website which is from another company than the company set to the
pricelist.
We also filter website in m2o widget to only show company's websites.

Closes #25109


With this new constraint, l10n module would need to force company:

l10n modules install will change the company currency, creating a pricelist for
that currency. Do not use user's company in that case as module install are
done with OdooBot (company 1).

Step to reproduce:

  • Active multi-company and create a new company
  • Switch to that company and try to install any l10n module not in EUR or USD
  • It will create a new pricelist for that company for that new currency
  • It will crash as module install are done as OdooBot which is in company 1.
    It will search websites in OdooBot company (self.env.user).
    It will then create the pricelist with company 1's website which is
    uncompatible with the new company, thus raising the constraint.

@rdeodoo
Copy link
Contributor Author

rdeodoo commented Mar 18, 2019

Wait forward-port of 28301 to remove part of diff for _default_website

@C3POdoo C3POdoo added the RD research & development, internal work label Mar 18, 2019
@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label Mar 18, 2019
@rdeodoo rdeodoo requested a review from JKE-be March 21, 2019 11:02
@rdeodoo
Copy link
Contributor Author

rdeodoo commented Mar 21, 2019

Your team can take it from there. Wait forward port so _default_website diff will be reduced

@rdeodoo rdeodoo force-pushed the master-imp-pricelist-mw-multicompany-rde branch from 84ac073 to d9dbc94 Compare March 26, 2019 22:09
@robodoo robodoo removed the CI 🤖 Robodoo has seen passing statuses label Mar 26, 2019
…pany

Before this commit, a pricelist could be easily misconfigured when
multi-company and multi-website were both activated.
Eg: website 2 is for company 1, create a pricelist and set website 2 and
    company 2, it would make no sense and code would not behave as expected.

Now, we prevent this type of misconfiguration by ensuring a pricelist can't
have a website which is from another company than the company set to the
pricelist.
We also filter website in m2o widget to only show company's websites.

Closes odoo#25109

---------------

With this new constraint, l10n module would need to force company:

l10n modules install will change the company currency, creating a pricelist for
that currency. Do not use user's company in that case as module install are
done with OdooBot (company 1).

Step to reproduce:
  - Active multi-company and create a new company
  - Switch to that company and try to install any l10n module not in EUR or USD
  - It will create a new pricelist for that company for that new currency
  - It will crash as module install are done as OdooBot which is in company 1.
    It will search websites in OdooBot company (self.env.user).
    It will then create the pricelist with company 1's website which is
    uncompatible with the new company, thus raising the constraint.
@rdeodoo rdeodoo force-pushed the master-imp-pricelist-mw-multicompany-rde branch from d9dbc94 to 852ea39 Compare March 26, 2019 22:13
@rdeodoo
Copy link
Contributor Author

rdeodoo commented Mar 26, 2019

Forward-port hit master. This PR got rebased and should be clean for review/merge.

@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label Mar 27, 2019
@JKE-be
Copy link
Contributor

JKE-be commented Mar 29, 2019

@robodoo r+

robodoo pushed a commit that referenced this pull request Mar 29, 2019
…pany

Before this commit, a pricelist could be easily misconfigured when
multi-company and multi-website were both activated.
Eg: website 2 is for company 1, create a pricelist and set website 2 and
    company 2, it would make no sense and code would not behave as expected.

Now, we prevent this type of misconfiguration by ensuring a pricelist can't
have a website which is from another company than the company set to the
pricelist.
We also filter website in m2o widget to only show company's websites.

Closes #25109

---------------

With this new constraint, l10n module would need to force company:

l10n modules install will change the company currency, creating a pricelist for
that currency. Do not use user's company in that case as module install are
done with OdooBot (company 1).

Step to reproduce:
  - Active multi-company and create a new company
  - Switch to that company and try to install any l10n module not in EUR or USD
  - It will create a new pricelist for that company for that new currency
  - It will crash as module install are done as OdooBot which is in company 1.
    It will search websites in OdooBot company (self.env.user).
    It will then create the pricelist with company 1's website which is
    uncompatible with the new company, thus raising the constraint.

closes #31929

Signed-off-by: Jérémy Kersten (jke) <jke@openerp.com>
@robodoo robodoo added r+ 👌 and removed r+ 👌 CI 🤖 Robodoo has seen passing statuses labels Mar 29, 2019
@robodoo
Copy link
Contributor

robodoo commented Mar 29, 2019

Staging failed: ci/runbot on 7830439a9307b4b8735da57f22aed751eef28d87 (view more at http://runbot.odoo.com/runbot/build/488921)

@rdeodoo
Copy link
Contributor Author

rdeodoo commented Mar 29, 2019

can you retry?

@JKE-be
Copy link
Contributor

JKE-be commented Mar 29, 2019

@robodoo r+

@JKE-be
Copy link
Contributor

JKE-be commented Mar 29, 2019

@robodoo retry

@robodoo
Copy link
Contributor

robodoo commented Mar 29, 2019

I'm sorry, @JKE-be. This PR is already reviewed, reviewing it again is useless.

@JKE-be
Copy link
Contributor

JKE-be commented Mar 29, 2019

@robodoo be kind and retry plz

@JKE-be JKE-be removed the error 🙅 label Mar 29, 2019
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses r+ 👌 labels Mar 29, 2019
robodoo pushed a commit that referenced this pull request Mar 29, 2019
…pany

Before this commit, a pricelist could be easily misconfigured when
multi-company and multi-website were both activated.
Eg: website 2 is for company 1, create a pricelist and set website 2 and
    company 2, it would make no sense and code would not behave as expected.

Now, we prevent this type of misconfiguration by ensuring a pricelist can't
have a website which is from another company than the company set to the
pricelist.
We also filter website in m2o widget to only show company's websites.

Closes #25109

---------------

With this new constraint, l10n module would need to force company:

l10n modules install will change the company currency, creating a pricelist for
that currency. Do not use user's company in that case as module install are
done with OdooBot (company 1).

Step to reproduce:
  - Active multi-company and create a new company
  - Switch to that company and try to install any l10n module not in EUR or USD
  - It will create a new pricelist for that company for that new currency
  - It will crash as module install are done as OdooBot which is in company 1.
    It will search websites in OdooBot company (self.env.user).
    It will then create the pricelist with company 1's website which is
    uncompatible with the new company, thus raising the constraint.

closes #31929

Signed-off-by: Jérémy Kersten (jke) <jke@openerp.com>
@robodoo
Copy link
Contributor

robodoo commented Mar 29, 2019

Merged, thanks!

@robodoo robodoo closed this Mar 29, 2019
@rdeodoo rdeodoo deleted the master-imp-pricelist-mw-multicompany-rde branch April 1, 2019 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI 🤖 Robodoo has seen passing statuses RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants