Skip to content
Permalink
Browse files

[FIX] hr_holidays: make _onchange_request_parameters handle timezones

At the end it constructs date_{from,to} (Datetime) by combining:

- request_date_{from,to} (Date) local to the user
- UTC time from date_{from,to} (extracted using time() into hour_{from,to})

This doesn't work in all cases, e.g.:

A leave in US/Pacific for one day (with request_unit_custom = True):
- request_date_from: 1st of Jan
- request_date_to:   1st of Jan
- hour_from:         15:00 (7:00 local)
- hour_to:           03:00 (19:00 local) <-- this happens on the 2nd of Jan in UTC

Before this patch it resulted in:
date_from: 1st of Jan 15:00
date_to:   1st of Jan 03:00

The correct value for date_to is:
date_to:   2nd of Jan 03:00

_adjust_date_based_on_tz takes care of adjusting the date based on the
hour and the user's timezone.

opw-2169863

closes #44534

Signed-off-by: jorenvo <jorenvo@users.noreply.github.com>
  • Loading branch information
jorenvo committed Jan 27, 2020
1 parent ff7c5e5 commit e0ee2e6004285044b0a617cc39639b31a034159d
Showing with 75 additions and 16 deletions.
  1. +32 −3 addons/hr_holidays/models/hr_leave.py
  2. +43 −13 addons/hr_holidays/tests/test_leave_requests.py
@@ -8,7 +8,7 @@

from collections import namedtuple

from datetime import datetime, time
from datetime import datetime, date, timedelta, time
from pytz import timezone, UTC

from odoo import api, fields, models, tools, SUPERUSER_ID
@@ -304,6 +304,9 @@ def _onchange_request_parameters(self):
# find last attendance coming before last_day
attendance_to = next((att for att in reversed(attendances) if int(att.dayofweek) <= self.request_date_to.weekday()), attendances[-1] if attendances else default_value)

compensated_request_date_from = self.request_date_from
compensated_request_date_to = self.request_date_to

if self.request_unit_half:
if self.request_date_from_period == 'am':
hour_from = float_to_time(attendance_from.hour_from)
@@ -317,13 +320,17 @@ def _onchange_request_parameters(self):
elif self.request_unit_custom:
hour_from = self.date_from.time()
hour_to = self.date_to.time()
compensated_request_date_from = self._adjust_date_based_on_tz(self.request_date_from, hour_from)
compensated_request_date_to = self._adjust_date_based_on_tz(self.request_date_to, hour_to)
else:
hour_from = float_to_time(attendance_from.hour_from)
hour_to = float_to_time(attendance_to.hour_to)

tz = self.env.user.tz if self.env.user.tz and not self.request_unit_custom else 'UTC' # custom -> already in UTC
self.date_from = timezone(tz).localize(datetime.combine(self.request_date_from, hour_from)).astimezone(UTC).replace(tzinfo=None)
self.date_to = timezone(tz).localize(datetime.combine(self.request_date_to, hour_to)).astimezone(UTC).replace(tzinfo=None)

self.date_from = timezone(tz).localize(datetime.combine(compensated_request_date_from, hour_from)).astimezone(UTC).replace(tzinfo=None)
self.date_to = timezone(tz).localize(datetime.combine(compensated_request_date_to, hour_to)).astimezone(UTC).replace(tzinfo=None)

self._onchange_leave_dates()

@api.onchange('holiday_status_id')
@@ -486,6 +493,28 @@ def _get_number_of_days(self, date_from, date_to, employee_id):

return self.env.user.company_id.resource_calendar_id.get_work_hours_count(date_from, date_to) / (today_hours or HOURS_PER_DAY)

def _adjust_date_based_on_tz(self, leave_date, hour):
""" request_date_{from,to} are local to the user's tz but hour_{from,to} are in UTC.
In some cases they are combined (assuming they are in the same tz) as a datetime. When
that happens it's possible we need to adjust one of the dates. This function adjust the
date, so that it can be passed to datetime().
E.g. a leave in US/Pacific for one day:
- request_date_from: 1st of Jan
- request_date_to: 1st of Jan
- hour_from: 15:00 (7:00 local)
- hour_to: 03:00 (19:00 local) <-- this happens on the 2nd of Jan in UTC
"""
user_tz = timezone(self.env.user.tz if self.env.user.tz else 'UTC')
request_date_to_utc = UTC.localize(datetime.combine(leave_date, hour)).astimezone(user_tz).replace(tzinfo=None)
if request_date_to_utc.date() < leave_date:
return leave_date + timedelta(days=1)
elif request_date_to_utc.date() > leave_date:
return leave_date - timedelta(days=1)
else:
return leave_date

####################################################
# ORM Overrides methods
####################################################
@@ -2,6 +2,7 @@
# Part of Odoo. See LICENSE file for full copyright and licensing details.
from datetime import date, datetime
from dateutil.relativedelta import relativedelta
from pytz import timezone, UTC

from odoo import fields
from odoo.exceptions import ValidationError
@@ -191,23 +192,52 @@ def test_employee_is_absent(self):
self.assertTrue(self.employee_emp.is_absent, "He should be considered absent")
self.assertFalse(self.employee_hrmanager.is_absent, "He should not be considered absent")

@mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail')
def test_leave_defaults_with_timezones(self):
""" Make sure that leaves start with correct defaults for non-UTC timezones """
self.user_employee.tz = 'US/Pacific'
def _test_leave_with_tz(self, tz, local_date_from, local_date_to, number_of_days):
self.user_employee.tz = tz
tz = timezone(tz)

# Mimic what is done by the calendar widget when clicking on a day. It
# will take the local datetime from 7:00 to 19:00 and then convert it
# to UTC before sending it. Values here are for PST (UTC -8) and
# represent a leave on 2019/1/1 from 7:00 to 19:00 local time.
webclient_values = {
'date_from': datetime(2019, 1, 1, 15, 0, 0),
'date_to': datetime(2019, 1, 2, 3, 0), # note that this is the next day in UTC
values = {
'date_from': tz.localize(local_date_from).astimezone(UTC).replace(tzinfo=None),
'date_to': tz.localize(local_date_to).astimezone(UTC).replace(tzinfo=None), # note that this can be the next day in UTC
}
values = self.env['hr.leave'].sudo(self.user_employee_id)._default_get_request_parameters(webclient_values)
values.update(self.env['hr.leave'].sudo(self.user_employee_id)._default_get_request_parameters(values))

# Dates should be local to the user, they do not and cannot be
# converted using the user's timezone.
first_of_january = date(2019, 1, 1)
self.assertEqual(values['request_date_from'], first_of_january)
self.assertEqual(values['request_date_to'], first_of_january)
# Dates should be local to the user
self.assertEqual(values['request_date_from'], local_date_from.date())
self.assertEqual(values['request_date_to'], local_date_to.date())

values.update({
'name': 'Test',
'employee_id': self.employee_emp_id,
'holiday_status_id': self.holidays_type_1.id,
})
leave = self.env['hr.leave'].sudo(self.user_employee_id).new(values)
leave._onchange_request_parameters()
self.assertEqual(leave.number_of_days, number_of_days)

@mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail')
def test_leave_defaults_with_timezones(self):
""" Make sure that leaves start with correct defaults for non-UTC timezones """
timezones_to_test = ('UTC', 'Pacific/Midway', 'US/Pacific', 'Asia/Taipei', 'Pacific/Kiritimati') # UTC, UTC -11, UTC -8, UTC +8, UTC +14

# January 2020
# Su Mo Tu We Th Fr Sa
# 1 2 3 4
# 5 6 7 8 9 10 11
# 12 13 14 15 16 17 18
# 19 20 21 22 23 24 25
# 26 27 28 29 30 31
local_date_from = datetime(2019, 1, 1, 7, 0, 0)
local_date_to = datetime(2019, 1, 1, 19, 0, 0)
for tz in timezones_to_test:
self._test_leave_with_tz(tz, local_date_from, local_date_to, 1)

# We, Th, Fr, Mo, Tu, We => 6 days
local_date_from = datetime(2019, 1, 1, 7, 0, 0)
local_date_to = datetime(2019, 1, 8, 19, 0, 0)
for tz in timezones_to_test:
self._test_leave_with_tz(tz, local_date_from, local_date_to, 6)

0 comments on commit e0ee2e6

Please sign in to comment.
You can’t perform that action at this time.