-
-
Notifications
You must be signed in to change notification settings - Fork 584
[18.0][IMP] base_tier_validation: support res.groups in addition to res.users in field-based validation #1081
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
base: 18.0
Are you sure you want to change the base?
Conversation
Hi @LoisRForgeFlow, |
7e58ca6
to
6425290
Compare
6425290
to
b0a4f98
Compare
0e382fc
to
f9db455
Compare
return self.env["res.users"] | ||
|
||
def _check_reviewers(self, resource): | ||
if not resource or resource._name not in ["res.users", "res.groups"]: |
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.
You don't actually seem to pass groups here. Shouldn't resource always be res.users
?
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.
@kevinkhao Just a reminder that this review is still pending to attend
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.
right! @StefanRijnhart fixed/mini refactored here cc064b5
…rs in field-based validation
9f96b57
to
cc064b5
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.
Thanks for the changes so far! One remaining comment.
if reviewer_field_val._name == "res.users": | ||
res = reviewer_field_val | ||
if not res: | ||
raise ValidationError( |
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.
Can you add a line in your test to check this error can be raised successfully? Should make codecov happy.
No description provided.