Skip to content

[19.0][MIG] hr_attendance#5705

Open
hbrunn wants to merge 2 commits into
OCA:19.0from
hbrunn:19.0-hr_attendance
Open

[19.0][MIG] hr_attendance#5705
hbrunn wants to merge 2 commits into
OCA:19.0from
hbrunn:19.0-hr_attendance

Conversation

@hbrunn

@hbrunn hbrunn commented Jun 9, 2026

Copy link
Copy Markdown
Member

No description provided.

@hbrunn hbrunn added this to the 19.0 milestone Jun 9, 2026
@hbrunn

hbrunn commented Jun 9, 2026

Copy link
Copy Markdown
Member Author

/ocabot migration hr_attendance

Depends on :

@legalsylvain

This comment was marked as duplicate.

@hbrunn

This comment was marked as off-topic.

@legalsylvain

This comment was marked as off-topic.

@hbrunn

This comment was marked as off-topic.

@legalsylvain

This comment was marked as off-topic.

@hbrunn

This comment was marked as off-topic.

@legalsylvain

This comment was marked as off-topic.

@hbrunn

hbrunn commented Jun 23, 2026

Copy link
Copy Markdown
Member Author

@legalsylvain done in #5745, now setting our whole discussion as off-topic

@legalsylvain

Copy link
Copy Markdown
Contributor

@legalsylvain done in #5745, now setting our whole discussion as off-topic

thanks a lot for this Job. I manually run the script a last time for the V19. new 19 PR should be handled by your script.


@openupgrade.migrate()
def migrate(env, version):
openupgrade.load_data(env, "hr_attendance", "19.0.2.0/noupdate_changes.xml")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For letting the record as it's in 19, we should remove the group hr_attendance.group_hr_attendance_manager from hr_attendance.hr_attendance_rule_attendance_admin record.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

---Models in module 'hr_attendance'---
obsolete model hr.attendance.overtime (renamed to hr.attendance.overtime.line)
new model hr.attendance.overtime.line (renamed from hr.attendance.overtime)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why adding these empty lines that makes the reading harder? I think the comment should be together with the analysis lines block.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well I think the empty lines improve readability. If that's so important to you, add it to https://oca.github.io/OpenUpgrade/090_contribute.html#how-to-contribute-new-migration-scripts and I'll bulk update the whole branch, as I've been doing this for a long time


# NOTHING TO DO

hr_attendance / hr.attendance / in_location (char) : NEW

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reorganize this line into in_city DEL comment for seeing both together and to understand the change.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hard disagree about reordering lines in the work file. I want to be able to say diff upgrade_analysis* to easily verify all lines from the generated file are in the work file (=only whitespace or comments in the diff)

# date is required in v19, fill with create_date if empty, possibly wrong
env.cr.execute(
f"""
UPDATE hr_attendance_overtime_line SET date=(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not get it the same from the nearest attendance record?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how do you propose to find the nearest record? create_date? that's just as prone to filling in a wrong date here.

I consider overtime lines without date garbage data which people should clean up, and do the update only to satisfy the non-null constraint

SQL_EMPLOYEE2TZ = """
(
SELECT
hr_employee.id employee_id,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find you new SQL style a bit difficult to read, and very vertical scroll intensive. Either indent the lines after the command (SELECT, FROM, etc), or put them in the same line as always.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not so new, but indeed was lacking indentation

WHERE
hr_attendance.employee_id=hr_attendance_overtime_line.employee_id
AND
hr_attendance.date=hr_attendance_overtime_line.date

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part may not fit according current code, and even more, we may have more than one record for the same date.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which code do you mean? and indeed this can lead to updating the same record multiple times, but how does this matter?

"""
Pre-fill hr.attendance#date
"""
openupgrade.add_fields(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why using add_fields instead of add_columns or a simply ALTER TABLE?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good question, changed to add_columns

@OCA-git-bot OCA-git-bot added mod:openupgrade_scripts Module openupgrade_scripts series:19.0 labels Jun 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants