From e9bd75f32dc82ae849c520bc5eeefa237b450e63 Mon Sep 17 00:00:00 2001 From: Dossogne Bertrand Date: Thu, 1 Feb 2024 16:09:11 +0100 Subject: [PATCH] [FIX] hr_holidays: correct validity check Before this commit, discrepancies between leaves and allocations for an specific employee and a specific time off type would have prevented the employee from taking any time off for that time off type. It would also affect their dashboard. __How to reproduce the issue:__ - time off type without negative amount - create 2 allocations: last year and this year - create a leave last year - remove last year allocation from DB - you cannot take leaves this year - dashboard amount is affected by last year's leave __Changes brought with this commit:__ This commit keeps the logic identical for negative time off types. However, for the others, the check ensuring the allocation validity for the leave will now check the discrepancies before and after a time off creation or modification. The error will be thrown if the discrepancies have changed, meaning that the leave had an effect on it. This way, any other issue will not prevent leave creation. Those leaves are still noticeable through the warning though, meaning that they can be noticed and dealt with accordingly. Part-of: odoo/odoo#152311 --- addons/hr_holidays/models/hr_employee.py | 4 +++- addons/hr_holidays/models/hr_leave.py | 27 +++++++++++++++++----- addons/hr_holidays/models/hr_leave_type.py | 8 ++++--- 3 files changed, 29 insertions(+), 10 deletions(-) diff --git a/addons/hr_holidays/models/hr_employee.py b/addons/hr_holidays/models/hr_employee.py index 61534eed7f2..fe47d01d89e 100644 --- a/addons/hr_holidays/models/hr_employee.py +++ b/addons/hr_holidays/models/hr_employee.py @@ -394,6 +394,9 @@ class HrEmployee(models.Model): ('employee_id', 'in', employees.ids), ('state', 'in', ['confirm', 'validate1', 'validate']), ] + if self.env.context.get('ignored_leave_ids'): + leaves_domain.append(('id', 'not in', self.env.context.get('ignored_leave_ids'))) + if not target_date: target_date = fields.Date.today() if ignore_future: @@ -454,7 +457,6 @@ class HrEmployee(models.Model): 'is_virtual': True, }), 'total_virtual_excess': 0, - 'total_real_excess': 0, 'exceeding_duration': 0, 'to_recheck_leaves': self.env['hr.leave'] }) diff --git a/addons/hr_holidays/models/hr_leave.py b/addons/hr_holidays/models/hr_leave.py index 6076404c5fa..99b1b12d2c1 100644 --- a/addons/hr_holidays/models/hr_leave.py +++ b/addons/hr_holidays/models/hr_leave.py @@ -749,18 +749,31 @@ Attempting to double-book your time off won't magically make your vacation 2x be if holiday.state in ['cancel', 'refuse', 'validate1', 'validate']: raise ValidationError(_("This modification is not allowed in the current state.")) - @api.constrains('date_from', 'date_to') def _check_validity(self): + sorted_leaves = defaultdict(lambda: self.env['hr.leave']) for leave in self: - leave_type = leave.holiday_status_id + sorted_leaves[(leave.holiday_status_id, leave.date_from.date())] |= leave + for (leave_type, date_from), leaves in sorted_leaves.items(): if leave_type.requires_allocation == 'no': continue - employees = leave._get_employees_from_holiday_type() - date_from = leave.date_from.date() + employees = self.env['hr.employee'] + for leave in leaves: + employees |= leave._get_employees_from_holiday_type() leave_data = leave_type.get_allocation_data(employees, date_from) - max_excess = leave_type.max_allowed_negative if leave_type.allows_negative else 0 + if leave_type.allows_negative: + max_excess = leave_type.max_allowed_negative + for employee in employees: + if leave_data[employee][0][1]['virtual_remaining_leaves'] < -max_excess: + raise ValidationError(_("There is no valid allocation to cover that request.")) + continue + + previous_leave_data = leave_type.with_context( + ignored_leave_ids=leaves.ids + ).get_allocation_data(employees, date_from) for employee in employees: - if leave_data[employee][0][1]['total_virtual_excess'] > max_excess: + previous_emp_data = previous_leave_data[employee][0][1]['virtual_excess_data'] + emp_data = leave_data[employee][0][1]['virtual_excess_data'] + if previous_emp_data != emp_data and len(emp_data) >= len(previous_emp_data): raise ValidationError(_("There is no valid allocation to cover that request.")) #################################################### @@ -907,6 +920,7 @@ Attempting to double-book your time off won't magically make your vacation 2x be self._check_double_validation_rules(employee_id, values.get('state', False)) holidays = super(HolidaysRequest, self.with_context(mail_create_nosubscribe=True)).create(vals_list) + holidays._check_validity() for holiday in holidays: if not self._context.get('leave_fast_create'): @@ -956,6 +970,7 @@ Attempting to double-book your time off won't magically make your vacation 2x be if 'date_to' in values: values['request_date_to'] = values['date_to'] result = super(HolidaysRequest, self).write(values) + self._check_validity() if not self.env.context.get('leave_fast_create'): for holiday in self: if employee_id: diff --git a/addons/hr_holidays/models/hr_leave_type.py b/addons/hr_holidays/models/hr_leave_type.py index cc908dc009b..c36749c2a0a 100644 --- a/addons/hr_holidays/models/hr_leave_type.py +++ b/addons/hr_holidays/models/hr_leave_type.py @@ -380,7 +380,9 @@ class HolidaysType(models.Model): elif not target_date: target_date = fields.Date.today() - allocations_leaves_consumed, extra_data = employees._get_consumed_leaves(self, target_date) + allocations_leaves_consumed, extra_data = employees.with_context( + ignored_leave_ids=self.env.context.get('ignored_leave_ids') + )._get_consumed_leaves(self, target_date) leave_type_requires_allocation = self.filtered(lambda lt: lt.requires_allocation == 'yes') for employee in employees: @@ -403,7 +405,6 @@ class HolidaysType(models.Model): 'holds_changes': False, 'total_virtual_excess': 0, 'virtual_excess_data': {}, - 'total_real_excess': 0, 'exceeding_duration': extra_data[employee][leave_type]['exceeding_duration'], 'request_unit': leave_type.request_unit, 'icon': leave_type.sudo().icon_id.url, @@ -418,6 +419,8 @@ class HolidaysType(models.Model): lt_info[1]['virtual_excess_data'].update({ excess_date.strftime('%Y-%m-%d'): excess_days }), + if not leave_type.allows_negative: + continue lt_info[1]['virtual_leaves_taken'] += amount lt_info[1]['virtual_remaining_leaves'] -= amount lt_info[1]['total_virtual_excess'] += amount @@ -427,7 +430,6 @@ class HolidaysType(models.Model): lt_info[1]['leaves_approved'] += amount lt_info[1]['leaves_taken'] += amount lt_info[1]['remaining_leaves'] -= amount - lt_info[1]['total_real_excess'] += amount allocations_now = self.env['hr.leave.allocation'] allocations_date = self.env['hr.leave.allocation'] allocations_with_remaining_leaves = self.env['hr.leave.allocation']