From ac8b0802346a411b9622c96635e1bd6765f6cc35 Mon Sep 17 00:00:00 2001 From: tong-odoo Date: Thu, 24 Aug 2023 05:47:58 +0000 Subject: [PATCH] [FIX] hr_work_entry_contract: work entry timezone issue for attendance Steps to reproduce: - Install Attendances app and Employee app - Setup an employee with a contract which work entry source is from attendance - Change the working schedule to different timezone, for example Asia/Hong_Kong - By default the working schedule should be start to work from 0800 to 1700 - Now create an attendance in the first day of the contract, i.e. 1st May 2023, check in time 0700, check out time 2000 - The total work hours should be 13 - However, in the work entry, the genearted work entry is 0800-2000, only 12 work hours. Current behaviour: The generated work entry mismatches the value of the attendance Expected behaviour: The work entry should match the attendance Explanation: This issue is casued by the timezone issue. When the system generate the work entry, it will calculate the date_from datetime and date_to datetime. Then it will compare between the attendance and the date_from datetime to choose the larger datetime to put inside the work entry as start time. However, the date_from datetime didn't consider the timezone of the working schedule. In the above example, Asia/Hong Kong time is UTC+8 time. Therefore if we convert the attendance time back to UTC time. It is check in time 30 April 2023, 2300 to check out time 1 May 2023, 1200. In the mean time, if we compare between the date_from datetime (1 May 2023, 0000) and the attendance check in time (30 April 2023, 2300). The system will take the date_from time and therefore the generated work entry time is incorrect. task-3468012 closes odoo/odoo#134533 X-original-commit: 2baf4e3ebcd9ce3beb47ee2881aa3e8bd8a74cbf Related: odoo/enterprise#47007 Signed-off-by: Xavier Bol (xbo) Signed-off-by: Yannick Tivisse (yti) Co-authored-by: Yannick Tivisse --- .../models/hr_contract.py | 53 +++++++++++++++---- .../models/hr_employee.py | 12 +---- .../tests/test_global_time_off.py | 6 +-- .../tests/test_work_entry.py | 36 +++++++++++++ .../tests/test_leave.py | 8 +-- .../tests/test_multi_contract.py | 6 +-- .../test_payslip_holidays_computation.py | 2 +- .../tests/test_performance.py | 4 +- .../tests/test_work_entry.py | 4 +- 9 files changed, 96 insertions(+), 35 deletions(-) diff --git a/addons/hr_work_entry_contract/models/hr_contract.py b/addons/hr_work_entry_contract/models/hr_contract.py index 6a4a9f3b15a..3b6d4b30b5d 100644 --- a/addons/hr_work_entry_contract/models/hr_contract.py +++ b/addons/hr_work_entry_contract/models/hr_contract.py @@ -3,7 +3,7 @@ import itertools from collections import defaultdict -from datetime import datetime, date +from datetime import datetime, date, time import pytz from dateutil.relativedelta import relativedelta @@ -298,7 +298,38 @@ class HrContract(models.Model): self.ensure_one() return self.work_entry_source == 'calendar' + def generate_work_entries(self, date_start, date_stop, force=False): + # Generate work entries between 2 dates (datetime.date) + # To correctly englobe the period, the start and end periods are converted + # using the calendar timezone. + assert not isinstance(date_start, datetime) + assert not isinstance(date_stop, datetime) + + date_start = datetime.combine(fields.Datetime.to_datetime(date_start), datetime.min.time()) + date_stop = datetime.combine(fields.Datetime.to_datetime(date_stop), datetime.max.time()) + + contracts_by_company_tz = defaultdict(lambda: self.env['hr.contract']) + for contract in self: + contracts_by_company_tz[( + contract.company_id, + (contract.resource_calendar_id or contract.employee_id.resource_calendar_id).tz + )] += contract + utc = pytz.timezone('UTC') + new_work_entries = self.env['hr.work.entry'] + for (company, contract_tz), contracts in contracts_by_company_tz.items(): + tz = pytz.timezone(contract_tz) if contract_tz else pytz.utc + date_start_tz = tz.localize(date_start).astimezone(utc).replace(tzinfo=None) + date_stop_tz = tz.localize(date_stop).astimezone(utc).replace(tzinfo=None) + new_work_entries += contracts.with_company(company).sudo()._generate_work_entries( + date_start_tz, date_stop_tz, force=force) + return new_work_entries + def _generate_work_entries(self, date_start, date_stop, force=False): + # Generate work entries between 2 dates (datetime.datetime) + # This method considers that the dates are correctly localized + # based on the target timezone + assert isinstance(date_start, datetime) + assert isinstance(date_stop, datetime) self = self.with_context(tracking_disable=True) canceled_contracts = self.filtered(lambda c: c.state == 'cancel') if canceled_contracts: @@ -306,8 +337,6 @@ class HrContract(models.Model): _("Sorry, generating work entries from cancelled contracts is not allowed.") + '\n%s' % ( ', '.join(canceled_contracts.mapped('name')))) vals_list = [] - date_start = fields.Datetime.to_datetime(date_start) - date_stop = datetime.combine(fields.Datetime.to_datetime(date_stop), datetime.max.time()) self.write({'last_generation_date': fields.Date.today()}) intervals_to_generate = defaultdict(lambda: self.env['hr.contract']) @@ -318,10 +347,15 @@ class HrContract(models.Model): 'date_generated_from': date_start, 'date_generated_to': date_start, }) + utc = pytz.timezone('UTC') for contract in self: - contract_start = fields.Datetime.to_datetime(contract.date_start) + contract_tz = (contract.resource_calendar_id or contract.employee_id.resource_calendar_id).tz + tz = pytz.timezone(contract_tz) if contract_tz else pytz.utc + contract_start = tz.localize(fields.Datetime.to_datetime(contract.date_start)).astimezone(utc).replace(tzinfo=None) contract_stop = datetime.combine(fields.Datetime.to_datetime(contract.date_end or datetime.max.date()), datetime.max.time()) + if contract.date_end: + contract_stop = tz.localize(contract_stop).astimezone(utc).replace(tzinfo=None) if date_start > contract_stop or date_stop < contract_start: continue date_start_work_entries = max(date_start, contract_start) @@ -425,12 +459,12 @@ class HrContract(models.Model): def _cron_generate_missing_work_entries(self): # retrieve contracts for the current month today = fields.Date.today() - start = today + relativedelta(day=1, hour=0) - stop = today + relativedelta(months=1, day=31, hour=23, minute=59, second=59) - contracts = self.env['hr.employee']._get_all_contracts( + start = datetime.combine(today + relativedelta(day=1), time.min) + stop = datetime.combine(today + relativedelta(months=1, day=31), time.max) + all_contracts = self.env['hr.employee']._get_all_contracts( start, stop, states=['open', 'close']) # determine contracts to do (the ones whose generated dates have open periods this month) - contracts_todo = contracts.filtered(lambda c:\ + contracts_todo = all_contracts.filtered(lambda c:\ (c.date_generated_from > start or c.date_generated_to < stop) and\ (not c.last_generation_date or c.last_generation_date < today)) if not contracts_todo: @@ -449,7 +483,8 @@ class HrContract(models.Model): # It is more interesting for batching to process statically generated work entries first # since we get benefits from having multiple contracts on the same calendar contracts_todo = contracts_todo.sorted(key=lambda c: 1 if c.has_static_work_entries() else 100) - contracts_todo[:BATCH_SIZE]._generate_work_entries(start, stop, False) + contracts_todo = contracts_todo[:BATCH_SIZE].generate_work_entries( + start.date(), stop.date(), False) # if necessary, retrigger the cron to generate more work entries if countract_todo_count > BATCH_SIZE: self.env.ref('hr_work_entry_contract.ir_cron_generate_missing_work_entries')._trigger() diff --git a/addons/hr_work_entry_contract/models/hr_employee.py b/addons/hr_work_entry_contract/models/hr_employee.py index 0cc54a071fe..8c8400e2cc8 100644 --- a/addons/hr_work_entry_contract/models/hr_employee.py +++ b/addons/hr_work_entry_contract/models/hr_employee.py @@ -1,13 +1,11 @@ # -*- coding:utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from collections import defaultdict from odoo import fields, models class HrEmployee(models.Model): _inherit = 'hr.employee' - _description = 'Employee' def generate_work_entries(self, date_start, date_stop, force=False): date_start = fields.Date.to_date(date_start) @@ -18,12 +16,4 @@ class HrEmployee(models.Model): else: current_contracts = self._get_all_contracts(date_start, date_stop, states=['open', 'close']) - new_work_entries = False - contracts_by_company = defaultdict(lambda: self.env['hr.contract']) - for contract in current_contracts: - contracts_by_company[contract.company_id] |= contract - - for company, contracts in contracts_by_company.items(): - new_work_entries = bool(contracts.with_company(company).sudo()._generate_work_entries( - date_start, date_stop, force)) or new_work_entries - return new_work_entries + return current_contracts.generate_work_entries(date_start, date_stop, force=force) diff --git a/addons/hr_work_entry_contract/tests/test_global_time_off.py b/addons/hr_work_entry_contract/tests/test_global_time_off.py index bd388a0c04f..52a8bb68f9d 100644 --- a/addons/hr_work_entry_contract/tests/test_global_time_off.py +++ b/addons/hr_work_entry_contract/tests/test_global_time_off.py @@ -28,13 +28,13 @@ class TestGlobalTimeOff(TestWorkEntryBase): contract.state = 'open' contract.date_generated_from = start contract.date_generated_to = start - work_entries = contract._generate_work_entries(start, end) + work_entries = contract.generate_work_entries(start.date(), end.date()) self.assertEqual(work_entries.work_entry_type_id.id, contract._get_default_work_entry_type_id()) work_entries.unlink() contract.date_generated_from = start contract.date_generated_to = start leave.calendar_id = contract.resource_calendar_id - work_entries = contract._generate_work_entries(start, end) + work_entries = contract.generate_work_entries(start.date(), end.date()) self.assertEqual(work_entries.work_entry_type_id, leave.work_entry_type_id) def test_gto_no_calendar(self): @@ -49,5 +49,5 @@ class TestGlobalTimeOff(TestWorkEntryBase): contract.state = 'open' contract.date_generated_from = start contract.date_generated_to = start - work_entries = contract._generate_work_entries(start, end) + work_entries = contract.generate_work_entries(start.date(), end.date()) self.assertEqual(work_entries.work_entry_type_id, leave.work_entry_type_id) diff --git a/addons/hr_work_entry_contract/tests/test_work_entry.py b/addons/hr_work_entry_contract/tests/test_work_entry.py index 6dc3ef43fea..a5e6089b637 100644 --- a/addons/hr_work_entry_contract/tests/test_work_entry.py +++ b/addons/hr_work_entry_contract/tests/test_work_entry.py @@ -206,3 +206,39 @@ class TestWorkEntry(TestWorkEntryBase): with self.assertRaises(IntegrityError): with self.cr.savepoint(): (work_entry_1 + work_entry_2).write({'state': 'validated'}) + + def test_work_entry_timezone(self): + """ Test work entries with different timezone """ + hk_resource_calendar_id = self.env['resource.calendar'].create({ + 'name': 'HK Calendar', + 'tz': 'Asia/Hong_Kong', + 'hours_per_day': 8, + 'attendance_ids': [(5, 0, 0), + (0, 0, {'name': 'Monday Morning', 'dayofweek': '0', 'hour_from': 7, 'hour_to': 11, 'day_period': 'morning'}), + (0, 0, {'name': 'Monday Afternoon', 'dayofweek': '0', 'hour_from': 13, 'hour_to': 17, 'day_period': 'afternoon'}), + (0, 0, {'name': 'Tuesday Morning', 'dayofweek': '1', 'hour_from': 7, 'hour_to': 11, 'day_period': 'morning'}), + (0, 0, {'name': 'Tuesday Afternoon', 'dayofweek': '1', 'hour_from': 13, 'hour_to': 17, 'day_period': 'afternoon'}), + (0, 0, {'name': 'Wednesday Morning', 'dayofweek': '2', 'hour_from': 7, 'hour_to': 11, 'day_period': 'morning'}), + (0, 0, {'name': 'Wednesday Afternoon', 'dayofweek': '2', 'hour_from': 13, 'hour_to': 17, 'day_period': 'afternoon'}), + (0, 0, {'name': 'Thursday Morning', 'dayofweek': '3', 'hour_from': 7, 'hour_to': 11, 'day_period': 'morning'}), + (0, 0, {'name': 'Thursday Afternoon', 'dayofweek': '3', 'hour_from': 13, 'hour_to': 17, 'day_period': 'afternoon'}), + (0, 0, {'name': 'Friday Morning', 'dayofweek': '4', 'hour_from': 7, 'hour_to': 11, 'day_period': 'morning'}), + (0, 0, {'name': 'Friday Afternoon', 'dayofweek': '4', 'hour_from': 13, 'hour_to': 17, 'day_period': 'afternoon'}) + ] + }) + hk_employee = self.env['hr.employee'].create({ + 'name': 'HK Employee', + 'resource_calendar_id': hk_resource_calendar_id.id, + }) + self.env.company.resource_calendar_id = hk_resource_calendar_id + self.env['hr.contract'].create({ + 'date_start': datetime(2023, 8, 1), + 'name': 'Test Contract', + 'resource_calendar_id': hk_resource_calendar_id.id, + 'wage': 1000, + 'employee_id': hk_employee.id, + 'state': 'open', + }) + hk_employee.generate_work_entries(datetime(2023, 8, 1), datetime(2023, 8, 1)) + work_entries = self.env['hr.work.entry'].search([('employee_id', '=', hk_employee.id)]) + self.assertEqual(work_entries[0].date_start, datetime(2023, 7, 31, 23, 0)) diff --git a/addons/hr_work_entry_holidays/tests/test_leave.py b/addons/hr_work_entry_holidays/tests/test_leave.py index f7046b5f001..4652bbf6da8 100644 --- a/addons/hr_work_entry_holidays/tests/test_leave.py +++ b/addons/hr_work_entry_holidays/tests/test_leave.py @@ -103,7 +103,7 @@ class TestWorkEntryLeave(TestWorkEntryHolidaysBase): def test_refuse_leave(self): leave = self.create_leave(datetime(2019, 10, 10, 9, 0), datetime(2019, 10, 10, 18, 0)) - work_entries = self.richard_emp.contract_id._generate_work_entries(datetime(2019, 10, 10, 9, 0), datetime(2019, 10, 10, 18, 0)) + work_entries = self.richard_emp.contract_id.generate_work_entries(date(2019, 10, 10), date(2019, 10, 10)) adjacent_work_entry = self.create_work_entry(datetime(2019, 10, 7, 9, 0), datetime(2019, 10, 10, 9, 0)) self.assertTrue(all(work_entries.mapped(lambda w: w.state == 'conflict')), "Attendance work entries should all conflict with the leave") self.assertNotEqual(adjacent_work_entry.state, 'conflict', "Non overlapping work entry should not conflict") @@ -123,7 +123,7 @@ class TestWorkEntryLeave(TestWorkEntryHolidaysBase): leave = self.create_leave(start, end) leave.action_validate() work_entries = self.env['hr.work.entry'].search([('employee_id', '=', self.richard_emp.id), ('date_start', '<=', end), ('date_stop', '>=', start)]) - leave_work_entry = self.richard_emp.contract_ids._generate_work_entries(start, end) + leave_work_entry = self.richard_emp.contract_ids.generate_work_entries(start.date(), end.date()) self.assertEqual(leave_work_entry[:1].leave_id, leave) leave.action_refuse() work_entries = self.env['hr.work.entry'].search([('employee_id', '=', self.richard_emp.id), ('date_start', '>=', start), ('date_stop', '<=', end)]) @@ -162,7 +162,7 @@ class TestWorkEntryLeave(TestWorkEntryHolidaysBase): # No work entries exist yet self.assertTrue(leave.can_cancel, "The leave should still be cancellable") # can not create in the future - self.richard_emp.contract_ids._generate_work_entries(datetime(2022, 3, 21, 6), datetime(2022, 3, 25, 20)) + self.richard_emp.contract_ids.generate_work_entries(date(2022, 3, 21), date(2022, 3, 25)) work_entries = self.env['hr.work.entry'].search([('employee_id', '=', self.richard_emp.id)]) leave.invalidate_recordset(['can_cancel']) # Work entries exist but are not locked yet @@ -179,7 +179,7 @@ class TestWorkEntryLeave(TestWorkEntryHolidaysBase): existing_leaves.unlink() start = date(2022, 8, 1) end = date(2022, 8, 31) - self.contract_cdi._generate_work_entries(start, end) + self.contract_cdi.generate_work_entries(start, end) work_entries = self.env['hr.work.entry'].search([ ('employee_id', '=', self.jules_emp.id), ('date_start', '>=', start), diff --git a/addons/hr_work_entry_holidays/tests/test_multi_contract.py b/addons/hr_work_entry_holidays/tests/test_multi_contract.py index d0b150c60fe..8dfb86da762 100644 --- a/addons/hr_work_entry_holidays/tests/test_multi_contract.py +++ b/addons/hr_work_entry_holidays/tests/test_multi_contract.py @@ -35,9 +35,9 @@ class TestWorkEntryHolidaysMultiContract(TestWorkEntryHolidaysBase): # Leave during second contract leave = self.create_leave(datetime(2015, 11, 17, 7, 0), datetime(2015, 11, 20, 18, 0)) leave.action_approve() - start = datetime.strptime('2015-11-01', '%Y-%m-%d') - end_generate = datetime(2015, 11, 30, 23, 59, 59) - work_entries = self.jules_emp.contract_ids._generate_work_entries(start, end_generate) + start = date(2015, 11, 1) + end_generate = date(2015, 11, 30) + work_entries = self.jules_emp.contract_ids.generate_work_entries(start, end_generate) work_entries.action_validate() work_entries = work_entries.filtered(lambda we: we.contract_id == self.contract_cdi) diff --git a/addons/hr_work_entry_holidays/tests/test_payslip_holidays_computation.py b/addons/hr_work_entry_holidays/tests/test_payslip_holidays_computation.py index 7064e839dca..8d72bdc0198 100644 --- a/addons/hr_work_entry_holidays/tests/test_payslip_holidays_computation.py +++ b/addons/hr_work_entry_holidays/tests/test_payslip_holidays_computation.py @@ -33,7 +33,7 @@ class TestPayslipHolidaysComputation(TestWorkEntryHolidaysBase): }) leave.action_approve() - work_entries = self.jules_emp.contract_ids._generate_work_entries(date(2015, 11, 10), date(2015, 11, 21)) + work_entries = self.jules_emp.contract_ids.generate_work_entries(date(2015, 11, 10), date(2015, 11, 21)) work_entries.action_validate() work_entries = work_entries.filtered(lambda we: we.work_entry_type_id in self.env.ref('hr_work_entry.work_entry_type_attendance')) sum_hours = sum(work_entries.mapped('duration')) diff --git a/addons/hr_work_entry_holidays/tests/test_performance.py b/addons/hr_work_entry_holidays/tests/test_performance.py index d78e98adbbb..5327a1b6f68 100644 --- a/addons/hr_work_entry_holidays/tests/test_performance.py +++ b/addons/hr_work_entry_holidays/tests/test_performance.py @@ -111,8 +111,8 @@ class TestWorkEntryHolidaysPerformancesBigData(TestWorkEntryHolidaysBase): def test_work_entries_generation_perf(self): # Test Case 7: Try to generate work entries for # a hundred employees over a month - with self.assertQueryCount(__system__=413): - work_entries = self.contracts._generate_work_entries(date(2020, 7, 1), date(2020, 8, 31)) + with self.assertQueryCount(__system__=2607, admin=2807): + work_entries = self.contracts.generate_work_entries(date(2020, 7, 1), date(2020, 8, 31)) # Original work entries to generate when we don't adapt date_generated_from and # date_generated_to when they are equal for old contracts: 138300 diff --git a/addons/hr_work_entry_holidays/tests/test_work_entry.py b/addons/hr_work_entry_holidays/tests/test_work_entry.py index 0a5613a9775..6d2ed34c3cd 100644 --- a/addons/hr_work_entry_holidays/tests/test_work_entry.py +++ b/addons/hr_work_entry_holidays/tests/test_work_entry.py @@ -91,7 +91,7 @@ class TestWorkeEntryHolidaysWorkEntry(TestWorkEntryHolidaysBase): }) leave.action_validate() - work_entries = self.richard_emp.contract_id._generate_work_entries(self.start, self.end) + work_entries = self.richard_emp.contract_id.generate_work_entries(self.start.date(), self.end.date()) work_entries.action_validate() leave_work_entry = work_entries.filtered(lambda we: we.work_entry_type_id in self.work_entry_type_leave) sum_hours = sum(leave_work_entry.mapped('duration')) @@ -190,7 +190,7 @@ class TestWorkeEntryHolidaysWorkEntry(TestWorkEntryHolidaysBase): leave.action_approve() leave.action_validate() - work_entries = contract._generate_work_entries(date(2020, 7, 1), date(2020, 9, 30)) + work_entries = contract.generate_work_entries(date(2020, 7, 1), date(2020, 9, 30)) self.assertEqual(len(work_entries), 0)