Skip to content
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

Timezone Conversions #378

Closed
Bram1903 opened this issue May 4, 2023 · 6 comments · Fixed by #379 or #380
Closed

Timezone Conversions #378

Bram1903 opened this issue May 4, 2023 · 6 comments · Fixed by #379 or #380
Assignees
Labels
Bug Something isn't working Enhancement New feature or request Help Wanted Extra attention is needed

Comments

@Bram1903
Copy link
Collaborator

Bram1903 commented May 4, 2023

Problem Explained

Currently, we are storingDateTime based on the location of the device the application is hosted on, but we don't take into account that if someone accesses this website from another timezone it doesn't show up properly.

We should store all the DateTime's as DateTime.UtcNow in the database, and convert the time back to a user his local time on request. Therefore we would be able to convert a UTC timestamp back to the user his timezone, so everyone sees a timestamp in their own local format.

I would recommend reading this very interesting article by Shay Rojansky, who is a member of the entity framework team at Microsoft, and lead developer of the PostgreSQL entity framework provider.

For example currently, because I'm using PostgreSQL I had to enable AppContext.SetSwitch("Npgsql.EnableLegacyTimestampBehavior", true);, which results in all the timestamps being converted to UTC now when writing to the Database, but because we are only showing the end user the local saved timezone for me all the timestamps are not converted properly, which is also a problem for people who are outside of the timezone the application is hosted on.

I suggest we start working on a proper conversion for timezones to boost this project in terms of professionalism, and production readiness.

Problem Example

Local console log time:

image

Website log time:

image

As you can see my current time was 14:20, but since the DateTime is being saved in UTC the website will show up in UTC 12:20.

As I said earlier this isn't only a problem when using PostgreSQL, but also when someone would be outside of the timezone the application is hosted on, which would make it impossible to use worldwide.

@Bram1903 Bram1903 added Bug Something isn't working Enhancement New feature or request Help Wanted Extra attention is needed labels May 4, 2023
@neozhu
Copy link
Owner

neozhu commented May 4, 2023

Thank you for your message. It's great that you are aware of the timezone issue in your project. It's true that the DateTimeService.cs file with the Now => DateTime.UtcNow code is a step in the right direction. To convert between local time and UTC time, you can use the TimeZoneInfo class in C#. This class provides methods to convert between different time zones and can be used to convert the UTC time to the local time of the user. You can also use the DateTimeOffset structure, which includes both the UTC time and the local time offset. I hope this helps! Let me know if you have any further questions.

@Bram1903
Copy link
Collaborator Author

Bram1903 commented May 4, 2023

Well, this isn't necessarily an issue with "my" project, but with the whole project in general. This is because, as far as I can see, we don't do any conversion, which means everybody always sees the time based on the device location the project is hosted on. Switching from DateTime.Now to DateTime.UtcNow in the DateTimeService.cs file is a really small step since only saving the utc now times isn't necessarily a solution. We also have to convert it back, per location properly. Otherwise, everyone will end up with UTC timestamps, rather than the converted timestamp based on their location.

I'll start working on it in the following days, and see how far I can come. I'll probably write some extension method that somehow gets the user his current timezone and converts the UTC timestamp to the client's timezone.

@Bram1903 Bram1903 changed the title Proper Time Zones Conversions Timezone Conversions May 4, 2023
@Bram1903 Bram1903 pinned this issue May 4, 2023
@Bram1903 Bram1903 linked a pull request May 5, 2023 that will close this issue
@Bram1903 Bram1903 reopened this May 5, 2023
@Bram1903
Copy link
Collaborator Author

Bram1903 commented May 5, 2023

@neozhu, I've found something pretty cool. This library sends a javascript call to the user his browser, and dynamically changes the times where specified to the browser his local time, so whoever watches the page sees the time based on their own TimeZone with little to no effort. I've come pretty far with implementing this, but I got stuck on a certain point and hoped you would be able to help me out.

On the Logs.Razor page we show the log timestamp in UTC, which is where we want to convert that time to the user his local time as follows:

We add a CellTemplate under the PropertyColumn where we have our timestamp containing the following line of code:

<ToLocal DateTime="context.Item.TimeStamp" Format="dd/mm/yyyy HH:MM:ss"></ToLocal>

Which results in the following screenshot:

image

Instead of:

image

Unfortunately, it goes wrong with DateTime="context.Item.TimeStamp", whereas when I change context.Item.TimeStamp to DateTime.UtcNow in order to test if it is capable of converting that to the proper timezone based on the browser it works perfectly! The weird thing is that it does change the format in both cases, but when using DateTime="context.Item.TimeStamp" it doesn't convert that time at all but still applies the correct date formatting. You can test this by changing: Format="dd/mm/yyyy HH:MM:ss" to Format="default", and you will see that it does apply the proper date formatting to DateTime="context.Item.TimeStamp", but doesn't correctly convert it to the proper timezone.

If we can get this to work, we are basically done with showing the proper times across the whole website, since it's really easy to implement, especially because we will have all the DateTime's in UTC. I hope you are able to figure out why it does work when using DateTime.UtcNow, but doesn't when using the timestamp from the LogDto model.

You can test this by using this branch!

@Bram1903
Copy link
Collaborator Author

Bram1903 commented May 5, 2023

I've been able to fix it! The issue was that the code didn't know the DateTime kind, which after specifying the kind works great!

Old:

DateTime="context.Item.TimeStamp"

New:

DateTime="DateTime.SpecifyKind(context.Item.TimeStamp, DateTimeKind.Utc)"

The converting now works flawlessly, as shown in the below screenshots!

Screenshot of how the time is being saved in the Database:
image

Screenshot of the log page:
image

Testing

I opened two browsers at the same time, and for one I manually modified the browser's timezone and set it to: America/Los_Angeles Below screenshot is the result:

The left side is my local timezone Europe/Amsterdam, and on the right, I have the modified browser which shows the results in America/Los_Angeles

image

@neozhu
Copy link
Owner

neozhu commented May 5, 2023

@Bram1903
I think it would be simpler to create two extension methods directly on the DateTime class, such as ToLocal() or ToUtc().

@Bram1903
Copy link
Collaborator Author

Bram1903 commented May 5, 2023

@Bram1903
I think it would be simpler to create two extension methods directly on the DateTime class, such as ToLocal() or ToUtc().

You mean instead of having to specify DateTime="DateTime.SpecifyKind(context.Item.TimeStamp, DateTimeKind.Utc)" We can just call our extension method, which will do the SpecifiyKind for us?

I can create a PR as a draft, so we can both work on it, without having to merge it instantly, so you are able to modify it a bit.

@Bram1903 Bram1903 linked a pull request May 5, 2023 that will close this issue
@Bram1903 Bram1903 unpinned this issue May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Enhancement New feature or request Help Wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants