-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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] add helper for getting a self env with the tech user #1824
[IMP] add helper for getting a self env with the tech user #1824
Conversation
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.
code review. Thanks
6a12997
to
e13fd3b
Compare
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.
Great! Just one question
base_technical_user/models/models.py
Outdated
return self_sudoer | ||
|
||
|
||
models.BaseModel.sudo_tech = sudo_tech |
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.
Why not doing like this:
class Base(models.AbstractModel):
_inherit = 'base'
def sudo_tech(self, raise_if_missing=False):
self_sudoer = self
tech_user = self.env.user.company_id.user_tech_id
if tech_user:
self_sudoer = self.sudo(tech_user.id)
elif raise_if_missing:
raise UserError("The technical user is missing in the company {}".format(self.env.user.company_id.name))
return self_sudoer
It's more "Odoo"?
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.
Sorry I mix the version. I was think that this was not possible on 10 and only work since 11. But it's work since 9. I fix it
e13fd3b
to
bbf07b7
Compare
bbf07b7
to
7540fe7
Compare
@simahawk fixed ! |
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.
Nice!
7540fe7
to
266d809
Compare
/ocabot merge minor |
This PR looks fantastic, let's merge it! |
Congratulations, your PR was merged at 9636d1a. Thanks a lot for contributing to OCA. ❤️ |
No description provided.