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

[FW][FIX] sql_db: flush when no uid in environment #48757

Conversation

fw-bot
Copy link
Contributor

@fw-bot fw-bot commented Apr 1, 2020

  • A bug has been introduced by the PR [FIX] odoo: don't commit outside of a checked call #46719
    This issue prevented flushes to database when no uid is set in the
    current environment.
    This happens when database changes are made in auth="none" routes.
    In a MonoDB setup this was working because Odoo consider None, False as
    null and was always bound to a database.

    Nothing prevents to write null in the columns create_uid and
    write_uid in database.
    The PR that introduced the bug wanted to block the cases where
    uid is an instance of the class RequestUID to avoid Cannot adapt type errors.
    So testing that uid is an integer isn't enough.

    Two fixes were possible here, either check if uid is an instance
    of the class RequestUID or if it is either an integer, False or None.

    To avoid noise and imports from odoo, we test that uid is an instance
    of None.
    The case where it is False is already tested in the python code:

      isinstance(env.uid, int)

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

Forward-Port-Of: #48685

- A bug has been introduced by the PR odoo#46719
  This issue prevented flushes to database when no `uid` is set in the
  current environment.
  This happens when database changes are made in `auth="none"` routes.
  In a MonoDB setup this was working because Odoo consider `None`, `False` as
  `null` and was always bound to a database.

  Nothing prevents to write `null` in the columns `create_uid` and
  `write_uid` in database.
  The PR that introduced the bug wanted to block the cases where
  `uid` is an instance of the class `RequestUID` to avoid `Cannot adapt
  type` errors.
  So testing that `uid` is an integer isn't enough.

  Two fixes were possible here, either check if `uid` is an instance
  of the class `RequestUID` or if it is either an integer, `False` or `None`.

  To avoid noise and imports from `odoo`, we test that `uid` is an instance
  of `None`.
  The case where it is `False` is already tested in the python code:

  ```python
	isinstance(env.uid, int)
  ```

X-original-commit: 276f22c
@robodoo robodoo added conflict There was an error while creating this forward-port PR forwardport This PR was created by @fw-bot seen 🙂 labels Apr 1, 2020
@fw-bot
Copy link
Contributor Author

fw-bot commented Apr 1, 2020

Ping @tbe-odoo
Cherrypicking e2504d9 of source #48685 failed

stdout:

On branch saas-13.2-13.0-flush-no-uid-fix-tbe-mk0m-fw
You are currently cherry-picking commit e2504d90e8f.

nothing to commit, working tree clean

stderr:

The previous cherry-pick is now empty, possibly due to conflict resolution.
If you wish to commit it anyway, use:

    git commit --allow-empty

Otherwise, please use 'git reset'

Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).

In the former case, you may want to edit this PR message as well.

@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label Apr 1, 2020
@C3POdoo C3POdoo added the RD research & development, internal work label Apr 1, 2020
@tbe-odoo tbe-odoo closed this Apr 2, 2020
@tbe-odoo tbe-odoo deleted the saas-13.2-13.0-flush-no-uid-fix-tbe-mk0m-fw branch April 2, 2020 07:10
@robodoo robodoo added closed 💔 and removed CI 🤖 Robodoo has seen passing statuses labels Apr 2, 2020
@tbe-odoo
Copy link
Contributor

tbe-odoo commented Apr 2, 2020

Forward ported manually

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflict There was an error while creating this forward-port PR forwardport This PR was created by @fw-bot RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants