From d12808bb9f121195c87e87264a0965ae3cffcd60 Mon Sep 17 00:00:00 2001 From: Yannick Tivisse Date: Wed, 8 Apr 2020 11:00:49 +0000 Subject: [PATCH] [FIX] hr_holidays: Split existing leaves for company leaves Purpose ======= Currently, when setting a global leave for a company, the leave validation raises an UserError if there is already a leave on those dates. Specification ============= Manage the conflicts for easy tests cases: - The global time off is taken in days - The conflicting leaves are all in days/half days For those use cases, refuse all the conflicting leaves and recreate other one before and/or after the global time off, according to the use case. Taskid: 2230549 X-original-commit: 20ef422fea7793fc87dbf51c43699bb7872036f9 --- addons/hr_holidays/models/hr_leave.py | 81 ++++- addons/hr_holidays/tests/__init__.py | 1 + .../hr_holidays/tests/test_company_leave.py | 280 ++++++++++++++++++ .../hr_holidays/tests/test_leave_requests.py | 3 + 4 files changed, 359 insertions(+), 6 deletions(-) create mode 100644 addons/hr_holidays/tests/test_company_leave.py diff --git a/addons/hr_holidays/models/hr_leave.py b/addons/hr_holidays/models/hr_leave.py index 649a0bfbeeb..9915b9825fa 100644 --- a/addons/hr_holidays/models/hr_leave.py +++ b/addons/hr_holidays/models/hr_leave.py @@ -107,7 +107,7 @@ class HolidaysRequest(models.Model): private_name = fields.Char('Time Off Description', groups='hr_holidays.group_hr_holidays_user') state = fields.Selection([ ('draft', 'To Submit'), - ('cancel', 'Cancelled'), + ('cancel', 'Cancelled'), # YTI This state seems to be unused. To remove ('confirm', 'To Approve'), ('refuse', 'Refused'), ('validate1', 'Second Approval'), @@ -939,10 +939,79 @@ class HolidaysRequest(models.Model): else: employees = holiday.department_id.member_ids - if self.env['hr.leave'].search_count([('date_from', '<=', holiday.date_to), ('date_to', '>', holiday.date_from), - ('state', 'not in', ['cancel', 'refuse']), ('holiday_type', '=', 'employee'), - ('employee_id', 'in', employees.ids)]): - raise ValidationError(_('You can not have 2 leaves that overlaps on the same day.')) + conflicting_leaves = self.env['hr.leave'].with_context( + tracking_disable=True, + mail_activity_automation_skip=True, + leave_fast_create=True + ).search([ + ('date_from', '<=', holiday.date_to), + ('date_to', '>', holiday.date_from), + ('state', 'not in', ['cancel', 'refuse']), + ('holiday_type', '=', 'employee'), + ('employee_id', 'in', employees.ids)]) + + if conflicting_leaves: + # YTI: More complex use cases could be managed in master + if holiday.leave_type_request_unit != 'day' or any(l.leave_type_request_unit == 'hour' for l in conflicting_leaves): + raise ValidationError(_('You can not have 2 leaves that overlaps on the same day.')) + + for conflicting_leave in conflicting_leaves: + if conflicting_leave.leave_type_request_unit == 'half_day' and conflicting_leave.request_unit_half: + conflicting_leave.action_refuse() + continue + # Leaves in days + split_leaves = self.env['hr.leave'] + target_state = conflicting_leave.state + conflicting_leave.action_refuse() + if conflicting_leave.date_from < holiday.date_from: + before_leave_vals = conflicting_leave.copy_data({ + 'date_from': conflicting_leave.date_from.date(), + 'date_to': holiday.date_from.date() + timedelta(days=-1), + })[0] + before_leave = self.env['hr.leave'].new(before_leave_vals) + before_leave._onchange_request_parameters() + # Could happen for part-time contract, that time off is not necessary + # anymore. + # Imagine you work on monday-wednesday-friday only. + # You take a time off on friday. + # We create a company time off on friday. + # By looking at the last attendance before the company time off + # start date to compute the date_to, you would have a date_from > date_to. + # Just don't create the leave at that time. That's the reason why we use + # new instead of create. As the leave is not actually created yet, the sql + # constraint didn't check date_from < date_to yet. + if before_leave.date_from < before_leave.date_to: + split_leaves |= self.env['hr.leave'].with_context( + tracking_disable=True, + mail_activity_automation_skip=True, + leave_fast_create=True + ).create(before_leave._convert_to_write(before_leave._cache)) + if conflicting_leave.date_to > holiday.date_to: + after_leave_vals = conflicting_leave.copy_data({ + 'date_from': holiday.date_to.date() + timedelta(days=1), + 'date_to': conflicting_leave.date_to.date(), + })[0] + after_leave = self.env['hr.leave'].new(after_leave_vals) + after_leave._onchange_request_parameters() + # Could happen for part-time contract, that time off is not necessary + # anymore. + if after_leave.date_from < after_leave.date_to: + split_leaves |= self.env['hr.leave'].with_context( + tracking_disable=True, + mail_activity_automation_skip=True, + leave_fast_create=True + ).create(after_leave._convert_to_write(after_leave._cache)) + for split_leave in split_leaves: + if target_state == 'draft': + continue + if target_state == 'confirm': + split_leave.action_confirm() + elif target_state == 'validate1': + split_leave.action_confirm() + split_leave.action_approve() + elif target_state == 'validate': + split_leave.action_confirm() + split_leave.action_validate() values = [holiday._prepare_holiday_values(employee) for employee in employees] leaves = self.env['hr.leave'].with_context( @@ -962,7 +1031,7 @@ class HolidaysRequest(models.Model): def action_refuse(self): current_employee = self.env['hr.employee'].search([('user_id', '=', self.env.uid)], limit=1) - if any(holiday.state not in ['confirm', 'validate', 'validate1'] for holiday in self): + if any(holiday.state not in ['draft', 'confirm', 'validate', 'validate1'] for holiday in self): raise UserError(_('Time off request must be confirmed or validated in order to refuse it.')) validated_holidays = self.filtered(lambda hol: hol.state == 'validate1') diff --git a/addons/hr_holidays/tests/__init__.py b/addons/hr_holidays/tests/__init__.py index bc355646af0..1fb97c40b74 100644 --- a/addons/hr_holidays/tests/__init__.py +++ b/addons/hr_holidays/tests/__init__.py @@ -10,3 +10,4 @@ from . import test_accrual_allocations from . import test_change_department from . import test_leave_requests from . import test_out_of_office +from . import test_company_leave diff --git a/addons/hr_holidays/tests/test_company_leave.py b/addons/hr_holidays/tests/test_company_leave.py new file mode 100644 index 00000000000..de99fa7987b --- /dev/null +++ b/addons/hr_holidays/tests/test_company_leave.py @@ -0,0 +1,280 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from datetime import date, datetime + +from odoo.tests import tagged +from odoo.tests.common import SavepointCase + +@tagged('company_leave') +class TestCompanyLeave(SavepointCase): + """ Test leaves for a whole company, conflict resolutions """ + + @classmethod + def setUpClass(cls): + super(TestCompanyLeave, cls).setUpClass() + cls.company = cls.env['res.company'].create({'name': 'A company'}) + + cls.bank_holiday = cls.env['hr.leave.type'].create({ + 'name': 'Bank Holiday', + 'responsible_id': cls.env.user.id, + 'company_id': cls.company.id, + }) + + cls.paid_time_off = cls.env['hr.leave.type'].create({ + 'name': 'Paid Time Off', + 'request_unit': 'day', + 'leave_validation_type': 'both', + 'company_id': cls.company.id, + }) + + cls.employee = cls.env['hr.employee'].create({ + 'name': 'My Employee', + 'company_id': cls.company.id, + }) + + def test_leave_whole_company_01(self): + # TEST CASE 1: Leaves taken in days. Take a 3 days leave + # Add a company leave on the second day. + # Check that leave is split into 2. + + leave = self.env['hr.leave'].create({ + 'name': 'Hol11', + 'employee_id': self.employee.id, + 'holiday_status_id': self.paid_time_off.id, + 'request_date_from': date(2020, 1, 7), + 'date_from': date(2020, 1, 7), + 'request_date_to': date(2020, 1, 9), + 'date_to': date(2020, 1, 9), + 'number_of_days': 3, + }) + leave._onchange_request_parameters() + + company_leave = self.env['hr.leave'].create({ + 'name': 'Bank Holiday', + 'holiday_type': 'company', + 'mode_company_id': self.company.id, + 'holiday_status_id': self.bank_holiday.id, + 'date_from': date(2020, 1, 8), + 'request_date_from': date(2020, 1, 8), + 'date_to': date(2020, 1, 8), + 'request_date_to': date(2020, 1, 8), + 'number_of_days': 1, + }) + company_leave._onchange_request_parameters() + + company_leave.action_validate() + + all_leaves = self.env['hr.leave'].search([('employee_id', '=', self.employee.id)], order='id') + self.assertEqual(len(all_leaves), 4) + # Original Leave + self.assertEqual(leave.state, 'refuse') + # before leave + self.assertEqual(all_leaves[1].date_from, datetime(2020, 1, 7, 7, 0)) + self.assertEqual(all_leaves[1].date_to, datetime(2020, 1, 7, 16, 0)) + self.assertEqual(all_leaves[1].number_of_days, 1) + self.assertEqual(all_leaves[1].state, 'confirm') + # After leave + self.assertEqual(all_leaves[2].date_from, datetime(2020, 1, 9, 7, 0)) + self.assertEqual(all_leaves[2].date_to, datetime(2020, 1, 9, 16, 0)) + self.assertEqual(all_leaves[2].number_of_days, 1) + self.assertEqual(all_leaves[2].state, 'confirm') + # Company Leave + self.assertEqual(all_leaves[3].date_from, datetime(2020, 1, 8, 7, 0)) + self.assertEqual(all_leaves[3].date_to, datetime(2020, 1, 8, 16, 0)) + self.assertEqual(all_leaves[3].number_of_days, 1) + self.assertEqual(all_leaves[3].state, 'validate') + + + def test_leave_whole_company_02(self): + # TEST CASE 2: Leaves taken in half-days. Take a 3 days leave + # Add a company leave on the second day + # Check that leave is split into 2 + self.paid_time_off.request_unit = 'half_day' + + leave = self.env['hr.leave'].create({ + 'name': 'Hol11', + 'employee_id': self.employee.id, + 'holiday_status_id': self.paid_time_off.id, + 'request_date_from': date(2020, 1, 7), + 'date_from': date(2020, 1, 7), + 'request_date_to': date(2020, 1, 9), + 'date_to': date(2020, 1, 9), + 'number_of_days': 3, + }) + leave._onchange_request_parameters() + + company_leave = self.env['hr.leave'].create({ + 'name': 'Bank Holiday', + 'holiday_type': 'company', + 'mode_company_id': self.company.id, + 'holiday_status_id': self.bank_holiday.id, + 'date_from': date(2020, 1, 8), + 'request_date_from': date(2020, 1, 8), + 'date_to': date(2020, 1, 8), + 'request_date_to': date(2020, 1, 8), + 'number_of_days': 1, + }) + company_leave._onchange_request_parameters() + + company_leave.action_validate() + + all_leaves = self.env['hr.leave'].search([('employee_id', '=', self.employee.id)], order='id') + self.assertEqual(len(all_leaves), 4) + # Original Leave + self.assertEqual(leave.state, 'refuse') + # before leave + self.assertEqual(all_leaves[1].date_from, datetime(2020, 1, 7, 7, 0)) + self.assertEqual(all_leaves[1].date_to, datetime(2020, 1, 7, 16, 0)) + self.assertEqual(all_leaves[1].number_of_days, 1) + self.assertEqual(all_leaves[1].state, 'confirm') + # After leave + self.assertEqual(all_leaves[2].date_from, datetime(2020, 1, 9, 7, 0)) + self.assertEqual(all_leaves[2].date_to, datetime(2020, 1, 9, 16, 0)) + self.assertEqual(all_leaves[2].number_of_days, 1) + self.assertEqual(all_leaves[2].state, 'confirm') + # Company Leave + self.assertEqual(all_leaves[3].date_from, datetime(2020, 1, 8, 7, 0)) + self.assertEqual(all_leaves[3].date_to, datetime(2020, 1, 8, 16, 0)) + self.assertEqual(all_leaves[3].number_of_days, 1) + self.assertEqual(all_leaves[3].state, 'validate') + + def test_leave_whole_company_03(self): + # TEST CASE 3: Leaves taken in half-days. Take a 0.5 days leave + # Add a company leave on the same day + # Check that leave refused + self.paid_time_off.request_unit = 'half_day' + + leave = self.env['hr.leave'].create({ + 'name': 'Hol11', + 'employee_id': self.employee.id, + 'holiday_status_id': self.paid_time_off.id, + 'request_date_from': date(2020, 1, 7), + 'request_date_to': date(2020, 1, 7), + 'number_of_days': 0.5, + 'request_unit_half': True, + 'request_date_from_period': 'am', + + }) + leave._onchange_request_parameters() + + company_leave = self.env['hr.leave'].create({ + 'name': 'Bank Holiday', + 'holiday_type': 'company', + 'mode_company_id': self.company.id, + 'holiday_status_id': self.bank_holiday.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': 1, + }) + company_leave._onchange_request_parameters() + + company_leave.action_validate() + + all_leaves = self.env['hr.leave'].search([('employee_id', '=', self.employee.id)], order='id') + self.assertEqual(len(all_leaves), 2) + # Original Leave + self.assertEqual(leave.state, 'refuse') + # Company Leave + self.assertEqual(all_leaves[1].date_from, datetime(2020, 1, 7, 7, 0)) + self.assertEqual(all_leaves[1].date_to, datetime(2020, 1, 7, 16, 0)) + self.assertEqual(all_leaves[1].number_of_days, 1) + self.assertEqual(all_leaves[1].state, 'validate') + + def test_leave_whole_company_04(self): + # TEST CASE 4: Leaves taken in days. Take a 1 days leave + # Add a company leave on the same day + # Check that leave is refused + self.paid_time_off.request_unit = 'day' + + leave = self.env['hr.leave'].create({ + 'name': 'Hol11', + 'employee_id': self.employee.id, + 'holiday_status_id': self.paid_time_off.id, + 'request_date_from': date(2020, 1, 9), + 'request_date_to': date(2020, 1, 9), + 'number_of_days': 1, + + }) + leave._onchange_request_parameters() + + company_leave = self.env['hr.leave'].create({ + 'name': 'Bank Holiday', + 'holiday_type': 'company', + 'mode_company_id': self.company.id, + 'holiday_status_id': self.bank_holiday.id, + 'date_from': date(2020, 1, 9), + 'request_date_from': date(2020, 1, 9), + 'date_to': date(2020, 1, 9), + 'request_date_to': date(2020, 1, 9), + 'number_of_days': 1, + }) + company_leave._onchange_request_parameters() + + company_leave.action_validate() + + all_leaves = self.env['hr.leave'].search([('employee_id', '=', self.employee.id)], order='id') + self.assertEqual(len(all_leaves), 2) + # Original Leave + self.assertEqual(leave.state, 'refuse') + # Company Leave + self.assertEqual(all_leaves[1].date_from, datetime(2020, 1, 9, 7, 0)) + self.assertEqual(all_leaves[1].date_to, datetime(2020, 1, 9, 16, 0)) + self.assertEqual(all_leaves[1].number_of_days, 1) + self.assertEqual(all_leaves[1].state, 'validate') + + def test_leave_whole_company_06(self): + # Test case 6: Leaves taken in days. But the employee + # only works on Monday, Wednesday and Friday + # Takes a time off for all the week (3 days), should be split + + self.employee.resource_calendar_id.write({'attendance_ids': [ + (5, 0, 0), + (0, 0, {'name': 'Monday Morning', 'dayofweek': '0', 'hour_from': 8, 'hour_to': 12, 'day_period': 'morning'}), + (0, 0, {'name': 'Monday Afternoon', 'dayofweek': '0', 'hour_from': 13, 'hour_to': 17, 'day_period': 'afternoon'}), + (0, 0, {'name': 'Wednesday Morning', 'dayofweek': '2', 'hour_from': 8, 'hour_to': 12, 'day_period': 'morning'}), + (0, 0, {'name': 'Wednesday Afternoon', 'dayofweek': '2', 'hour_from': 13, 'hour_to': 17, 'day_period': 'afternoon'}), + (0, 0, {'name': 'Friday Morning', 'dayofweek': '4', 'hour_from': 8, 'hour_to': 12, 'day_period': 'morning'}), + (0, 0, {'name': 'Friday Afternoon', 'dayofweek': '4', 'hour_from': 13, 'hour_to': 17, 'day_period': 'afternoon'}) + ]}) + + leave = self.env['hr.leave'].create({ + 'name': 'Hol11', + 'employee_id': self.employee.id, + 'holiday_status_id': self.paid_time_off.id, + 'request_date_from': date(2020, 1, 6), + 'request_date_to': date(2020, 1, 10), + 'number_of_days': 3, + }) + leave._onchange_request_parameters() + + company_leave = self.env['hr.leave'].create({ + 'name': 'Bank Holiday', + 'holiday_type': 'company', + 'mode_company_id': self.company.id, + 'holiday_status_id': self.bank_holiday.id, + 'date_from': date(2020, 1, 10), + 'request_date_from': date(2020, 1, 10), + 'date_to': date(2020, 1, 10), + 'request_date_to': date(2020, 1, 10), + 'number_of_days': 1, + }) + company_leave._onchange_request_parameters() + company_leave.action_validate() + + all_leaves = self.env['hr.leave'].search([('employee_id', '=', self.employee.id)], order='id') + self.assertEqual(len(all_leaves), 3) + # Original Leave + self.assertEqual(leave.state, 'refuse') + # before leave + self.assertEqual(all_leaves[1].date_from, datetime(2020, 1, 6, 7, 0)) + self.assertEqual(all_leaves[1].date_to, datetime(2020, 1, 9, 16, 0)) + self.assertEqual(all_leaves[1].number_of_days, 2) + self.assertEqual(all_leaves[1].state, 'confirm') + # Company Leave + self.assertEqual(all_leaves[2].date_from, datetime(2020, 1, 10, 7, 0)) + self.assertEqual(all_leaves[2].date_to, datetime(2020, 1, 10, 16, 0)) + self.assertEqual(all_leaves[2].number_of_days, 1) + self.assertEqual(all_leaves[2].state, 'validate') diff --git a/addons/hr_holidays/tests/test_leave_requests.py b/addons/hr_holidays/tests/test_leave_requests.py index fd9652d6d9a..5f239c1c6e1 100644 --- a/addons/hr_holidays/tests/test_leave_requests.py +++ b/addons/hr_holidays/tests/test_leave_requests.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. + from datetime import datetime, date from dateutil.relativedelta import relativedelta from pytz import timezone, UTC @@ -8,9 +9,11 @@ from odoo import fields from odoo.exceptions import ValidationError from odoo.tools import mute_logger from odoo.tests.common import Form +from odoo.tests import tagged from odoo.addons.hr_holidays.tests.common import TestHrHolidaysCommon +@tagged('leave_requests') class TestLeaveRequests(TestHrHolidaysCommon): def _check_holidays_status(self, holiday_status, ml, lt, rl, vrl):