launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/mainsite-links into lp:launchpad with lp:~wgrant/launchpad/kill-facet-link-titles as a prerequisite.

Commit message:
Add an app.mainsite_only.canonical_url feature flag to force all facet vhost links to mainsite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/mainsite-links/+merge/208276

Add an app.mainsite_only.canonical_url feature flag to force all facet vhost links to mainsite.

This lets us flatten the vhosts for some users to see how it works out.
-- 
https://code.launchpad.net/~wgrant/launchpad/mainsite-links/+merge/208276
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/mainsite-links into lp:launchpad.

=== modified file 'lib/lp/app/browser/launchpad.py'
--- lib/lp/app/browser/launchpad.py	2014-02-26 05:37:25 +0000
+++ lib/lp/app/browser/launchpad.py	2014-02-26 05:37:25 +0000
@@ -111,6 +111,7 @@
 from lp.registry.interfaces.role import IPersonRoles
 from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
 from lp.services.config import config
+from lp.services.features import getFeatureFlag
 from lp.services.helpers import intOrZero
 from lp.services.identity.interfaces.account import AccountStatus
 from lp.services.propertycache import cachedproperty
@@ -469,32 +470,37 @@
     def overview(self):
         target = ''
         text = 'Launchpad Home'
-        return Link(target, text)
+        return Link(target, text, site='mainsite')
 
     def translations(self):
-        target = ''
         text = 'Translations'
-        return Link(target, text)
+        target = 'translations' if self.mainsite_only else ''
+        site = 'mainsite' if self.mainsite_only else 'translations'
+        return Link(target, text, site=site)
 
     def bugs(self):
-        target = ''
         text = 'Bugs'
-        return Link(target, text)
+        target = 'bugs' if self.mainsite_only else ''
+        site = 'mainsite' if self.mainsite_only else 'bugs'
+        return Link(target, text, site=site)
 
     def answers(self):
-        target = ''
         text = 'Answers'
-        return Link(target, text)
+        target = 'questions' if self.mainsite_only else ''
+        site = 'mainsite' if self.mainsite_only else 'answers'
+        return Link(target, text, site=site)
 
     def specifications(self):
-        target = ''
         text = 'Blueprints'
-        return Link(target, text)
+        target = 'specs' if self.mainsite_only else ''
+        site = 'mainsite' if self.mainsite_only else 'blueprints'
+        return Link(target, text, site=site)
 
     def branches(self):
-        target = ''
         text = 'Code'
-        return Link(target, text)
+        target = '+code' if self.mainsite_only else ''
+        site = 'mainsite' if self.mainsite_only else 'code'
+        return Link(target, text, site=site)
 
 
 class LoginStatus:

=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py	2014-02-26 05:37:25 +0000
+++ lib/lp/registry/browser/product.py	2014-02-26 05:37:25 +0000
@@ -152,7 +152,6 @@
     PillarNavigationMixin,
     PillarViewMixin,
     )
-from lp.registry.browser.productseries import get_series_branch_error
 from lp.registry.interfaces.pillar import IPillarNameSet
 from lp.registry.interfaces.product import (
     IProduct,
@@ -1569,6 +1568,7 @@
 
     def validate(self, data):
         """See `LaunchpadFormView`."""
+        from lp.registry.browser.productseries import get_series_branch_error
         branch = data.get('branch')
         if branch is not None:
             message = get_series_branch_error(self.context, branch)

=== modified file 'lib/lp/registry/browser/productseries.py'
--- lib/lp/registry/browser/productseries.py	2014-02-26 05:37:25 +0000
+++ lib/lp/registry/browser/productseries.py	2014-02-26 05:37:25 +0000
@@ -210,10 +210,9 @@
         """Return a link to view the branches related to this series."""
         # XXX wgrant 2014-02-26 bug=183433: Override to go to the
         # branches for the product. This is inconsistent and weird. Ew.
-        text = 'Code'
-        summary = 'View related branches of code'
-        link = canonical_url(self.context.product, rootsite='code')
-        return Link(link, text, summary=summary)
+        link = super(ProductSeriesFacets, self).branches()
+        link.target = canonical_url(self.context.product, rootsite='code')
+        return link
 
 
 class IProductSeriesInvolved(Interface):

=== modified file 'lib/lp/services/webapp/__init__.py'
--- lib/lp/services/webapp/__init__.py	2014-02-26 05:37:25 +0000
+++ lib/lp/services/webapp/__init__.py	2014-02-26 05:37:25 +0000
@@ -37,6 +37,7 @@
     'Utf8PreferredCharsets',
     ]
 
+from lp.services.features import getFeatureFlag
 from lp.services.webapp.escaping import structured
 from lp.services.webapp.menu import (
     ApplicationMenu,
@@ -95,42 +96,40 @@
 
     defaultlink = 'overview'
 
-    def _filterLink(self, name, link):
-        if link.site is None:
-            if name == 'specifications':
-                link.site = 'blueprints'
-            elif name == 'branches':
-                link.site = 'code'
-            elif name == 'translations':
-                link.site = 'translations'
-            elif name == 'answers':
-                link.site = 'answers'
-            elif name == 'bugs':
-                link.site = 'bugs'
-            else:
-                link.site = 'mainsite'
-        return link
+    @property
+    def mainsite_only(self):
+        return getFeatureFlag('app.mainsite_only.canonical_url')
 
     def overview(self):
         text = 'Overview'
-        return Link('', text)
+        return Link('', text, site='mainsite')
 
     def branches(self):
         text = 'Code'
-        return Link('', text)
+        target = '+branches' if self.mainsite_only else ''
+        site = 'mainsite' if self.mainsite_only else 'code'
+        return Link(target, text, site=site)
 
     def bugs(self):
         text = 'Bugs'
-        return Link('', text)
+        target = '+bugs' if self.mainsite_only else ''
+        site = 'mainsite' if self.mainsite_only else 'bugs'
+        return Link(target, text, site=site)
 
     def specifications(self):
         text = 'Blueprints'
-        return Link('', text)
+        target = '+specs' if self.mainsite_only else ''
+        site = 'mainsite' if self.mainsite_only else 'blueprints'
+        return Link(target, text, site=site)
 
     def translations(self):
         text = 'Translations'
-        return Link('', text)
+        target = '+translations' if self.mainsite_only else ''
+        site = 'mainsite' if self.mainsite_only else 'translations'
+        return Link(target, text, site=site)
 
     def answers(self):
         text = 'Answers'
-        return Link('', text)
+        target = '+questions' if self.mainsite_only else ''
+        site = 'mainsite' if self.mainsite_only else 'answers'
+        return Link(target, text, site=site)

=== modified file 'lib/lp/services/webapp/doc/canonical_url.txt'
--- lib/lp/services/webapp/doc/canonical_url.txt	2013-04-10 04:12:57 +0000
+++ lib/lp/services/webapp/doc/canonical_url.txt	2014-02-26 05:37:25 +0000
@@ -410,6 +410,17 @@
     >>> canonical_url(country_instance, rootsite='code')
     u'http://code.launchpad.dev/countries/England'
 
+Webapp vhost overrides can be ignored by setting the
+app.mainsite_only.canonical_url feature flag, so all links end up on
+mainsite. Non-webapp vhosts (eg. api and feeds) are unaffected.
+
+    >>> from lp.services.features.testing import MemoryFeatureFixture
+    >>> with MemoryFeatureFixture({'app.mainsite_only.canonical_url': 'on'}):
+    ...     canonical_url(country_instance, rootsite='code')
+    ...     canonical_url(country_instance, rootsite='api')
+    u'http://launchpad.dev/countries/England'
+    u'http://api.launchpad.dev/countries/England'
+
 Overriding the rootsite from the specified request:
 
     >>> canonical_url(country_instance, mandrill_request)
@@ -444,6 +455,9 @@
     u'http://code.launchpad.dev/countries/England'
     >>> canonical_url(country_instance, mandrill_request, rootsite='code')
     u'http://code.launchpad.dev/countries/England'
+    >>> with MemoryFeatureFixture({'app.mainsite_only.canonical_url': 'on'}):
+    ...     canonical_url(country_instance)
+    u'http://launchpad.dev/countries/England'
 
 
 == canonical_url and named views ==

=== modified file 'lib/lp/services/webapp/menu.py'
--- lib/lp/services/webapp/menu.py	2014-02-26 05:37:25 +0000
+++ lib/lp/services/webapp/menu.py	2014-02-26 05:37:25 +0000
@@ -347,13 +347,8 @@
     # See IFacetMenu.
     defaultlink = None
 
-    def _filterLink(self, name, link):
-        """Hook to allow subclasses to alter links based on the name used."""
-        return link
-
     def _get_link(self, name):
-        return IFacetLink(
-            self._filterLink(name, MenuBase._get_link(self, name)))
+        return IFacetLink(MenuBase._get_link(self, name))
 
     def initLink(self, linkname, request_url=None):
         link = super(FacetMenu, self).initLink(linkname, request_url)

=== modified file 'lib/lp/services/webapp/publisher.py'
--- lib/lp/services/webapp/publisher.py	2014-01-30 15:04:06 +0000
+++ lib/lp/services/webapp/publisher.py	2014-02-26 05:37:25 +0000
@@ -706,6 +706,14 @@
             raise NoCanonicalUrl(obj, obj)
         rootsite = obj_urldata.rootsite
 
+    # The app.mainsite_only.canonical_url feature flag forces facet
+    # vhost links to mainsite.
+    facet_rootsites = (
+        'answers', 'blueprints', 'code', 'bugs', 'translations')
+    if (getFeatureFlag('app.mainsite_only.canonical_url')
+            and rootsite in facet_rootsites):
+        rootsite = 'mainsite'
+
     # The request is needed when there's no rootsite specified.
     if request is None:
         # Look for a request from the interaction.