From b6ef46f71ce52b69020016d6376b83f77ae2ec45 Mon Sep 17 00:00:00 2001 From: Philippe Wauthy Date: Wed, 17 Nov 2021 09:12:53 +0000 Subject: [PATCH] [IMP] hr_holidays: show conflicting time off in validation error message Description of the issue/feature this PR addresses: In hr_holidays, there is no information about the conflicting time off when a second time off is scheduled at the same time. There is a validation error saying "You can not set 2 time off that overlaps on the same day for the same employee". The validation error should display the conflicting time off Current behavior before PR: The validation error displays the following: "You can not set 2 time off that overlaps on the same day for the same employee" Desired behavior after PR is merged: The validation error will display the following: "You can not set 2 time off that overlaps on the same day for the same employee. Existing time off: Mitchell Admin / Trip with Family : 24.00 hours / from 06/09/2021 to 08/09/2021 / To Approve" task-2641682 closes odoo/odoo#76452 Related: odoo/enterprise#22490 Signed-off-by: Kevin Baptiste --- addons/hr_holidays/models/hr_leave.py | 78 +++++++++++++++---- .../hr_holidays/tests/test_company_leave.py | 8 ++ addons/hr_holidays/views/hr_views.xml | 4 +- .../tests/test_performance.py | 2 + .../tests/test_work_entry.py | 2 + 5 files changed, 78 insertions(+), 16 deletions(-) diff --git a/addons/hr_holidays/models/hr_leave.py b/addons/hr_holidays/models/hr_leave.py index 9b1068e892c..ded99b4548e 100644 --- a/addons/hr_holidays/models/hr_leave.py +++ b/addons/hr_holidays/models/hr_leave.py @@ -16,6 +16,7 @@ from odoo.addons.resource.models.resource import float_to_time, HOURS_PER_DAY from odoo.exceptions import AccessError, UserError, ValidationError from odoo.tools import float_compare from odoo.tools.float_utils import float_round +from odoo.tools.misc import format_date from odoo.tools.translate import _ from odoo.osv import expression @@ -81,11 +82,6 @@ class HolidaysRequest(models.Model): lt = self.env['hr.leave.type'].browse(defaults.get('holiday_status_id')) defaults['state'] = 'confirm' if lt and lt.leave_validation_type != 'no_validation' else 'draft' - now = fields.Datetime.now() - if 'date_from' not in defaults: - defaults.update({'date_from': now}) - if 'date_to' not in defaults: - defaults.update({'date_to': now}) return defaults def _default_get_request_parameters(self, values): @@ -651,17 +647,68 @@ class HolidaysRequest(models.Model): def _check_date(self): if self.env.context.get('leave_skip_date_check', False): return - for holiday in self.filtered('employee_id'): + + all_employees = self.employee_id | self.employee_ids + all_leaves = self.search([ + ('date_from', '<', max(self.mapped('date_to'))), + ('date_to', '>', min(self.mapped('date_from'))), + ('employee_id', 'in', all_employees.ids), + ('id', 'not in', self.ids), + ('state', 'not in', ['cancel', 'refuse']), + ]) + for holiday in self: domain = [ ('date_from', '<', holiday.date_to), ('date_to', '>', holiday.date_from), - ('employee_id', '=', holiday.employee_id.id), ('id', '!=', holiday.id), ('state', 'not in', ['cancel', 'refuse']), ] - nholidays = self.search_count(domain) - if nholidays: - raise ValidationError(_('You can not set 2 time off that overlaps on the same day for the same employee.')) + + employee_ids = (holiday.employee_id | holiday.employee_ids).ids + search_domain = domain + [('employee_id', 'in', employee_ids)] + conflicting_holidays = all_leaves.filtered_domain(search_domain) + + if conflicting_holidays: + conflicting_holidays_list = [] + # Do not display the name of the employee if the conflicting holidays have an employee_id.user_id equivalent to the user id + holidays_only_have_uid = bool(holiday.employee_id) + holiday_states = dict(conflicting_holidays.fields_get(allfields=['state'])['state']['selection']) + for conflicting_holiday in conflicting_holidays: + conflicting_holiday_data = {} + conflicting_holiday_data['employee_name'] = conflicting_holiday.employee_id.name + conflicting_holiday_data['date_from'] = format_date(self.env, min(conflicting_holiday.mapped('date_from'))) + conflicting_holiday_data['date_to'] = format_date(self.env, min(conflicting_holiday.mapped('date_to'))) + conflicting_holiday_data['state'] = holiday_states[conflicting_holiday.state] + if conflicting_holiday.employee_id.user_id.id != self.env.uid: + holidays_only_have_uid = False + if conflicting_holiday_data not in conflicting_holidays_list: + conflicting_holidays_list.append(conflicting_holiday_data) + if not conflicting_holidays_list: + return + conflicting_holidays_strings = [] + if holidays_only_have_uid: + for conflicting_holiday_data in conflicting_holidays_list: + conflicting_holidays_string = _('From %(date_from)s To %(date_to)s - %(state)s', + date_from=conflicting_holiday_data['date_from'], + date_to=conflicting_holiday_data['date_to'], + state=conflicting_holiday_data['state']) + conflicting_holidays_strings.append(conflicting_holidays_string) + raise ValidationError(_('You can not set two time off that overlap on the same day.\nExisting time off:\n%s') % + ('\n'.join(conflicting_holidays_strings))) + for conflicting_holiday_data in conflicting_holidays_list: + conflicting_holidays_string = _('%(employee_name)s - From %(date_from)s To %(date_to)s - %(state)s', + employee_name=conflicting_holiday_data['employee_name'], + date_from=conflicting_holiday_data['date_from'], + date_to=conflicting_holiday_data['date_to'], + state=conflicting_holiday_data['state']) + conflicting_holidays_strings.append(conflicting_holidays_string) + conflicting_employees = set(employee_ids) - set(conflicting_holidays.employee_id.ids) + # Only one employee has a conflicting holiday + if len(conflicting_employees) == len(employee_ids) - 1: + raise ValidationError(_('You can not set two time off that overlap on the same day for the same employee.\nExisting time off:\n%s') % + ('\n'.join(conflicting_holidays_strings))) + raise ValidationError(_('You can not set two time off that overlap on the same day for the same employees.\nExisting time off:\n%s') % + ('\n'.join(conflicting_holidays_strings))) @api.constrains('state', 'number_of_days', 'holiday_status_id') def _check_holidays(self): @@ -943,10 +990,13 @@ class HolidaysRequest(models.Model): now = fields.Datetime.now() if not self.user_has_groups('hr_holidays.group_hr_holidays_user'): - if any(hol.state not in ['draft', 'confirm'] for hol in self): - raise UserError(error_message % state_description_values.get(self[:1].state)) - if any(hol.date_from < now for hol in self): - raise UserError(_('You cannot delete a time off which is in the past')) + for hol in self: + if hol.state not in ['draft', 'confirm']: + raise UserError(error_message % state_description_values.get(self[:1].state)) + if hol.date_from < now: + raise UserError(_('You cannot delete a time off which is in the past')) + if hol.employee_ids and not hol.employee_id: + raise UserError(_('You cannot delete a time off assigned to several employees')) else: for holiday in self.filtered(lambda holiday: holiday.state not in ['draft', 'cancel', 'confirm']): raise UserError(error_message % (state_description_values.get(holiday.state),)) diff --git a/addons/hr_holidays/tests/test_company_leave.py b/addons/hr_holidays/tests/test_company_leave.py index ef1ae0448e3..3b1e4de93ff 100644 --- a/addons/hr_holidays/tests/test_company_leave.py +++ b/addons/hr_holidays/tests/test_company_leave.py @@ -152,7 +152,9 @@ class TestCompanyLeave(TransactionCase): 'name': 'Hol11', 'employee_id': self.employee.id, 'holiday_status_id': self.paid_time_off.id, + 'date_from': date(2020, 1, 7), 'request_date_from': date(2020, 1, 7), + 'date_to': date(2020, 1, 7), 'request_date_to': date(2020, 1, 7), 'number_of_days': 0.5, 'request_unit_half': True, @@ -196,7 +198,9 @@ class TestCompanyLeave(TransactionCase): 'name': 'Hol11', 'employee_id': self.employee.id, 'holiday_status_id': self.paid_time_off.id, + 'date_from': datetime.now(), 'request_date_from': date(2020, 1, 9), + 'date_to': datetime.now(), 'request_date_to': date(2020, 1, 9), 'number_of_days': 1, @@ -247,7 +251,9 @@ class TestCompanyLeave(TransactionCase): 'name': 'Hol11', 'employee_id': self.employee.id, 'holiday_status_id': self.paid_time_off.id, + 'date_from': date(2020, 1, 6), 'request_date_from': date(2020, 1, 6), + 'date_to': date(2020, 1, 10), 'request_date_to': date(2020, 1, 10), 'number_of_days': 3, }) @@ -297,7 +303,9 @@ class TestCompanyLeave(TransactionCase): 'employee_id': employee.id, 'holiday_status_id': self.paid_time_off.id, 'request_date_from': date(2020, 3, 29), + 'date_from': date(2020, 3, 29), 'request_date_to': date(2020, 4, 1), + 'date_to': date(2020, 4, 1), 'number_of_days': 3, } for employee in employees[0:15]]) leaves._compute_date_from_to() diff --git a/addons/hr_holidays/views/hr_views.xml b/addons/hr_holidays/views/hr_views.xml index b0ea90b536e..2b8c38b50e0 100644 --- a/addons/hr_holidays/views/hr_views.xml +++ b/addons/hr_holidays/views/hr_views.xml @@ -173,7 +173,7 @@