launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #30012
[Merge] ~cjwatson/launchpad:date-widget-time-zone-name into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:date-widget-time-zone-name into launchpad:master.
Commit message:
Refactor time zone name handling in DateTimeWidget
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/443006
`DateTimeWidget` currently works out the time zone name to display by calling `.tzname(None)` on a time zone object. This works with `pytz`, but it won't work once we switch to `dateutil.tz`, and there doesn't seem to be a reasonable way to get it to work with that time zone provider without poking about in private attributes; the best we could do would return a time zone abbreviation string rather than the original time zone name the user asked for (e.g. "CET" rather than "Europe/Paris"), which isn't very satisfactory.
Since the whole approach of expecting to be able to recover the original time zone name from a time zone object seems fundamentally dodgy, a better approach is to explicitly keep track of the original time zone name to start with. This needs some changes to `LaunchBag`, but the result should be much more robust and can easily be made to work with any time zone provider.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:date-widget-time-zone-name into launchpad:master.
diff --git a/lib/lp/app/widgets/date.py b/lib/lp/app/widgets/date.py
index acaa0f2..e024dfa 100644
--- a/lib/lp/app/widgets/date.py
+++ b/lib/lp/app/widgets/date.py
@@ -17,8 +17,9 @@ __all__ = [
"DatetimeDisplayWidget",
]
-from datetime import datetime, timezone
+from datetime import datetime, timezone, tzinfo
+import pytz
from zope.browserpage import ViewPageTemplateFile
from zope.component import getUtility
from zope.datetime import DateTimeError, parse
@@ -40,7 +41,6 @@ from lp.services.webapp.interfaces import ILaunchBag
class DateTimeWidget(TextWidget):
"""A date and time selection widget with popup selector.
- >>> import pytz
>>> from zope.schema import Field
>>> from lp.services.webapp.servers import LaunchpadTestRequest
>>> field = Field(__name__="foo", title="Foo")
@@ -63,7 +63,7 @@ class DateTimeWidget(TextWidget):
If there is a required time zone, then that overrides the user or system
default, and the user is not invited to change the time zone:
- >>> widget.required_time_zone = pytz.timezone("America/Los_Angeles")
+ >>> widget.required_time_zone_name = "America/Los_Angeles"
>>> print(widget()) # doctest: +ELLIPSIS
<BLANKLINE>
<...in time zone: America/Los_Angeles...
@@ -114,7 +114,7 @@ class DateTimeWidget(TextWidget):
"""
timeformat = "%Y-%m-%d %H:%M:%S"
- required_time_zone = None
+ required_time_zone_name = None # type: str
display_zone = True
from_date = None
to_date = None
@@ -126,7 +126,7 @@ class DateTimeWidget(TextWidget):
def __init__(self, context, request):
super().__init__(context, request)
launchbag = getUtility(ILaunchBag)
- self.system_time_zone = launchbag.time_zone
+ self.system_time_zone_name = launchbag.time_zone_name
@property
def supported_input_formats(self):
@@ -158,7 +158,17 @@ class DateTimeWidget(TextWidget):
return [o.strip() for o in outputs]
@property
- def time_zone(self):
+ def time_zone_name(self) -> str:
+ """The name of the widget time zone for display in the widget."""
+ if self.required_time_zone_name is not None:
+ return self.required_time_zone_name
+ assert (
+ self.system_time_zone_name is not None
+ ), "DateTime widget needs a time zone."
+ return self.system_time_zone_name
+
+ @property
+ def time_zone(self) -> tzinfo:
"""The widget time zone.
This will either give you the user's time zone, or the system
@@ -167,7 +177,7 @@ class DateTimeWidget(TextWidget):
externally-defined time zone. For example, when a person will join a
conference in the time zone in which the conference is being held.
- >>> import pytz
+ >>> from datetime import tzinfo
>>> from zope.publisher.browser import TestRequest
>>> from zope.schema import Field
>>> field = Field(__name__="foo", title="Foo")
@@ -176,12 +186,12 @@ class DateTimeWidget(TextWidget):
The time zone is a time zone object, not the string representation
of that.
- >>> print(type(widget.time_zone))
- <class 'datetime.timezone'>
+ >>> isinstance(widget.time_zone, tzinfo)
+ True
- The widget required_time_zone is None by default.
+ The widget required_time_zone_name is None by default.
- >>> print(widget.required_time_zone)
+ >>> print(widget.required_time_zone_name)
None
The widget "system time zone" is generally UTC. It is the logged in
@@ -189,41 +199,32 @@ class DateTimeWidget(TextWidget):
user. Although this isn't used directly, it influences the outcome
of widget.time_zone.
- >>> print(repr(widget.system_time_zone))
- datetime.timezone.utc
+ >>> print(widget.system_time_zone_name)
+ UTC
- When there is no required_time_zone, then we get the system time
- zone.
+ When there is no required_time_zone_name, then we get the system
+ time zone.
- >>> print(widget.required_time_zone)
+ >>> print(widget.required_time_zone_name)
None
+ >>> print(widget.time_zone_name)
+ UTC
>>> print(repr(widget.time_zone))
datetime.timezone.utc
- When there is a required_time_zone, we get it:
+ When there is a required_time_zone_name, we get it:
- >>> widget.required_time_zone = pytz.timezone("Africa/Maseru")
+ >>> widget.required_time_zone_name = "Africa/Maseru"
+ >>> print(widget.time_zone_name)
+ Africa/Maseru
>>> print(widget.time_zone)
Africa/Maseru
"""
- if self.required_time_zone is not None:
- return self.required_time_zone
- assert (
- self.system_time_zone is not None
- ), "DateTime widget needs a time zone."
- return self.system_time_zone
-
- @property
- def time_zone_name(self):
- """The name of the widget time zone for display in the widget."""
- # XXX cjwatson 2023-03-09: In Python < 3.6, `timezone.utc.tzname`
- # returns "UTC+00:00" rather than "UTC". Drop this once we require
- # Python >= 3.6.
- if self.time_zone is timezone.utc:
- return "UTC"
+ if self.time_zone_name == "UTC":
+ return timezone.utc
else:
- return self.time_zone.tzname(None)
+ return pytz.timezone(self.time_zone_name)
def _align_date_constraints_with_time_zone(self):
"""Ensure that from_date and to_date use the widget time zone."""
@@ -383,14 +384,13 @@ class DateTimeWidget(TextWidget):
def _parseInput(self, input):
"""Convert a string to a datetime value.
- >>> import pytz
>>> from zope.publisher.browser import TestRequest
>>> from zope.schema import Field
>>> field = Field(__name__="foo", title="Foo")
>>> widget = DateTimeWidget(field, TestRequest())
- >>> widget.required_time_zone = timezone.utc
- >>> widget.time_zone
- datetime.timezone.utc
+ >>> widget.required_time_zone_name = "UTC"
+ >>> widget.time_zone_name
+ 'UTC'
The widget converts an empty string to the missing value:
@@ -404,7 +404,7 @@ class DateTimeWidget(TextWidget):
But it will handle other time zones:
- >>> widget.required_time_zone = pytz.timezone("Australia/Perth")
+ >>> widget.required_time_zone_name = "Australia/Perth"
>>> print(widget._parseInput("2006-01-01 12:00:00"))
2006-01-01 12:00:00+08:00
@@ -434,7 +434,6 @@ class DateTimeWidget(TextWidget):
def _toFormValue(self, value):
"""Convert a date to its string representation.
- >>> import pytz
>>> from zope.publisher.browser import TestRequest
>>> from zope.schema import Field
>>> field = Field(__name__="foo", title="Foo")
@@ -455,7 +454,7 @@ class DateTimeWidget(TextWidget):
The date value will be converted to the widget's time zone
before being displayed:
- >>> widget.required_time_zone = pytz.timezone("America/New_York")
+ >>> widget.required_time_zone_name = "America/New_York"
>>> widget._toFormValue(dt)
'2006-01-01 07:00:00'
"""
@@ -497,7 +496,6 @@ class DateWidget(DateTimeWidget):
The DateWidget subclass can limit requests to date ranges:
>>> from datetime import date
- >>> import pytz
>>> from zope.publisher.browser import TestRequest
>>> from zope.schema import Field
>>> field = Field(__name__="foo", title="Foo")
@@ -509,7 +507,7 @@ class DateWidget(DateTimeWidget):
>>> "[[2004,04,05],[2004,04,10]]" in widget()
True
- This widget ignores required_time_zone and system_time_zone and
+ This widget ignores required_time_zone_name and system_time_zone_name and
interprets everything as UTC. This does not matter, because it is only
picking the date, and it will always be rendered as a date sans time
zone even if it is stored as a datetime.
@@ -517,11 +515,11 @@ class DateWidget(DateTimeWidget):
>>> widget.time_zone
datetime.timezone.utc
- >>> widget.system_time_zone = pytz.timezone("America/New_York")
+ >>> widget.system_time_zone_name = "America/New_York"
>>> widget.time_zone
datetime.timezone.utc
- >>> widget.required_time_zone = pytz.timezone("America/Los_Angeles")
+ >>> widget.required_time_zone_name = "America/Los_Angeles"
>>> widget.time_zone
datetime.timezone.utc
@@ -536,7 +534,7 @@ class DateWidget(DateTimeWidget):
"""
timeformat = "%Y-%m-%d"
- time_zone = timezone.utc
+ time_zone_name = "UTC"
# ZPT that renders our widget
__call__ = ViewPageTemplateFile("templates/date.pt")
diff --git a/lib/lp/app/widgets/templates/datetime.pt b/lib/lp/app/widgets/templates/datetime.pt
index d53d9ba..1042d65 100644
--- a/lib/lp/app/widgets/templates/datetime.pt
+++ b/lib/lp/app/widgets/templates/datetime.pt
@@ -9,7 +9,7 @@
disabled view/disabled_flag" />
<tal:display_zone condition="view/display_zone">
<span> in time zone: <tal:tz replace="view/time_zone_name" />
- <a tal:condition="not: view/required_time_zone"
+ <a tal:condition="not: view/required_time_zone_name"
href="/people/+me/+editlocation">
<img src="/@@/edit"/>
</a></span>
diff --git a/lib/lp/blueprints/browser/sprint.py b/lib/lp/blueprints/browser/sprint.py
index a89a362..41148c0 100644
--- a/lib/lp/blueprints/browser/sprint.py
+++ b/lib/lp/blueprints/browser/sprint.py
@@ -291,9 +291,9 @@ class SprintAddView(LaunchpadFormView):
self.widgets["time_ends"].timeformat = timeformat
time_zone_widget = self.widgets["time_zone"]
if time_zone_widget.hasValidInput():
- tz = pytz.timezone(time_zone_widget.getInputValue())
- self.widgets["time_starts"].required_time_zone = tz
- self.widgets["time_ends"].required_time_zone = tz
+ tz_name = time_zone_widget.getInputValue()
+ self.widgets["time_starts"].required_time_zone_name = tz_name
+ self.widgets["time_ends"].required_time_zone_name = tz_name
def validate(self, data):
time_starts = data.get("time_starts")
@@ -373,11 +373,11 @@ class SprintEditView(LaunchpadEditFormView):
time_zone_widget = self.widgets["time_zone"]
# What time zone are the start and end values relative to?
if time_zone_widget.hasValidInput():
- tz = pytz.timezone(time_zone_widget.getInputValue())
+ tz_name = time_zone_widget.getInputValue()
else:
- tz = pytz.timezone(self.context.time_zone)
- self.widgets["time_starts"].required_time_zone = tz
- self.widgets["time_ends"].required_time_zone = tz
+ tz_name = self.context.time_zone
+ self.widgets["time_starts"].required_time_zone_name = tz_name
+ self.widgets["time_ends"].required_time_zone_name = tz_name
def validate(self, data):
time_starts = data.get("time_starts")
diff --git a/lib/lp/blueprints/browser/sprintattendance.py b/lib/lp/blueprints/browser/sprintattendance.py
index 2964ffd..194f340 100644
--- a/lib/lp/blueprints/browser/sprintattendance.py
+++ b/lib/lp/blueprints/browser/sprintattendance.py
@@ -44,11 +44,10 @@ class BaseSprintAttendanceAddView(LaunchpadFormView):
def setUpWidgets(self):
LaunchpadFormView.setUpWidgets(self)
- tz = pytz.timezone(self.context.time_zone)
self.starts_widget = self.widgets["time_starts"]
self.ends_widget = self.widgets["time_ends"]
- self.starts_widget.required_time_zone = tz
- self.ends_widget.required_time_zone = tz
+ self.starts_widget.required_time_zone_name = self.context.time_zone
+ self.ends_widget.required_time_zone_name = self.context.time_zone
# We don't need to display seconds
timeformat = "%Y-%m-%d %H:%M"
self.starts_widget.timeformat = timeformat
@@ -57,8 +56,9 @@ class BaseSprintAttendanceAddView(LaunchpadFormView):
# after the sprint. We will accept a time just before or just after
# and map those to the beginning and end times, respectively, in
# self.getDates().
- from_date = self.context.time_starts.astimezone(tz)
- to_date = self.context.time_ends.astimezone(tz)
+ time_zone = pytz.timezone(self.context.time_zone)
+ from_date = self.context.time_starts.astimezone(time_zone)
+ to_date = self.context.time_ends.astimezone(time_zone)
self.starts_widget.from_date = from_date - timedelta(days=1)
self.starts_widget.to_date = to_date
self.ends_widget.from_date = from_date
diff --git a/lib/lp/services/webapp/doc/launchbag.rst b/lib/lp/services/webapp/doc/launchbag.rst
index 304707d..41e7b6e 100644
--- a/lib/lp/services/webapp/doc/launchbag.rst
+++ b/lib/lp/services/webapp/doc/launchbag.rst
@@ -79,20 +79,23 @@ will be cookie@xxxxxxxxxxx.
cookie@xxxxxxxxxxx
-time_zone
----------
+time_zone_name and time_zone
+----------------------------
-The time_zone attribute gives the user's time zone, as an pytz object.
+The time_zone_name attribute gives the name of the user's time zone; the
+time_zone attribute gives it as a tzinfo object.
>>> from lp.testing.factory import LaunchpadObjectFactory
>>> factory = LaunchpadObjectFactory()
>>> person = factory.makePerson()
>>> ignored = login_person(person)
+ >>> launchbag.time_zone_name
+ 'UTC'
>>> launchbag.time_zone
- <UTC>
+ datetime.timezone.utc
-It's cached, so even if the user's time zone is changed, it will stay
-the same. This is to optimize the look-up time, since some pages look it
+They're cached, so even if the user's time zone is changed, they will stay
+the same. This is to optimize the look-up time, since some pages look them
up a lot of times.
>>> person.setLocation(
@@ -101,11 +104,15 @@ up a lot of times.
... "Europe/Paris",
... launchbag.user,
... )
+ >>> launchbag.time_zone_name
+ 'UTC'
>>> launchbag.time_zone
- <UTC>
+ datetime.timezone.utc
After the LaunchBag has been cleared, the correct time zone is returned.
>>> launchbag.clear()
+ >>> launchbag.time_zone_name
+ 'Europe/Paris'
>>> launchbag.time_zone
<... 'Europe/Paris' ...>
diff --git a/lib/lp/services/webapp/interfaces.py b/lib/lp/services/webapp/interfaces.py
index c6ef41a..ad0a103 100644
--- a/lib/lp/services/webapp/interfaces.py
+++ b/lib/lp/services/webapp/interfaces.py
@@ -335,6 +335,7 @@ class ILaunchBag(Interface):
user = Attribute("Currently authenticated IPerson, or None")
login = Attribute("The login used by the authenticated person, or None")
+ time_zone_name = Attribute("The name of the user's time zone")
time_zone = Attribute("The user's time zone")
developer = Bool(
diff --git a/lib/lp/services/webapp/launchbag.py b/lib/lp/services/webapp/launchbag.py
index a9d12d7..f20dfe0 100644
--- a/lib/lp/services/webapp/launchbag.py
+++ b/lib/lp/services/webapp/launchbag.py
@@ -30,8 +30,6 @@ from lp.services.webapp.interaction import get_current_principal
from lp.services.webapp.interfaces import ILoggedInEvent, IOpenLaunchBag
from lp.soyuz.interfaces.distroarchseries import IDistroArchSeries
-_utc_tz = timezone.utc
-
@implementer(IOpenLaunchBag)
class LaunchBag:
@@ -92,6 +90,7 @@ class LaunchBag:
setattr(store, attribute, None)
store.login = None
store.time_zone = None
+ store.time_zone_name = None
@property
def person(self):
@@ -147,13 +146,22 @@ class LaunchBag:
return getattr(self._store, "bugtask", None)
@property
- def time_zone(self):
- if getattr(self._store, "time_zone", None) is None:
+ def time_zone_name(self):
+ if getattr(self._store, "time_zone_name", None) is None:
if self.user:
- self._store.time_zone = pytz.timezone(self.user.time_zone)
+ self._store.time_zone_name = self.user.time_zone
else:
# fall back to UTC
- self._store.time_zone = _utc_tz
+ self._store.time_zone_name = "UTC"
+ return self._store.time_zone_name
+
+ @property
+ def time_zone(self):
+ if getattr(self._store, "time_zone", None) is None:
+ if self.time_zone_name == "UTC":
+ self._store.time_zone = timezone.utc
+ else:
+ self._store.time_zone = pytz.timezone(self.time_zone_name)
return self._store.time_zone
