[IMP] hr_holidays: Remove leave splitting
Currently when a global leave is removed or modified in such a way that
the duration of an existing leave increases, the leave is instead split
into multiple leaves with a gap in between such that it no longer
includes the period that was previously covered by the global leave.
However, this splitting is very error-prone and from a user perspective
is not even always the behaviour that is expected, especially because
editing global leaves is a very low-threshold operation during which it
is in no way indicated that existing leaves might be impacted.
Also note that this is very specific edge case, that in normal operation
should almost never occur. It is therefore best to simply use an
approach that has a minimal impact on the codebase complexity and
produces no unexpected results for the end user.
Therefore in this commit, we handle this case in the following way:
- If a global leave is created or extended in duration, we simply
recompute the impacted leave durations (as was the case before).
- If a global leave is shortened in duration or deleted, the durations
of the affected leaves are also recomputed, but:
- In the case that existing allocations can cover this increase in
leave duration, the leave is left 'as is', but the user is
notified that additional allocation time has been taken.
- In the case that existing allocations cannot cover this increase,
the user is again notified, but the leave is additionally reset to
the 'draft' state (to prevent the allocation from going negative,
which is impossible).
With this approach, it now up to the user to handle this specific edge
case, while no information is changed or lost, which was possible to
happen with the previous approach.
Part-of: odoo/odoo#119317
This commit is contained in:
@@ -12,7 +12,6 @@ from datetime import datetime, timedelta, time
|
||||
from pytz import timezone, UTC
|
||||
|
||||
from odoo.addons.base.models.ir_model import MODULE_UNINSTALL_FLAG
|
||||
from odoo.tools import date_utils
|
||||
|
||||
from odoo import api, Command, fields, models, tools
|
||||
from odoo.addons.base.models.res_partner import _tz_get
|
||||
@@ -1475,60 +1474,6 @@ class HolidaysRequest(models.Model):
|
||||
if not is_officer and (state == 'validate' and val_type == 'hr') and holiday.holiday_type == 'employee':
|
||||
raise UserError(_('You must either be a Time off Officer or Time off Manager to approve this leave'))
|
||||
|
||||
###################################################
|
||||
# Time Off modification methods
|
||||
###################################################
|
||||
|
||||
def _split_leave_on_gto(self, gto): #gto = global time off
|
||||
self.ensure_one()
|
||||
|
||||
leave_start = date_utils.start_of(self.date_from, 'day')
|
||||
leave_end = date_utils.end_of(self.date_to - timedelta(seconds=1), 'day')
|
||||
gto_start = date_utils.start_of(gto['date_from'], 'day')
|
||||
gto_end = date_utils.end_of(gto['date_to'], 'day')
|
||||
leave_tz = timezone(self.employee_id.resource_id.tz)
|
||||
|
||||
if gto_start <= leave_start\
|
||||
and gto_end > leave_start\
|
||||
and gto_end < leave_end:
|
||||
self.write({
|
||||
'date_from': leave_tz.localize(gto_end + timedelta(seconds=1))\
|
||||
.astimezone(UTC).replace(tzinfo=None)
|
||||
})
|
||||
return self.env['hr.leave']
|
||||
if gto_start > leave_start\
|
||||
and gto_end < leave_end:
|
||||
copys = {
|
||||
'date_from': self.date_from,
|
||||
'date_to': leave_tz.localize(gto_start - timedelta(seconds=1))\
|
||||
.astimezone(UTC).replace(tzinfo=None)
|
||||
}
|
||||
self.write({
|
||||
'date_from': leave_tz.localize(gto_end + timedelta(seconds=1))\
|
||||
.astimezone(UTC).replace(tzinfo=None)
|
||||
})
|
||||
return self.copy(copys)
|
||||
if gto_start > leave_start\
|
||||
and gto_start < leave_end\
|
||||
and gto_end >= leave_end:
|
||||
self.write({
|
||||
'date_to': leave_tz.localize(gto_start - timedelta(seconds=1))\
|
||||
.astimezone(UTC).replace(tzinfo=None)
|
||||
})
|
||||
return self.env['hr.leave']
|
||||
|
||||
def split_leave(self, time_domain_dict):
|
||||
self.ensure_one()
|
||||
|
||||
new_leaves = self.env['hr.leave']
|
||||
for global_time_off in sorted(
|
||||
filter(lambda r: r['date_to'] > self.date_from and r['date_from'] < self.date_to, time_domain_dict),
|
||||
key=lambda r: r['date_from']):
|
||||
new_leave = self._split_leave_on_gto(global_time_off)
|
||||
if new_leave:
|
||||
new_leaves |= new_leave
|
||||
return new_leaves
|
||||
|
||||
# ------------------------------------------------------------
|
||||
# Activity methods
|
||||
# ------------------------------------------------------------
|
||||
|
||||
@@ -67,22 +67,20 @@ class CalendarLeaves(models.Model):
|
||||
sick_time_status = self.env.ref('hr_holidays.holiday_status_sl')
|
||||
for previous_duration, leave, state in zip(previous_durations, leaves, previous_states):
|
||||
duration_difference = previous_duration - leave.number_of_days
|
||||
if duration_difference > 0 and leave['holiday_allocation_id'] and leave.number_of_days == 0.0:
|
||||
message = False
|
||||
if duration_difference > 0 and leave['holiday_allocation_id']:
|
||||
message = _("Due to a change in global time offs, you have been granted %s day(s) back.", duration_difference)
|
||||
leave._notify_change(message)
|
||||
if leave.number_of_days > previous_duration\
|
||||
and leave.holiday_status_id not in sick_time_status:
|
||||
new_leaves = leave.split_leave(time_domain_dict)
|
||||
leaves |= new_leaves
|
||||
previous_states += [state] * len(new_leaves)
|
||||
|
||||
leaves_to_cancel = self.env['hr.leave']
|
||||
for state, leave in zip(previous_states, leaves):
|
||||
leave.write({'state': state})
|
||||
if leave.number_of_days == 0.0:
|
||||
leaves_to_cancel |= leave
|
||||
|
||||
leaves_to_cancel._force_cancel(_("a new public holiday completely overrides this leave."), 'mail.mt_comment')
|
||||
message = _("Due to a change in global time offs, %s extra day(s) have been taken from your allocation. Please review this leave if you need it to be changed.", -1 * duration_difference)
|
||||
try:
|
||||
leave.write({'state': state})
|
||||
leave._check_holidays()
|
||||
except ValidationError:
|
||||
leave.action_refuse()
|
||||
message = _("Due to a change in global time offs, this leave no longer has the required amount of available allocation and has been set to refused. Please review this leave.")
|
||||
if message:
|
||||
leave._notify_change(message)
|
||||
|
||||
def _convert_timezone(self, utc_naive_datetime, tz_from, tz_to):
|
||||
"""
|
||||
|
||||
@@ -758,7 +758,7 @@ class TestLeaveRequests(TestHrHolidaysCommon):
|
||||
'date_from': '2021-12-15 00:00:00',
|
||||
'date_to': '2021-12-15 23:59:59',
|
||||
})
|
||||
self.assertEqual(time_off_1.number_of_days, 2)
|
||||
self.assertEqual(time_off_1.number_of_days, 5)
|
||||
self.assertEqual(time_off_2.number_of_days, 4)
|
||||
|
||||
def test_time_off_recovery_on_unlink(self):
|
||||
@@ -776,9 +776,9 @@ class TestLeaveRequests(TestHrHolidaysCommon):
|
||||
})
|
||||
self.assertEqual(time_off.number_of_days, 4)
|
||||
global_time_off.unlink()
|
||||
self.assertEqual(time_off.number_of_days, 3)
|
||||
self.assertEqual(time_off.number_of_days, 5)
|
||||
|
||||
def test_time_off_auto_cancel(self):
|
||||
def test_time_off_duration_zero(self):
|
||||
time_off = self.env['hr.leave'].create({
|
||||
'name': 'Holiday Request',
|
||||
'employee_id': self.employee_emp_id,
|
||||
@@ -786,12 +786,15 @@ class TestLeaveRequests(TestHrHolidaysCommon):
|
||||
'date_from': '2021-11-15 00:00:00',
|
||||
'date_to': '2021-11-19 23:59:59',
|
||||
})
|
||||
self.assertEqual(time_off.number_of_days, 5)
|
||||
self.env['resource.calendar.leaves'].create({
|
||||
'name': 'Global Time Off',
|
||||
'date_from': '2021-11-15 00:00:00',
|
||||
'date_to': '2021-11-19 23:59:59',
|
||||
})
|
||||
self.assertEqual(time_off.active, False)
|
||||
self.assertEqual(time_off.active, True)
|
||||
self.assertEqual(time_off.number_of_days, 0)
|
||||
|
||||
def test_holiday_type_requires_no_allocation(self):
|
||||
# holiday_type_2 initially requires an allocation
|
||||
# Once an allocation is granted and a leave is taken,
|
||||
|
||||
Reference in New Issue
Block a user