[FIX] hr: time off type error fixed #4894
Conversation
|
This PR targets the un-managed branch odoo-dev/odoo:master-hr-onboarding-abshe, it needs to be retargeted before it can be merged. |
Mahmoudk3m
left a comment
Mahmoudk3m
left a comment
There was a problem hiding this comment.
Thanks for your work 🚀
The fix is good but I have some comments about the commit message:
You wrote the wrong module name in the commit message
for bug fixes we need:
- Steps to reproduce and bug description
- Cause of the bug
- How you fixed it
Also, for bug fixes, we need to add tests to prevent it from happening again.
abdullahsherif
force-pushed
the
master-hr-onboarding-traceback_on_time_off_type-abshe
branch
from
0d0aa75 to
e414881
Compare
Mahmoudk3m
left a comment
Mahmoudk3m
left a comment
There was a problem hiding this comment.
Helloo.
The commit message is still a bit wrong:
you're missing the module name.
bug fix tasks should have: (bug, cause, fix)
Also you're still missing the tests :)
Thanks for your work 🚀
abdullahsherif
force-pushed
the
master-hr-onboarding-traceback_on_time_off_type-abshe
branch
from
e414881 to
08d5ab5
Compare
Mahmoudk3m
left a comment
Mahmoudk3m
left a comment
There was a problem hiding this comment.
Hello,
tests for search methods should actually assert something about the results, not just call .search() and ignore the return value.
So if you try to remove your fix and run the test it'll still pass.
| 'requires_allocation': True, | ||
| }) | ||
|
|
||
| leave_type.search(domain=[('virtual_remaining_leaves', '>', 0)]) |
There was a problem hiding this comment.
| leave_type.search(domain=[('virtual_remaining_leaves', '>', 0)]) | |
| self.env['hr.leave.type'].search(domain=[('virtual_remaining_leaves', '>', 0)]) |
It should be self.env['hr.lreave.type'] instead of leave_type.search. Even though it does the same thing.
Bug: traceback error when going throw normal flows Cause: missing parameter on a function call Fix: added the missing parameter Steps to reproduce: Time Off >> Configuration >> Time Off Types >> Open any type, navigate to the Smart button Time Off >> Click on New >> Time Off Type Task #5379901
abdullahsherif
force-pushed
the
master-hr-onboarding-traceback_on_time_off_type-abshe
branch
from
08d5ab5 to
5a2a39a
Compare
Mahmoudk3m
deleted the
master-hr-onboarding-traceback_on_time_off_type-abshe
branch
this fixes the traceback error occuring when trying to select time off types via some flows
Task #5379901