Skip to content

[FIX] hr_holidays: fix error when selecting time off type#5150

Draft
SebVde wants to merge 1 commit into
master-hr-onboarding-sevanfrom
master-hr-onboarding-time_off_type_error-sevan
Draft

[FIX] hr_holidays: fix error when selecting time off type#5150
SebVde wants to merge 1 commit into
master-hr-onboarding-sevanfrom
master-hr-onboarding-time_off_type_error-sevan

Conversation

@SebVde
Copy link
Copy Markdown

@SebVde SebVde commented May 6, 2026

There was a TypeError after allocating a time off for an employee, validating it, creating the time off for this employee, and clicking on the Time Type selection.

Task: 6191029

@robodoo
Copy link
Copy Markdown

robodoo commented May 6, 2026

This PR targets the un-managed branch odoo-dev/odoo:master-hr-onboarding-sevan, it needs to be retargeted before it can be merged.

Copy link
Copy Markdown

@lebm-odoo lebm-odoo left a comment

Choose a reason for hiding this comment

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

Helloooo, little comment, and you should write a test for this search method as it is a fix 👀

def _search_virtual_remaining_leaves(self, operator, value):
def is_valid(work_entry_type):
return not work_entry_type.requires_allocation or op(work_entry_type.virtual_remaining_leaves)
return not work_entry_type.requires_allocation or op(work_entry_type.virtual_remaining_leaves, work_entry_type.requires_allocation)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The purpose of a search method is to implement the search behavior on a field (that is not stored in db but computed for instance) and so here it's on virtual_remaining_leaves. So you have to implement each comparisons: (computed_value, operator, comparison_value)
so if someone search for work entry types with virtual remaining leaves under 15. It will compute for each work entry the virtual_remaining_leave and compare it with the < operator with 15.
So with your fix, the traceback is fixed but the search will not work.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Oops, I misunderstood the method 😅

@SebVde SebVde force-pushed the master-hr-onboarding-time_off_type_error-sevan branch from cf3d314 to f62e68c Compare May 8, 2026 08:42
@lebm-odoo
Copy link
Copy Markdown

And don't forget to read the main message above the review 👀 a test still missing to be sure the _search method will never fail again 😄
For every fix, you should do at least one test that is failing before your fix and pass after it. (if it's possible, writing test before the dev process is even better to be sure that the test will not just fit your fix case)

@SebVde SebVde force-pushed the master-hr-onboarding-time_off_type_error-sevan branch 2 times, most recently from 8e4f918 to f760374 Compare May 8, 2026 14:15
@SebVde
Copy link
Copy Markdown
Author

SebVde commented May 8, 2026

I added a test method, let me know if it is sufficient for this fix 🤓

Copy link
Copy Markdown

@lebm-odoo lebm-odoo left a comment

Choose a reason for hiding this comment

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

Helloooo, I think the test can be a bit better 😄

Comment thread addons/hr_holidays/tests/test_hr_work_entry_type.py Outdated
@SebVde SebVde force-pushed the master-hr-onboarding-time_off_type_error-sevan branch from f760374 to befc5f3 Compare May 12, 2026 11:35
There was a TypeError after allocating a time off for an employee, validating it, creating the time off for this employee, and clicking on the Time Type selection.
The bug was due to the private _search method.

Added a test to ensure the search on work entry types works.

Task: 6191029
@SebVde SebVde force-pushed the master-hr-onboarding-time_off_type_error-sevan branch from befc5f3 to 5ece274 Compare May 12, 2026 11:44
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.

3 participants