-
-
Notifications
You must be signed in to change notification settings - Fork 316
[18.0][MIG] base_user_effective_permissions: Migration to 18.0 #354
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?
[18.0][MIG] base_user_effective_permissions: Migration to 18.0 #354
Conversation
e61b84b to
d35c607
Compare
d35c607 to
aedf873
Compare
aedf873 to
ab6a1ae
Compare
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
Currently translated at 100.0% (26 of 26 strings) Translation: server-backend-16.0/server-backend-16.0-base_user_effective_permissions Translate-URL: https://translation.odoo-community.org/projects/server-backend-16-0/server-backend-16-0-base_user_effective_permissions/it/
ab6a1ae to
11a0a46
Compare
|
Hey @OCA/tools-maintainers would be great if someone could have a look at this module. |
|
@hbrunn : could you take a look on this migration ? |
hbrunn
left a comment
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.
/ocabot migration base_user_effective_permissions
| f"{operation}_domain": IrRule._compute_domain( | ||
| for operation in operations: | ||
| try: | ||
| model.check_access(operation) |
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.
this is not the equivalent of v16's model.check_access_rights - use ir.model.access#check instead
| vals[f"{operation}_permission"] = True | ||
| except Exception: | ||
| vals[f"{operation}_permission"] = False | ||
| try: |
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.
under which conditions is this expected to raise? I'd expect something to be very wrong when this happens, and then we shouldn't squelch the exception
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 didn't answer this question
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.
The _compute_domain() method can raise an AccessError when:
- The user doesn't have access to certain ir.rule records
- Domain evaluation fails due to missing fields or invalid domain syntax
- Security restrictions prevent domain computation for specific models
In these cases, setting an empty domain "[]" is appropriate as it indicates no domain-based restrictions beyond the basic access rights check already performed by ir.model.access#check.
However, you're right that we shouldn't squelch all exceptions. I'll update the code to catch only AccessError
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.
obvious copy&paste from coding assistant, no interest to continue discussion
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.
@hbrunn here I accept that I used AI but its only to clean up the wording, since sometimes I struggle to words things well. Sorry if that caused any confusion.
Next onwards I will try to best to answer well.
11a0a46 to
1f9939e
Compare
|
@hbrunn : I’ve made the changes as per your suggestion. Could you please review? Thanks!” |
1f9939e to
2e88e57
Compare
No description provided.