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] account: ensure non empty VALUES in query #162043

Open
wants to merge 1 commit into
base: 17.0
Choose a base branch
from

Conversation

aj-fuentes
Copy link
Contributor

@aj-fuentes aj-fuentes commented Apr 16, 2024

When the companies of system user are all archived we get an invalid SQL syntax on the output of _from. For normal users having their company_id archived is not a standard flow, but system user (Odoo Bot) is an archived user, thus it can be associated to an archived company (usually the id=1 one).

The invalid query is necessary during the installation of some modules. This operation runs as system user.

Steps reproduce:

  1. On a clean DB with just account installed, add an extra company.
  2. Archive company 1 (after switching admin/demo/portal to comp2)
  3. Set allowed companies of system user to just company 1
  4. Try to install gamification_sale_crm

The validation of the domain in for gamification records fail due to the invalid query.

Compare:

>>> self.env['res.currency']._get_query_currency_table(self.env.companies.ids, fields.Date.today())
'(VALUES ) AS currency_table(company_id, rate, precision)'
>>> self.env['res.currency']._get_query_currency_table((self.env.companies | self.env.company).ids, fields.Date.today())
'(VALUES (1, 1.0, 2)) AS currency_table(company_id, rate, precision)'

Setting:

>>> self.env.user
res.users(1,)
>>> self.env.company
res.company(1,)
>>> self.env.company.active
False
>>> self.env.user.with_context(active_test=False).company_ids
res.company(1,)
>>> self.env.companies
res.company()

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

When the companies of system user are all archived we get an invalid SQL
syntax on the output of `_from`. For normal users having their
company_id archived is not a standard flow, but system user (Odoo Bot)
is an archived user, thus it can be associated to an archived company
(usually the id=1 one).

The invalid query is necessary during the installation of some modules.
This operation runs as system user.

Steps reproduce:
1. On a clean DB with just account installed, add an extra company.
2. Archive company 1 (after switching admin/demo/portal to comp2)
3. Set allowed companies of system user to just company 1
4. Try to install gamification_sale_crm

The validation of the domain in for gamification records fail due to the
invalid query.

Compare:
```
>>> self.env['res.currency']._get_query_currency_table(self.env.companies.ids, fields.Date.today())
'(VALUES ) AS currency_table(company_id, rate, precision)'
>>> self.env['res.currency']._get_query_currency_table((self.env.companies | self.env.company).ids, fields.Date.today())
'(VALUES (1, 1.0, 2)) AS currency_table(company_id, rate, precision)'
```

Setting:
```
>>> self.env.user
res.users(1,)
>>> self.env.company
res.company(1,)
>>> self.env.company.active
False
>>> self.env.user.with_context(active_test=False).company_ids
res.company(1,)
```
@robodoo
Copy link
Contributor

robodoo commented Apr 16, 2024

@aj-fuentes
Copy link
Contributor Author

Hi @svfu-odoo and @william-andre I'm not sure if you'd prefer to handle this issue also in env['res.currency']._get_query_currency_table directly. Maybe raising an error if company_ids is empty, or:

if not company_ids:
   return '(VALUES (NULL::int, NULL::float, NULL::int)) AS currency_table(company_id, rate, precision)'

related to b06da79

@C3POdoo C3POdoo added the RD research & development, internal work label Apr 16, 2024
Copy link
Contributor

@william-andre william-andre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rco-odoo Is it expected that self.env.company can not be in self.env.companies?

@aj-fuentes
Copy link
Contributor Author

I updated the PR description with:

>>> self.env.companies
res.company()

It's easily reproducible locally. It looks like self.env.companies only holds active companies. No idea if that's expected or not.

@aj-fuentes aj-fuentes marked this pull request as ready for review April 16, 2024 13:43
@C3POdoo C3POdoo requested review from a team, alialfie and vlst-odoo and removed request for a team April 16, 2024 13:53
@Feyensv
Copy link
Contributor

Feyensv commented Apr 16, 2024

@rco-odoo Is it expected that self.env.company can not be in self.env.companies?

Seems to have been introduced by #102586, maybe to confirm with DLE 👀

@KangOl
Copy link
Contributor

KangOl commented Apr 30, 2024

//cc @beledouxdenis @rco-odoo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants