launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/new-person-edit-timezone into lp:launchpad.

Commit message:
Make Person.time_zone always be non-None, allowing us to easily show the edit widget even for users who have never set their time zone.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1568806 in Launchpad itself: "Cant set my timezone to my LP profile in a gui way."
  https://bugs.launchpad.net/launchpad/+bug/1568806

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/new-person-edit-timezone/+merge/293188

Make Person.time_zone always be non-None, allowing us to easily show the edit widget even for users who have never set their time zone.

Fixing bug 933699 would also be a valid approach to this, but that's rather more work, and I think it makes sense in any event for the Person.time_zone property to deal with picking a reasonable default rather than callers having to do so.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/new-person-edit-timezone into lp:launchpad.

=== modified file 'lib/lp/app/browser/tales.py'
--- lib/lp/app/browser/tales.py	2016-02-04 04:39:30 +0000
+++ lib/lp/app/browser/tales.py	2016-04-28 01:50:42 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Implementation of the lp: htmlform: fmt: namespaces in TALES."""
@@ -1199,9 +1199,7 @@
 
     def local_time(self):
         """Return the local time for this person."""
-        time_zone = 'UTC'
-        if self._context.time_zone is not None:
-            time_zone = self._context.time_zone
+        time_zone = self._context.time_zone
         return datetime.now(pytz.timezone(time_zone)).strftime('%T %Z')
 
     def url(self, view_name=None, rootsite='mainsite'):

=== modified file 'lib/lp/app/doc/tales.txt'
--- lib/lp/app/doc/tales.txt	2016-02-06 01:41:00 +0000
+++ lib/lp/app/doc/tales.txt	2016-04-28 01:50:42 +0000
@@ -487,8 +487,11 @@
     >>> test_tales("person/fmt:local-time", person=sample_person)
     '... AWST'
 
-    >>> print mark.time_zone
+    >>> from zope.security.proxy import removeSecurityProxy
+    >>> print removeSecurityProxy(mark).location
     None
+    >>> mark.time_zone
+    u'UTC'
 
     >>> test_tales("person/fmt:local-time", person=mark)
     '... UTC'

=== modified file 'lib/lp/app/widgets/date.py'
--- lib/lp/app/widgets/date.py	2013-04-10 08:05:17 +0000
+++ lib/lp/app/widgets/date.py	2016-04-28 01:50:42 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """These widgets use the a YUI2 calendar widget to allow for
@@ -178,9 +178,8 @@
 
         The widget "system time zone" is generally UTC. It is the logged in
         users time zone, with a fallback to UTC if there is no logged in
-        user, or if the logged in user has not given us a time zone.
-        Although this isn't used directly, it influences the outcome of
-        widget.time_zone.
+        user. Although this isn't used directly, it influences the outcome
+        of widget.time_zone.
 
           >>> print widget.system_time_zone
           UTC

=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2016-03-09 02:33:27 +0000
+++ lib/lp/registry/browser/person.py	2016-04-28 01:50:42 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Person-related view classes."""
@@ -210,10 +210,8 @@
 from lp.services.geoip.interfaces import IRequestPreferredLanguages
 from lp.services.gpg.interfaces import (
     GPG_DATABASE_READONLY_FEATURE_FLAG,
-    GPG_WRITE_TO_GPGSERVICE_FEATURE_FLAG,
     GPGKeyNotFoundError,
     GPGReadOnly,
-    IGPGClient,
     IGPGHandler,
     )
 from lp.services.identity.interfaces.account import (
@@ -2567,8 +2565,8 @@
             self.key_already_imported = True
             return
 
-        # Launchpad talks to the keyserver directly to check if the key has been
-        # uploaded to the key server.
+        # Launchpad talks to the keyserver directly to check if the key has
+        # been uploaded to the key server.
         try:
             key = gpghandler.retrieveKey(self.fingerprint)
         except GPGKeyNotFoundError:
@@ -4012,10 +4010,7 @@
 
     @property
     def initial_values(self):
-        if self.context.time_zone is None:
-            return {}
-        else:
-            return dict(time_zone=self.context.time_zone)
+        return {'time_zone': self.context.time_zone}
 
     @property
     def next_url(self):

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2016-04-11 08:10:46 +0000
+++ lib/lp/registry/model/person.py	2016-04-28 01:50:42 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Implementation classes for a Person."""
@@ -768,7 +768,7 @@
     def time_zone(self):
         """See `IHasLocation`."""
         if self.location is None:
-            return None
+            return u'UTC'
         # Wrap the location with a security proxy to make sure the user has
         # enough rights to see it.
         return ProxyFactory(self.location).time_zone

=== modified file 'lib/lp/registry/stories/location/personlocation-edit.txt'
--- lib/lp/registry/stories/location/personlocation-edit.txt	2016-01-26 15:47:37 +0000
+++ lib/lp/registry/stories/location/personlocation-edit.txt	2016-04-28 01:50:42 +0000
@@ -20,7 +20,8 @@
 A user can set their own time zone:
 
     >>> self_browser = setupBrowser(auth="Basic zzz@xxxxxxx:test")
-    >>> self_browser.open('http://launchpad.dev/~zzz/+editlocation')
+    >>> self_browser.open('http://launchpad.dev/~zzz')
+    >>> self_browser.getLink('Set location and time zone').click()
     >>> self_browser.getControl(name='field.time_zone').value = [
     ...     'Europe/Madrid']
     >>> self_browser.getControl('Update').click()
@@ -33,6 +34,7 @@
 And when they come back to change it later, they'll see it there as the
 selected value.
 
-    >>> self_browser.open('http://launchpad.dev/~zzz/+editlocation')
+    >>> self_browser.open('http://launchpad.dev/~zzz')
+    >>> self_browser.getLink('Set location and time zone').click()
     >>> print self_browser.getControl(name='field.time_zone').value
     ['Europe/Madrid']

=== modified file 'lib/lp/registry/stories/webservice/xx-person.txt'
--- lib/lp/registry/stories/webservice/xx-person.txt	2016-04-12 10:50:30 +0000
+++ lib/lp/registry/stories/webservice/xx-person.txt	2016-04-28 01:50:42 +0000
@@ -58,7 +58,7 @@
     sub_teams_collection_link: u'http://.../~salgado/sub_teams'
     super_teams_collection_link: u'http://.../~salgado/super_teams'
     team_owner_link: None
-    time_zone: None
+    time_zone: u'UTC'
     visibility: u'Public'
     web_link: u'http://launchpad.../~salgado'
     wiki_names_collection_link: u'http://.../~salgado/wiki_names'
@@ -124,7 +124,7 @@
     super_teams_collection_link: u'http://.../~ubuntu-team/super_teams'
     team_description: u'This Team is responsible for the Ubuntu Distribution'
     team_owner_link: u'http://.../~mark'
-    time_zone: None
+    time_zone: u'UTC'
     visibility: u'Public'
     web_link: u'http://launchpad.../~ubuntu-team'
     wiki_names_collection_link: u'http://.../~ubuntu-team/wiki_names'

=== modified file 'lib/lp/registry/stories/webservice/xx-personlocation.txt'
--- lib/lp/registry/stories/webservice/xx-personlocation.txt	2012-02-16 20:37:55 +0000
+++ lib/lp/registry/stories/webservice/xx-personlocation.txt	2016-04-28 01:50:42 +0000
@@ -10,7 +10,7 @@
 
     >>> jdub = webservice.get("/~jdub").jsonBody()
     >>> print jdub['time_zone']
-    None
+    UTC
     >>> print jdub['latitude']
     None
     >>> print jdub['longitude']
@@ -20,7 +20,7 @@
 latitude/longitude read via the Web API will still be None.
 
     >>> print webservice.get("/~jdub").jsonBody()['time_zone']
-    None
+    UTC
     >>> print webservice.named_post(
     ...     '/~jdub', 'setLocation', {},
     ...     latitude='-34.6', longitude='157.0', time_zone='Australia/Sydney')

=== modified file 'lib/lp/registry/templates/person-portlet-contact-details.pt'
--- lib/lp/registry/templates/person-portlet-contact-details.pt	2015-02-27 01:11:06 +0000
+++ lib/lp/registry/templates/person-portlet-contact-details.pt	2016-04-28 01:50:42 +0000
@@ -177,7 +177,7 @@
       </dd>
     </dl>
 
-    <dl id="timezone" tal:condition="context/time_zone">
+    <dl id="timezone">
       <dt>Time zone:
         <a tal:replace="structure overview_menu/editlocation/fmt:icon" />
       </dt>

=== modified file 'lib/lp/services/webapp/launchbag.py'
--- lib/lp/services/webapp/launchbag.py	2015-07-08 16:05:11 +0000
+++ lib/lp/services/webapp/launchbag.py	2016-04-28 01:50:42 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """
@@ -151,7 +151,7 @@
     @property
     def time_zone(self):
         if getattr(self._store, "time_zone", None) is None:
-            if self.user and self.user.time_zone:
+            if self.user:
                 self._store.time_zone = pytz.timezone(self.user.time_zone)
             else:
                 # fall back to UTC