[FW][FIX] dates: avoid timezone issues by using UTC everywhere #3398
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
14 tasks
|
@LucasLefevre @pro-odoo cherrypicking of pull request #3375 failed. stderr: Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?). In the former case, you may want to edit this PR message as well. More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port |
14 tasks
Remove `any` Task: 3666703
This commit prepares the ground for a fix (next 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 ----------- This commit prepares the ground for the real fix (next 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. we introduce a wrapper around the Date native object for two reasons: - the business code should not know or care about timezones/UTC - manipulating Date is error prone. It's easy to mess up local vs UTC (as this bug fix has shown) With this [REF] commit, the behavior does not change. We still mix localised (`new DateTime(...)`) and UTC (`DateTime.fromTimestamp(...)`). The change comes with the next commit. For the future, we could also improve the `DateTime` API, compared to the poor `Date` API. For stable version however, we keep the changes to the minimum and keep the Date API Task: 3666703
(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. Task: 3666703
LucasLefevre
force-pushed
the
saas-16.3-15.0-naive-datetime-lul-YnIc-fw
branch
from
1442a35
to
eda6968
Compare
robodoo
pushed a commit
that referenced
this pull request
Jan 9, 2024
Remove `any` Task: 3666703 Part-of: #3398
robodoo
pushed a commit
that referenced
this pull request
Jan 9, 2024
This commit prepares the ground for a fix (next 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 ----------- This commit prepares the ground for the real fix (next 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. we introduce a wrapper around the Date native object for two reasons: - the business code should not know or care about timezones/UTC - manipulating Date is error prone. It's easy to mess up local vs UTC (as this bug fix has shown) With this [REF] commit, the behavior does not change. We still mix localised (`new DateTime(...)`) and UTC (`DateTime.fromTimestamp(...)`). The change comes with the next commit. For the future, we could also improve the `DateTime` API, compared to the poor `Date` API. For stable version however, we keep the changes to the minimum and keep the Date API Task: 3666703 Part-of: #3398
robodoo
pushed a commit
that referenced
this pull request
Jan 9, 2024
(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 #3398 Task: 3666703 Signed-off-by: Pierre Rousseau (pro) <pro@odoo.com> Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
14 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The issue
Steps to reproduce:
=> it becomes "3/7/2023"
Other steps to reproduce:
=> 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:
but then set the time as if it was localized.
The fix
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.
we introduce a wrapper around the Date native object for two reasons:
(as this bug fix has shown)
For the future, we could also improve the
DateTimeAPI, compared to the poorDateAPI. For stable version however, we keep the changes to the minimum andkeep the Date API
Task: : 3666703
review checklist
Forward-Port-Of: #3396
Forward-Port-Of: #3375