-
-
Notifications
You must be signed in to change notification settings - Fork 667
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
[13.0][IMP] hr_employee_calendar_planning: Set calendar_ids default value to cover all use cases #1263
[13.0][IMP] hr_employee_calendar_planning: Set calendar_ids default value to cover all use cases #1263
Conversation
Hi @pedrobaeza, |
I don't think injecting specific sources is the right patch. Instead, a default should exist for covering all cases. |
c654097
to
94ed880
Compare
94ed880
to
546c87d
Compare
With this you can remove also the |
0b9966e
to
81c053e
Compare
Changes done and test fixed |
…o cover all use cases TT44093
81c053e
to
372b3eb
Compare
…calendar_id fields to allow removal TT44093
c8dfe45
to
045dde5
Compare
Changes done. |
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.
/ocabot merge patch
Hey, thanks for contributing! Proceeding to merge this for you. |
Congratulations, your PR was merged at d857c71. Thanks a lot for contributing to OCA. ❤️ |
Set calendar_ids default value to cover all use cases.
Steps to reproduce the error (before):
An error is displayed when trying to create the employee because there is no calendar defined.
Please @pedrobaeza and @chienandalu can you review it?
@Tecnativa TT44093