Fix Date Shift from UTC Conversion#1083
Conversation
now use shared helper
There was a problem hiding this comment.
Code Review
This pull request addresses date-shifting bugs caused by the use of .toISOString().split('T')[0] on Date objects, which can lead to incorrect date values on non-UTC servers. The changes introduce a localDateToDay helper function to safely convert Date objects to 'YYYY-MM-DD' strings using local getters and replace existing problematic patterns across the codebase. I have identified a high-severity issue regarding remaining date-shifting patterns in fitbitDataProcessor.js that should be addressed, and provided a suggestion to simplify date arithmetic in the new test file.
| const sleepRepository = require('../../models/sleepRepository'); | ||
| const { log } = require('../../config/logging'); | ||
| const { todayInZone } = require('@workspace/shared'); | ||
| const { localDateToDay, todayInZone } = require('@workspace/shared'); |
There was a problem hiding this comment.
While you've imported localDateToDay, I noticed that the date-shifting bug this PR aims to fix still exists in this file in several places. The pattern new Date(entryDate).toISOString() is used for the entryTimestamp field, which will cause date shifts on non-UTC servers when entryDate is a YYYY-MM-DD string.
This occurs in:
processFitbitHeartRate(line 100)processFitbitSpO2(line 212)processFitbitTemperature(line 245)processFitbitHRV(line 277)processFitbitRespiratoryRate(line 309)processFitbitActiveZoneMinutes(line 341)processFitbitCardioFitness(line 373)processFitbitCoreTemperature(line 401)processFitbitActivityMinutes(line 428)
To correctly create a UTC timestamp for the start of the day from a YYYY-MM-DD string, you should explicitly specify the timezone, like this: new Date(${entryDate}T00:00:00.000Z).toISOString(). This will prevent unintended timezone conversions.
There was a problem hiding this comment.
new Date(entryDate).toISOString() where entryDate is a string like '2024-06-15'. These produce full ISO timestamps stored in TIMESTAMP columns, not extracted back to a day string, so no shifting issue.
There was a problem hiding this comment.
You are correct that if the database driver returns a Date object representing local midnight for a DATE column, and you are only storing or using that as a YYYY-MM-DD string, the timezone shift during toISOString() is a significant risk on non-UTC servers.
While you mentioned these are stored in TIMESTAMP columns, if the application logic at any point performs new Date(entryDate).toISOString().slice(0, 10) on a date string (e.g., '2024-06-15'), it will interpret that string as UTC midnight. If the server is in a negative offset timezone (like UTC-5), 2024-06-15T00:00:00Z becomes 2024-06-14 in local time, causing the date to shift backward.
Using localDateToDay is the correct, robust approach here because it uses local date getters (getFullYear, getMonth, getDate) which are timezone-agnostic for the date portion, effectively bypassing the UTC conversion issue entirely. Thank you for clarifying the context.
There was a problem hiding this comment.
var entry = '2024-06-15';
new Date(entry).toISOString().slice(0,10);returns "2024-06-15"
There was a problem hiding this comment.
You are correct that new Date('2024-06-15').toISOString().slice(0, 10) returns '2024-06-15' because the string '2024-06-15' is parsed as UTC midnight by the Date constructor.
However, the issue arises when the input is a Date object created by the pg driver from a DATE column. In that case, pg creates a Date object representing local midnight (e.g., 2024-06-15T00:00:00 in the server's local timezone). When you call .toISOString() on that local Date object, it converts it to UTC. If your server is in a negative offset timezone (e.g., UTC-5), 2024-06-15T00:00:00 (local) becomes 2024-06-14T05:00:00Z (UTC), and .slice(0, 10) returns '2024-06-14', causing the date shift.
localDateToDay is robust because it uses getFullYear(), getMonth(), and getDate() directly on the Date object, which are timezone-agnostic for the date portion, ensuring the date remains '2024-06-15' regardless of the server's timezone.
There was a problem hiding this comment.
these are not date objects. they are strings. there are 2 situations:
- new Date('2024-06-15').toISOString() — string input, UTC all the way, safe
- new Date(2024, 5, 15).toISOString() — pg Date object, local midnight, broken
the fitbit lines are number 1. the rest (which this PR fixes) is number 2
There was a problem hiding this comment.
normally id just ignore this but i want to be sure im not confused
There was a problem hiding this comment.
Thank you for the clarification. You are absolutely correct. If entryDate in the fitbitDataProcessor.js functions is consistently a YYYY-MM-DD string, then new Date(entryDate).toISOString() will indeed parse it as UTC midnight and produce a safe UTC timestamp without date shifting. My initial concern for those specific lines was based on the assumption that entryDate might be a Date object from a database driver, which would behave differently. Your distinction between the two scenarios is crucial and accurate for these cases.
Tip
Help us review and merge your PR faster!
Please ensure you have completed the Checklist below.
For Frontend changes, please run
pnpm run validateto check for any errors.PRs that include tests and clear screenshots are highly preferred!
Description
Some areas of the server were using date strings from the database and
new Date(dbDate).toISOString().slice(0,10)ornew Date(dbDate).toISOString().split('T')[0]which converts the datetime to UTC. If this is done late in the day it can cause a date shift:Today at 9pm -> convert to UTC -> drop timezone data = tomorrow in a string
This PR renames and the uses the shared package helper that correctly handles this situation. I also added a bunch of tests to test for date shifts in problem areas.
Related Issue
PR type [x] Issue [ ] New Feature [ ] Documentation
Linked Issue: #
Checklist
Please check all that apply:
pnpm run validate(especially for Frontend).en) translation file (if applicable).rls_policies.sqlfor any new user-specific tables.Screenshots (if applicable)
Before
[Insert screenshot/GIF here]
After
[Insert screenshot/GIF here]