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
[IMP] hr_holidays: restrict allocation type to leave type #158067
base: master
Are you sure you want to change the base?
Conversation
4e7456b
to
baf5479
Compare
093dfd2
to
8974caf
Compare
9246dd7
to
5a91dbb
Compare
raise ValidationError(_( | ||
"Fields modification is limited for leave types linked to at least one allocation or leave." | ||
)) |
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.
This error message could be more descriptive. I would add in which of the fields that are being modified are problematic
raise ValidationError(_( | ||
"The modification you're trying to do to negative amounts will bring some discrepancies to some employees." | ||
)) |
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.
Also this error message is a little hard to read to me. Does this mean that the changes being done result in some employees being further in negative than they are allowed to be?
@@ -320,6 +321,43 @@ def _search(self, domain, offset=0, limit=None, order=None, access_rights_uid=No | |||
return leaves._as_query() | |||
return super()._search(domain, offset, limit, order, access_rights_uid) | |||
|
|||
def _has_discrepancies(self, employees, target_date=None): |
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.
Can you make this method name more descriptive? This method checks whether an employee has taken more negative time off than is allowed right? Maybe just _check_negative_time_off
and raise the error in this method, instead of returning a boolean?
eaa64d7
to
b228ef8
Compare
b228ef8
to
2e1cbfd
Compare
Before this commit, the allocation type determining if the allocation is regular or linked to an accrual was chosen from the allocation directly. This means that for a same leave type, both allocation type could have been used. However, as both are very different in the way we compute the remaining leaves (for the time off dashboard for example) this might lead to some display issues. This commit moves the allocation type selection to the leave type, ensuring only one type can be used instead. This commit also removed the unused field "accruals_ids". Finally, this commit set a constraint on the fields `requires_allocation` and `allocation_type` to avoid modifying them once either some leaves or some allocations have been set for that leave type. The negative amount settings modification is also limited to ensure that the new amount set is not bringing discrepancies into the database. task-3412869
2e1cbfd
to
f07d33b
Compare
Before this commit, the allocation type determining if the allocation is regular or linked to an accrual was chosen from the allocation directly. This means that for a same leave type, both allocation type could have been used. However, as both are very different in the way we compute the remaining leaves (for the time off dashboard for example) this might lead to some display issues.
This commit moves the allocation type selection to the leave type, ensuring only one type can be used instead.
task-3412869