-
-
Notifications
You must be signed in to change notification settings - Fork 747
[18.0][OU-ADD] hr_holidays #5023
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
/ocabot migration hr_holidays |
a0a2990
to
04af498
Compare
6d153ec
to
ec1b791
Compare
ec1b791
to
861a206
Compare
861a206
to
923762e
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.
Hi @BhaveshHeliconia thank you for this PR, there is still some work to do IMO
DEL ir.ui.view: hr_holidays.hr_departure_wizard_view_form | ||
DEL ir.ui.view: hr_holidays.hr_leave_report_search_view | ||
NEW mail.activity.type: hr_holidays.mail_act_leave_allocation_second_approval (noupdate) | ||
DEL mail.message.subtype: hr_holidays.mt_leave_home_working (noupdate) |
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 one you need to delete manually since it is noupdate.
hr_holidays / hr.employee / current_leave_state (selection): selection_keys is now '['cancel', 'confirm', 'refuse', 'validate', 'validate1']' ('['cancel', 'confirm', 'draft', 'refuse', 'validate', 'validate1']') | ||
hr_holidays / hr.employee / hr_icon_display (False) : selection_keys is now '['presence_absent', 'presence_archive', 'presence_holiday_absent', 'presence_holiday_present', 'presence_out_of_working_hour', 'presence_present', 'presence_undetermined']' ('['presence_absent', 'presence_absent_active', 'presence_holiday_absent', 'presence_holiday_present', 'presence_present', 'presence_to_define', 'presence_undetermined']') |
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 may want to precise that these 2 are Nothing to do since they are computed non stored fields
hr_holidays / hr.employee / hr_icon_display (False) : selection_keys is now '['presence_absent', 'presence_archive', 'presence_holiday_absent', 'presence_holiday_present', 'presence_out_of_working_hour', 'presence_present', 'presence_undetermined']' ('['presence_absent', 'presence_absent_active', 'presence_holiday_absent', 'presence_holiday_present', 'presence_present', 'presence_to_define', 'presence_undetermined']') | ||
hr_holidays / hr.leave / active (boolean) : DEL | ||
hr_holidays / hr.leave / category_id (many2one) : DEL relation: hr.employee.category | ||
hr_holidays / hr.leave / employee_id (many2one) : now required |
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 need to set an employee in case you had leaves without employee.
It should be decided what should be done of this hr_leaves which were not assigned to an employee but either to category of employees, department, company since these are not supported anymore. Should these be deleted ?
hr_holidays / hr.leave / holiday_type (selection) : DEL required, selection_keys: ['category', 'company', 'department', 'employee'] | ||
hr_holidays / hr.leave / linked_request_ids (one2many) : DEL relation: hr.leave | ||
hr_holidays / hr.leave / mode_company_id (many2one) : DEL relation: res.company | ||
hr_holidays / hr.leave / multi_employee (boolean) : DEL | ||
hr_holidays / hr.leave / parent_id (many2one) : DEL relation: hr.leave | ||
hr_holidays / hr.leave / report_note (text) : DEL |
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.
These should be handled together with above comment about what should be done with multi employee hr_leave which are not allowed anymore
hr_holidays / hr.leave.allocation / actual_lastcall (date) : NEW | ||
hr_holidays / hr.leave.allocation / carried_over_days_expiration_date (date): NEW | ||
hr_holidays / hr.leave.allocation / category_id (many2one) : DEL relation: hr.employee.category | ||
hr_holidays / hr.leave.allocation / employee_id (many2one) : now required |
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 same as above for hr.leave applies to hr.leave.allocation where multi-employee allocations are not allowed anymore
openupgrade.logged_query( | ||
env.cr, | ||
""" | ||
UPDATE hr_leave SET request_hour_to = request_hour_to_float | ||
WHERE request_hour_to_float IS NOT NULL; | ||
UPDATE hr_leave SET request_hour_from = request_hour_from_float | ||
WHERE request_hour_from_float IS NOT NULL; | ||
|
||
""", | ||
) |
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 should be done in pre-migration
openupgrade.logged_query( | ||
env.cr, | ||
""" | ||
ALTER TABLE hr_leave DROP COLUMN request_hour_to_float; | ||
ALTER TABLE hr_leave DROP COLUMN request_hour_from_float; | ||
|
||
""", | ||
) |
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 will be handled by ORM during upgrade
openupgrade.logged_query( | ||
env.cr, | ||
""" | ||
ALTER TABLE hr_leave ADD COLUMN request_hour_to_float float; | ||
ALTER TABLE hr_leave ADD COLUMN request_hour_from_float float; | ||
""", | ||
) | ||
openupgrade.logged_query( | ||
env.cr, | ||
""" | ||
UPDATE hr_leave SET request_hour_to_float = request_hour_to::float | ||
WHERE request_hour_to IS NOT NULL; | ||
UPDATE hr_leave SET request_hour_from_float = request_hour_from::float | ||
WHERE request_hour_from IS NOT NULL; | ||
|
||
""", | ||
) |
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.
Here you should pre-create destination column (request_hour_to / request_hour_from) and copy data inside instead of creating temporary column
openupgrade.logged_query( | ||
env.cr, | ||
""" | ||
DELETE FROM ir_model_fields_selection | ||
WHERE field_id IN ( | ||
SELECT f.id | ||
FROM ir_model_fields f | ||
JOIN ir_model m ON f.model_id = m.id | ||
WHERE f.name = 'request_hour_to' AND m.model = 'hr.leave' | ||
); | ||
DELETE FROM ir_model_fields_selection | ||
WHERE field_id IN ( | ||
SELECT f.id | ||
FROM ir_model_fields f | ||
JOIN ir_model m ON f.model_id = m.id | ||
WHERE f.name = 'request_hour_from' AND m.model = 'hr.leave' | ||
); | ||
""", | ||
) | ||
|
||
# Remove the Many2many relation table if it exists | ||
openupgrade.logged_query( | ||
env.cr, | ||
""" | ||
DROP TABLE IF EXISTS hr_employee_hr_leave_allocation_rel CASCADE | ||
""", | ||
) | ||
|
||
# Optionally drop the column from the table (not usually required for M2M) | ||
openupgrade.logged_query( | ||
env.cr, | ||
""" | ||
ALTER TABLE hr_leave_allocation | ||
DROP COLUMN IF EXISTS employee_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.
I think that all these steps are unecessary as will be performed by ORM during upgrade
openupgrade.logged_query( | ||
env.cr, | ||
""" | ||
DELETE FROM ir_rule | ||
WHERE id = ( | ||
SELECT res_id FROM ir_model_data | ||
WHERE module = 'hr_holidays' | ||
AND name = 'hr_leave_allocation_rule_multicompany' | ||
) | ||
""", | ||
) |
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.
Here you better use delete_records_safely_by_xml_id() function from openupgradelib
Depends on