Fix timezone bug when clicking heatmap

[mayswind]

I click date block in heatmap, but gitea shows the actions in other date. There are two actions in 2021-03-23 00:10 (GMT+8) and 2021-03-23 00:21 (GMT+8), but only if I click 2021-03-22, the actions in 2021-03-23 would be displayed (see the below screenshot).

I enabled sql log, then and I find the query date range is 1616457600 (2021-03-23 08:00:00 in GMT+8) to 1616543999 (2021-03-24 07:59:59 in GMT+8) when I click "2021-03-23" in heatmap (see the below log). The time zone of the container which runs gitea is GMT+8. I runns the official docker image gitea/gitea:1.14.0-rc2.

2021/03/24 14:47:11 ...m.io/xorm/core/db.go:286:afterProcess() [I] [SQL] SELECT `id`, `user_id`, `op_type`, `act_user_id`, `repo_id`, `comment_id`, `is_deleted`, `ref_name`, `is_private`, `content`, `created_unix` FROM `action` WHERE user_id=? AND is_deleted=? AND created_unix>=? AND created_unix<=? ORDER BY `id` DESC LIMIT 20 [1 false 1616457600 1616543999] - 1.064742ms

You can reproduce this bug when the time zone of your server/container is not UTC.

[noerw]

I don't think this is the right fix. This problem has several layers (I don't fully understand it, I hope I summarize correctly):

1. all activities assume the same server timezone, meaning they potentially store the incorrect time for users in other timezones
2. the heatmap assumes timestamps in user's / browser timezone, but we pass in timestamp that is the start of day in server timezone, meaning the activities show up on the wrong day in the heatmap (or maybe the heatmap assumes UTC? need to doublecheck)
3. the click event uses the timestamp from the heatmap, which is incorrect due to (2)

If I understand correctly, your changes would adress problem (3).. but it doesn't consider the browser timezone.

Imho the correct fix would be to actually only store UTC timestamps in the DB (and convert already stored timestamps on activities to UTC via a migration).
To address problem (1), a user setting indicating their local timezone could be added (defaulting to server timezone)

This is a longstanding issue with several proposed patches in the comments, did you take a look at this? #6087

[mayswind]

I don't think this is the right fix. This problem has several layers (I don't fully understand it, I hope I summarize correctly):

1. all activities assume the same server timezone, meaning they potentially store the incorrect time for users in other timezones
2. the heatmap assumes timestamps in user's / browser timezone, but we pass in timestamp that is the start of day in server timezone, meaning the activities show up on the wrong day in the heatmap (or maybe the heatmap assumes UTC? need to doublecheck)
3. the click event uses the timestamp from the heatmap, which is incorrect due to (2)

If I understand correctly, your changes would adress problem (3).. but it doesn't consider the browser timezone.

Imho the correct fix would be to actually only store UTC timestamps in the DB (and convert already stored timestamps on activities to UTC via a migration).
To address problem (1), a user setting indicating their local timezone could be added (defaulting to server timezone)

This is a longstanding issue with several proposed patches in the comments, did you take a look at this? #6087

I think it is really worth to do the work in #6087, but this pr and #13935 has nothing to do with #6087. #13935 just makes heatmap clickable, and passes "Date" parameter (converted to UTC time zone in frontend) to backend.
I think what the server should do in this pr and #13935 is just converting the "Date" parameter to unix time correctly, and this pr makes this job correct.

[silverwind]

To really fix all time zone issues, we need to:

• remove the server time zone setting
• make all data sent and received by the server UTC

Essentially the server must not be aware of a time zone and if client wants to send/display another time zone, they need to convert from/to UTC when talking with the server.

The server can of course accept received ISO format with time zone info but needs to ensure it's converted to UTC before hitting the database.

[noerw]

Ok, I may have misunderstood the scope of this PR ;)
This fix seems to be a good stopgap for the issue, but to fix related issues like #6087 we should really only store UTC.

[6543]

🚀