-
-
Notifications
You must be signed in to change notification settings - Fork 392
[18.0][FIX] hr_timesheet_sheet_attendance: prevent AccessError for non-admin users #858
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
[18.0][FIX] hr_timesheet_sheet_attendance: prevent AccessError for non-admin users #858
Conversation
…n-admin users Non-admin users could get an AccessError when creating a timesheet sheet because it triggers a write on hr.attendance.
| ] | ||
| ) | ||
| attendances._compute_sheet_id() | ||
| attendances.sudo()._compute_sheet_id() |
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.
Add compute_sudo=True to the field instead?
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.
I tried adding compute_sudo=True to sheet_id = fields.Many2one(...), but it didn’t work.
Since _compute_sheet_id is called from create, it seems that create needs to be executed with sudo().
| Returns last attendance record""" | ||
|
|
||
| return self.employee_id._attendance_action_change() | ||
| return self.employee_id.sudo()._attendance_action_change() |
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 double-check if this is a valid fix. Wouldn't this allow updating attendance records of other employees?
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 part seems to require sudo() to write to hr.attendance.
The write access to hr.attendance is handled the same way in Odoo’s standard code as well.
https://github.com/odoo/odoo/blob/ad1c367d9d3e14cb7196416e117a13b8a11e9d12/addons/hr_attendance/controllers/main.py#L135-L139
yostashiro
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.
Code review. LGTM.
AungKoKoLin1997
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.
LGTM
|
This PR has the |
|
Hey @kanda999 thank you for your contribution! |
|
What a great day to merge this nice PR. Let's do it! |
|
It looks like something changed on |
|
Congratulations, your PR was merged at a3618e9. Thanks a lot for contributing to OCA. ❤️ |
Non-admin users could get an AccessError when creating a timesheet sheet because it triggers a write on hr.attendance.
@qrtl QT6155