← Back to team overview

launchpad-reviewers team mailing list archive

  1. launchpad-reviewers team
  2. Mailing list archive
  3. Message #00804

[Merge] lp:~sinzui/launchpad/disable-gmaps-0 into lp:launchpad/devel

  Thread Previous • Date Previous • • Thread Next 
Curtis Hovey has proposed merging lp:~sinzui/launchpad/disable-gmaps-0 into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


This is my branch to disable Google maps.

    lp:~sinzui/launchpad/disable-gmaps-0
    Diff size: 467
    Launchpad bug:
          https://bugs.launchpad.net/bugs/624981
    Test command: ./bin/test -vv \
          -t personlocation, -t team-map -t xx-person-home
    Pre-implementation: Edwin
    Target release: 10.09


Disable Google maps
-------------------

Launchpad users who have enabled maps are seeing a popup stating that "the
Google Maps API server rejected your request". Google recognises this as a
problem with their service:
http://www.google.com/support/forum/p/base/thread?hl=en&tid=462f63cbd84b4464

Launchpad users can avoid the message by unchecking the "[X] Display map"
checkbox. If this problem persists, we will consider removing maps from
Launchpad until the issue is addresses.


Rules
-----

The small maps shown on user and team pages is governed in the view by
    self.request.needs_gmap2
This value is set by person visibility and the user's choice to enable/disable
maps. We can add a third condition that we can control per environment to
enable maps.

    * Add a switch to enable/disable Google maps
      * The switch should be easy to re-enable in any environment--
        Sounds like a feature-flag.


QA
--

    * Visit your profile page.
    * Verify maps are not displayed, but you can visit the page to
      set your timezone..


Lint
----

Linting changed files:
  lib/lp/registry/browser/__init__.py
  lib/lp/registry/browser/person.py
  lib/lp/registry/browser/team.py
  lib/lp/registry/stories/location/personlocation.txt
  lib/lp/registry/stories/location/team-map.txt
  lib/lp/registry/stories/person/xx-person-home.txt
  lib/lp/registry/templates/person-portlet-map.pt
  lib/lp/registry/templates/team-portlet-map.pt



Test
----

    * lib/lp/registry/stories/location/personlocation.txt
      * Added a test to verify the map is not shown when the gmap2 feature
        flag is not on. Revised a test to enable gmap2 so the test could
        continue verifying the maps users see.
      * Fixed long lines and headers.
    * lib/lp/registry/stories/location/team-map.txt
      * Added a test to verify the map is not shown when the gmap2 feature
        flag is not on. Revised a test to enable gmap2 so the test could
        continue verifying the maps users see.
      * Fixed long lines and headers.
    * lib/lp/registry/stories/person/xx-person-home.txt
      * Removed team test from person story...the team has its own story.
      * Fixed long lines and headers.


Implementation
--------------

    * lib/lp/registry/browser/__init__.py
      * Added MapMixin that provides access to the rudimentary feature flag.
        This method can be refactored to use the real feature when it is
        ready.
    * lib/lp/registry/browser/person.py
      * Use MapMixin in PersonIndexView.
    * lib/lp/registry/browser/team.py
      * Use MapMixin in TeamMapView.
    * lib/lp/registry/templates/person-portlet-map.pt
      * Added an edit icon so that the user can always edit his timezone.
      * Do not show the map if gmap2 is not enabled.
    * lib/lp/registry/templates/team-portlet-map.pt
      * Do not show the map if gmap2 is not enabled.
-- 
https://code.launchpad.net/~sinzui/launchpad/disable-gmaps-0/+merge/33971
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/disable-gmaps-0 into lp:launchpad/devel.

=== modified file 'lib/lp/registry/browser/__init__.py'
--- lib/lp/registry/browser/__init__.py	2010-08-20 20:31:18 +0000
+++ lib/lp/registry/browser/__init__.py	2010-08-27 23:01:14 +0000
@@ -7,6 +7,7 @@
 
 __all__ = [
     'get_status_counts',
+    'MapMixin',
     'MilestoneOverlayMixin',
     'RegistryEditFormView',
     'RegistryDeleteViewMixin',
@@ -19,6 +20,7 @@
 from storm.store import Store
 from zope.component import getUtility
 
+from canonical.cachedproperty import cachedproperty
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.webapp.launchpadform import (
     action,
@@ -256,3 +258,19 @@
     @action("Change", name='change')
     def change_action(self, action, data):
         self.updateContextFromData(data)
+
+
+class MapMixin:
+
+    @cachedproperty
+    def gmap2_enabled(self):
+        # XXX sinzui 2010-08-27 bug=625556: This is a hack to use
+        # feature flags, which are not ready for general use in the production
+        # code, but has just enough to support this use case:
+        # Do not enable gmap2 if Google's service is not operational.
+        from lp.services.features.flags import FeatureController
+
+        def in_scope(value):
+            return True
+
+        return FeatureController(in_scope).getFlag('gmap2') == 'on'

=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2010-08-26 22:44:30 +0000
+++ lib/lp/registry/browser/person.py	2010-08-27 23:01:14 +0000
@@ -244,6 +244,7 @@
 from lp.code.browser.sourcepackagerecipelisting import HasRecipesMenuMixin
 from lp.code.errors import InvalidNamespace
 from lp.code.interfaces.branchnamespace import IBranchNamespaceSet
+from lp.registry.browser import MapMixin
 from lp.registry.browser.branding import BrandingChangeView
 from lp.registry.browser.mailinglists import enabled_with_active_mailing_list
 from lp.registry.browser.menu import (
@@ -3324,7 +3325,7 @@
         return self.state is EmailAddressVisibleState.ALLOWED
 
 
-class PersonIndexView(XRDSContentNegotiationMixin, PersonView):
+class PersonIndexView(XRDSContentNegotiationMixin, MapMixin, PersonView):
     """View class for person +index and +xrds pages."""
 
     xrds_template = ViewPageTemplateFile(
@@ -3337,7 +3338,8 @@
         # the location is set, visible, and the viewing user wants to see it.
         launchpad_views = get_launchpad_views(self.request.cookies)
         self._small_map = launchpad_views['small_maps']
-        if (self.has_visible_location and self._small_map):
+        if (self.gmap2_enabled
+            and self.has_visible_location and self._small_map):
             self.request.needs_gmap2 = True
         if self.request.method == "POST":
             self.processForm()

=== modified file 'lib/lp/registry/browser/team.py'
--- lib/lp/registry/browser/team.py	2010-08-22 19:14:23 +0000
+++ lib/lp/registry/browser/team.py	2010-08-27 23:01:14 +0000
@@ -69,6 +69,7 @@
     LaunchpadRadioWidget,
     )
 from lp.app.errors import UnexpectedFormData
+from lp.registry.browser import MapMixin
 from lp.registry.browser.branding import BrandingChangeView
 from lp.registry.interfaces.mailinglist import (
     IMailingList,
@@ -1031,7 +1032,7 @@
         self.request.response.addInfoNotification(msg)
 
 
-class TeamMapView(LaunchpadView):
+class TeamMapView(MapMixin, LaunchpadView):
     """Show all people with known locations on a map.
 
     Also provides links to edit the locations of people in the team without
@@ -1044,7 +1045,7 @@
     def initialize(self):
         # Tell our base-layout to include Google's gmap2 javascript so that
         # we can render the map.
-        if self.mapped_participants_count > 0:
+        if self.gmap2_enabled and self.mapped_participants_count > 0:
             self.request.needs_gmap2 = True
 
     @cachedproperty

=== modified file 'lib/lp/registry/stories/location/personlocation.txt'
--- lib/lp/registry/stories/location/personlocation.txt	2010-07-15 10:55:27 +0000
+++ lib/lp/registry/stories/location/personlocation.txt	2010-08-27 23:01:14 +0000
@@ -1,4 +1,5 @@
-= Person Locations =
+Person Locations
+================
 
 People can have a location and time zone in Launchpad.  In some cases, a
 person has a time zone, but no location. We test that their home page renders
@@ -16,18 +17,38 @@
     >>> print extract_text(
     ...     find_tag_by_id(anon_browser.contents, 'portlet-map'))
 
+If a person has a location, but the gmap2 feature is not enabled, the user
+sees the timezone, but no map.
+
+    >>> login('test@xxxxxxxxxxxxx')
+    >>> yyy = factory.makePerson(name='yyy', time_zone='Europe/London',
+    ...                          latitude=52.2, longitude=0.3)
+    >>> logout()
+
+    >>> anon_browser.open('http://launchpad.dev/~yyy')
+    >>> markup = str(anon_browser.contents)
+    >>> print extract_text(
+    ...     find_tag_by_id(markup, 'portlet-map'), skip_tags=[])
+    Location
+    Time zone: Europe/London...
+
+    >>> 'src="http://maps.google.com/maps' in markup
+    False
+
 If a person has a location, there is a little map portlet in their
 profile page. We can't test all the google javascript, but we can make sure
-there's a map, and the scripts are loaded.
+there's a map, and the scripts are loaded  when the gmap2 feature is enabled
+for users.
 
-    >>> login('test@xxxxxxxxxxxxx')
-    >>> yyy = factory.makePerson(name='yyy', time_zone='Europe/London',
-    ...                          latitude=52.2, longitude=0.3)
-    >>> logout()
+    >>> from lp.services.features.model import FeatureFlag, getFeatureStore
+    >>> ignore = getFeatureStore().add(FeatureFlag(
+    ...     scope=u'default', flag=u'gmap2', value=u'on', priority=1))
+    >>> transaction.commit()
 
     >>> anon_browser.open('http://launchpad.dev/~yyy')
     >>> markup = str(anon_browser.contents)
-    >>> print extract_text(find_tag_by_id(markup, 'portlet-map'), skip_tags=[])
+    >>> print extract_text(
+    ...     find_tag_by_id(markup, 'portlet-map'), skip_tags=[])
     Location
     Time zone: Europe/London...
     Y.lp.app.mapping.renderPersonMapSmall(...

=== modified file 'lib/lp/registry/stories/location/team-map.txt'
--- lib/lp/registry/stories/location/team-map.txt	2010-07-15 10:55:27 +0000
+++ lib/lp/registry/stories/location/team-map.txt	2010-08-27 23:01:14 +0000
@@ -1,4 +1,27 @@
-== The map of a team's members ==
+The map of a team's members
+===========================
+
+Maps are disabled
+-----------------
+
+Users cannot see maps when the gmap2 feature is disbaled for them
+
+    >>> user_browser.open('http://launchpad.dev/~guadamen')
+    >>> body = find_main_content(user_browser.contents)
+    >>> mapdiv = find_tag_by_id(str(body), 'team_map_div')
+    >>> 'lp.app.mapping.renderTeamMapSmall(' in str(body)
+    False
+
+
+Maps are enabled
+----------------
+
+Users can see maps when the gmap2 feature is enabled for them.
+
+    >>> from lp.services.features.model import FeatureFlag, getFeatureStore
+    >>> ignore = getFeatureStore().add(FeatureFlag(
+    ...     scope=u'default', flag=u'gmap2', value=u'on', priority=1))
+    >>> transaction.commit()
 
 If a team has members that have locations, then you should see a portlet
 with their locations displayed.
@@ -37,7 +60,9 @@
       <participant
         displayname="Colin Watson"
         name="kamion"
-        logo_html="&lt;img alt=&quot;&quot; width=&quot;64&quot; height=&quot;64&quot; src=&quot;/@@/person-logo&quot; /&gt;"
+        logo_html="&lt;img alt=&quot;&quot;
+            width=&quot;64&quot; height=&quot;64&quot;
+            src=&quot;/@@/person-logo&quot; /&gt;"
         url="/~kamion"
         local_time="..."
         lat="52.2"
@@ -72,7 +97,9 @@
       <participant
         displayname="Colin Watson"
         name="kamion"
-        logo_html="&lt;img alt=&quot;&quot; width=&quot;64&quot; height=&quot;64&quot; src=&quot;/@@/person-logo&quot; /&gt;"
+        logo_html="&lt;img alt=&quot;&quot;
+            width=&quot;64&quot; height=&quot;64&quot;
+                src=&quot;/@@/person-logo&quot; /&gt;"
         url="/~kamion"
         local_time="..."
         lat="52.2"
@@ -90,7 +117,8 @@
     http://launchpad.dev/~guadamen/+map
 
 
-== +mapdata ==
++mapdata
+--------
 
 The display name of all team participants will be escaped to prevent
 XSS attacks on any callsite of +mapdata.
@@ -106,5 +134,6 @@
     >>> anon_browser.open('http://launchpad.dev/~guadamen/+mapdata')
     >>> print anon_browser.contents
     <?xml version="1.0"...
-    ...displayname="&amp;lt;script&amp;gt;alert('Colin &amp;quot;nasty&amp;quot;');&amp;lt;/script&amp;gt;"
+    ...displayname="&amp;lt;script&amp;gt;alert('Colin
+        &amp;quot;nasty&amp;quot;');&amp;lt;/script&amp;gt;"
     ...

=== modified file 'lib/lp/registry/stories/person/xx-person-home.txt'
--- lib/lp/registry/stories/person/xx-person-home.txt	2010-06-24 15:30:55 +0000
+++ lib/lp/registry/stories/person/xx-person-home.txt	2010-08-27 23:01:14 +0000
@@ -1,4 +1,5 @@
-= Personal Home Pages =
+Personal Home Pages
+===================
 
 Launchpad creates profiles for people that have contributed to free
 software (e.g. in a bug import or a translation template upload). It's
@@ -15,7 +16,8 @@
     2006-12-13 when importing the Portuguese...
 
 
-== Email address disclosure ==
+Email address disclosure
+------------------------
 
 Mark has a registered email address, and he has chosen to disclose it to
 the world. Anonymous users cannot see Mark's address
@@ -51,9 +53,11 @@
     testing@xxxxxxxxxxxxx
 
 
-== Open ID link ==
+Open ID link
+------------
 
-When a person visits his or her own page, they'll see their OpenID login URL.
+When a person visits his or her own page, they'll see their OpenID login
+URL.
 
     >>> user_browser.open('http://launchpad.dev/~no-priv')
     >>> print extract_text(
@@ -80,7 +84,8 @@
     LinkNotFoundError
 
 
-== Jabber IDs ==
+Jabber IDs
+----------
 
 A person's jabber IDs are only show to authenticated users.
 
@@ -95,11 +100,12 @@
     Jabber: &lt;email address hidden&gt;
 
 
-== OpenPGP keys ==
+OpenPGP keys
+------------
 
-In order to avoid email harvesters to find a person's email addresses just by
-following the link to that person's OpenPGP keys, only authenticated users can
-see the key ID with a link to the keyserver.
+In order to avoid email harvesters to find a person's email addresses
+just by following the link to that person's OpenPGP keys, only
+authenticated users can see the key ID with a link to the keyserver.
 
     >>> user_browser.open('http://launchpad.dev/~name16')
     >>> print find_tag_by_id(user_browser.contents, 'pgp-keys')
@@ -112,18 +118,19 @@
     <dd> 12345678...
 
 
-== Languages ==
+Languages
+---------
 
-The contact details portlet shows the languages that the user speaks.
-No Privileges Person can see the languages that mark speaks.
+The contact details portlet shows the languages that the user speaks. No
+Privileges Person can see the languages that mark speaks.
 
     >>> user_browser.open('http://launchpad.dev/~carlos')
     >>> print extract_text(find_tag_by_id(user_browser.contents, 'languages'))
     Languages:
     Catalan, English, Spanish
 
-When viewing his own page, No Privileges Person sees his languages and can
-edit them.
+When viewing his own page, No Privileges Person sees his languages and
+can edit them.
 
     >>> user_browser.open('http://launchpad.dev/~no-priv')
     >>> print extract_text(find_tag_by_id(user_browser.contents, 'languages'))
@@ -131,7 +138,8 @@
     English
 
 
-== Summary Pagelets ==
+Summary Pagelets
+----------------
 
 A person's homepage also lists Karma information:
 
@@ -154,6 +162,7 @@
 
     >>> print extract_text(find_tag_by_id(browser.contents, 'karma-total'))
     130
+
     >>> print extract_text(find_tag_by_id(browser.contents, 'member-since'))
     2005-06-06
 
@@ -164,7 +173,8 @@
     2005-06-06
 
 
-=== Time zones ===
+Time zones
+..........
 
 The user's time zone is displayed next to their location details:
 
@@ -174,26 +184,20 @@
     Location
     Time zone: Europe/London...
 
-If the user does not have location data set then the portlet will not
-be shown.
+If the user does not have location data set then the portlet will not be
+shown.
 
     >>> browser.open('http://launchpad.dev/~bac')
     >>> print extract_text(
     ...     find_tag_by_id(browser.contents, 'portlet-map'))
 
-Teams don't have a time zone field.
-
-    >>> browser.open('http://launchpad.dev/~guadamen')
-    >>> 'Time zone' in extract_text(
-    ...     find_tag_by_id(browser.contents, 'portlet-map'))
-    False
-
-
-== Table of contributions ==
-
-A person's home page also displays a table with the contributions made by that
-person. This table includes 5 projects in which this person is most active
-and also the areas in which (s)he worked on each project.
+
+Table of contributions
+----------------------
+
+A person's home page also displays a table with the contributions made
+by that person. This table includes 5 projects in which this person is
+most active and also the areas in which (s)he worked on each project.
 
     >>> anon_browser.open('http://launchpad.dev/~name16')
     >>> table = find_tag_by_id(anon_browser.contents, 'contributions')
@@ -220,8 +224,8 @@
     >>> anon_browser.getLink('Recent activities')
     <Link text='Recent activities' url='http://launchpad.dev/~name16/+karma'>
 
-If the person hasn't made any contributions, the table is not present in its
-page.
+If the person hasn't made any contributions, the table is not present in
+its page.
 
     >>> anon_browser.open('http://launchpad.dev/~jdub')
     >>> print find_tag_by_id(anon_browser.contents, 'contributions')
@@ -237,9 +241,9 @@
 Unactivated profiles
 --------------------
 
-Many profiles are created for users who contributed to projects that were
-imported into Launchpad. Any user can see an unclaimed profile and a link
-to request a claim the profile.
+Many profiles are created for users who contributed to projects that
+were imported into Launchpad. Any user can see an unclaimed profile and
+a link to request a claim the profile.
 
     >>> anon_browser.open('https://launchpad.dev/~jvprat')
     >>> print anon_browser.title
@@ -252,13 +256,13 @@
     >>> anon_browser.getLink('Are you Jordi Vilalta')
     <Link text='Are you Jordi Vilalta?' url='.../people/+requestmerge...'>
 
-It is possible for the preferred email address to be set if it is associated
-with an Ubuntu Single Signon account. Anonymous and logged in users cannot
-see this, but admins like Foo Bar can.
+It is possible for the preferred email address to be set if it is
+associated with an Ubuntu Single Signon account. Anonymous and logged in
+users cannot see this, but admins like Foo Bar can.
 
     >>> from zope.component import getUtility
     >>> from canonical.launchpad.interfaces.emailaddress import (
-    ... EmailAddressStatus, IEmailAddressSet)
+    ...     EmailAddressStatus, IEmailAddressSet)
 
     >>> login('admin@xxxxxxxxxxxxx')
     >>> address = getUtility(IEmailAddressSet).getByEmail('jvprat@xxxxxxxxxx')
@@ -279,3 +283,5 @@
     ...     find_tag_by_id(admin_browser.contents, 'email-addresses'))
     jvprat@xxxxxxxxxx
     Change e-mail settings
+
+

=== modified file 'lib/lp/registry/templates/person-portlet-map.pt'
--- lib/lp/registry/templates/person-portlet-map.pt	2009-11-16 21:39:26 +0000
+++ lib/lp/registry/templates/person-portlet-map.pt	2010-08-27 23:01:14 +0000
@@ -14,8 +14,11 @@
     <div tal:condition="context/time_zone">
       <strong>Time zone:</strong>
       <span tal:replace="context/time_zone">UTC</span>
+      <a tal:replace="structure overview_menu/editlocation/fmt:icon" />
     </div>
 
+
+    <tal:gmap2 condition="view/gmap2_enabled">
     <div style="width: 400px;" tal:condition="context/latitude">
       <div id="person_map_actions"
         style="position:relative; z-index: 9999;
@@ -28,6 +31,7 @@
         <a tal:replace="structure overview_menu/editlocation/fmt:link-icon" />
       </div>
     </div>
+    </tal:gmap2>
 
     <tal:comment condition="nothing">
       Only the user can see the editlocation image and link.

=== modified file 'lib/lp/registry/templates/team-portlet-map.pt'
--- lib/lp/registry/templates/team-portlet-map.pt	2009-08-25 02:01:55 +0000
+++ lib/lp/registry/templates/team-portlet-map.pt	2010-08-27 23:01:14 +0000
@@ -5,7 +5,8 @@
   omit-tag="">
 
 <div class="portlet" id="portlet-map" style="margin-bottom: 0px;"
-     tal:define="link context/menu:overview/map">
+     tal:define="link context/menu:overview/map"
+     tal:condition="view/gmap2_enabled">
 <table>
 <tr><td>
   <h2>
  Thread Previous • Date Previous • • Thread Next