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][MIG] hr_leave_custom_hour_interval: Migration to 17.0 #1

Closed
wants to merge 8 commits into from

Conversation

xaviedoanhduy
Copy link
Owner

@xaviedoanhduy xaviedoanhduy commented Jun 10, 2024

The failing test not related to this module

@xaviedoanhduy xaviedoanhduy force-pushed the 17.0-mig-hr_leave_custom_hour_interval branch from 51a6520 to 824fc89 Compare June 10, 2024 12:03
@xaviedoanhduy xaviedoanhduy marked this pull request as ready for review June 10, 2024 12:04
Comment on lines 4 to 5
* `Trobz <https://trobz.com>`_:
* Duy Do Anh <[email protected]>

Choose a reason for hiding this comment

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

@xaviedoanhduy xaviedoanhduy force-pushed the 17.0-mig-hr_leave_custom_hour_interval branch 6 times, most recently from 17ea0d2 to b7bc00a Compare June 11, 2024 11:18
def _compute_date_from_to(self):
return super()._compute_date_from_to()

def action_validate(self):

Choose a reason for hiding this comment

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

@xaviedoanhduy could you please share your ideas about this override?

exclude_public_holidays is not existed btw

Copy link
Owner Author

Choose a reason for hiding this comment

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

hi @dzungtran89, I am trying to fix failed tests in the hr_holidays_public module when overriding the action_validate function.
some pr is exists do the same in here

Copy link
Owner Author

Choose a reason for hiding this comment

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

you can get the error log in here

Choose a reason for hiding this comment

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

  File "/__w/hr-holidays/hr-holidays/hr_holidays_public/tests/test_holidays_calculation.py", line 188, in test_number_of_hours_excluding_employee_2
    leave_request.action_validate()
  File "/__w/hr-holidays/hr-holidays/hr_leave_custom_hour_interval/models/hr_leave.py", line 47, in action_validate
    return super().action_validate()
  File "/__w/hr-holidays/hr-holidays/hr_holidays_public/models/hr_leave.py", line 22, in action_validate
    return super().action_validate()
  File "/opt/odoo/addons/hr_holidays/models/hr_leave.py", line 1385, in action_validate
    employee_requests._validate_leave_request()
  File "/opt/odoo/addons/hr_holidays/models/hr_leave.py", line 1112, in _validate_leave_request
    holiday.message_post(
  File "/opt/odoo/addons/mail/models/mail_thread.py", line 2096, in message_post
    raise ValueError(_("Posting a message should be done on a business document. Use message_notify to send a notification to an user."))
ValueError: Posting a message should be done on a business document. Use message_notify to send a notification to an user.

You could take some times to investigate the issue on the module hr_holidays_public, the module hr_leave_custom_hour_interval is not relevant

btw, A new commit is quite recent odoo/odoo/commit/9f1aaec55fff6 which triggers a message post in the action_validate

Copy link
Owner Author

@xaviedoanhduy xaviedoanhduy Jun 12, 2024

Choose a reason for hiding this comment

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

hi @dzungtran89, the overide about action_validate fuction is removed from this PR. so now I can create PR with OCA right?

Choose a reason for hiding this comment

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

yes, please go ahead

@xaviedoanhduy xaviedoanhduy force-pushed the 17.0-mig-hr_leave_custom_hour_interval branch from b7bc00a to db76320 Compare June 12, 2024 05:23
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.

6 participants