launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/faceted-breadcrumbs into lp:launchpad.

Commit message:
Replace vhost breadcrumbs with facet breadcrumbs, in preparation for vhost abolition.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/faceted-breadcrumbs/+merge/206916

Each page currently has breadcrumbs generated from the object hierarchy, with an extra domain breadcrumb injected after the primary structural object (eg. project, distribution, distribution source package, or person). For example, at the top of this page we have "Launchpad itself » Code » faceted-breadcrumbs » Merge into devel", with "Code" being the injected breadcrumb linking to https://code.launchpad.net/launchpad.

This breadcrumb is actually generated from the vhost rather than the view's facet, yielding weirdness like https://code.launchpad.net/launchpad/+bug/1234 -- despite the fact that IBug only has views on the bug facet, we get a Code breadcrumb injected because I happened to view it from code.launchpad.net.

This branch alters the breadcrumb generation to use each view's declared facet instead of the current vhost, a prerequisite for flattening the webapp vhosts into launchpad.net. There's no functional change today besides fixing the weirdness I described above.
-- 
https://code.launchpad.net/~wgrant/launchpad/faceted-breadcrumbs/+merge/206916
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/faceted-breadcrumbs into lp:launchpad.

=== modified file 'lib/lp/app/browser/launchpad.py'
--- lib/lp/app/browser/launchpad.py	2013-09-11 06:05:44 +0000
+++ lib/lp/app/browser/launchpad.py	2014-02-18 12:29:07 +0000
@@ -30,7 +30,6 @@
 import time
 import urllib
 
-from lazr.uri import URI
 from zope import i18n
 from zope.component import (
     getGlobalSiteManager,
@@ -138,9 +137,9 @@
     ILaunchpadRoot,
     INavigationMenu,
     )
+from lp.services.webapp.menu import get_facet
 from lp.services.webapp.publisher import RedirectionView
 from lp.services.webapp.url import urlappend
-from lp.services.webapp.vhosts import allvhosts
 from lp.services.worlddata.interfaces.country import ICountrySet
 from lp.services.worlddata.interfaces.language import ILanguageSet
 from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet
@@ -261,21 +260,18 @@
             if breadcrumb is not None:
                 breadcrumbs.append(breadcrumb)
 
-        host = URI(self.request.getURL()).host
-        mainhost = allvhosts.configs['mainsite'].hostname
-        if (len(breadcrumbs) != 0 and
-            host != mainhost and
+        facet = get_facet(self._naked_context_view)
+        if (len(breadcrumbs) != 0 and facet is not None and
             self.vhost_breadcrumb):
-            # We have breadcrumbs and we're not on the mainsite, so we'll
-            # sneak an extra breadcrumb for the vhost we're on.
-            vhost = host.split('.')[0]
+            # We have breadcrumbs and we're on a custom facet, so we'll
+            # sneak an extra breadcrumb for the facet we're on.
 
             # Iterate over the context of our breadcrumbs in reverse order and
-            # for the first one we find an adapter named after the vhost we're
+            # for the first one we find an adapter named after the facet we're
             # on, generate an extra breadcrumb and insert it in our list.
             for idx, breadcrumb in reversed(list(enumerate(breadcrumbs))):
                 extra_breadcrumb = queryAdapter(
-                    breadcrumb.context, IBreadcrumb, name=vhost)
+                    breadcrumb.context, IBreadcrumb, name=facet)
                 if extra_breadcrumb is not None:
                     breadcrumbs.insert(idx + 1, extra_breadcrumb)
                     break

=== modified file 'lib/lp/blueprints/browser/configure.zcml'
--- lib/lp/blueprints/browser/configure.zcml	2012-12-01 15:51:37 +0000
+++ lib/lp/blueprints/browser/configure.zcml	2014-02-18 12:29:07 +0000
@@ -10,13 +10,13 @@
     i18n_domain="launchpad">
 
   <adapter
-      name="blueprints"
+      name="specifications"
       provides="lp.services.webapp.interfaces.IBreadcrumb"
       for="lp.blueprints.interfaces.specificationtarget.IHasSpecifications"
       factory="lp.blueprints.browser.specificationtarget.BlueprintsVHostBreadcrumb"
       permission="zope.Public"/>
   <adapter
-      name="blueprints"
+      name="specifications"
       provides="lp.services.webapp.interfaces.IBreadcrumb"
       for="lp.registry.interfaces.person.IPerson"
       factory="lp.blueprints.browser.specificationtarget.BlueprintsVHostBreadcrumb"

=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml	2012-10-08 01:59:53 +0000
+++ lib/lp/code/browser/configure.zcml	2014-02-18 12:29:07 +0000
@@ -1013,7 +1013,7 @@
         template="../templates/active-reviews.pt"/>
 
     <adapter
-        name="code"
+        name="branches"
         provides="lp.services.webapp.interfaces.IBreadcrumb"
         for="lp.code.interfaces.hasbranches.IHasBranches"
         factory="lp.code.browser.branchlisting.CodeVHostBreadcrumb"

=== modified file 'lib/lp/services/webapp/tests/test_breadcrumbs.py'
--- lib/lp/services/webapp/tests/test_breadcrumbs.py	2012-08-08 07:22:51 +0000
+++ lib/lp/services/webapp/tests/test_breadcrumbs.py	2014-02-18 12:29:07 +0000
@@ -118,18 +118,16 @@
         self.assertEquals(breadcrumb.text, 'breadcrumb test')
 
 
-class TestExtraVHostBreadcrumbsOnHierarchyView(BaseBreadcrumbTestCase):
-    """How our breadcrumbs behave when using a vhost other than the main one?
+class TestExtraFacetBreadcrumbsOnHierarchyView(BaseBreadcrumbTestCase):
+    """How our breadcrumbs behave when using a facet other than the main one?
 
-    When we go to bugs.lp.net/ubuntu, we only traversed the Ubuntu distro, so
+    When we go to lp.net/ubuntu/+bugs, we only traversed the Ubuntu distro, so
     that's what we'd have a breadcrumb for, but we also want to generate a
-    breadcrumb for bugs on Ubuntu, given that we're on the bugs vhost.
-
-    The behaviour is similar to other vhosts; read on for more.
+    breadcrumb for bugs on Ubuntu, given that we're in the bugs facet.
     """
 
     def setUp(self):
-        super(TestExtraVHostBreadcrumbsOnHierarchyView, self).setUp()
+        super(TestExtraFacetBreadcrumbsOnHierarchyView, self).setUp()
         login('test@xxxxxxxxxxxxx')
         self.product = self.factory.makeProduct(name='crumb-tester')
         self.product_url = canonical_url(self.product)