Merge lp:~salgado/launchpad/real-breadcrumbs-2 into lp:launchpad

[Guilherme Salgado]

[Guilherme Salgado (salgado)]

= Summary =

In 3.0 we'll have real breadcrumbs, and this branch will make it
possible to construct them.

== Proposed fix ==

Basically, what this branch does is have the URL of all breadcrumbs
default to the object's canonical_url on the mainsite, as well as
sneaking an extra breadcrumb item for the object on the vhost we are on
(in case we are on a vhost other than the mainsite).

E.g. the breadcrumbs for bugs.lp.net/ubuntu/+bug/1 will be
  Ubuntu > Bugs on Ubuntu > Bug #1

This is accomplished by looking up IBreadcrumbBuilder adapters with
name=vhost for the traversed objects, starting from the end. When one is
found, we use it to generate the extra breadcrumb and insert the
breadcrumb right after the current one.

For some types of objects we will not want the URL on their breadcrumbs
to point to the mainsite, so they can just specify the rootsite they
want in their custom IBreadcrumbBuilder adapter.

== Pre-implementation notes ==

== Implementation details ==

In lib/canonical/launchpad/webapp/publisher.py I reorganized some code
slightly to make it more readable.

The XXX I left in lib/canonical/launchpad/webapp/breadcrumb.py I'm
planning to fix as soon as I get this through review. I didn't do it
here as it would pollute the diff.

== Tests ==

./bin/test -vvt test_breadcrumbs -t hierarchical-menu.txt

== Demo and Q/A ==

https://bugs.launchpad.dev/firefox/+bug/1

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/code/browser/branch.py
  lib/canonical/launchpad/webapp/publisher.py
  lib/canonical/launchpad/webapp/tests/test_breadcrumbs.py
  lib/lp/registry/configure.zcml
  lib/canonical/launchpad/browser/launchpad.py
  lib/canonical/launchpad/webapp/breadcrumb.py
  lib/lp/registry/browser/pillar.py
  lib/canonical/lazr/testing/menus.py
  lib/canonical/launchpad/doc/hierarchical-menu.txt

== Pylint notices ==

lib/lp/code/browser/branch.py
    48: [F0401] Unable to import 'lazr.uri' (No module named uri)

lib/canonical/launchpad/browser/launchpad.py
    103: [F0401] Unable to import 'lazr.uri' (No module named uri)

--
Guilherme Salgado <email address hidden>

[Brad Crittenden (bac)]

Hi Salgado,

This is a very cool branch and I enjoyed looking at it. I've made a
few comments below. The things to fix are straightforward so I'm
marking it Approved but it's really merge-conditional.

I look forward to having these bread crumbs fully implemented!

> === modified file 'lib/canonical/launchpad/browser/launchpad.py'
> --- lib/canonical/launchpad/browser/launchpad.py 2009-08-11 04:35:26 +0000
+++ lib/canonical/launchpad/browser/launchpad.py 2009-08-18 14:36:16 +0000
> @@ -33,7 +33,6 @@
> import time
> import urllib
> from datetime import timedelta, datetime
> -from urlparse import urlunsplit
>
> from zope.datetime import parseDatetimetz, tzinfo, DateTimeError
> from zope.component import getUtility, queryAdapter
> @@ -102,7 +101,7 @@
> from canonical.launchpad.webapp.publisher import RedirectionView
> from canonical.launchpad.webapp.authorization import check_permission
> from lazr.uri import URI
> -from canonical.launchpad.webapp.url import urlparse, urlappend
> +from canonical.launchpad.webapp.url import urlappend
> from canonical.launchpad.webapp.vhosts import allvhosts
> from canonical.widgets.project import ProjectScopeWidget
>
> @@ -216,58 +215,50 @@
> class Hierarchy(LaunchpadView):
> """The hierarchy part of the location bar on each page."""
>
> + @property
> + def objects(self):
> + """The objects for which we want breadcrumbs."""
> + return self.request.traversed_objects
> +
> def items(self):
> """Return a list of `IBreadcrumb` objects visible in the hierarchy.
>
> The list starts with the breadcrumb closest to the hierarchy root.
> """
> - urlparts = urlparse(self.request.getURL(0, path_only=False))
> - baseurl = urlunsplit((urlparts[0], urlparts[1], '', '', ''))
> -
> - # Construct a list of complete URLs for each URL path segment.
> - pathurls = []
> - working_url = baseurl
> - for segment in urlparts[2].split('/'):
> - working_url = urlappend(working_url, segment)
> - # Segments starting with '+' should be ignored because they
> - # will never correspond to an object in navigation.
> - if segment.startswith('+'):
> - continue
> - pathurls.append(working_url)
> -
> - # We assume a 1:1 relationship between the traversed_objects list and
> - # the URL path segments. Note that there may be more segments than
> - # there are objects.
> - object_urls = zip(self.request.traversed_objects, pathurls)
> - return self._breadcrumbs(object_urls)
> -
> - def _breadcrumbs(self, object_urls):
> - """Generate the breadcrumb list.
> -
> - :param object_urls: A sequence of (object, url) pairs.
> - :return: A list of 'IBreadcrumb' objects.
> - """
> breadcrumbs = []
> - for obj, url in object_urls:
> - crumb = self.breadcrumb_for(obj, url)
> + for builder in self._getBreadcrumbBuilders():
> + crumb = builder.make_breadcrumb()
> if crumb is not None:
> breadcrumbs.append(crumb)
> return breadcrumbs...

review: Approve

[Guilherme Salgado (salgado)]

On Tue, 2009-08-18 at 20:19 +0000, Brad Crittenden wrote:
> Review: Approve
> Hi Salgado,
>
> This is a very cool branch and I enjoyed looking at it. I've made a
> few comments below. The things to fix are straightforward so I'm
> marking it Approved but it's really merge-conditional.

Thanks, Brad.

>
> I look forward to having these bread crumbs fully implemented!

So do I. I think they'll be a significant improvement to LP.

> > === modified file 'lib/canonical/launchpad/webapp/publisher.py'
> > --- lib/canonical/launchpad/webapp/publisher.py 2009-07-30 08:07:21 +0000
> > +++ lib/canonical/launchpad/webapp/publisher.py 2009-08-12 20:22:21 +0000
> > @@ -504,10 +504,10 @@
> > 'step for "%s".' % (view_name, obj.__class__.__name__))
> > urlparts.insert(0, view_name)
> >
> > - if request is None and rootsite is not None:
> > + if request is None:
> > + if rootsite is None:
>
> I see rootsite is set to ICanonicalUrlDate(obj).rootsite if it is
> passed as None and it also may be None, which is the only way you'd
> get here. That's all correct?

Yep; I just re-arranged the code here to make it more readable.

>
> > + rootsite = 'mainsite'
> > root_url = allvhosts.configs[rootsite].rooturl
> > - elif request is None:
> > - root_url = allvhosts.configs['mainsite'].rooturl
> > else:
> > root_url = request.getRootURL(rootsite)
> >
>

> > === added file 'lib/canonical/launchpad/webapp/tests/test_breadcrumbs.py'
> > --- lib/canonical/launchpad/webapp/tests/test_breadcrumbs.py 1970-01-01 00:00:00 +0000
> > +++ lib/canonical/launchpad/webapp/tests/test_breadcrumbs.py 2009-08-18 14:36:16 +0000
> > @@ -0,0 +1,81 @@
> > +# Copyright 2009 Canonical Ltd. This software is licensed under the
> > +# GNU Affero General Public License version 3 (see the file LICENSE).
> > +
> > +__metaclass__ = type
> > +
> > +import unittest
> > +
> > +from canonical.launchpad.webapp.publisher import canonical_url
> > +from canonical.launchpad.webapp.tests import BaseBreadcrumbTestCase
> > +from lp.testing import login
> > +
> > +
> > +class TestExtraVHostBreadcrumbsOnHierarchyView(BaseBreadcrumbTestCase):
> > + """How our breadcrumbs behave when using a vhost other the main one?
>
> typo: other than the

Fixed

>
> > + When we go to bugs.lp.net/ubuntu, 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.
> > + """
> > +
> > + def setUp(self):
> > + super(TestExtraVHostBreadcrumbsOnHierarchyView, self).setUp()
> > + login('<email address hidden>')
> > + self.product = self.factory.makeProduct(name='crumb-tester')
> > + self.product_bug = self.factory.makeBug(product=self.product)
> > + self.product_bug_url = canonical_url(self.product_bug)
> > + self.source_package = self.factory.makeSourcePackage()
> > + self.package_bug = self.factory.makeBugTask(
> > + target=self.source_package)
> > + s...

[Guilherme Salgado (salgado)]

Also, after reading Gavin's tests for the new bugtask breadcrumbs again, I realized I was not testing some things properly; I was testing using bug URLs when in fact I want to test with bugtask URLs as that's what happens on the web UI. The diff below changes that

=== modified file 'lib/canonical/launchpad/webapp/tests/test_breadcrumbs.py'
--- lib/canonical/launchpad/webapp/tests/test_breadcrumbs.py 2009-08-18 14:36:16 +0000
+++ lib/canonical/launchpad/webapp/tests/test_breadcrumbs.py 2009-08-19 12:35:17 +0000
@@ -11,7 +11,7 @@

 class TestExtraVHostBreadcrumbsOnHierarchyView(BaseBreadcrumbTestCase):
- """How our breadcrumbs behave when using a vhost other the main one?
+ """How our breadcrumbs behave when using a vhost other than the main one?

     When we go to bugs.lp.net/ubuntu, we only traversed the Ubuntu distro, so
     that's what we'd have a breadcrumb for, but we also want to generate a
@@ -24,12 +24,15 @@
         super(TestExtraVHostBreadcrumbsOnHierarchyView, self).setUp()
         login('<email address hidden>')
         self.product = self.factory.makeProduct(name='crumb-tester')
- self.product_bug = self.factory.makeBug(product=self.product)
- self.product_bug_url = canonical_url(self.product_bug)
+ self.product_url = canonical_url(self.product)
+ self.product_bugs_url = canonical_url(self.product, rootsite='bugs')
+ product_bug = self.factory.makeBug(product=self.product)
+ self.product_bugtask = product_bug.default_bugtask
+ self.product_bugtask_url = canonical_url(self.product_bugtask)
         self.source_package = self.factory.makeSourcePackage()
- self.package_bug = self.factory.makeBugTask(
+ self.package_bugtask = self.factory.makeBugTask(
             target=self.source_package)
- self.package_bug_url = canonical_url(self.package_bug)
+ self.package_bugtask_url = canonical_url(self.package_bugtask)

     def test_root_on_mainsite(self):
         urls = self._getBreadcrumbsURLs('http://launchpad.dev/', [self.root])
@@ -37,9 +40,8 @@

     def test_product_on_mainsite(self):
         urls = self._getBreadcrumbsURLs(
- 'http://launchpad.dev/%s' % self.product.name,
- [self.root, self.product])
- self.assertEquals(urls, ['http://launchpad.dev/crumb-tester'])
+ self.product_url, [self.root, self.product])
+ self.assertEquals(urls, [self.product_url])

     def test_root_on_vhost(self):
         urls = self._getBreadcrumbsURLs(
@@ -48,33 +50,31 @@

     def test_product_on_vhost(self):
         urls = self._getBreadcrumbsURLs(
- 'http://bugs.launchpad.dev/%s' % self.product.name,
- [self.root, self.product])
- self.assertEquals(
- urls, ['http://launchpad.dev/crumb-tester',
- 'http://bugs.launchpad.dev/crumb-tester'])
+ self.product_bugs_url, [self.root, self.product])
+ self.assertEquals(urls, [self.product_url, self.product_bugs_url])

- def test_product_bug(self):
+ def test_product_bugtask(self):
         urls = self._getBreadcrumbsURLs(
- self.product_bug_url, [self.root, self.product, self.pr...

=== modified file 'lib/canonical/launchpad/browser/launchpad.py'
--- lib/canonical/launchpad/browser/launchpad.py	2009-08-11 04:35:26 +0000
+++ lib/canonical/launchpad/browser/launchpad.py	2009-08-18 14:36:16 +0000
@@ -33,7 +33,6 @@
 import time
 import urllib
 from datetime import timedelta, datetime
-from urlparse import urlunsplit
 
 from zope.datetime import parseDatetimetz, tzinfo, DateTimeError
 from zope.component import getUtility, queryAdapter
@@ -102,7 +101,7 @@
 from canonical.launchpad.webapp.publisher import RedirectionView
 from canonical.launchpad.webapp.authorization import check_permission
 from lazr.uri import URI
-from canonical.launchpad.webapp.url import urlparse, urlappend
+from canonical.launchpad.webapp.url import urlappend
 from canonical.launchpad.webapp.vhosts import allvhosts
 from canonical.widgets.project import ProjectScopeWidget
 
@@ -216,58 +215,50 @@
 class Hierarchy(LaunchpadView):
     """The hierarchy part of the location bar on each page."""
 
+    @property
+    def objects(self):
+        """The objects for which we want breadcrumbs."""
+        return self.request.traversed_objects
+
     def items(self):
         """Return a list of `IBreadcrumb` objects visible in the hierarchy.
 
         The list starts with the breadcrumb closest to the hierarchy root.
         """
-        urlparts = urlparse(self.request.getURL(0, path_only=False))
-        baseurl = urlunsplit((urlparts[0], urlparts[1], '', '', ''))
-
-        # Construct a list of complete URLs for each URL path segment.
-        pathurls = []
-        working_url = baseurl
-        for segment in urlparts[2].split('/'):
-            working_url = urlappend(working_url, segment)
-            # Segments starting with '+' should be ignored because they
-            # will never correspond to an object in navigation.
-            if segment.startswith('+'):
-                continue
-            pathurls.append(working_url)
-
-        # We assume a 1:1 relationship between the traversed_objects list and
-        # the URL path segments.  Note that there may be more segments than
-        # there are objects.
-        object_urls = zip(self.request.traversed_objects, pathurls)
-        return self._breadcrumbs(object_urls)
-
-    def _breadcrumbs(self, object_urls):
-        """Generate the breadcrumb list.
-
-        :param object_urls: A sequence of (object, url) pairs.
-        :return: A list of 'IBreadcrumb' objects.
-        """
         breadcrumbs = []
-        for obj, url in object_urls:
-            crumb = self.breadcrumb_for(obj, url)
+        for builder in self._getBreadcrumbBuilders():
+            crumb = builder.make_breadcrumb()
             if crumb is not None:
                 breadcrumbs.append(crumb)
         return breadcrumbs
 
-    def breadcrumb_for(self, obj, url):
-        """Return the breadcrumb for the an object, using the supplied URL.
-
-        :return: An `IBreadcrumb` object, or None if a breadcrumb adaptation
-            for the object doesn't exist.
-        """
-        # If the object has an IBreadcrumbBuilder adaptation then the
-        # object is intended to be shown in the hierarchy.
-        builder = queryAdapter(obj, IBreadcrumbBuilder)
-        if builder is not None:
-            # The breadcrumb builder hasn't been given a URL yet.
-            builder.url = url
-            return builder.make_breadcrumb()
-        return None
+    def _getBreadcrumbBuilders(self):
+        builders = []
+        for obj in self.objects:
+            builder = queryAdapter(obj, IBreadcrumbBuilder)
+            if builder is not None:
+                builders.append(builder)
+
+        host = URI(self.request.getURL()).host
+        if (len(builders) == 0
+            or host == allvhosts.configs['mainsite'].hostname):
+            return builders
+
+        # If we got this far it means 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]
+
+        # Iterate over the context of our builders in reverse order and for
+        # the first one we find an adapter named after the vhost we're on,
+        # generate an extra breadcrumb and insert it in our list.
+        for idx in reversed(xrange(len(builders))):
+            builder = builders[idx]
+            extra_builder = queryAdapter(
+                builder.context, IBreadcrumbBuilder, name=vhost)
+            if extra_builder is not None:
+                builders.insert(idx + 1, extra_builder)
+                break
+        return builders
 
     def render(self):
         """Render the hierarchy HTML.

=== modified file 'lib/canonical/launchpad/doc/hierarchical-menu.txt'
--- lib/canonical/launchpad/doc/hierarchical-menu.txt	2009-03-24 12:43:49 +0000
+++ lib/canonical/launchpad/doc/hierarchical-menu.txt	2009-08-17 19:22:06 +0000
@@ -129,11 +129,9 @@
 IBreadcrumbBuilder factory.  The factory adapts a context object and
 produces an IBreadcrumb object for that context.
 
-The builder holds a temporary copy of a breadcrumb until the breadcrumb
-is in a usable state.  This allows us to adapt the breadcrumb context
-object in one place, and to defer setting the breadcrumb's URL.  We can
-ask the builder for a finished breadcrumb after both the text and the
-URL have been specified.
+The builder holds a temporary copy of a breadcrumb until the breadcrumb is in
+a usable state.  We can ask the builder for a finished breadcrumb after both
+the text and the URL have been specified.
 
     >>> from canonical.launchpad.webapp.interfaces import IBreadcrumb
     >>> from zope.interface.verify import verifyObject
@@ -143,7 +141,6 @@
     True
 
     >>> builder.text = 'Joy of cooking'
-    >>> builder.url = 'http://launchpad.dev/joy-of-cooking'
 
     >>> breadcrumb = builder.make_breadcrumb()
     >>> verifyObject(IBreadcrumb, breadcrumb)
@@ -153,18 +150,6 @@
         url='http://launchpad.dev/joy-of-cooking'
         text='Joy of cooking'>
 
-The builder enforces the requirement for breadcrumbs to have a valid
-URL. The builder raises an error if a URL is not explicitly given by the
-developer.
-
-    >>> builder = BreadcrumbBuilder(cookbook)
-    >>> builder.text = 'Oops!'
-
-    >>> builder.make_breadcrumb()
-    Traceback (most recent call last):
-    ...
-    AssertionError: The builder has not been given a valid breadcrumb URL.
-
 The breadcrumb's attributes can be overridden with subclassing and
 Python properties.
 
@@ -238,16 +223,14 @@
     >>> from canonical.launchpad.browser.launchpad import Hierarchy
 
     >>> class CustomHierarchy(Hierarchy):
-    ...     def items(self):
-    ...         breadcrumb = self.breadcrumb_for(
-    ...             recipe, "http://launchpad.dev/some-spammy-url")
-    ...         # Omitted the check for 'breadcrumb is None'.
-    ...         return [breadcrumb]
+    ...     @property
+    ...     def objects(self):
+    ...         return [recipe]
 
     >>> spammy_hierarchy = CustomHierarchy(root, request)
     >>> spammy_hierarchy.items()
     [<Breadcrumb
-        url='http://launchpad.dev/some-spammy-url'
+        url='http://launchpad.dev/joy-of-cooking/spam'
         text='Spam'
         icon='<img src="/@@/recipe"/>'>]
 
@@ -273,7 +256,7 @@
     >>> print_hierarchy(hierarchy.render())
     Location: Joy of cooking > Spam
 
-The Launchpad Homepage displays no items in it's location bar.  We are
+The Launchpad Homepage displays no items in its location bar.  We are
 considered to be on the home page if there are no breadcrumbs.
 
     # Simulate a visit to the site root

=== modified file 'lib/canonical/launchpad/webapp/breadcrumb.py'
--- lib/canonical/launchpad/webapp/breadcrumb.py	2009-06-25 05:30:52 +0000
+++ lib/canonical/launchpad/webapp/breadcrumb.py	2009-08-17 20:20:17 +0000
@@ -15,6 +15,7 @@
 from zope.component import queryAdapter
 from zope.interface import implements
 
+from canonical.launchpad.webapp import canonical_url
 from canonical.launchpad.webapp.interfaces import (
     IBreadcrumb, IBreadcrumbBuilder)
 
@@ -38,6 +39,9 @@
             self.__class__.__name__, self.url, self.text, icon_repr)
 
 
+# XXX: salgado, 2009-08-17: Since this adapter now provides a default
+# value for the 'url' attribute, we could easily convert it into an
+# adapter for IBreadcrumb, just changing the Hierarchy view.
 class BreadcrumbBuilder:
     """See `IBreadcrumbBuilder`.
 
@@ -45,13 +49,17 @@
     """
     implements(IBreadcrumbBuilder)
 
+    rootsite = 'mainsite'
     text = None
-    url = None
 
     def __init__(self, context):
         self.context = context
 
     @property
+    def url(self):
+        return canonical_url(self.context, rootsite=self.rootsite)
+
+    @property
     def icon(self):
         """See `IBreadcrumb`."""
         # Get the <img> tag from the path adapter.

=== modified file 'lib/canonical/launchpad/webapp/interfaces.py'
--- lib/canonical/launchpad/webapp/interfaces.py	2009-08-10 21:07:02 +0000
+++ lib/canonical/launchpad/webapp/interfaces.py	2009-08-18 14:36:16 +0000
@@ -264,7 +264,10 @@
     # We subclass IBreadcrumb to minimize interface drift.
 
     def make_breadcrumb():
-        """Return an object implementing the `IBreadcrumb` interface."""
+        """Return an object implementing the `IBreadcrumb` interface.
+
+        If for any reason no IBreadcrumb object can be created, return None.
+        """
 
 
 #

=== modified file 'lib/canonical/launchpad/webapp/publisher.py'
--- lib/canonical/launchpad/webapp/publisher.py	2009-07-30 08:07:21 +0000
+++ lib/canonical/launchpad/webapp/publisher.py	2009-08-12 20:22:21 +0000
@@ -504,10 +504,10 @@
                     'step for "%s".' % (view_name, obj.__class__.__name__))
         urlparts.insert(0, view_name)
 
-    if request is None and rootsite is not None:
+    if request is None:
+        if rootsite is None:
+            rootsite = 'mainsite'
         root_url = allvhosts.configs[rootsite].rooturl
-    elif request is None:
-        root_url = allvhosts.configs['mainsite'].rooturl
     else:
         root_url = request.getRootURL(rootsite)
 

=== modified file 'lib/canonical/launchpad/webapp/tests/__init__.py'
--- lib/canonical/launchpad/webapp/tests/__init__.py	2009-06-25 05:39:50 +0000
+++ lib/canonical/launchpad/webapp/tests/__init__.py	2009-08-18 14:36:16 +0000
@@ -10,6 +10,10 @@
 
 from lp.testing import TestCase
 
+from canonical.launchpad.webapp.tests._breadcrumbs import (
+    BaseBreadcrumbTestCase)
+
+
 class DummyWebServiceConfiguration:
     """A totally vanilla web service configuration."""
     implements(IWebServiceConfiguration)

=== added file 'lib/canonical/launchpad/webapp/tests/_breadcrumbs.py'
--- lib/canonical/launchpad/webapp/tests/_breadcrumbs.py	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/webapp/tests/_breadcrumbs.py	2009-08-18 14:36:16 +0000
@@ -0,0 +1,33 @@
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+from zope.component import getMultiAdapter
+
+from canonical.lazr.testing.menus import make_fake_request
+from canonical.launchpad.webapp.publisher import RootObject
+from canonical.testing import DatabaseFunctionalLayer
+from lp.testing import TestCaseWithFactory
+
+
+class BaseBreadcrumbTestCase(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(BaseBreadcrumbTestCase, self).setUp()
+        self.root = RootObject()
+
+    def _getHierarchyView(self, url, traversed_objects):
+        request = make_fake_request(url, traversed_objects)
+        return getMultiAdapter((self.root, request), name='+hierarchy')
+
+    def _getBreadcrumbsTexts(self, url, traversed_objects):
+        view = self._getHierarchyView(url, traversed_objects)
+        return [crumb.text for crumb in view.items()]
+
+    def _getBreadcrumbsURLs(self, url, traversed_objects):
+        view = self._getHierarchyView(url, traversed_objects)
+        return [crumb.url for crumb in view.items()]
+

=== added file 'lib/canonical/launchpad/webapp/tests/test_breadcrumbs.py'
--- lib/canonical/launchpad/webapp/tests/test_breadcrumbs.py	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/webapp/tests/test_breadcrumbs.py	2009-08-18 14:36:16 +0000
@@ -0,0 +1,81 @@
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+import unittest
+
+from canonical.launchpad.webapp.publisher import canonical_url
+from canonical.launchpad.webapp.tests import BaseBreadcrumbTestCase
+from lp.testing import login
+
+
+class TestExtraVHostBreadcrumbsOnHierarchyView(BaseBreadcrumbTestCase):
+    """How our breadcrumbs behave when using a vhost other the main one?
+
+    When we go to bugs.lp.net/ubuntu, 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.
+    """
+
+    def setUp(self):
+        super(TestExtraVHostBreadcrumbsOnHierarchyView, self).setUp()
+        login('[e-mail]')
+        self.product = self.factory.makeProduct(name='crumb-tester')
+        self.product_bug = self.factory.makeBug(product=self.product)
+        self.product_bug_url = canonical_url(self.product_bug)
+        self.source_package = self.factory.makeSourcePackage()
+        self.package_bug = self.factory.makeBugTask(
+            target=self.source_package)
+        self.package_bug_url = canonical_url(self.package_bug)
+
+    def test_root_on_mainsite(self):
+        urls = self._getBreadcrumbsURLs('http://launchpad.dev/', [self.root])
+        self.assertEquals(urls, [])
+
+    def test_product_on_mainsite(self):
+        urls = self._getBreadcrumbsURLs(
+            'http://launchpad.dev/%s' % self.product.name,
+            [self.root, self.product])
+        self.assertEquals(urls, ['http://launchpad.dev/crumb-tester'])
+
+    def test_root_on_vhost(self):
+        urls = self._getBreadcrumbsURLs(
+            'http://bugs.launchpad.dev/', [self.root])
+        self.assertEquals(urls, [])
+
+    def test_product_on_vhost(self):
+        urls = self._getBreadcrumbsURLs(
+            'http://bugs.launchpad.dev/%s' % self.product.name,
+            [self.root, self.product])
+        self.assertEquals(
+            urls, ['http://launchpad.dev/crumb-tester',
+                   'http://bugs.launchpad.dev/crumb-tester'])
+
+    def test_product_bug(self):
+        urls = self._getBreadcrumbsURLs(
+            self.product_bug_url, [self.root, self.product, self.product_bug])
+        self.assertEquals(
+            urls, ['http://launchpad.dev/crumb-tester',
+                   'http://bugs.launchpad.dev/crumb-tester'])
+
+    def test_package_bug(self):
+        target = self.package_bug.target
+        distro_url = canonical_url(target.distribution)
+        distroseries_url = canonical_url(target.distroseries)
+        package_url = canonical_url(target)
+        package_bugs_url = canonical_url(target, rootsite='bugs')
+        urls = self._getBreadcrumbsURLs(
+            self.package_bug_url,
+            [self.root, target.distribution, target.distroseries, target,
+             self.package_bug])
+        self.assertEquals(
+            urls,
+            [distro_url, distroseries_url, package_url, package_bugs_url,
+             self.package_bug_url])
+
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/canonical/lazr/testing/menus.py'
--- lib/canonical/lazr/testing/menus.py	2009-08-13 00:51:50 +0000
+++ lib/canonical/lazr/testing/menus.py	2009-08-18 12:21:51 +0000
@@ -60,7 +60,8 @@
         SERVER_URL=server_url,
         PATH_INFO=path_info)
     request._traversed_names = path_info.split('/')[1:]
-    request.traversed_objects = traversed_objects
+    if traversed_objects is not None:
+        request.traversed_objects = traversed_objects
     # After making the request, setup a new interaction.
     endInteraction()
     newInteraction(request)

=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py	2009-08-17 17:09:23 +0000
+++ lib/lp/bugs/browser/bugtarget.py	2009-08-18 14:36:16 +0000
@@ -56,6 +56,7 @@
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.interfaces.temporaryblobstorage import (
     ITemporaryStorageManager)
+from canonical.launchpad.webapp.breadcrumb import BreadcrumbBuilder
 from canonical.launchpad.webapp.interfaces import ILaunchBag, NotFoundError
 from lp.bugs.interfaces.bug import (
     CreateBugParams, IBugAddForm, IFrontPageBugAddForm, IProjectBugAddForm)
@@ -1344,3 +1345,10 @@
         """The URL the user is sent to when clicking the "cancel" link."""
         return canonical_url(self.context)
 
+
+class BugTargetOnBugsVHostBreadcrumbBuilder(BreadcrumbBuilder):
+    rootsite = 'bugs'
+
+    @property
+    def text(self):
+        return 'Bugs on %s' % self.context.name

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2009-08-14 16:33:17 +0000
+++ lib/lp/bugs/browser/bugtask.py	2009-08-18 14:36:16 +0000
@@ -3497,6 +3497,8 @@
 class BugTaskBreadcrumbBuilder(BreadcrumbBuilder):
     """Builds a breadcrumb for an `IBugTask`."""
 
+    rootsite = 'bugs'
+
     @property
     def text(self):
         return self.context.bug.displayname

=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml	2009-08-13 10:14:48 +0000
+++ lib/lp/bugs/configure.zcml	2009-08-18 14:36:16 +0000
@@ -871,4 +871,10 @@
         <allow
             interface="lp.bugs.interfaces.bugnotification.IBugNotificationSet"/>
     </securedutility>
+    <adapter
+        name="bugs"
+        provides="canonical.launchpad.webapp.interfaces.IBreadcrumbBuilder"
+        for="lp.bugs.interfaces.bugtarget.IBugTarget"
+        factory="lp.bugs.browser.bugtarget.BugTargetOnBugsVHostBreadcrumbBuilder"
+        permission="zope.Public"/>
 </configure>

=== modified file 'lib/lp/bugs/stories/bug-release-management/nomination-navigation.txt'
--- lib/lp/bugs/stories/bug-release-management/nomination-navigation.txt	2009-08-14 14:50:19 +0000
+++ lib/lp/bugs/stories/bug-release-management/nomination-navigation.txt	2009-08-18 18:00:01 +0000
@@ -11,7 +11,8 @@
     ... 'http://bugs.launchpad.dev/ubuntu/+source/mozilla-firefox/+bug/1'
     ... '/nominations/2/+editstatus')
     >>> print_location(admin_browser.contents)
-    Hierarchy: Launchpad > Ubuntu > ?mozilla-firefox? package > Bug #1
+    Hierarchy: Launchpad > Ubuntu > ?mozilla-firefox? package > 
+        Bugs on mozilla-firefox > Bug #1
     Tabs:
     * Overview - http://launchpad.dev/ubuntu/+source/mozilla-firefox
     * Code - http://code.launchpad.dev/ubuntu/+source/mozilla-firefox

=== modified file 'lib/lp/bugs/stories/bugs/xx-bug-activity.txt'
--- lib/lp/bugs/stories/bugs/xx-bug-activity.txt	2009-08-14 09:26:51 +0000
+++ lib/lp/bugs/stories/bugs/xx-bug-activity.txt	2009-08-18 18:00:01 +0000
@@ -12,7 +12,8 @@
 the main heading repeats the bug number for clarity:
 
     >>> print_location(anon_browser.contents)
-    Hierarchy: Launchpad > Debian > ?mozilla-firefox? package > Bug #3
+    Hierarchy: Launchpad > Debian > ?mozilla-firefox? package >
+        Bugs on mozilla-firefox > Bug #3
     Tabs:
     * Overview - http://launchpad.dev/debian/+source/mozilla-firefox
     * Code - http://code.launchpad.dev/debian/+source/mozilla-firefox

=== modified file 'lib/lp/bugs/tests/test_breadcrumbs.py'
--- lib/lp/bugs/tests/test_breadcrumbs.py	2009-08-14 16:33:17 +0000
+++ lib/lp/bugs/tests/test_breadcrumbs.py	2009-08-18 14:36:16 +0000
@@ -5,67 +5,38 @@
 
 import unittest
 
-from zope.component import getMultiAdapter
-
-from canonical.lazr.testing.menus import make_fake_request
-from canonical.launchpad.webapp.publisher import RootObject
-from canonical.testing import DatabaseFunctionalLayer
-from lp.testing import (
-    ANONYMOUS, TestCaseWithFactory, login)
-
-
-class TestBugTaskBreadcrumbBuilder(TestCaseWithFactory):
-
-    layer = DatabaseFunctionalLayer
+from canonical.launchpad.webapp.publisher import canonical_url
+from canonical.launchpad.webapp.tests import BaseBreadcrumbTestCase
+from lp.testing import ANONYMOUS, login
+
+
+class TestBugTaskBreadcrumbBuilder(BaseBreadcrumbTestCase):
 
     def setUp(self):
         super(TestBugTaskBreadcrumbBuilder, self).setUp()
-        self.product = self.factory.makeProduct(
+        product = self.factory.makeProduct(
             name='crumb-tester', displayname="Crumb Tester")
-        self.bug = self.factory.makeBug(product=self.product)
-        self.root = RootObject()
-
-    def test_bugtask_on_mainsite(self):
-        request = make_fake_request(
-            'http://launchpad.dev/%s/+bug/%d' % (
-                self.product.name, self.bug.id),
-            [self.root, self.product, self.bug.default_bugtask])
-        hierarchy = getMultiAdapter((self.root, request), name='+hierarchy')
-        self.assertEquals(
-            ['http://launchpad.dev/crumb-tester',
-             'http://launchpad.dev/crumb-tester/+bug/%d' % self.bug.id],
-            [crumb.url for crumb in hierarchy.items()])
-        self.assertEquals(
-            ["Crumb Tester", "Bug #%d" % self.bug.id],
-            [crumb.text for crumb in hierarchy.items()])
-
-    def test_bugtask_on_vhost(self):
-        request = make_fake_request(
-            'http://bugs.launchpad.dev/%s/+bug/%d' % (
-                self.product.name, self.bug.id),
-            [self.root, self.product, self.bug.default_bugtask])
-        hierarchy = getMultiAdapter((self.root, request), name='+hierarchy')
-        self.assertEquals(
-            ['http://bugs.launchpad.dev/crumb-tester',
-             'http://bugs.launchpad.dev/crumb-tester/+bug/%d' % self.bug.id],
-            [crumb.url for crumb in hierarchy.items()])
-        self.assertEquals(
-            ["Crumb Tester", "Bug #%d" % self.bug.id],
-            [crumb.text for crumb in hierarchy.items()])
-
-    def test_bugtask_child_on_vhost(self):
-        request = make_fake_request(
-            'http://bugs.launchpad.dev/%s/+bug/%d/+activity' % (
-                self.product.name, self.bug.id),
-            [self.root, self.product, self.bug.default_bugtask])
-        hierarchy = getMultiAdapter((self.root, request), name='+hierarchy')
-        self.assertEquals(
-            ['http://bugs.launchpad.dev/crumb-tester',
-             'http://bugs.launchpad.dev/crumb-tester/+bug/%d' % self.bug.id],
-            [crumb.url for crumb in hierarchy.items()])
-        self.assertEquals(
-            ["Crumb Tester", "Bug #%d" % self.bug.id],
-            [crumb.text for crumb in hierarchy.items()])
+        self.bug = self.factory.makeBug(product=product)
+        self.bugtask_url = canonical_url(
+            self.bug.default_bugtask, rootsite='bugs')
+        self.traversed_objects = [
+            self.root, product, self.bug.default_bugtask]
+
+    def test_bugtask(self):
+        urls = self._getBreadcrumbsURLs(
+            self.bugtask_url, self.traversed_objects)
+        self.assertEquals(urls[-1], self.bugtask_url)
+        texts = self._getBreadcrumbsTexts(
+            self.bugtask_url, self.traversed_objects)
+        self.assertEquals(texts[-1], "Bug #%d" % self.bug.id)
+
+    def test_bugtask_child(self):
+        url = canonical_url(
+            self.bug.default_bugtask, rootsite='bugs', view_name='+activity')
+        urls = self._getBreadcrumbsURLs(url, self.traversed_objects)
+        self.assertEquals(urls[-1], self.bugtask_url)
+        texts = self._getBreadcrumbsTexts(url, self.traversed_objects)
+        self.assertEquals(texts[-1], "Bug #%d" % self.bug.id)
 
     def test_bugtask_private_bug(self):
         # A breadcrumb is not generated for a bug that the user does
@@ -73,16 +44,15 @@
         login('[e-mail]')
         self.bug.setPrivate(True, self.bug.owner)
         login(ANONYMOUS)
-        request = make_fake_request(
-            'http://bugs.launchpad.dev/%s/+bug/%d/+activity' % (
-                self.product.name, self.bug.id),
-            [self.root, self.product, self.bug.default_bugtask])
-        hierarchy = getMultiAdapter((self.root, request), name='+hierarchy')
-        self.assertEquals(
-            ['http://bugs.launchpad.dev/crumb-tester'],
-            [crumb.url for crumb in hierarchy.items()])
-        self.assertEquals(
-            ["Crumb Tester"], [crumb.text for crumb in hierarchy.items()])
+        url = canonical_url(
+            self.bug.default_bugtask, rootsite='bugs', view_name='+activity')
+        self.assertEquals(
+            ['http://launchpad.dev/crumb-tester',
+             'http://bugs.launchpad.dev/crumb-tester'],
+            self._getBreadcrumbsURLs(url, self.traversed_objects))
+        self.assertEquals(
+            ["Crumb Tester", "Bugs on crumb-tester"],
+            self._getBreadcrumbsTexts(url, self.traversed_objects))
 
 
 def test_suite():

=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py	2009-08-13 07:29:51 +0000
+++ lib/lp/code/browser/branch.py	2009-08-18 12:21:51 +0000
@@ -115,11 +115,10 @@
 class BranchHierarchy(Hierarchy):
     """The hierarchy for a branch should be the product if there is one."""
 
-    def items(self):
+    @property
+    def objects(self):
         """See `Hierarchy`."""
-        return self._breadcrumbs(
-            (obj, canonical_url(obj))
-            for obj in IHasBranchTarget(self.context).target.components)
+        return IHasBranchTarget(self.context).target.components
 
 
 class BranchNavigation(Navigation):

=== modified file 'lib/lp/soyuz/stories/packaging/package-pages-navigation.txt'
--- lib/lp/soyuz/stories/packaging/package-pages-navigation.txt	2009-07-06 21:52:17 +0000
+++ lib/lp/soyuz/stories/packaging/package-pages-navigation.txt	2009-08-18 18:00:01 +0000
@@ -31,7 +31,7 @@
 
     >>> anon_browser.open('http://bugs.launchpad.dev/ubuntu/+source/alsa-utils')
     >>> print_location(anon_browser.contents)
-    Hierarchy: Launchpad > Ubuntu > ?alsa-utils? package
+    Hierarchy: Launchpad > Ubuntu > ?alsa-utils? package > Bugs on alsa-utils
     Tabs:
     * Overview - http://launchpad.dev/ubuntu/+source/alsa-utils
     * Code - http://code.launchpad.dev/ubuntu/+source/alsa-utils

=== modified file 'lib/lp/soyuz/stories/ppa/xx-ppa-navigation.txt'
--- lib/lp/soyuz/stories/ppa/xx-ppa-navigation.txt	2009-07-01 13:16:44 +0000
+++ lib/lp/soyuz/stories/ppa/xx-ppa-navigation.txt	2009-08-18 18:00:01 +0000
@@ -109,7 +109,7 @@
 The user could return to the 'PPA' overview by using the breadcrumb link.
 
     >>> print anon_browser.getLink('Default PPA').url
-    http://launchpad.dev/%7Ecprov/+archive/ppa
+    http://launchpad.dev/~cprov/+archive/ppa
 
 The user can navigate to an individual build details: