Skip to content

[17.0][MIG] hr_holidays_team_manager: Migration to 17.0#1450

Open
BhaveshHeliconia wants to merge 7 commits intoOCA:17.0from
HeliconiaIO:17.0-mig-hr_holidays_team_manager
Open

[17.0][MIG] hr_holidays_team_manager: Migration to 17.0#1450
BhaveshHeliconia wants to merge 7 commits intoOCA:17.0from
HeliconiaIO:17.0-mig-hr_holidays_team_manager

Conversation

@BhaveshHeliconia
Copy link
Contributor

No description provided.

@BhaveshHeliconia BhaveshHeliconia mentioned this pull request Mar 3, 2025
24 tasks
@github-actions
Copy link

github-actions bot commented Jul 6, 2025

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jul 6, 2025
@BhaveshHeliconia BhaveshHeliconia force-pushed the 17.0-mig-hr_holidays_team_manager branch from 50eb6f7 to 1bc4322 Compare July 9, 2025 12:16
@github-actions github-actions bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jul 13, 2025
@github-actions
Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Nov 16, 2025
@BhaveshHeliconia BhaveshHeliconia force-pushed the 17.0-mig-hr_holidays_team_manager branch from 1bc4322 to 3801023 Compare November 17, 2025 09:03
@github-actions github-actions bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label Nov 23, 2025
Copy link
Contributor

@alexey-pelykh alexey-pelykh left a comment

Choose a reason for hiding this comment

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

Thanks for the migration. The code and tests look good overall -- search() signature updated correctly, Command API applied, and action_validate properly adapted for 17.0. One thing to double-check: in hr_holidays_security.xml, the implied_ids for group_hr_holidays_officer was changed from (4, ref(...)) (link) to Command.set(...) (replace). The 16.0 version used link semantics to add the user group without disturbing other implied groups that might be set by third-party modules. Consider using Command.link() here instead to preserve the same behavior.


<record id="group_hr_holidays_officer" model="res.groups">
<field name="name">Time off/Holiday Officer</field>
<field
Copy link
Contributor

Choose a reason for hiding this comment

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

In 16.0 this was (4, ref('hr_holidays.group_hr_holidays_user')) which uses link/add semantics. Command.set() replaces all implied groups, which could remove implied groups added by other installed modules. Should this be Command.link(ref('hr_holidays.group_hr_holidays_user')) instead to preserve the original behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you're right — it should use Command.link() to preserve the original link semantics.
I've replaced Command.set(ref('hr_holidays.group_hr_holidays_user')) with Command.link(ref('hr_holidays.group_hr_holidays_user')).
Thanks for pointing this out and for the review.

@BhaveshHeliconia BhaveshHeliconia force-pushed the 17.0-mig-hr_holidays_team_manager branch from 3801023 to 5115421 Compare March 5, 2026 05:08
@BhaveshHeliconia BhaveshHeliconia force-pushed the 17.0-mig-hr_holidays_team_manager branch from 5115421 to c4971ed Compare March 5, 2026 05:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants