#3027 Adjust activity heatmap and logs for timezone (#1642)
Merged by pingou. Opened by adamwill.
adamwill/pagure activity-timezone-funtimes  into  master

Download 3027.patch

As discussed in #1642, there is a problem with the user activity
log page in the web UI caused by a timezone mismatch. The
library used to generate the heatmap assumes any date / time you
feed it is in the browser timezone. Pagure ultimately feeds it
the 'date' value of PagureLog instances, converted to a Unix
timestamp. This timestamp reflects 00:00 on the date in question
in the UTC timezone. If the browser timezone is behind UTC, the
heatmap library treats the event as falling on the previous
day, because in the browser timezone, that precise time falls on
the previous day.

It would be relatively easy to 'fix' this if all we wanted to do
was have the heatmap reflect what date each event occurred on in
the UTC timezone, as the activity log currently does. However, I
don't think that's the best fix. I think the best behaviour here
is for both the heatmap and the activity log to be relative to
the browser timezone. As a Pagure user in a timezone several
hours behind UTC, if I make a commit in the late afternoon of
Wednesday, I'd expect the heatmap and the activity log to show
that commit as occuring on Wednesday, not Thursday.

So, this commit dumps the use of the 'date' value in this code,
and works with the 'date_created' value instead, which is a
precise UTC date and time (as opposed to just the date relative
to UTC). We allow a timezone offset to be passed to the relevant
methods, and figure out what date the events fell on if that
offset is applied
, rather than just what date they fell on in
the UTC timezone. To facilitate this we add a custom method to
the database model, date_offset, which gives the date when an
arbitrary offset (in minutes, positive or negative) is applied.
We tweak the relevant functions in pagure.lib to get all the
necessary data from the database, then use the date_offset
method to adjust and refine the results before returning.

Fixes https://pagure.io/pagure/issue/1642

Looks good and tests are passing, so all good to me.

Thanks! :)

Pull-Request has been merged by pingou

Just for the record, my explanation of what the old code did was slightly wrong in one regard. When producing a timestamp to feed to the heatmap library, Pagure did not in fact always use 'the timestamp for 00:00 UTC on the date stored in the database'. It used 'the timestamp for 00:00 server local time on the date stored in the database'. I think the local time of the main 'real world' production servers (this one, and the Fedora package one) is UTC, but if you were using a local test server, this would affect its behaviour a bit.

So the key factor in determining whether the heatmap would show the event on a different date from the one stored in the database (and shown in the activity log) was the difference between the client's browser timezone and the server's local time, not the difference between the client's browser timezone and UTC.

This is just a note for the record, though, it doesn't mean the fix was wrong in any way or require any further changes. AFAICT, we are now not using this 'date' value in the database model at all, as really it's fundamentally problematic. In fact it might be a good idea to get rid of it, or at least put large warning signs around it.

Bonus note! The reason for the above is I think to do with how datetime.Date objects work. This is the type Pagure uses for the PagureLog.date database model attribute; it effectively gets set to 'a datetime.Date instance for the date when the event happened in UTC timezone'.

Dates are inherently naive - they cannot have a timezone. So while we use 'the current date in UTC timezone' to create it, the fact that the date was 'relative' to UTC then immediately gets lost. All the information that is really stored in the database is the year, month and date.

When creating the timestamp to send to the heatmap library, the old code got the date from the database as d (sqlalchemy would provide it as a Date instance again) and called d.strftime('%s'). Python documents that when calling strftime on a Date instance, "Format codes referring to hours, minutes or seconds will see 0 values." It doesn't explicitly state what it does about the timezone, but clearly it just uses the system's local timezone, so you get a timestamp for 00:00 on the date in question in the system's local time. This does seem like the 'natural' thing for it to do, given that the Date instance cannot have a 'native' timezone.

Turns out this wasn't really sufficient. See https://pagure.io/pagure/pull-request/3030 .

Metadata