Skip to content
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

[17.0][FIX] hr_holidays_public: fixed action_validate() override #116

Merged

Conversation

dalonsod
Copy link

Forward-port of #115

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 Jul 21, 2024
@pedrobaeza pedrobaeza added this to the 17.0 milestone Jul 21, 2024
@pedrobaeza
Copy link
Member

Was this already fixed?

@dalonsod
Copy link
Author

Tested in 17.0 runboat without this fix the bug case (trying to validate leaves that belong to more than one employee):

imagen

still fires an error:

imagen

Then, the fix seems to be required.

@dalonsod
Copy link
Author

Anyway I'll take a look at existing merge conflicts ASAP and update this PR

@dalonsod dalonsod force-pushed the 17.0-fix-hr_holidays_public-validatesingleton branch from c6c347d to 9726640 Compare July 23, 2024 14:33
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 17.0-ocabot-merge-pr-116-by-pedrobaeza-bump-patch, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Jul 23, 2024
Signed-off-by pedrobaeza
@OCA-git-bot
Copy link
Contributor

@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 17.0-ocabot-merge-pr-116-by-pedrobaeza-bump-patch.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@pedrobaeza
Copy link
Member

Can you check CI?

@dalonsod
Copy link
Author

Checked, and I don't know how to fix it. It fails when hr_holidays addon is trying to notify leave approval in a way I don't understand which modification in current code could lead to this error.

@dariodelzozzo could you help us?

@github-actions github-actions bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jul 28, 2024
@xaviedoanhduy
Copy link
Contributor

xaviedoanhduy commented Sep 30, 2024

Hello @dalonsod, this issue has been fixed already in this commit, So can you need to rebase this PR for tests to be green?

@pedrobaeza
Copy link
Member

The rebase is done by the bot, so trying again

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 17.0-ocabot-merge-pr-116-by-pedrobaeza-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit f81ecc6 into OCA:17.0 Sep 30, 2024
2 of 5 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at fdfbef9. Thanks a lot for contributing to OCA. ❤️

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.

5 participants