Skip to content

fix: strip timezone information from a date in dau response #11962

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jan 31, 2024

Conversation

Emyrk
Copy link
Member

@Emyrk Emyrk commented Jan 31, 2024

Timezone information is lost, so do not forward it to the client.

See issue for debugging details

Closes #11943

Emyrk added 2 commits January 31, 2024 10:37
Timezone information is lost, so do not forward it to the client.
@Emyrk Emyrk marked this pull request as ready for review January 31, 2024 19:53
@Emyrk Emyrk requested a review from johnstcn January 31, 2024 19:55
Comment on lines -2187 to 2205
func TimezoneOffsetHour(loc *time.Location) int {
// TimezoneOffsetHourWithTime is implemented to match the javascript 'getTimezoneOffset()' function.
// This is the amount of time between this date evaluated in UTC and evaluated in the 'loc'
// The trivial case of times being on the same day is:
// 'time.Now().UTC().Hour() - time.Now().In(loc).Hour()'
func TimezoneOffsetHourWithTime(now time.Time, loc *time.Location) int {
if loc == nil {
// Default to UTC time to be consistent across all callers.
loc = time.UTC
}
_, offsetSec := time.Now().In(loc).Zone()
// Convert to hours
return offsetSec / 60 / 60
_, offsetSec := now.In(loc).Zone()
// Convert to hours and flip the sign
return -1 * offsetSec / 60 / 60
}

func TimezoneOffsetHour(loc *time.Location) int {
return TimezoneOffsetHourWithTime(time.Now(), loc)
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value was flipped from the javascript implementation. Given this value comes from the browser, I made it match the javascript. I fixed all the tests to be deterministic so we can keep them on.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also verified this is the same on Postgres

@Emyrk Emyrk merged commit ac64155 into main Jan 31, 2024
@Emyrk Emyrk deleted the stevenmasley/dau_date branch January 31, 2024 22:01
@github-actions github-actions bot locked and limited conversation to collaborators Jan 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Active user count on deployment page has date off by one day
2 participants