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
1=== modified file 'configs/development/build-from-branch.zcml'
2--- configs/development/build-from-branch.zcml 2010-04-23 03:33:33 +0000
3+++ configs/development/build-from-branch.zcml 2010-04-23 03:33:35 +0000
4@@ -89,6 +89,18 @@
5 name="+recipes"
6 template="../../lib/lp/code/templates/sourcepackagerecipe-listing.pt"/>
7
8+ <browser:page
9+ for="lp.code.interfaces.sourcepackagerecipe.ISourcePackageRecipe"
10+ name="+hierarchy"
11+ class="lp.code.browser.sourcepackagerecipe.SourcePackageRecipeHierarchy"
12+ template="../../lib/lp/app/templates/launchpad-hierarchy.pt"
13+ permission="zope.Public"/>
14+
15+ <adapter
16+ provides="canonical.launchpad.webapp.interfaces.IBreadcrumb"
17+ for="lp.code.interfaces.sourcepackagerecipe.ISourcePackageRecipe"
18+ factory="canonical.launchpad.webapp.breadcrumb.NameBreadcrumb"
19+ permission="zope.Public"/>
20 </facet>
21 <securedutility
22 name="BuildableDistroSeries"
23
24=== modified file 'lib/canonical/launchpad/browser/launchpad.py'
25--- lib/canonical/launchpad/browser/launchpad.py 2010-04-05 22:31:01 +0000
26+++ lib/canonical/launchpad/browser/launchpad.py 2010-04-23 03:33:35 +0000
27@@ -225,6 +225,8 @@
28 class Hierarchy(LaunchpadView):
29 """The hierarchy part of the location bar on each page."""
30
31+ vhost_breadcrumb = True
32+
33 @property
34 def objects(self):
35 """The objects for which we want breadcrumbs."""
36@@ -238,13 +240,15 @@
37 """
38 breadcrumbs = []
39 for obj in self.objects:
40- breadcrumb = queryAdapter(obj, IBreadcrumb)
41+ breadcrumb = IBreadcrumb(obj, None)
42 if breadcrumb is not None:
43 breadcrumbs.append(breadcrumb)
44
45 host = URI(self.request.getURL()).host
46 mainhost = allvhosts.configs['mainsite'].hostname
47- if len(breadcrumbs) != 0 and host != mainhost:
48+ if (len(breadcrumbs) != 0 and
49+ host != mainhost and
50+ self.vhost_breadcrumb):
51 # We have breadcrumbs and we're not on the mainsite, so we'll
52 # sneak an extra breadcrumb for the vhost we're on.
53 vhost = host.split('.')[0]
54
55=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
56--- lib/lp/code/browser/sourcepackagerecipe.py 2010-04-23 03:33:33 +0000
57+++ lib/lp/code/browser/sourcepackagerecipe.py 2010-04-23 03:33:35 +0000
58@@ -23,17 +23,19 @@
59 from lazr.restful.interface import use_template
60 from zope.component import getUtility
61 from zope.event import notify
62-from zope.interface import Interface
63+from zope.interface import implements, Interface
64 from zope.schema import Choice, List, Text
65 from zope.schema.vocabulary import SimpleVocabulary, SimpleTerm
66
67 from canonical.database.constants import UTC_NOW
68+from canonical.launchpad.browser.launchpad import Hierarchy
69 from canonical.launchpad.interfaces import ILaunchBag
70 from canonical.launchpad.webapp import (
71 action, canonical_url, ContextMenu, custom_widget,
72 enabled_with_permission, LaunchpadEditFormView, LaunchpadFormView,
73 LaunchpadView, Link, NavigationMenu)
74 from canonical.launchpad.webapp.authorization import check_permission
75+from canonical.launchpad.webapp.breadcrumb import Breadcrumb
76 from canonical.widgets.itemswidgets import LabeledMultiCheckBoxWidget
77 from lp.buildmaster.interfaces.buildbase import BuildStatus
78 from lp.code.interfaces.sourcepackagerecipe import (
79@@ -45,6 +47,50 @@
80 from lp.registry.interfaces.pocket import PackagePublishingPocket
81
82
83+class IRecipesForPerson(Interface):
84+ """A marker interface for source package recipe sets."""
85+
86+
87+class RecipesForPersonBreadcrumb(Breadcrumb):
88+ """A Breadcrumb that will handle the "Recipes" link for recipe breadcrumbs.
89+ """
90+
91+ rootsite = 'code'
92+ text = 'Recipes'
93+
94+ implements(IRecipesForPerson)
95+
96+ @property
97+ def url(self):
98+ return canonical_url(self.context, view_name="+recipes")
99+
100+
101+class SourcePackageRecipeHierarchy(Hierarchy):
102+ """"Hierarchy for Source Package Recipe."""
103+
104+ vhost_breadcrumb = False
105+
106+ @property
107+ def objects(self):
108+ """See `Hierarchy`."""
109+ traversed = list(self.request.traversed_objects)
110+
111+ # Pop the root object
112+ yield traversed.pop(0)
113+
114+ recipe = traversed.pop(0)
115+ while not ISourcePackageRecipe.providedBy(recipe):
116+ yield recipe
117+ recipe = traversed.pop(0)
118+
119+ # Pop in the "Recipes" link to recipe listings.
120+ yield RecipesForPersonBreadcrumb(recipe.owner)
121+ yield recipe
122+
123+ for item in traversed:
124+ yield item
125+
126+
127 class SourcePackageRecipeNavigationMenu(NavigationMenu):
128 """Navigation menu for sourcepackage recipes."""
129
130@@ -81,10 +127,12 @@
131 """Default view of a SourcePackageRecipe."""
132
133 @property
134- def title(self):
135- return self.context.name
136+ def page_title(self):
137+ return "%(name)s\'s %(recipe_name)s recipe" % {
138+ 'name': self.context.owner.displayname,
139+ 'recipe_name': self.context.name}
140
141- label = title
142+ label = page_title
143
144 @property
145 def builds(self):
146
147=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
148--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2010-04-23 03:33:33 +0000
149+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2010-04-23 03:33:35 +0000
150@@ -81,7 +81,8 @@
151 browser.getControl('Create Recipe').click()
152
153 pattern = """\
154- daily .*
155+ Master Chef's daily recipe
156+ .*
157
158 Description
159 Make some food!
160@@ -162,7 +163,8 @@
161 browser.getControl('Update Recipe').click()
162
163 pattern = """\
164- fings .*
165+ Master Chef's fings recipe
166+ .*
167
168 Description
169 This is stuff
170@@ -182,7 +184,6 @@
171 pattern, main_text)
172
173
174-
175 class TestSourcePackageRecipeView(TestCaseForRecipe):
176
177 layer = DatabaseFunctionalLayer
178@@ -193,33 +194,26 @@
179 recipe=recipe, distroseries=self.squirrel, archive=self.ppa))
180 build.buildstate = BuildStatus.FULLYBUILT
181 build.datebuilt = datetime(2010, 03, 16, tzinfo=utc)
182- self.assertContainsRe(dedent("""\
183- Master Chef
184- Branches
185+
186+ self.assertTextMatchesExpressionIgnoreWhitespace("""\
187+ Master Chef Recipes cake_recipe
188 Description
189 This recipe .*changes.
190+
191 Recipe information
192- Owner:
193- Master Chef
194- Base branch:
195- lp://dev/~chef/chocolate/cake
196- Debian version:
197- 1.0
198- Distribution series:
199- Secret Squirrel
200+ Owner: Master Chef
201+ Base branch: lp://dev/~chef/chocolate/cake
202+ Debian version: 1.0
203+ Distribution series: Secret Squirrel
204+
205 Build records
206- Status
207- Time
208- Distribution series
209- Archive
210- Successful build
211- on 2010-03-16
212- Secret Squirrel
213- Secret PPA
214+ Status Time Distribution series Archive
215+ Successful build on 2010-03-16 Secret Squirrel Secret PPA
216 Request build\(s\)
217+
218 Recipe contents
219 # bzr-builder format 0.2 deb-version 1.0
220- lp://dev/~chef/chocolate/cake"""), self.getMainText(recipe))
221+ lp://dev/~chef/chocolate/cake""", self.getMainText(recipe))
222
223 def assertContainsRe(self, regex, text):
224 """Assert that the text contains the specified regex."""
225@@ -229,28 +223,20 @@
226 def test_index_no_builds(self):
227 """A message should be shown when there are no builds."""
228 recipe = self.makeRecipe()
229- self.assertContainsRe(dedent("""\
230+ self.assertTextMatchesExpressionIgnoreWhitespace("""\
231 Build records
232- Status
233- Time
234- Distribution series
235- Archive
236- This recipe has not been built yet."""), self.getMainText(recipe))
237+ Status Time Distribution series Archive
238+ This recipe has not been built yet.""", self.getMainText(recipe))
239
240 def test_index_no_suitable_builders(self):
241 recipe = self.makeRecipe()
242 removeSecurityProxy(self.factory.makeSourcePackageRecipeBuild(
243 recipe=recipe, distroseries=self.squirrel, archive=self.ppa))
244- self.assertContainsRe(dedent("""\
245+ self.assertTextMatchesExpressionIgnoreWhitespace("""
246 Build records
247- Status
248- Time
249- Distribution series
250- Archive
251- No suitable builders
252- Secret Squirrel
253- Secret PPA
254- Request build\(s\)"""), self.getMainText(recipe))
255+ Status Time Distribution series Archive
256+ No suitable builders Secret Squirrel Secret PPA
257+ Request build\(s\)""", self.getMainText(recipe))
258
259 def makeBuildJob(self, recipe):
260 """Return a build associated with a buildjob."""
261@@ -264,19 +250,16 @@
262 recipe = self.makeRecipe()
263 self.makeBuildJob(recipe)
264 self.factory.makeBuilder()
265- self.assertContainsRe(dedent("""\
266+ pattern = """\
267 Build records
268- Status
269- Time
270- Distribution series
271- Archive
272- Pending build
273- in .*
274- \(estimated\)
275- Secret Squirrel
276- Secret PPA
277+ Status Time Distribution series Archive
278+ Pending build in .* \(estimated\) Secret Squirrel Secret PPA
279 Request build\(s\)
280- Recipe contents"""), self.getMainText(recipe))
281+
282+ Recipe contents"""
283+ main_text = self.getMainText(recipe)
284+ self.assertTextMatchesExpressionIgnoreWhitespace(
285+ pattern, main_text)
286
287 def test_builds(self):
288 """Ensure SourcePackageRecipeView.builds is as described."""
289@@ -311,11 +294,11 @@
290 def test_request_builds_page(self):
291 """Ensure the +request-builds page is sane."""
292 recipe = self.makeRecipe()
293- text = self.getMainText(recipe, '+request-builds')
294- self.assertEqual(dedent(u"""\
295+ pattern = dedent("""\
296 Request builds for cake_recipe
297 Master Chef
298- Branches
299+ Recipes
300+ cake_recipe
301 Request builds for cake_recipe
302 Archive:
303 Secret PPA (chef/ppa)
304@@ -329,7 +312,9 @@
305 Guada2005
306 Secret Squirrel
307 or
308- Cancel"""), text)
309+ Cancel""")
310+ main_text = self.getMainText(recipe, '+request-builds')
311+ self.assertEqual(pattern, main_text)
312
313 def test_request_builds_action(self):
314 """Requesting a build creates pending builds."""