Code

Opened 10 months ago

Closed 10 months ago

Last modified 6 months ago

#20663 closed Bug (fixed)

Admin Now and Today buttons use confusing time / timezone

Reported by: imfletcher Owned by: aaugustin
Component: contrib.admin Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

The Django Admin has the default Today and Now buttons for datetime fields.

These seem to use the local time, but store it as UTC in the db without converting it. Meaning, if its 10PM local time, the Now button will populate with 22:00 but will store as 22:00 +0:00:00. This is confusing as it makes it very hard to understand what is going on.

It would be better if the buttons used UTC's current date/time and stored as such, or use the default timezone and store the appropriate offset.

PS sorry if dupe, i tried searching but admin, today, and now are hard terms to search on.

Attachments (0)

Change History (22)

comment:1 Changed 10 months ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

This is a known problem (sort of). I'm not sure if there's already a ticket about. It's hard to fix because JavaScript doesn't have any support for time zones.

In order to clarify what we're talking about, could you tell me the values of the USE_TZ and TIME_ZONE settings in your app?

comment:2 Changed 10 months ago by imfletcher

I suppose the hard part is there is no timezone settings for auth_user, so you can't specify the Admin User's timezone separate from the server settings.

Settings:
TIME_ZONE = 'UTC'
USE_TZ = True

comment:3 Changed 10 months ago by aaugustin

  • Triage Stage changed from Unreviewed to Accepted

I'm a bit surprised that you set TIME_ZONE = 'UTC'. I suppose your application is used by users in multiple time zones, and UTC is the least controversial choice?

The admin buttons should use the "current time zone". Then it's up to developers to ensure that their users have the correct "current time zone". This is how Django supports multiple time zones. If you don't do anything special, the "current time zone" is the same as the "default time zone" defined by the TIME_ZONE setting.

Then all we need is a way to determine the current time in an arbitrary time zone in JavaScript. Unfortunately, I'm not aware of a JavaScript library to access the tzinfo database. If there's one, I'm not sure how to maintain it up to date. This is an open problem!

I'm accepting the ticket because it's clearly an UX bug. Now, we need a plan to fix it, and such a plan should be discussed on the DevelopersMailingList first.

comment:4 Changed 10 months ago by loic84

How about a context variable, and something along these lines in base.html:

<script type="text/javascript">window.__admin_timezone__ = "{% filter escapejs %}{{ TIMEZONE }}{% endfilter %}";</script>

comment:5 Changed 10 months ago by aaugustin

How does that solve the problem?

comment:6 Changed 10 months ago by loic84

Server side knows about the timezone for the current request (either set by a middleware or coming from settings.TIME_ZONE) so it can give the expected offset to the JS.

The now button would then use UTC + the given offset instead of local time.

Now would then be, "now as the server will treat it" as opposed to "now the time on your wrist watch".

Or am I missing something obvious?

Version 0, edited 10 months ago by loic84 (next)

comment:7 Changed 10 months ago by aaugustin

Technically, an offset from UTC isn't sufficient. If you don't use the time zone definition from the tzinfo database, you're going to hit bugs around DST transitions (eg. load the form before the transition, click the "now" button after the transition). Since web pages are supposed to be short lived, an offset might be acceptable, but I'm always reluctant to make a feature 99% correct. The whole point of time zone support was to be 100% correct even around DST transitions.


I realize my initial explanation wasn't very clear, so I'll expand a bit. The problem isn't UTC vs. local time. It's "browser time zone" vs. "current time zone".

When a value is submitted through a form, Django interprets it in the "current time zone". Currently, if the "browser time zone" differs from the "current time zone", Django won't interpret correctly the value sent by the browser. Assuming we have a way to detect when the "browser time zone" differs from the "current time zone" (which, in the reporter's case, is UTC because that's his "default time zone" and he didn't do anything special), the "now" button should prepopulate with now in the "current time zone" rather than in the "browser time zone".

Note that this problem already existed before I added support for time zone, and it also exists when USE_TZ is False. If the browser's time zone isn't the same as TIME_ZONE, you hit the same issue.

comment:8 Changed 10 months ago by imfletcher

i set server to UTC because my app defaults timezones for users, and every user must choose when they signup. so no one uses the default server setting. UTC seemed reasonable given thats how the stuff is stored in the db anyway.

IMO the only 100% solution here is to have some ability to tell the admin site how to activate the admin user's timezone (there can be multiple admins in diff tz's). without a timezone column in auth_user (which is kind of curious but i'm sure a deep issue), i'm not sure how to handle it other than perhaps a tzinfo dropdown in the header so a user can choose (and a cookie to save it).

comment:9 Changed 10 months ago by loic84

  • Easy pickings set
  • Has patch set
  • Needs documentation set
  • Needs tests set
  • Patch needs improvement set

comment:10 Changed 10 months ago by aaugustin

  • Owner changed from nobody to aaugustin
  • Status changed from new to assigned

comment:11 Changed 10 months ago by ramiro

See #7717, #14253 and #18768 for some previous, related discussion about the topic.

comment:12 Changed 10 months ago by loic84

Since there have been a lot of previous discussion on the topic, I'm gonna explain in detail the reasoning behind this patch.

As Aymeric describes in 7, the issue at hand is "browser time zone" vs. "current time zone".

  • "browser time zone" is usually the time zone on the user's computer, it's a time zone we basically know nothing about on the backend.
  • "current time zone" is the time zone on the backend for the current request, which by default comes from settings.TIME_ZONE unless manipulated by timezone.activate() (i.e. in a middleware).

For whatever value we find in the date/time input field, the backend will assume it's given to him in the "current time zone", which again, is the only time zone it knows about.

When both "browser time zone" and "current time zone" are the same, all is well since there is no ambiguity.

However when those two differ, as things are now, you start having issues. For example, if your field controls the publishing of an article (pub_date), you get the weird behavior where you ask to publish something now and nothing happens, because "now" for you is not the same as "now" for the backend.

The only way to get it right is to remove all ambiguity introduced by naive datetime. My patch fixes this ambiguity by making sure that both the backend and the JavaScript agree to talk in the "current time zone".

A more ambitious solution to those naive datetime would be to have a new SelectDateWidget with an additional dropdown menu for the time zone and maybe explicit shortcuts like: "now (backend)" and "now (local)".

I hope that helps.

comment:13 Changed 10 months ago by aaugustin

I reviewed previous tickets on the topic, here are my conclusions:

  • the correct fix is to have "now" pre-populate the field in the server's time zone, one way or another;
  • displaying the offset between "browser time" and "server time" would improve usability — "why does it put 11:07 in the field? It's 16:07 here!"; ideally this would appear only when using the auto-fill options and when the offset isn't 0.

comment:14 Changed 10 months ago by aaugustin

In fact, it makes sense to display the offset as soon and __admin_timezone__ and (new Date()).getTimezoneOffset() don't match. The latter appears to be in minutes with the POSIX convention, ie. -60 * X for UTC + X hours.

comment:15 Changed 10 months ago by loic84

I updated the branch to add a warning when there is a time zone mismatch. I'm open to suggestions for better wording.

As soon as we are good with the implementation, particularly the markup, I'll start work on the selenium tests.

comment:16 Changed 10 months ago by loic84

I've tried to come up with selenium tests but it turned out to be problematic because both the LiveServer (which runs in a different thread) and the browser (because of selenium) run in the default time zone (America/Chicago).

I tried changing the time zone for either selenium or the LiveServer with a custom setUpClass() but I didn't manage.

I came up with the following JavaScript monkey-patch which makes the browser pretend to be running in a different time zone (at least for our purpose), but I failed to inject it before the DOM ready event.

# UTC offset in minutes as given by JavaScript Date.getTimezoneOffset().
browser_offset = 480  # Singapore.
server_offset = datetime.now(timezone.get_current_timezone()).utcoffset().total_seconds() / 60

monkeypatch = """
Date.prototype.getTimezoneOffset = function() { return %d; };
var originalGetTime = Date.prototype.getTime;
Date.prototype.getTime = function() { return originalGetTime.apply(this) - %d; };
console.log('monkeypatch');
""" % (browser_offset, (browser_offset + server_offset) * 60 * 1000)

self.selenium.execute_script(monkeypatch)

I'm pretty much out of ideas. In case anyone want to have a go at this, the basic testcase looks like this: https://gist.github.com/loic/5899006.

comment:17 Changed 10 months ago by loic84

Found a workaround and updated the branch: https://github.com/loic/django/compare/ticket20663.

comment:18 Changed 10 months ago by loic84

  • Needs tests unset
  • Version changed from 1.5 to master

comment:19 Changed 10 months ago by loic84

  • Needs documentation unset
  • Patch needs improvement unset

comment:20 Changed 10 months ago by Aymeric Augustin <aymeric.augustin@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 7e6d852bac4de2d5ed2d5ddeabf71482d644ef51:

Fixed #20663 -- "Today" and "now" admin shortcuts.

Changed the shortcuts next to date and time intput widgets
to account for the current timezone.

Refs #7717, #14253 and #18768.

comment:21 Changed 10 months ago by aaugustin

  • Triage Stage changed from Accepted to Ready for checkin

Fantastic. I've made very minor tweaks and pushed the patch. Thank you so much!

comment:22 Changed 6 months ago by Loic Bistuer <loic.bistuer@…>

In 757945b47de45b6eaa0b0e14f87936261a60d227:

Fixed failing test around DST change.

The timezone arithmetic done in JS can be off by one hour around DST
change. We work around this issue by adding one extra hour to the test
error margin when we detect a DST change is near.

Refs #20663.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.