-
-
Notifications
You must be signed in to change notification settings - Fork 822
[19.0][MIG] hr_attendance #5705
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
9 changes: 9 additions & 0 deletions
9
...e_scripts/scripts/hr_attendance/19.0.2.0/hr_attendance_overtime_rule_data-forcecreate.xml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| <odoo> | ||
| <!-- force create rule records --> | ||
| <record id="hr_attendance_overtime_employee_schedule_rule" position="attributes"> | ||
| <attribute name="forcecreate">1</attribute> | ||
| </record> | ||
| <record id="hr_attendance_overtime_non_working_days_rule" position="attributes"> | ||
| <attribute name="forcecreate">1</attribute> | ||
| </record> | ||
| </odoo> |
80 changes: 80 additions & 0 deletions
80
openupgrade_scripts/scripts/hr_attendance/19.0.2.0/post-migration.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| # Copyright 2026 Hunki Enterprises BV | ||
| # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl). | ||
|
|
||
| from openupgradelib import openupgrade | ||
|
|
||
| from odoo.fields import Command | ||
|
|
||
| _deleted_xmlids = [ | ||
| "hr_attendance.hr_attendance_overtime_rule_employee_company", | ||
| "hr_attendance.hr_attendance_rule_attendance_officer_overtime_restrict", | ||
| "hr_attendance.hr_attendance_rule_attendance_overtime_admin", | ||
| "hr_attendance.hr_attendance_rule_attendance_overtime_simple_user", | ||
| ] | ||
|
|
||
|
|
||
| def cleanup_hr_attendance_rule_attendance_admin(env): | ||
| """ | ||
| Remove old group from hr_attendance_rule_attendance_admin | ||
| """ | ||
| env.ref("hr_attendance.hr_attendance_rule_attendance_admin").write( | ||
| { | ||
| "groups": [ | ||
| Command.unlink(env.ref("hr_attendance.group_hr_attendance_manager").id) | ||
| ], | ||
| } | ||
| ) | ||
|
|
||
|
|
||
| def hr_attendance_overtime_rule(env): | ||
| """ | ||
| Load noupdate, forcecreate=False data for overtime rules, adjust with previous | ||
| values | ||
| """ | ||
| openupgrade.load_data( | ||
| env, | ||
| "hr_attendance", | ||
| "data/hr_attendance_overtime_rule_data.xml", | ||
| xml_transformation_filename="19.0.2.0/hr_attendance_overtime_rule_data-forcecreate.xml", | ||
| ) | ||
| main_company = env.ref("base.main_company") | ||
| default_rule = env.ref( | ||
| "hr_attendance.hr_attendance_overtime_employee_schedule_rule" | ||
| ) | ||
| default_rule.employer_tolerance = main_company.overtime_company_threshold / 60.0 | ||
| default_rule.employee_tolerance = main_company.overtime_employee_threshold / 60.0 | ||
| for company in env["res.company"].search([]): | ||
| rule = default_rule | ||
| if ( | ||
| main_company.overtime_company_threshold | ||
| != company.overtime_company_threshold | ||
| or main_company.overtime_employee_threshold | ||
| != company.overtime_employee_threshold | ||
| ): | ||
| rule = default_rule.copy( | ||
| { | ||
| "employer_tolerance": company.overtime_company_threshold / 60.0, | ||
| "employee_tolerance": company.overtime_employee_threshold / 60.0, | ||
| "ruleset_id": default_rule.ruleset_id.copy( | ||
| { | ||
| "company_id": company.id, | ||
| "rule_ids": [ | ||
| Command.link(other_rule.copy().id) | ||
| for other_rule in default_rule.ruleset_id.rule_ids | ||
| if other_rule != default_rule | ||
| ], | ||
| } | ||
| ).id, | ||
| } | ||
| ) | ||
| env["hr.version"].search( | ||
| [("company_id", "=", company.id)] | ||
| ).ruleset_id = rule.ruleset_id | ||
|
|
||
|
|
||
| @openupgrade.migrate() | ||
| def migrate(env, version): | ||
| openupgrade.load_data(env, "hr_attendance", "19.0.2.0/noupdate_changes.xml") | ||
|
MiquelRForgeFlow marked this conversation as resolved.
|
||
| cleanup_hr_attendance_rule_attendance_admin(env) | ||
| hr_attendance_overtime_rule(env) | ||
| openupgrade.delete_records_safely_by_xml_id(env, _deleted_xmlids) | ||
169 changes: 169 additions & 0 deletions
169
openupgrade_scripts/scripts/hr_attendance/19.0.2.0/pre-migration.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,169 @@ | ||
| # Copyright 2026 Hunki Enterprises BV | ||
| # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl). | ||
|
|
||
| from openupgradelib import openupgrade | ||
|
|
||
| _renamed_models = [ | ||
| ("hr.attendance.overtime", "hr.attendance.overtime.line"), | ||
| ] | ||
|
|
||
| _renamed_tables = [ | ||
| ("hr_attendance_overtime", "hr_attendance_overtime_line"), | ||
| ] | ||
|
|
||
| _renamed_fields = [ | ||
| ("hr.attendance", "hr_attendance", "in_city", "in_location"), | ||
| ("hr.attendance", "hr_attendance", "out_city", "out_location"), | ||
| ( | ||
| "hr.attendance.overtime.line", | ||
| "hr_attendance_overtime_line", | ||
| "duration_real", | ||
| "manual_duration", | ||
| ), | ||
| ] | ||
|
|
||
| SQL_EMPLOYEE2TZ = """ | ||
| ( | ||
| SELECT | ||
| hr_employee.id employee_id, | ||
| COALESCE( | ||
| MIN(resource_calendar.tz), | ||
| MIN(resource_resource.tz), | ||
| MIN(company_resource_calendar.tz) | ||
| ) zone | ||
| FROM | ||
| hr_employee | ||
| JOIN | ||
| resource_resource | ||
| ON hr_employee.resource_id=resource_resource.id | ||
| LEFT JOIN | ||
| resource_calendar | ||
| ON resource_resource.calendar_id=resource_calendar.id | ||
| JOIN | ||
| res_company | ||
| ON hr_employee.company_id=res_company.id | ||
| LEFT JOIN | ||
| resource_calendar company_resource_calendar | ||
| ON res_company.resource_calendar_id=company_resource_calendar.id | ||
| GROUP BY | ||
| hr_employee.id | ||
| ) employee2zone | ||
| """ | ||
|
|
||
|
|
||
| def hr_attendance_date(env): | ||
| """ | ||
| Pre-fill hr.attendance#date | ||
| """ | ||
| openupgrade.add_columns( | ||
| env, [("hr.attendance", "date", "date", None, "hr_attendance")] | ||
| ) | ||
| env.cr.execute( | ||
| f""" | ||
| UPDATE | ||
| hr_attendance | ||
| SET | ||
| date=(check_in AT TIME ZONE COALESCE(employee2zone.zone, 'utc'))::date | ||
| FROM | ||
| {SQL_EMPLOYEE2TZ} | ||
| WHERE | ||
| employee2zone.employee_id=hr_attendance.employee_id | ||
| AND date IS NULL | ||
| """ | ||
| ) | ||
|
|
||
|
|
||
| def hr_attendance_overtime_line_fields(env): | ||
| """ | ||
| Pre-fill new fields of hr.attendance.overtime.line | ||
| """ | ||
| openupgrade.add_columns( | ||
| env, | ||
| [ | ||
| ( | ||
| "hr.attendance.overtime.line", | ||
| "status", | ||
| "char", | ||
| "approved", | ||
| "hr_attendance_overtime_line", | ||
| ), | ||
| ( | ||
| "hr.attendance.overtime.line", | ||
| "time_start", | ||
| "datetime", | ||
| None, | ||
| "hr_attendance_overtime_line", | ||
| ), | ||
| ( | ||
| "hr.attendance.overtime.line", | ||
| "time_stop", | ||
| "datetime", | ||
| None, | ||
| "hr_attendance_overtime_line", | ||
| ), | ||
| ], | ||
| ) | ||
| # date is required in v19, fill with create_date if empty, possibly wrong | ||
| env.cr.execute( | ||
| f""" | ||
| UPDATE | ||
| hr_attendance_overtime_line | ||
| SET | ||
| date=( | ||
| create_date AT TIME ZONE COALESCE(employee2zone.zone, 'utc') | ||
| )::date | ||
| FROM | ||
| {SQL_EMPLOYEE2TZ} | ||
| WHERE | ||
| employee2zone.employee_id=hr_attendance_overtime_line.employee_id | ||
| AND | ||
| date IS NULL | ||
| """ | ||
| ) | ||
| # time_start, time_stop need to match some check_in, check_out from hr_attendance | ||
| env.cr.execute( | ||
| """ | ||
| UPDATE | ||
| hr_attendance_overtime_line | ||
| SET | ||
| time_start=hr_attendance.check_in, | ||
| time_stop=hr_attendance.check_out | ||
| FROM | ||
| hr_attendance | ||
| WHERE | ||
| hr_attendance.employee_id=hr_attendance_overtime_line.employee_id | ||
| AND | ||
| hr_attendance.date=hr_attendance_overtime_line.date | ||
| """ | ||
| ) | ||
| # for companies with manager approval, set status from state on attendance | ||
| env.cr.execute( | ||
| """ | ||
| UPDATE | ||
| hr_attendance_overtime_line | ||
| SET | ||
| status=hr_attendance.overtime_status | ||
| FROM | ||
| hr_employee, res_company, hr_attendance | ||
| WHERE | ||
| hr_attendance.check_in=hr_attendance_overtime_line.time_start | ||
| AND | ||
| hr_attendance.employee_id=hr_attendance_overtime_line.employee_id | ||
| AND | ||
| hr_attendance_overtime_line.employee_id=hr_employee.id | ||
| AND | ||
| hr_employee.company_id=res_company.id | ||
| AND | ||
| res_company.attendance_overtime_validation = 'by_manager' | ||
| """ | ||
| ) | ||
|
|
||
|
|
||
| @openupgrade.migrate() | ||
| def migrate(env, version): | ||
| openupgrade.rename_models(env.cr, _renamed_models) | ||
| openupgrade.rename_tables(env.cr, _renamed_tables) | ||
| openupgrade.rename_fields(env, _renamed_fields) | ||
| # order matters | ||
| hr_attendance_date(env) | ||
| hr_attendance_overtime_line_fields(env) |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 don't agree with this logic.
Why are you setting the fields on the default rule and then copying that rule to every company?
I don't think the fields should be set on the default rule.
If the default rule does not have a company_id set, why should it take the values from the main company?
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 results in the default rule without company applying to all companies that have the same settings as the main company, which probably is the most common case. you only get specific rules for companies that have different settings than the main company.
what would your alternative do?
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.
Option 1: If you create or duplicate the rule for the
main_company, then don't set these fields on the default rule. Otherwise, we assume that the values from the main company are the same for all other companies. In other words, don't modify the default rule; instead, create a new rule for each company that has these fields set.Option 2: If the default rule without a
company_idis intended to preserve the field values from themain_company, then don't create or duplicate another rule for themain_company.Example:
My Company has these fields set.
C2 has these fields set, but with values different from My Company.
C3 does not have these fields set.
With Option 1, I would expect the following:
Don't modify the default rule, so it can be used by C3.
Don't create a rule for C3, because the default rule is shared.
Create a dedicated rule for My Company and C2, with the specific values configured for each company.
What I see as incorrect behavior:
A rule is created for C3, even though this company does not have these fields set.
The default rule receives the same values as My Company, but another rule is also created for My Company. Instead, it should use the default rule rather than creating a new one.
I prefer Option 1. What do you think?
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 believe i implemented option 2, and i think that's the good one. would expect no new rule for main company in your example
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.
OK, if you want to use Option 2, that's fine. However, in this case a new rule is still created for My Company. Please check my screenshot; this is a database migrated from v18.
The rule is created because, in the if statement, you compare values that have been divided by 60 when assigned to the default rule, while the values on the company fields are not divided by 60. As a result, they are never equal, and the code always enters the block that copies the rule.
default_rule.employer_tolerance != company.overtime_company_thresholdIt should be:
default_rule.employer_tolerance != (company.overtime_company_threshold / 60.0)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, your code would never create a new rule though. I think d698ab8 gets it right
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, but after testing the latest changes, I'm not convinced by the results.
Following my example:
Currently, a new rule is created for C3, but I would expect no rule to be created for this company.
The default rule (without company_id set) takes the values from main_company, but this rule is shared. As a result, other companies inherit values from main_company incorrectly.
I made these changes locally, and the behavior is correct according to my expectations:
I think this behavior is more correct because the default rule remains shared and is used by companies that do not have specific values configured, while companies with customized values get their own dedicated rules.
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.
well, I disagree this is the most natural expectation. I think we should treat everything that's different from the main company the exception, and whatever the main company does should be the default for new companies
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.
Okay, while another person can review this and decide whether the values configured on main_company can be used by other companies.
Please avoid creating rules for companies that do not have these fields set, because the default rule without a company_id should be used instead.
In my example, no rule should be created for C3.
Could you apply this 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.
we need this rule because otherwise the default one will be active. remember the goal here is to get v19 to behave in a similar way it was in v18