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

Proposed by Paul Hummer
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~rockstar/launchpad/recipe-index-redux
Merge into: lp:launchpad
Prerequisite: lp:~rockstar/launchpad/delete-recipe
Diff against target: 314 lines (+108/-59)
4 files modified
configs/development/build-from-branch.zcml (+12/-0)
lib/canonical/launchpad/browser/launchpad.py (+6/-2)
lib/lp/code/browser/sourcepackagerecipe.py (+52/-4)
lib/lp/code/browser/tests/test_sourcepackagerecipe.py (+38/-53)
To merge this branch: bzr merge lp:~rockstar/launchpad/recipe-index-redux
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Review via email: mp+23976@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

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.

Revision history for this message
Tim Penhey (thumper) :
review: Needs Fixing
Revision history for this message
Paul Hummer (rockstar) wrote :

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:

Revision history for this message
Tim Penhey (thumper) wrote :

Land it!

LAND IT NOW!!!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
89 name="+recipes"89 name="+recipes"
90 template="../../lib/lp/code/templates/sourcepackagerecipe-listing.pt"/>90 template="../../lib/lp/code/templates/sourcepackagerecipe-listing.pt"/>
9191
92 <browser:page
93 for="lp.code.interfaces.sourcepackagerecipe.ISourcePackageRecipe"
94 name="+hierarchy"
95 class="lp.code.browser.sourcepackagerecipe.SourcePackageRecipeHierarchy"
96 template="../../lib/lp/app/templates/launchpad-hierarchy.pt"
97 permission="zope.Public"/>
98
99 <adapter
100 provides="canonical.launchpad.webapp.interfaces.IBreadcrumb"
101 for="lp.code.interfaces.sourcepackagerecipe.ISourcePackageRecipe"
102 factory="canonical.launchpad.webapp.breadcrumb.NameBreadcrumb"
103 permission="zope.Public"/>
92 </facet>104 </facet>
93 <securedutility105 <securedutility
94 name="BuildableDistroSeries"106 name="BuildableDistroSeries"
95107
=== 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 @@
225class Hierarchy(LaunchpadView):225class Hierarchy(LaunchpadView):
226 """The hierarchy part of the location bar on each page."""226 """The hierarchy part of the location bar on each page."""
227227
228 vhost_breadcrumb = True
229
228 @property230 @property
229 def objects(self):231 def objects(self):
230 """The objects for which we want breadcrumbs."""232 """The objects for which we want breadcrumbs."""
@@ -238,13 +240,15 @@
238 """240 """
239 breadcrumbs = []241 breadcrumbs = []
240 for obj in self.objects:242 for obj in self.objects:
241 breadcrumb = queryAdapter(obj, IBreadcrumb)243 breadcrumb = IBreadcrumb(obj, None)
242 if breadcrumb is not None:244 if breadcrumb is not None:
243 breadcrumbs.append(breadcrumb)245 breadcrumbs.append(breadcrumb)
244246
245 host = URI(self.request.getURL()).host247 host = URI(self.request.getURL()).host
246 mainhost = allvhosts.configs['mainsite'].hostname248 mainhost = allvhosts.configs['mainsite'].hostname
247 if len(breadcrumbs) != 0 and host != mainhost:249 if (len(breadcrumbs) != 0 and
250 host != mainhost and
251 self.vhost_breadcrumb):
248 # We have breadcrumbs and we're not on the mainsite, so we'll252 # We have breadcrumbs and we're not on the mainsite, so we'll
249 # sneak an extra breadcrumb for the vhost we're on.253 # sneak an extra breadcrumb for the vhost we're on.
250 vhost = host.split('.')[0]254 vhost = host.split('.')[0]
251255
=== 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 @@
23from lazr.restful.interface import use_template23from lazr.restful.interface import use_template
24from zope.component import getUtility24from zope.component import getUtility
25from zope.event import notify25from zope.event import notify
26from zope.interface import Interface26from zope.interface import implements, Interface
27from zope.schema import Choice, List, Text27from zope.schema import Choice, List, Text
28from zope.schema.vocabulary import SimpleVocabulary, SimpleTerm28from zope.schema.vocabulary import SimpleVocabulary, SimpleTerm
2929
30from canonical.database.constants import UTC_NOW30from canonical.database.constants import UTC_NOW
31from canonical.launchpad.browser.launchpad import Hierarchy
31from canonical.launchpad.interfaces import ILaunchBag32from canonical.launchpad.interfaces import ILaunchBag
32from canonical.launchpad.webapp import (33from canonical.launchpad.webapp import (
33 action, canonical_url, ContextMenu, custom_widget,34 action, canonical_url, ContextMenu, custom_widget,
34 enabled_with_permission, LaunchpadEditFormView, LaunchpadFormView,35 enabled_with_permission, LaunchpadEditFormView, LaunchpadFormView,
35 LaunchpadView, Link, NavigationMenu)36 LaunchpadView, Link, NavigationMenu)
36from canonical.launchpad.webapp.authorization import check_permission37from canonical.launchpad.webapp.authorization import check_permission
38from canonical.launchpad.webapp.breadcrumb import Breadcrumb
37from canonical.widgets.itemswidgets import LabeledMultiCheckBoxWidget39from canonical.widgets.itemswidgets import LabeledMultiCheckBoxWidget
38from lp.buildmaster.interfaces.buildbase import BuildStatus40from lp.buildmaster.interfaces.buildbase import BuildStatus
39from lp.code.interfaces.sourcepackagerecipe import (41from lp.code.interfaces.sourcepackagerecipe import (
@@ -45,6 +47,50 @@
45from lp.registry.interfaces.pocket import PackagePublishingPocket47from lp.registry.interfaces.pocket import PackagePublishingPocket
4648
4749
50class IRecipesForPerson(Interface):
51 """A marker interface for source package recipe sets."""
52
53
54class RecipesForPersonBreadcrumb(Breadcrumb):
55 """A Breadcrumb that will handle the "Recipes" link for recipe breadcrumbs.
56 """
57
58 rootsite = 'code'
59 text = 'Recipes'
60
61 implements(IRecipesForPerson)
62
63 @property
64 def url(self):
65 return canonical_url(self.context, view_name="+recipes")
66
67
68class SourcePackageRecipeHierarchy(Hierarchy):
69 """"Hierarchy for Source Package Recipe."""
70
71 vhost_breadcrumb = False
72
73 @property
74 def objects(self):
75 """See `Hierarchy`."""
76 traversed = list(self.request.traversed_objects)
77
78 # Pop the root object
79 yield traversed.pop(0)
80
81 recipe = traversed.pop(0)
82 while not ISourcePackageRecipe.providedBy(recipe):
83 yield recipe
84 recipe = traversed.pop(0)
85
86 # Pop in the "Recipes" link to recipe listings.
87 yield RecipesForPersonBreadcrumb(recipe.owner)
88 yield recipe
89
90 for item in traversed:
91 yield item
92
93
48class SourcePackageRecipeNavigationMenu(NavigationMenu):94class SourcePackageRecipeNavigationMenu(NavigationMenu):
49 """Navigation menu for sourcepackage recipes."""95 """Navigation menu for sourcepackage recipes."""
5096
@@ -81,10 +127,12 @@
81 """Default view of a SourcePackageRecipe."""127 """Default view of a SourcePackageRecipe."""
82128
83 @property129 @property
84 def title(self):130 def page_title(self):
85 return self.context.name131 return "%(name)s\'s %(recipe_name)s recipe" % {
132 'name': self.context.owner.displayname,
133 'recipe_name': self.context.name}
86134
87 label = title135 label = page_title
88136
89 @property137 @property
90 def builds(self):138 def builds(self):
91139
=== 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 @@
81 browser.getControl('Create Recipe').click()81 browser.getControl('Create Recipe').click()
8282
83 pattern = """\83 pattern = """\
84 daily .*84 Master Chef's daily recipe
85 .*
8586
86 Description87 Description
87 Make some food!88 Make some food!
@@ -162,7 +163,8 @@
162 browser.getControl('Update Recipe').click()163 browser.getControl('Update Recipe').click()
163164
164 pattern = """\165 pattern = """\
165 fings .*166 Master Chef's fings recipe
167 .*
166168
167 Description169 Description
168 This is stuff170 This is stuff
@@ -182,7 +184,6 @@
182 pattern, main_text)184 pattern, main_text)
183185
184186
185
186class TestSourcePackageRecipeView(TestCaseForRecipe):187class TestSourcePackageRecipeView(TestCaseForRecipe):
187188
188 layer = DatabaseFunctionalLayer189 layer = DatabaseFunctionalLayer
@@ -193,33 +194,26 @@
193 recipe=recipe, distroseries=self.squirrel, archive=self.ppa))194 recipe=recipe, distroseries=self.squirrel, archive=self.ppa))
194 build.buildstate = BuildStatus.FULLYBUILT195 build.buildstate = BuildStatus.FULLYBUILT
195 build.datebuilt = datetime(2010, 03, 16, tzinfo=utc)196 build.datebuilt = datetime(2010, 03, 16, tzinfo=utc)
196 self.assertContainsRe(dedent("""\197
197 Master Chef198 self.assertTextMatchesExpressionIgnoreWhitespace("""\
198 Branches199 Master Chef Recipes cake_recipe
199 Description200 Description
200 This recipe .*changes.201 This recipe .*changes.
202
201 Recipe information203 Recipe information
202 Owner:204 Owner: Master Chef
203 Master Chef205 Base branch: lp://dev/~chef/chocolate/cake
204 Base branch:206 Debian version: 1.0
205 lp://dev/~chef/chocolate/cake207 Distribution series: Secret Squirrel
206 Debian version:208
207 1.0
208 Distribution series:
209 Secret Squirrel
210 Build records209 Build records
211 Status210 Status Time Distribution series Archive
212 Time211 Successful build on 2010-03-16 Secret Squirrel Secret PPA
213 Distribution series
214 Archive
215 Successful build
216 on 2010-03-16
217 Secret Squirrel
218 Secret PPA
219 Request build\(s\)212 Request build\(s\)
213
220 Recipe contents214 Recipe contents
221 # bzr-builder format 0.2 deb-version 1.0215 # bzr-builder format 0.2 deb-version 1.0
222 lp://dev/~chef/chocolate/cake"""), self.getMainText(recipe))216 lp://dev/~chef/chocolate/cake""", self.getMainText(recipe))
223217
224 def assertContainsRe(self, regex, text):218 def assertContainsRe(self, regex, text):
225 """Assert that the text contains the specified regex."""219 """Assert that the text contains the specified regex."""
@@ -229,28 +223,20 @@
229 def test_index_no_builds(self):223 def test_index_no_builds(self):
230 """A message should be shown when there are no builds."""224 """A message should be shown when there are no builds."""
231 recipe = self.makeRecipe()225 recipe = self.makeRecipe()
232 self.assertContainsRe(dedent("""\226 self.assertTextMatchesExpressionIgnoreWhitespace("""\
233 Build records227 Build records
234 Status228 Status Time Distribution series Archive
235 Time229 This recipe has not been built yet.""", self.getMainText(recipe))
236 Distribution series
237 Archive
238 This recipe has not been built yet."""), self.getMainText(recipe))
239230
240 def test_index_no_suitable_builders(self):231 def test_index_no_suitable_builders(self):
241 recipe = self.makeRecipe()232 recipe = self.makeRecipe()
242 removeSecurityProxy(self.factory.makeSourcePackageRecipeBuild(233 removeSecurityProxy(self.factory.makeSourcePackageRecipeBuild(
243 recipe=recipe, distroseries=self.squirrel, archive=self.ppa))234 recipe=recipe, distroseries=self.squirrel, archive=self.ppa))
244 self.assertContainsRe(dedent("""\235 self.assertTextMatchesExpressionIgnoreWhitespace("""
245 Build records236 Build records
246 Status237 Status Time Distribution series Archive
247 Time238 No suitable builders Secret Squirrel Secret PPA
248 Distribution series239 Request build\(s\)""", self.getMainText(recipe))
249 Archive
250 No suitable builders
251 Secret Squirrel
252 Secret PPA
253 Request build\(s\)"""), self.getMainText(recipe))
254240
255 def makeBuildJob(self, recipe):241 def makeBuildJob(self, recipe):
256 """Return a build associated with a buildjob."""242 """Return a build associated with a buildjob."""
@@ -264,19 +250,16 @@
264 recipe = self.makeRecipe()250 recipe = self.makeRecipe()
265 self.makeBuildJob(recipe)251 self.makeBuildJob(recipe)
266 self.factory.makeBuilder()252 self.factory.makeBuilder()
267 self.assertContainsRe(dedent("""\253 pattern = """\
268 Build records254 Build records
269 Status255 Status Time Distribution series Archive
270 Time256 Pending build in .* \(estimated\) Secret Squirrel Secret PPA
271 Distribution series
272 Archive
273 Pending build
274 in .*
275 \(estimated\)
276 Secret Squirrel
277 Secret PPA
278 Request build\(s\)257 Request build\(s\)
279 Recipe contents"""), self.getMainText(recipe))258
259 Recipe contents"""
260 main_text = self.getMainText(recipe)
261 self.assertTextMatchesExpressionIgnoreWhitespace(
262 pattern, main_text)
280263
281 def test_builds(self):264 def test_builds(self):
282 """Ensure SourcePackageRecipeView.builds is as described."""265 """Ensure SourcePackageRecipeView.builds is as described."""
@@ -311,11 +294,11 @@
311 def test_request_builds_page(self):294 def test_request_builds_page(self):
312 """Ensure the +request-builds page is sane."""295 """Ensure the +request-builds page is sane."""
313 recipe = self.makeRecipe()296 recipe = self.makeRecipe()
314 text = self.getMainText(recipe, '+request-builds')297 pattern = dedent("""\
315 self.assertEqual(dedent(u"""\
316 Request builds for cake_recipe298 Request builds for cake_recipe
317 Master Chef299 Master Chef
318 Branches300 Recipes
301 cake_recipe
319 Request builds for cake_recipe302 Request builds for cake_recipe
320 Archive:303 Archive:
321 Secret PPA (chef/ppa)304 Secret PPA (chef/ppa)
@@ -329,7 +312,9 @@
329 Guada2005312 Guada2005
330 Secret Squirrel313 Secret Squirrel
331 or314 or
332 Cancel"""), text)315 Cancel""")
316 main_text = self.getMainText(recipe, '+request-builds')
317 self.assertEqual(pattern, main_text)
333318
334 def test_request_builds_action(self):319 def test_request_builds_action(self):
335 """Requesting a build creates pending builds."""320 """Requesting a build creates pending builds."""