Merge lp:~rockstar/launchpad/recipe-index-redux into lp:launchpad

[Paul Hummer]

This branch does two small things in the API. The first is the change the page
title for the page to reflect the owner as well as the name of the recipe.
This was requested in a review of an earlier branch, and was quite easy to fix.

The second thing this branch does is change the Breadcrumbs to appear like so:

    Paul Hummer >> Recipes >> recipe_name

...where previously it looked like this:

    Paul Hummer >> Branches

I hacked it up a bit, and when I asked sinzui for guidance, he pointed me to
some things that didn't help, and then ultimately at the only place that really
messes with breadcrumb items: BranchHierarchy. I was already using that as
inspiration for what I was doing...

Here's what I had to do to make this change:

  1. Define the breadcrumb adapter for SourcePackageRecipe to NameBreadcrumb.
  2. Add a flag to Hierarchy that allows me to turn off the default
  VHostBreadcrumb from rendering (thus removing
  3. Add a SourcePackageRecipeSet item (with accompanying special breadcrumb
  class and adapter) to render the "Recipes" link with a url.
  4. Create a SourcePackageRecipeHierarchy that knows about
  SourcePackageRecipeSet and its breadcrumb, and knows to render it before
  rending the recipe.

[Tim Penhey (thumper)]

IRecipiesForPerson instead of ISourcePackageRecipeSet

so the __init__ should take a person, not a recipe

SourcePackageRecipeSetBreadcrumb needs a blank line after it. Actually lets merge the two classes as the class's entire purpose is to provide a breadcrumb, so it should just inherit from Breadcrumb, and provide the other interface. That way it doesn't need an adapter.

[Tim Penhey (thumper)]

review: Needs Fixing

[Paul Hummer (rockstar)]

All fixed! Here's the incremental.

=== modified file 'configs/development/build-from-branch.zcml'
--- configs/development/build-from-branch.zcml 2010-04-23 01:23:47 +0000
+++ configs/development/build-from-branch.zcml 2010-04-23 03:10:20 +0000
@@ -103,8 +103,8 @@
             permission="zope.Public"/>
         <adapter
             provides="canonical.launchpad.webapp.interfaces.IBreadcrumb"
- for="lp.code.browser.sourcepackagerecipe.ISourcePackageRecipeSet"
- factory="lp.code.browser.sourcepackagerecipe.SourcePackageRecipeSetBreadcrumb"
+ for="lp.code.browser.sourcepackagerecipe.IRecipesForPerson"
+ factory="lp.code.browser.sourcepackagerecipe.RecipesForPersonBreadcrumb"
             permission="zope.Public"/>
     </facet>
   <securedutility

=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
--- lib/lp/code/browser/sourcepackagerecipe.py 2010-04-23 01:23:47 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py 2010-04-23 03:08:40 +0000
@@ -47,29 +47,24 @@
 from lp.registry.interfaces.pocket import PackagePublishingPocket

-class ISourcePackageRecipeSet(Interface):
+class IRecipesForPerson(Interface):
     """A marker interface for source package recipe sets."""

-class SourcePackageRecipeSet:
- """A simple class from SourcePackageRecipeSet.
-
- This class is only used for making breadcrumbs.
+class RecipesForPersonBreadcrumb(Breadcrumb):
+ """A Breadcrumb that will handle the "Recipes" link for recipe breadcrumbs.
     """

- implements(ISourcePackageRecipeSet)
-
- def __init__(self, recipe):
- self.recipe = recipe
-
-
-class SourcePackageRecipeSetBreadcrumb(Breadcrumb):
     rootsite = 'code'
     text = 'Recipes'

+ implements(IRecipesForPerson)
+
     @property
     def url(self):
- return canonical_url(self.context.recipe.owner) + '/+recipes'
+ # Yes, this looks odd, but basically, the breadcrumb code wraps this
+ # class in an instance of itself (because it's doing double duty).
+ return canonical_url(self.context.context) + '/+recipes'

 class SourcePackageRecipeHierarchy(Hierarchy):
@@ -91,7 +86,7 @@
             recipe = traversed.pop(0)

         # Pop in the "Recipes" link to recipe listings.
- yield SourcePackageRecipeSet(recipe)
+ yield RecipesForPersonBreadcrumb(recipe.owner)
         yield recipe

         for item in traversed:

[Tim Penhey (thumper)]

Land it!

LAND IT NOW!!!

review: Approve

=== modified file 'configs/development/build-from-branch.zcml'
--- configs/development/build-from-branch.zcml	2010-04-23 03:33:33 +0000
+++ configs/development/build-from-branch.zcml	2010-04-23 03:33:35 +0000
@@ -89,6 +89,18 @@
             name="+recipes"
             template="../../lib/lp/code/templates/sourcepackagerecipe-listing.pt"/>
 
+        <browser:page
+            for="lp.code.interfaces.sourcepackagerecipe.ISourcePackageRecipe"
+            name="+hierarchy"
+            class="lp.code.browser.sourcepackagerecipe.SourcePackageRecipeHierarchy"
+            template="../../lib/lp/app/templates/launchpad-hierarchy.pt"
+            permission="zope.Public"/>
+
+        <adapter
+            provides="canonical.launchpad.webapp.interfaces.IBreadcrumb"
+            for="lp.code.interfaces.sourcepackagerecipe.ISourcePackageRecipe"
+            factory="canonical.launchpad.webapp.breadcrumb.NameBreadcrumb"
+            permission="zope.Public"/>
     </facet>
   <securedutility
     name="BuildableDistroSeries"

=== modified file 'lib/canonical/launchpad/browser/launchpad.py'
--- lib/canonical/launchpad/browser/launchpad.py	2010-04-05 22:31:01 +0000
+++ lib/canonical/launchpad/browser/launchpad.py	2010-04-23 03:33:35 +0000
@@ -225,6 +225,8 @@
 class Hierarchy(LaunchpadView):
     """The hierarchy part of the location bar on each page."""
 
+    vhost_breadcrumb = True
+
     @property
     def objects(self):
         """The objects for which we want breadcrumbs."""
@@ -238,13 +240,15 @@
         """
         breadcrumbs = []
         for obj in self.objects:
-            breadcrumb = queryAdapter(obj, IBreadcrumb)
+            breadcrumb = IBreadcrumb(obj, None)
             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:
+        if (len(breadcrumbs) != 0 and
+            host != mainhost 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]

=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
--- lib/lp/code/browser/sourcepackagerecipe.py	2010-04-23 03:33:33 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py	2010-04-23 03:33:35 +0000
@@ -23,17 +23,19 @@
 from lazr.restful.interface import use_template
 from zope.component import getUtility
 from zope.event import notify
-from zope.interface import Interface
+from zope.interface import implements, Interface
 from zope.schema import Choice, List, Text
 from zope.schema.vocabulary import SimpleVocabulary, SimpleTerm
 
 from canonical.database.constants import UTC_NOW
+from canonical.launchpad.browser.launchpad import Hierarchy
 from canonical.launchpad.interfaces import ILaunchBag
 from canonical.launchpad.webapp import (
     action, canonical_url, ContextMenu, custom_widget,
     enabled_with_permission, LaunchpadEditFormView, LaunchpadFormView,
     LaunchpadView, Link, NavigationMenu)
 from canonical.launchpad.webapp.authorization import check_permission
+from canonical.launchpad.webapp.breadcrumb import Breadcrumb
 from canonical.widgets.itemswidgets import LabeledMultiCheckBoxWidget
 from lp.buildmaster.interfaces.buildbase import BuildStatus
 from lp.code.interfaces.sourcepackagerecipe import (
@@ -45,6 +47,50 @@
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 
 
+class IRecipesForPerson(Interface):
+    """A marker interface for source package recipe sets."""
+
+
+class RecipesForPersonBreadcrumb(Breadcrumb):
+    """A Breadcrumb that will handle the "Recipes" link for recipe breadcrumbs.
+    """
+
+    rootsite = 'code'
+    text = 'Recipes'
+
+    implements(IRecipesForPerson)
+
+    @property
+    def url(self):
+        return canonical_url(self.context, view_name="+recipes")
+
+
+class SourcePackageRecipeHierarchy(Hierarchy):
+    """"Hierarchy for Source Package Recipe."""
+
+    vhost_breadcrumb = False
+
+    @property
+    def objects(self):
+        """See `Hierarchy`."""
+        traversed = list(self.request.traversed_objects)
+
+        # Pop the root object
+        yield traversed.pop(0)
+
+        recipe = traversed.pop(0)
+        while not ISourcePackageRecipe.providedBy(recipe):
+            yield recipe
+            recipe = traversed.pop(0)
+
+        # Pop in the "Recipes" link to recipe listings.
+        yield RecipesForPersonBreadcrumb(recipe.owner)
+        yield recipe
+
+        for item in traversed:
+            yield item
+
+
 class SourcePackageRecipeNavigationMenu(NavigationMenu):
     """Navigation menu for sourcepackage recipes."""
 
@@ -81,10 +127,12 @@
     """Default view of a SourcePackageRecipe."""
 
     @property
-    def title(self):
-        return self.context.name
+    def page_title(self):
+        return "%(name)s\'s %(recipe_name)s recipe" % {
+            'name': self.context.owner.displayname,
+            'recipe_name': self.context.name}
 
-    label = title
+    label = page_title
 
     @property
     def builds(self):

=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2010-04-23 03:33:33 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2010-04-23 03:33:35 +0000
@@ -81,7 +81,8 @@
         browser.getControl('Create Recipe').click()
 
         pattern = """\
-            daily .*
+            Master Chef's daily recipe
+            .*
 
             Description
             Make some food!
@@ -162,7 +163,8 @@
         browser.getControl('Update Recipe').click()
 
         pattern = """\
-            fings .*
+            Master Chef's fings recipe
+            .*
 
             Description
             This is stuff
@@ -182,7 +184,6 @@
             pattern, main_text)
 
 
-
 class TestSourcePackageRecipeView(TestCaseForRecipe):
 
     layer = DatabaseFunctionalLayer
@@ -193,33 +194,26 @@
             recipe=recipe, distroseries=self.squirrel, archive=self.ppa))
         build.buildstate = BuildStatus.FULLYBUILT
         build.datebuilt = datetime(2010, 03, 16, tzinfo=utc)
-        self.assertContainsRe(dedent("""\
-            Master Chef
-            Branches
+
+        self.assertTextMatchesExpressionIgnoreWhitespace("""\
+            Master Chef Recipes cake_recipe
             Description
             This recipe .*changes.
+
             Recipe information
-            Owner:
-            Master Chef
-            Base branch:
-            lp://dev/~chef/chocolate/cake
-            Debian version:
-            1.0
-            Distribution series:
-            Secret Squirrel
+            Owner: Master Chef
+            Base branch: lp://dev/~chef/chocolate/cake
+            Debian version: 1.0
+            Distribution series: Secret Squirrel
+
             Build records
-            Status
-            Time
-            Distribution series
-            Archive
-            Successful build
-            on 2010-03-16
-            Secret Squirrel
-            Secret PPA
+            Status Time Distribution series Archive
+            Successful build on 2010-03-16 Secret Squirrel Secret PPA
             Request build\(s\)
+
             Recipe contents
             # bzr-builder format 0.2 deb-version 1.0
-            lp://dev/~chef/chocolate/cake"""), self.getMainText(recipe))
+            lp://dev/~chef/chocolate/cake""", self.getMainText(recipe))
 
     def assertContainsRe(self, regex, text):
         """Assert that the text contains the specified regex."""
@@ -229,28 +223,20 @@
     def test_index_no_builds(self):
         """A message should be shown when there are no builds."""
         recipe = self.makeRecipe()
-        self.assertContainsRe(dedent("""\
+        self.assertTextMatchesExpressionIgnoreWhitespace("""\
             Build records
-            Status
-            Time
-            Distribution series
-            Archive
-            This recipe has not been built yet."""), self.getMainText(recipe))
+            Status Time Distribution series Archive
+            This recipe has not been built yet.""", self.getMainText(recipe))
 
     def test_index_no_suitable_builders(self):
         recipe = self.makeRecipe()
         removeSecurityProxy(self.factory.makeSourcePackageRecipeBuild(
             recipe=recipe, distroseries=self.squirrel, archive=self.ppa))
-        self.assertContainsRe(dedent("""\
+        self.assertTextMatchesExpressionIgnoreWhitespace("""
             Build records
-            Status
-            Time
-            Distribution series
-            Archive
-            No suitable builders
-            Secret Squirrel
-            Secret PPA
-            Request build\(s\)"""), self.getMainText(recipe))
+            Status Time Distribution series Archive
+            No suitable builders Secret Squirrel Secret PPA
+            Request build\(s\)""", self.getMainText(recipe))
 
     def makeBuildJob(self, recipe):
         """Return a build associated with a buildjob."""
@@ -264,19 +250,16 @@
         recipe = self.makeRecipe()
         self.makeBuildJob(recipe)
         self.factory.makeBuilder()
-        self.assertContainsRe(dedent("""\
+        pattern = """\
             Build records
-            Status
-            Time
-            Distribution series
-            Archive
-            Pending build
-            in .*
-            \(estimated\)
-            Secret Squirrel
-            Secret PPA
+            Status Time Distribution series Archive
+            Pending build in .* \(estimated\) Secret Squirrel Secret PPA
             Request build\(s\)
-            Recipe contents"""), self.getMainText(recipe))
+
+            Recipe contents"""
+        main_text = self.getMainText(recipe)
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            pattern, main_text)
 
     def test_builds(self):
         """Ensure SourcePackageRecipeView.builds is as described."""
@@ -311,11 +294,11 @@
     def test_request_builds_page(self):
         """Ensure the +request-builds page is sane."""
         recipe = self.makeRecipe()
-        text = self.getMainText(recipe, '+request-builds')
-        self.assertEqual(dedent(u"""\
+        pattern = dedent("""\
             Request builds for cake_recipe
             Master Chef
-            Branches
+            Recipes
+            cake_recipe
             Request builds for cake_recipe
             Archive:
             Secret PPA (chef/ppa)
@@ -329,7 +312,9 @@
             Guada2005
             Secret Squirrel
             or
-            Cancel"""), text)
+            Cancel""")
+        main_text = self.getMainText(recipe, '+request-builds')
+        self.assertEqual(pattern, main_text)
 
     def test_request_builds_action(self):
         """Requesting a build creates pending builds."""