Summary

Localized timezone support (Review Board)

Review Request #2725 — Created Dec. 1, 2011 and submitted March 21, 2012, 12:27 p.m.

Information

Owner

ddruska

Repository

Review Board

Branch

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Adds per-user timezone support, relying on the USE_TZ feature from the Django dev branch.

Using this method, datetime's are stored in the database in UTC and are timezone-aware. Datetime objects are stored with tzinfo=utc, so that they can be easily converted into any other timezone when retrieved from the database.
Middleware was created in the accounts app which activates the user's timezone in each session. A timezone field is added to the user model to store their local time, which they can change in "My account". The user profile and admin forms use TimeZoneField, which is pre-populated with all the available timezone choices. Templates which show dates and/or times are changed to display the user's local time with the {% load tz %}{% localtime on %} tags.

Testing Done

Issues

Description	From	Last Updated
"timezone"	chipx86	Dec. 2, 2011, 10:14 a.m.
One per line, ending with commas.	chipx86	Dec. 2, 2011, 10:14 a.m.
Should be after the siteconfig stuff. Alphabetical order.	chipx86	Dec. 2, 2011, 10:14 a.m.
Group these together. pytz is a third-party module.	chipx86	Dec. 2, 2011, 10:14 a.m.
Mind adding a docstring on this?	chipx86	Dec. 2, 2011, 10:14 a.m.
It's possible that there isn't a Profile yet. For example, if the user just logged in for the very first …	chipx86	Dec. 2, 2011, 10:14 a.m.
Why did these move?	chipx86	Dec. 4, 2011, 4 a.m.
Get the profile first, in a separate call. Should be self.request.user.get_profile().	chipx86	Dec. 2, 2011, 10:15 a.m.
Move semicolon out of the trans block.	ME medanat	Dec. 2, 2011, 10:05 a.m.
"Time Zone:" if we're going to be consistent with other places.	chipx86	Dec. 3, 2011, 1:02 a.m.
These should be single-space indented.	mike_conley	Dec. 3, 2011, 2:07 a.m.
Just to check, will the result we get back from the API for comparison be in the expected timezone?	chipx86	Jan. 25, 2012, 10:33 a.m.
Given that this e-mail goes out to admins, I don't think we want to apply the user's timezone to it. …	chipx86	Dec. 3, 2011, 1:38 a.m.

reviewboard/templates/accounts/prefs.html (Diff revision 1)
The issue has been dropped. Show all issues
```
Move semicolon out of the trans block.
```
1. ME
  
  medanat Dec. 1, 2011, 2 p.m.
  And by semicolon I mean colon.
2. chipx86 Dec. 1, 2011, 3:57 p.m.
  Actually, it should stay where it is. That may localize differently.

reviewboard/accounts/backends.py (Diff revision 1)
The issue has been resolved. Show all issues
```
"timezone"
```
reviewboard/accounts/evolutions/__init__.py (Diff revision 1)
The issue has been resolved. Show all issues
```
One per line, ending with commas.
```
reviewboard/accounts/forms.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Should be after the siteconfig stuff. Alphabetical order.
```
reviewboard/accounts/middleware.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Group these together. pytz is a third-party module.
```
reviewboard/accounts/middleware.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Mind adding a docstring on this?
```

reviewboard/accounts/middleware.py (Diff revision 1)

The issue has been resolved. Show all issues

It's possible that there isn't a Profile yet. For example, if the user just logged in for the very first time, and is using something like LDAP auth, the User will exist but the Profile won't have been created just yet.

reviewboard/reviews/datagrids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Why did these move?
```
1. DD
  
  ddruska Dec. 2, 2011, 12:23 p.m.
  I saw that they weren't in alphabetical order. Why are they in the order they are?

reviewboard/reviews/datagrids.py (Diff revision 1)

The issue has been resolved. Show all issues

Get the profile first, in a separate call.

Should be self.request.user.get_profile().

reviewboard/reviews/models.py (Diff revision 1)

I feel like we do this so often that we should really have a util function that does it.

reviewboard/templates/accounts/prefs.html (Diff revision 1)
The issue has been resolved. Show all issues
```
"Time Zone:" if we're going to be consistent with other places.
```

reviewboard/templates/diffviewer/view_diff.html (Diff revision 1)

The issue has been resolved. Show all issues

Just to check, will the result we get back from the API for comparison be in the expected timezone?

DD

ddruska Dec. 3, 2011, 5:07 a.m.
```
I'll check on that.
```

mike_conley Jan. 25, 2012, 10:29 a.m.

Was this issue ever resolved or investigated?

ddruska Jan. 25, 2012, 10:33 a.m.

It should have NO localtime tags, anything that isn't shown to the viewer is in UTC.

reviewboard/templates/notifications/new_user_email.html (Diff revision 1)

The issue has been resolved. Show all issues

Given that this e-mail goes out to admins, I don't think we want to apply the user's timezone to it. We should, however, make sure to list the timezone in the format. That's using "T"

Dave:

This looks pretty good!  Just found one further nit below.

-Mike

reviewboard/reviews/models.py (Diff revision 1)
```
Seconded.
```
reviewboard/templates/accounts/prefs.html (Diff revision 1)
The issue has been resolved. Show all issues
```
These should be single-space indented.
```

Change Summary:

Added utc aware datetime helper method, fixed other stuff from review.

Diff:

Revision 2 (+113 -22)

Show changes

	reviewboard/settings.py
	reviewboard/accounts/backends.py
	reviewboard/accounts/forms.py
	reviewboard/accounts/middleware.py
	reviewboard/accounts/models.py
	reviewboard/accounts/views.py
	reviewboard/accounts/evolutions/__init__.py
	reviewboard/accounts/evolutions/timezone.py
	17 more

Change Summary:

Added the timezone tag to each individual date within a template. Removed the tag from registerForUpdates. Removed localtime from admin pages and instead added T in the date format.

Diff:

Revision 3 (+112 -33)

Show changes

	reviewboard/settings.py
	reviewboard/accounts/backends.py
	reviewboard/accounts/forms.py
	reviewboard/accounts/middleware.py
	reviewboard/accounts/models.py
	reviewboard/accounts/views.py
	reviewboard/accounts/evolutions/__init__.py
	reviewboard/accounts/evolutions/timezone.py
	15 more

Ship it!

This looks good to me.  Great work, Dave!

-Mike

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (e7184fd). Thanks!

Status:: Re-opened

Status:: Completed