Skip to content

Commit

Permalink
[FIX] dates: use UTC to represent naive date times
Browse files Browse the repository at this point in the history
(see also the previous commit)

The issue
---------

Steps to reproduce:

- change your machine timezone to Jakarta
- enter "3/8/2023" in a cell
=> it becomes "3/7/2023"

Other steps to reproduce:

- change your machine timezone to Jakarta
- npm run test
=> some tests are failing.

Spreadsheet date times are naive date times, they do not carry any timezone
information. A date(time) in spreadsheet is just a number with a format
after all.

We are currently using javascript native Date objects everywhere in the code to
represent spreadsheet dates.
And we are mixing UTC (new Date(timestamp)) and localized Dates
(new Date(year, month, day, ...)) at some places. This is a mistake.

Because of an (un)lucky combination of mistakes, it mostly works by chance for
most timezones.
But going back to the faulty Jakarta case: one can notice that
INITIAL_1900_DAY is UTC+0707 while INITIAL_JS_DAY is UTC+0700 (7 minutes
offset). The timezone offset changed over time.

Some of the mistakes where we mix up things:

- INITIAL_1900_DAY is localized, but INITIAL_JS_DAY is UTC.
- numberToJsDate: we create the date from the value as if it was UTC,
  but then set the time as if it was localized.

This commit
-----------

The idea to fix the issue is to avoid mixing localized and UTC Dates.
To represent naive date times, we will always use UTC Date objects.

closes #3397

Task: 3666703
X-original-commit: 651f9bb
Signed-off-by: Pierre Rousseau (pro) <pro@odoo.com>
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
  • Loading branch information
LucasLefevre committed Jan 10, 2024
1 parent 6de11b4 commit a9deab9
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 19 deletions.
42 changes: 24 additions & 18 deletions src/helpers/dates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,27 @@ export interface InternalDate {
readonly jsDate?: ReadonlyDateTime;
}

/**
* A DateTime object that can be used to manipulate spreadsheet dates.
* Conceptually, a spreadsheet date is simply a number with a date format,
* and it is timezone-agnostic.
* This DateTime object consistently uses UTC time to represent a naive date and time.
*/
export class DateTime {
private jsDate: Date;
constructor(year: number, month: number, day: number, hours = 0, minutes = 0, seconds = 0) {
this.jsDate = new Date(year, month, day, hours, minutes, seconds, 0);
this.jsDate = new Date(Date.UTC(year, month, day, hours, minutes, seconds, 0));
}

static fromTimestamp(timestamp: number) {
const date = new Date(timestamp);
return new DateTime(
date.getFullYear(),
date.getMonth(),
date.getDate(),
date.getHours(),
date.getMinutes(),
date.getSeconds()
date.getUTCFullYear(),
date.getUTCMonth(),
date.getUTCDate(),
date.getUTCHours(),
date.getUTCMinutes(),
date.getUTCSeconds()
);
}

Expand All @@ -54,51 +60,51 @@ export class DateTime {
}

getFullYear() {
return this.jsDate.getFullYear();
return this.jsDate.getUTCFullYear();
}

getMonth() {
return this.jsDate.getMonth();
return this.jsDate.getUTCMonth();
}

getDate() {
return this.jsDate.getDate();
return this.jsDate.getUTCDate();
}

getDay() {
return this.jsDate.getDay();
return this.jsDate.getUTCDay();
}

getHours() {
return this.jsDate.getHours();
return this.jsDate.getUTCHours();
}

getMinutes() {
return this.jsDate.getMinutes();
return this.jsDate.getUTCMinutes();
}

getSeconds() {
return this.jsDate.getSeconds();
return this.jsDate.getUTCSeconds();
}

setFullYear(year: number) {
this.jsDate.setFullYear(year);
}

setDate(date: number) {
this.jsDate.setDate(date);
this.jsDate.setUTCDate(date);
}

setHours(hours: number) {
this.jsDate.setHours(hours);
this.jsDate.setUTCHours(hours);
}

setMinutes(minutes: number) {
this.jsDate.setMinutes(minutes);
this.jsDate.setUTCMinutes(minutes);
}

setSeconds(seconds: number) {
this.jsDate.setSeconds(seconds);
this.jsDate.setUTCSeconds(seconds);
}
}

Expand Down
4 changes: 3 additions & 1 deletion tests/functions/module_date.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ import {
evaluateGridText,
} from "../test_helpers/helpers";

// All these tests should pass no matter the machine timezone.

describe("DATE formula", () => {
test("functional tests on cell arguments CHECK TIMEZONE IF FAILS", () => {
test("functional tests on cell arguments", () => {
// prettier-ignore
const grid = {
// YEAR / MONTH / DAY
Expand Down

0 comments on commit a9deab9

Please sign in to comment.