search

open-sauced / app

🍕 Insights into your entire open source ecosystem.
https://pizza.new
Apache License 2.0
414 stars 222 forks source link

Bug: user timezones don't get updated with daylight savings time changes #2312

Open isabensusan opened 9 months ago

isabensusan commented 9 months ago

Describe the bug

Jesse from Xebia brought this up: There was a time shift in the Netherlands for winter time (same logic as daylight savings in the US), but the local time displayed in user's profiles didn't get updated.

The TZ on myprofile still shows UTC2, but since it's CET-Winter-time (Amsterdam), it should show UTC1 at the moment.

Jesse's example -- his profile shows that the time for him in Amsterdam is 7:37pm, but if you look at the actual time in Amsterdam at the moment it's 6:37pm because of the time shift (these two images were taken two minutes apart):

image image

Steps to reproduce

  1. Go to Jesse's profile: https://app.opensauced.pizza/user/jessehouwing
  2. See the local time info
  3. Google "time in Amsterdam"
  4. See time difference

Browsers

No response

Additional context (Is this in dev or production?)

No response

Code of Conduct

Contributing Docs

github-actions[bot] commented 9 months ago

Thanks for the issue, our team will look into it as soon as possible! If you would like to work on this issue, please wait for us to decide if it's ready. The issue will be ready to work on once we remove the "needs triage" label.

To claim an issue that does not have the "needs triage" label, please leave a comment that says ".take". If you have any questions, please reach out to us on Discord or follow up on the issue itself.

For full info on how to contribute, please check out our contributors guide.

nickytonline commented 9 months ago

We have the list of timezones hard-coded which do not take changes into account like daylight savings time.

https://github.com/open-sauced/app/blob/cef3efc502ff664692a7ca06e03c1797fae41b56/lib/utils/timezones.ts#L3-L1181

I'd suggest (and I think we discussed this at some point @jpmcb) that hitting an API would be better and we could cache the list for an indeterminate amount of time.

timezonedb.com or maybe the Google timezone API. Open to others recommendations.

bdougie commented 9 months ago

timezonedb.com or maybe the Google timezone API. Open to others recommendations.

Seems like the Google timezone has what we needed, but we should consider timezonedb at a later date if we need that flexibility. I don't have experience with paying for an API like this, and it is not clear if commercials use cost more.

nickytonline commented 9 months ago

Yeah Google is probably a safe bet, but I’ll let others chime in. (@brandonroberts @jpmcb)

jpmcb commented 9 months ago

I'd suggest (and I think we discussed this at some point @jpmcb) that hitting an API would be better and we could cache the list for an indeterminate amount of time.

Yes, that's part of the puzzle. But this would encompass a much bigger change to the way our data is structure and how we encapsulate user's timezones: right now, we store timezones in the API as raw text fields (like "Mountain Standard Time" in my user's row or "Central Standard Time" in Brandon's row in the database). I believe these are propagating through from this raw list of timezones in the frontend you mentioned.

Those should probably be UTC offset timezones (like UTC−07:00 for mountain time) since they are totally agnostic to different jurisdiction's interpretation of timezones.

Then, with a UTC offset, we could hit any number of different time sync services (like amazon has a public NTP server we could hit, although probably overkill for our purposes) to get the right localized timezone.

So, TLDR: there's more work to get this correct from the data perspective than just changing these types.

davidlj95 commented 4 months ago

Hi! I was surprised to see (UTC+01:00) when selecting Madrid's time zone in the app and came across this issue. I've dealt with time in some apps in the pass and is definitely a PIA 🥲 Some things I can say from my experience after reading the issue and the hardcoded timezones code:

Issues

Conceptual: UTC offset != time zone

As Moment.js explains on their site

A UTC offset is a value that represents how far a particular date and time is from UTC. It is expressed in the format HH:mm most of the time.

A time zone is a geographical region where all people observe a legally mandated standard time.

A time zone usually has more than one offset from UTC due to daylight saving time. Source code in case website link breaks

So when saying for instance:

Then, with a UTC offset, we could hit any number of different time sync services [...] to get the right localized timezone.

It's not possible! Because many people in different parts of the world may be in the same UTC offset (some time ahead/behind UTC). That's misconception 2 about time zones.

A weird case is for instance is Newfoundland, a city in Canada. Despite most of Canada uses UTC offsets from -07:00 to -04:00 the offset they use is -03:30 🙃 There are also funny time zones around

Mindblown? 🤯 Well, the above paragraph and linked map are not 100% correct either though 🤓 Because it's not taking into account daylight saving time (DST). When it's active, Canadians use UTC offsets from -07:00 to -03:00 (-07:00 for some given they do not have DST). Except Newfoundlanders, who use a -02:30 UTC offset for their time.

So to summarize...

Time zone data allows to calculate UTC offsets from there. Those UTC offsets may vary due to different reasons, like DST. And that therefore includes the time instant to localize. A date in winter won't have DST whilst a date in summer will. Can be others too. Like for instance going from using DST to not using DST at a certain point in time (or viceversa 😛 ).

Specific: hardcoded time zones & UTC offsets

Right now the hardcoded time zones file contains a hardcoded UTC offset and a text containing it. This is my timezone for instance:

https://github.com/open-sauced/app/blob/eb4d77fab6dc797fdba57ed6d33e9d5f292a710b/lib/utils/timezones.ts#L538-L545

Given we're right now in daylight saving time (DST), the UTC offset is actually (UTC+02:00) and not (UTC+01:00). The offset field is correct though. In this case it's due to DST. But as mentioned in the issue, an external source should be used for that in order to keep updated with the available time zones and their rules to get UTC offset for a given time moment.

Solutions

First of all, using time zones and not UTC offsets when localizing a time instant (ie: a Unix time stamp or a date in UTC). As explained above, a UTC offset is a different concept from a time zone. Having stored time zone names in the API is a great choice then.

Where to fetch time zones data though? Given all of that mess, querying an API like proposed Google's Time Zone API one makes sense to have all those rules up to date. However, those rules do not change so often. So using a time zones database can save 1 precious API query round trip time. Most used is IANA's Time Zone Database AFAIK. Kind of an industry standard as used in many many softwares around directly or indirectly. Updating the library providing this database from time to time would be more than enough to keep those time rules updated.

Talking about libraries, I saw date-fns is used. Seems that date-fns-tz brings time zones support to date-fns. Seems it uses browsers' Intl API so will indeed reduce your bundle size given you won't need to serve that database as it's using the user's browsers one*! There are polyfills also if support for old browsers is needed.

Which probably will use OS' one. Which probably will be IANA's one. Unless if using Windows which they use their own one AFAIK.

I'll give it a try and raise a PR if have some time 😃

Apologies for the bible being written here, I get confused about this topic often, so writing about it helps training myself about it (and about writing too 😛 )

Cool resources about managing time

Some links that have been useful about the topic: