-
-
Notifications
You must be signed in to change notification settings - Fork 140
[15.0][ADD] hr_attendance_rest_time_included: New module #240
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: 15.0
Are you sure you want to change the base?
[15.0][ADD] hr_attendance_rest_time_included: New module #240
Conversation
96d925f to
156f5c5
Compare
pedrobaeza
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.
@eduezerouali-tecnativa you have to add at attendance reason level, when being "Sign out" one, a check for saying if it's a rest time included in the attendance.
|
I see you have added it as a configuration option inside settings, but it should be at attendance reason level, as it can be more than one reason. |
pedrobaeza
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.
Apart from the comments inline, include in the ROADMAP that this module is not totally compatible with hr_attendance_geolocation, as I have seen that the sign out coordinates are stored in the main attendance record while signing out the rest time. On upper versions, where the geolocation is included, we will need to check how to make it compatible.
| class HrAttendance(models.Model): | ||
| _inherit = "hr.attendance" | ||
|
|
||
| hr_attendance_rest_time = fields.One2many( |
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 inside hr.attendance, so the prefix hr_attendance is useless, redundant and takes a lot of space, and on the other side, use _ids suffix convention for o2m:
| hr_attendance_rest_time = fields.One2many( | |
| rest_time_ids = fields.One2many( |
| _name = "hr.attendance.rest_time" | ||
| _inherit = "hr.attendance" |
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.
For indicating the intention of inheritance with new model, better to put before _inherit and then _name:
| _name = "hr.attendance.rest_time" | |
| _inherit = "hr.attendance" | |
| _inherit = "hr.attendance" | |
| _name = "hr.attendance.rest_time" |
| def _attendance_action_change(self): | ||
| self.ensure_one() | ||
| RestTime = self.env["hr.attendance.rest_time"] | ||
|
|
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.
Don't add empty lines inside a method (it applies to all the code).
hr_attendance_rest_time_included/report/hr_attendance_report_views.xml
Outdated
Show resolved
Hide resolved
8e897da to
35c4406
Compare
|
All changes apply ! |
pedrobaeza
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.
Apart from the inline comments, we are almost there. Some other changes:
-
It seems the original chatter is being rewritten when checking out/in the rest time:
-
There's no navigation nor visualization of the rest time attendances. Put a button next to the rest time column to go to them, and also a smart-button on the main attendance record, both only visible when there are rest time attendances.
hr_attendance_rest_time_included/models/hr_attendance_rest_time.py
Outdated
Show resolved
Hide resolved
hr_attendance_rest_time_included/views/hr_attendance_reason_views.xml
Outdated
Show resolved
Hide resolved
hr_attendance_rest_time_included/models/hr_attendance_reason.py
Outdated
Show resolved
Hide resolved
|
Small comments already fix (not push yet). About the chatter, this is only available if you install |
No need of the glue module then, but also a mention in the ROADMAP. |
35c4406 to
5f4ef82
Compare
|
add changes, plus the mention to ROADMAP |
5f4ef82 to
e875458
Compare
|
ping @pedrobaeza |
pedrobaeza
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.
Can you put another icon like the clock for the button to go the the breaks?
e875458 to
0f2d31d
Compare
|
done |
|
added Spanish translations, not 100% if it goes in an other commit or i should just squash them |
|
It should go in the same commit as the other, and anyway, if in a separate commit, it shouldn't be |
3071d7c to
8199ec8
Compare
okay, i got that from an other module commit as reference to add translation without using weblate. Anyways i just add it to the commit. Thank you! |
bbc523f to
faec668
Compare
| <group name="breaks"> | ||
| <group> | ||
| <field | ||
| name="rest_time_ids" |
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 adding this if it's invisible?
faec668 to
a7bb04c
Compare
a7bb04c to
aa0d717
Compare
aa0d717 to
d729f42
Compare

cc @Tecanativa TT59859
ping @pedrobaeza @juancarlosonate-tecnativa @david-banon-tecnativa
This module allows tracking employee rest times (breaks) that are included in the employee’s working time.