Launchpad itself

Merge lp:~wallyworld/launchpad/recipe-find-related-branches into lp:launchpad

  1. recipe-find-related-branches
  2. Merge into devel
Proposed by Ian Booth
Status: Merged
Approved by: Ian Booth
Approved revision: no longer in the source branch.
Merged at revision: 12111
Proposed branch: lp:~wallyworld/launchpad/recipe-find-related-branches
Merge into: lp:launchpad
Diff against target: 610 lines (+458/-4)
4 files modified
lib/lp/code/browser/sourcepackagerecipe.py (+107/-2)
lib/lp/code/browser/tests/test_sourcepackagerecipe.py (+282/-2)
lib/lp/code/templates/sourcepackagerecipe-new.pt (+3/-0)
lib/lp/code/templates/sourcepackagerecipe-related-branches.pt (+66/-0)
To merge this branch: bzr merge lp:~wallyworld/launchpad/recipe-find-related-branches
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Steve Kowalik (community) code* Approve
Review via email: mp+42097@code.launchpad.net

Commit message

[r=stevenk,thumper][ui=none][bug=670452] Add display of related branches to recipe add and edit pages

Description of the change

= Summary =

Bug 670452 - add a facility to SourcePackageRecipe add/edit pages to show branches related to the base branch of the recipe being added/edited.

= Implementation =

A custom widget was created: RelatedBranchesWidget. A new 'related-branches' form field added to the recipe Add and Edit views is specified to use the widget for rendering. The widget uses the ViewPageTemplateFile class to render some tales markup in ../templates/sourcepackagerecipe-related-branches.pt

What is rendered is a collapsible section called "Related Branches". This has 2 branches listings - related package branches and related series branches.

= Screenshot =

http://people.canonical.com/~ianb/recipe-related-branches.png

= Tests =

bin/test -vvt test_sourcepackagerecipe

New tests were added:
  test_new_recipe_with_package_branches
  test_new_recipe_with_series_branches
  test_new_recipe_view_related_branches
  test_new_recipe_with_related_branches
  test_edit_recipe_with_package_branches
  test_edit_recipe_with_series_branches
  test_edit_recipe_view_related_branches
  test_edit_recipe_with_related_branches

The tests checked that if there were no related branches, the new widget was no rendered, and also checked the contents of the page when related branches were present.

A small drive by improvement was made to the test_create_recipe_no_distroseries test.

= Lint =

Linting changed files:
  lib/lp/code/browser/sourcepackagerecipe.py
  lib/lp/code/browser/tests/test_sourcepackagerecipe.py
  lib/lp/code/templates/sourcepackagerecipe-related-branches.pt

./lib/lp/code/browser/tests/test_sourcepackagerecipe.py
     511: E501 line too long (80 characters)
    1023: E501 line too long (85 characters)
    1037: E501 line too long (89 characters)
    1049: E501 line too long (85 characters)
    1064: E501 line too long (89 characters)
    1080: E501 line too long (85 characters)
     511: Line exceeds 78 characters.
    1023: Line exceeds 78 characters.
    1037: Line exceeds 78 characters.
    1049: Line exceeds 78 characters.
    1064: Line exceeds 78 characters.
    1067: Line exceeds 78 characters.
    1080: Line exceeds 78 characters.
./lib/lp/code/templates/sourcepackagerecipe-related-branches.pt
       1: unbound prefix

To post a comment you must log in.
Revision history for this message
Steve Kowalik (stevenk) wrote :

This looks great! My only concerns are the lint errors, and the (it seems to me) excessive use of removeSecurityProxy.

review: Approve (code*)
Revision history for this message
Ian Booth (wallyworld) wrote :

lint errors (almost all line length) were there already and since this
code base is relatively new, I figured I'd leave well enough alone.

I'll see if I can tweak the zcml instead of using removeSecurityProxy

Thanks.

On 03/12/10 15:01, Steve Kowalik wrote:
> Review: Approve code*
> This looks great! My only concerns are the lint errors, and the (it seems to me) excessive use of removeSecurityProxy.

Revision history for this message
Ian Booth (wallyworld) wrote :

I had a look at the 3 usages of removeSecurityProxy in the new code:

The ones in createRelatedBranches are required:

naked_product is required for ICanHasLinkedBranch(naked_product)
sourcepackage is required for ICanHasLinkedBranch(naked_sourcepackage)

In checkRelatedBranches:

naked_branch is not required so has been removed

On 03/12/10 15:07, Ian Booth wrote:
> lint errors (almost all line length) were there already and since this
> code base is relatively new, I figured I'd leave well enough alone.
>
> I'll see if I can tweak the zcml instead of using removeSecurityProxy
>
> Thanks.
>
> On 03/12/10 15:01, Steve Kowalik wrote:
>> Review: Approve code*
>> This looks great! My only concerns are the lint errors, and the (it seems to me) excessive use of removeSecurityProxy.

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

I have two issues with this branch.

1) What do we show when we can't find any related branches?

2) Creating a widget is overkill for what is needed, which is some text on the page.

review: Needs Fixing
Revision history for this message
Ian Booth (wallyworld) wrote :

On 06/12/10 14:22, Tim Penhey wrote:
> Review: Needs Fixing
> I have two issues with this branch.
>
> 1) What do we show when we can't find any related branches?
>

Nothing. The page appears as it does now before the branch. Do you want
some text to say something like "No related branches"? I didn't think
that was needed but can easily add it.

> 2) Creating a widget is overkill for what is needed, which is some text on the page.

The page template uses this construct to render the form:

<div metal:use-macro="context/@@launchpad_form/form" />

The first implementation this branch involved replacing the above with a
macro:

<div metal:use-macro="context/@@+sourcepackagerecipe-macros/recipe-form"/>

which:

- expanded the default metal form definition to explicitly list the
required form fields to display:
- adds the related branches markup after all the form fields are specified

eg

<div metal:use-macro="context/@@launchpad_form/form">
  <tal:product-name define="widget nocall:widgets/name">
    <metal:block use-macro="context/@@launchpad_form/widget_row" />
  </tal:product-name>
  <tal:product-name define="widget nocall:widgets/description">
    <metal:block use-macro="context/@@launchpad_form/widget_row" />
  </tal:product-name>
  <another form field...>
  <another form field...>

  <!-- markup for related branches goes here -->
  <table>
  xxxx
  </table>

</div>

The macro was used for the add and edit page templates. Because the edit
form used the generic edit template, a new sourcepackagerecipe-edit page
template was created.

I thought that expanding the default form element and explicitly listing
the form fields in the page template was perhaps not the best approach
since a change to the form schema would require editing several
different files. The current implementation programatically adds the
custom field used to display the related branches to the form schema and
is more robust to changes in the underlying form.

I can revert to this approach is that's considered better.

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

On Tue, 07 Dec 2010 03:11:15 you wrote:
> On 06/12/10 14:22, Tim Penhey wrote:
> > Review: Needs Fixing
> > I have two issues with this branch.
> >
> > 1) What do we show when we can't find any related branches?
>
> Nothing. The page appears as it does now before the branch. Do you want
> some text to say something like "No related branches"? I didn't think
> that was needed but can easily add it.

No, I think that nothing is better. I take it that the fieldset isn't rendered
at all when there are no branches?

> > 2) Creating a widget is overkill for what is needed, which is some text
> > on the page.
>
> The page template uses this construct to render the form:
>
> <div metal:use-macro="context/@@launchpad_form/form" />
>
> The first implementation this branch involved replacing the above with a
> macro:
>
> <div metal:use-macro="context/@@+sourcepackagerecipe-macros/recipe-form"/>

There is a simpler way, which I happen to have done with my "ppa creation
during new recipe" branch, which does expand all the rendered widgets.

The form macro defines a number of slots which can be replaced. However with
my new change (which I know you didn't know about), it is easier to just add a
simple rendering, and have the methods to get the branches on the Add view
class. Perhaps you might want to merge my branch and make yours dependent on
it?

This is one of the problems of having multiple people hacking on the same
area.

Revision history for this message
Ian Booth (wallyworld) wrote :

>>>
>>> 1) What do we show when we can't find any related branches?
>>
>> Nothing. The page appears as it does now before the branch. Do you want
>> some text to say something like "No related branches"? I didn't think
>> that was needed but can easily add it.
>
> No, I think that nothing is better. I take it that the fieldset isn't rendered
> at all when there are no branches?
>

Correct. There's a tal condition wrapping the fieldset.

>
> There is a simpler way, which I happen to have done with my "ppa creation
> during new recipe" branch, which does expand all the rendered widgets.
>
> The form macro defines a number of slots which can be replaced. However with
> my new change (which I know you didn't know about), it is easier to just add a
> simple rendering, and have the methods to get the branches on the Add view
> class. Perhaps you might want to merge my branch and make yours dependent on
> it?
>
> This is one of the problems of having multiple people hacking on the same
> area.

Thanks for the info. I'll use the new code to improve my implementation.

Revision history for this message
Ian Booth (wallyworld) wrote :

>
>>> 2) Creating a widget is overkill for what is needed, which is some text
>>> on the page.
>>
>> The page template uses this construct to render the form:
>>
>> <div metal:use-macro="context/@@launchpad_form/form" />
>>
>> The first implementation this branch involved replacing the above with a
>> macro:
>>
>> <div metal:use-macro="context/@@+sourcepackagerecipe-macros/recipe-form"/>
>
> There is a simpler way, which I happen to have done with my "ppa creation
> during new recipe" branch, which does expand all the rendered widgets.

The custom widget is the way to do it with least code changes. It's not
just a simple field rendering as is the case with the ppa stuff -
there's an entire page template file which renders the tables and links
etc. Using another method eg a macro involves editing zcml and adding an
extra page template for the edit form etc. So I'd like to land it as is
if possible.

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

Can you move the functionality in the render method into the setupWidgets?

Apart from that, you are good to go.

review: Approve
Revision history for this message
Ian Booth (wallyworld) wrote :

> Can you move the functionality in the render method into the setupWidgets?
>
> Apart from that, you are good to go.

Done

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
--- lib/lp/code/browser/sourcepackagerecipe.py 2010-12-17 04:48:01 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py 2010-12-20 00:01:04 +0000
@@ -14,6 +14,7 @@
14 'SourcePackageRecipeView',14 'SourcePackageRecipeView',
15 ]15 ]
1616
17from operator import attrgetter
1718
18from bzrlib.plugins.builder.recipe import (19from bzrlib.plugins.builder.recipe import (
19 ForbiddenInstructionError,20 ForbiddenInstructionError,
@@ -24,6 +25,9 @@
24from lazr.lifecycle.snapshot import Snapshot25from lazr.lifecycle.snapshot import Snapshot
25from lazr.restful.interface import use_template26from lazr.restful.interface import use_template
26from storm.locals import Store27from storm.locals import Store
28from z3c.ptcompat import ViewPageTemplateFile
29from zope.app.form.browser.widget import Widget
30from zope.app.form.interfaces import IView
27from zope.component import getUtility31from zope.component import getUtility
28from zope.event import notify32from zope.event import notify
29from zope.formlib import form33from zope.formlib import form
@@ -33,6 +37,7 @@
33 providedBy,37 providedBy,
34 )38 )
35from zope.schema import (39from zope.schema import (
40 Field,
36 Choice,41 Choice,
37 List,42 List,
38 Text,43 Text,
@@ -75,10 +80,12 @@
75 )80 )
76from lp.code.errors import (81from lp.code.errors import (
77 BuildAlreadyPending,82 BuildAlreadyPending,
83 NoLinkedBranch,
78 NoSuchBranch,84 NoSuchBranch,
79 PrivateBranchRecipe,85 PrivateBranchRecipe,
80 TooNewRecipeFormat,86 TooNewRecipeFormat,
81 )87 )
88from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
82from lp.code.interfaces.sourcepackagerecipe import (89from lp.code.interfaces.sourcepackagerecipe import (
83 ISourcePackageRecipe,90 ISourcePackageRecipe,
84 ISourcePackageRecipeSource,91 ISourcePackageRecipeSource,
@@ -88,6 +95,7 @@
88 ISourcePackageRecipeBuildSource,95 ISourcePackageRecipeBuildSource,
89 )96 )
90from lp.registry.interfaces.pocket import PackagePublishingPocket97from lp.registry.interfaces.pocket import PackagePublishingPocket
98from lp.services.propertycache import cachedproperty
91from lp.soyuz.model.archive import Archive99from lp.soyuz.model.archive import Archive
92100
93101
@@ -360,7 +368,95 @@
360 self.setFieldError('recipe_text', str(error))368 self.setFieldError('recipe_text', str(error))
361369
362370
363class SourcePackageRecipeAddView(RecipeTextValidatorMixin, LaunchpadFormView):371class RelatedBranchesWidget(Widget):
372 """A widget to render the related branches for a recipe."""
373 implements(IView)
374
375 __call__ = ViewPageTemplateFile(
376 '../templates/sourcepackagerecipe-related-branches.pt')
377
378 related_package_branches = []
379 related_series_branches = []
380
381 def hasInput(self):
382 return True
383
384 def setRenderedValue(self, value):
385 self.related_package_branches = value['related_package_branches']
386 self.related_series_branches = value['related_series_branches']
387
388
389class RecipeRelatedBranchesMixin(LaunchpadFormView):
390 """A class to find related branches for a recipe's base branch."""
391
392 custom_widget('related-branches', RelatedBranchesWidget)
393
394 def extendFields(self):
395 """See `LaunchpadFormView`.
396
397 Adds a related branches field to the form.
398 """
399 self.form_fields += form.Fields(Field(__name__='related-branches'))
400 self.form_fields['related-branches'].custom_widget = (
401 self.custom_widgets['related-branches'])
402 self.widget_errors['related-branches'] = ''
403
404 def setUpWidgets(self, context=None):
405 # Adds a new related branches widget.
406 super(RecipeRelatedBranchesMixin, self).setUpWidgets(context)
407 self.widgets['related-branches'].display_label = False
408 self.widgets['related-branches'].setRenderedValue(dict(
409 related_package_branches=self.related_package_branches,
410 related_series_branches=self.related_series_branches))
411
412 @cachedproperty
413 def related_series_branches(self):
414 """Find development branches related to the base branch's product."""
415 result = set()
416 branch_to_check = self.getBranch()
417 product = branch_to_check.product
418
419 # We include the development focus branch.
420 dev_focus_branch = ICanHasLinkedBranch(product).branch
421 if dev_focus_branch is not None:
422 result.add(dev_focus_branch)
423
424 # Now any branches for the product's series.
425 for series in product.series:
426 try:
427 branch = ICanHasLinkedBranch(series).branch
428 if branch is not None:
429 result.add(branch)
430 except NoLinkedBranch:
431 # If there's no branch for a particular series, we don't care.
432 pass
433 # We don't want to include the source branch.
434 result.discard(branch_to_check)
435 return sorted(result, key=attrgetter('unique_name'))
436
437 @cachedproperty
438 def related_package_branches(self):
439 """Find branches for the base branch's product's distro src pkgs."""
440 result = set()
441 branch_to_check = self.getBranch()
442 product = branch_to_check.product
443
444 for sourcepackage in product.distrosourcepackages:
445 try:
446 branch = ICanHasLinkedBranch(sourcepackage).branch
447 if branch is not None:
448 result.add(branch)
449 except NoLinkedBranch:
450 # If there's no branch for a particular source package,
451 # we don't care.
452 pass
453 # We don't want to include the source branch.
454 result.discard(branch_to_check)
455 return sorted(result, key=attrgetter('unique_name'))
456
457
458class SourcePackageRecipeAddView(RecipeRelatedBranchesMixin,
459 RecipeTextValidatorMixin, LaunchpadFormView):
364 """View for creating Source Package Recipes."""460 """View for creating Source Package Recipes."""
365461
366 title = label = 'Create a new source package recipe'462 title = label = 'Create a new source package recipe'
@@ -390,6 +486,10 @@
390 # all confused when we want to create a new PPA.486 # all confused when we want to create a new PPA.
391 archive_widget._displayItemForMissingValue = False487 archive_widget._displayItemForMissingValue = False
392488
489 def getBranch(self):
490 """The branch on which the recipe is built."""
491 return self.context
492
393 @property493 @property
394 def initial_values(self):494 def initial_values(self):
395 return {495 return {
@@ -461,10 +561,15 @@
461 self.setFieldError('ppa_name', error)561 self.setFieldError('ppa_name', error)
462562
463563
464class SourcePackageRecipeEditView(RecipeTextValidatorMixin,564class SourcePackageRecipeEditView(RecipeRelatedBranchesMixin,
565 RecipeTextValidatorMixin,
465 LaunchpadEditFormView):566 LaunchpadEditFormView):
466 """View for editing Source Package Recipes."""567 """View for editing Source Package Recipes."""
467568
569 def getBranch(self):
570 """The branch on which the recipe is built."""
571 return self.context.base_branch
572
468 @property573 @property
469 def title(self):574 def title(self):
470 return 'Edit %s source package recipe' % self.context.name575 return 'Edit %s source package recipe' % self.context.name
471576
=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2010-12-13 04:33:22 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2010-12-20 00:01:04 +0000
@@ -7,10 +7,12 @@
7__metaclass__ = type7__metaclass__ = type
88
99
10from BeautifulSoup import BeautifulSoup
10from datetime import (11from datetime import (
11 datetime,12 datetime,
12 timedelta,13 timedelta,
13 )14 )
15from operator import attrgetter
14from textwrap import dedent16from textwrap import dedent
1517
16from mechanize import LinkNotFoundError18from mechanize import LinkNotFoundError
@@ -28,6 +30,7 @@
28 get_radio_button_text_for_field,30 get_radio_button_text_for_field,
29 )31 )
30from canonical.launchpad.webapp import canonical_url32from canonical.launchpad.webapp import canonical_url
33from canonical.launchpad.webapp.interfaces import ILaunchpadRoot
31from canonical.testing.layers import (34from canonical.testing.layers import (
32 DatabaseFunctionalLayer,35 DatabaseFunctionalLayer,
33 LaunchpadFunctionalLayer,36 LaunchpadFunctionalLayer,
@@ -40,15 +43,18 @@
40from lp.code.browser.sourcepackagerecipebuild import (43from lp.code.browser.sourcepackagerecipebuild import (
41 SourcePackageRecipeBuildView,44 SourcePackageRecipeBuildView,
42 )45 )
46from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
43from lp.code.interfaces.sourcepackagerecipe import MINIMAL_RECIPE_TEXT47from lp.code.interfaces.sourcepackagerecipe import MINIMAL_RECIPE_TEXT
44from lp.code.tests.helpers import recipe_parser_newest_version48from lp.code.tests.helpers import recipe_parser_newest_version
45from lp.registry.interfaces.person import TeamSubscriptionPolicy49from lp.registry.interfaces.person import TeamSubscriptionPolicy
46from lp.registry.interfaces.pocket import PackagePublishingPocket50from lp.registry.interfaces.pocket import PackagePublishingPocket
47from lp.services.propertycache import clear_property_cache51from lp.services.propertycache import clear_property_cache
48from lp.soyuz.model.processor import ProcessorFamily52from lp.soyuz.model.processor import ProcessorFamily
53from lp.testing.views import create_initialized_view
49from lp.testing import (54from lp.testing import (
50 ANONYMOUS,55 ANONYMOUS,
51 BrowserTestCase,56 BrowserTestCase,
57 celebrity_logged_in,
52 login,58 login,
53 person_logged_in,59 person_logged_in,
54 )60 )
@@ -86,6 +92,139 @@
86 ' Secret Squirrel changes.', branches=[cake_branch],92 ' Secret Squirrel changes.', branches=[cake_branch],
87 daily_build_archive=self.ppa)93 daily_build_archive=self.ppa)
8894
95 def createRelatedBranches(self, base_branch=None, nr_series_branches=5,
96 nr_package_branches=5):
97 """Create a recipe base branch and some others associated with it.
98 The other branches are:
99 - series branches: a set of branches associated with product
100 series of the same product as the recipe base branch.
101 - package branches: a set of branches associated with packagesource
102 entities of the same product as the recipe base branch.
103 """
104 related_series_branches = set()
105 related_package_branches = set()
106 if base_branch is None:
107 naked_product = removeSecurityProxy(
108 self.factory.makeProduct(owner=self.chef))
109 else:
110 naked_product = removeSecurityProxy(base_branch.product)
111
112 if nr_series_branches > 0:
113 # Add a development branch
114 naked_product.development_focus.name = 'trunk'
115 devel_branch = self.factory.makeProductBranch(
116 product=naked_product, name='trunk', owner=self.chef)
117 linked_branch = ICanHasLinkedBranch(naked_product)
118 linked_branch.setBranch(devel_branch)
119 related_series_branches.add(devel_branch)
120
121 # Add some product series
122 for x in range(nr_series_branches-1):
123 branch = self.factory.makeBranch(
124 product=naked_product, owner=self.chef)
125 self.factory.makeProductSeries(
126 product=naked_product, branch=branch)
127 related_series_branches.add(branch)
128
129 if nr_package_branches > 0:
130 distro = self.factory.makeDistribution()
131 distroseries = self.factory.makeDistroSeries(
132 distribution=distro)
133
134 for x in range(nr_package_branches):
135 sourcepackagename = self.factory.makeSourcePackageName()
136
137 suitesourcepackage = self.factory.makeSuiteSourcePackage(
138 sourcepackagename=sourcepackagename,
139 distroseries=distroseries,
140 pocket=PackagePublishingPocket.RELEASE)
141 naked_sourcepackage = removeSecurityProxy(
142 suitesourcepackage)
143
144 branch = self.factory.makePackageBranch(
145 owner=self.chef, sourcepackagename=sourcepackagename,
146 distroseries=distroseries)
147 linked_branch = ICanHasLinkedBranch(naked_sourcepackage)
148 with celebrity_logged_in('admin'):
149 linked_branch.setBranch(branch, self.chef)
150
151 series = self.factory.makeProductSeries(
152 product=naked_product)
153 self.factory.makePackagingLink(
154 distroseries=distroseries, productseries=series,
155 sourcepackagename=sourcepackagename)
156 related_package_branches.add(branch)
157
158 if base_branch is None:
159 # Create the 'source' branch ie the base branch of a recipe.
160 base_branch = self.factory.makeProductBranch(
161 product=naked_product)
162 return (
163 base_branch,
164 sorted(related_series_branches, key=attrgetter('unique_name')),
165 sorted(related_package_branches, key=attrgetter('unique_name')))
166
167 def checkRelatedBranches(self, related_series_branches,
168 related_package_branches, browser_contents):
169 """Check that the browser contents contain the correct branch info."""
170 login(ANONYMOUS)
171 soup = BeautifulSoup(browser_contents)
172
173 # The related branches collapsible section needs to be there.
174 related_branches = soup.find('fieldset', {'id': 'related-branches'})
175 self.assertIsNot(related_branches, None)
176
177 # Check the related package branches.
178 branch_table = soup.find(
179 'table', {'id': 'related-package-branches-listing'})
180 rows = branch_table.tbody.findAll('tr')
181
182 package_branches_info = []
183 root_url = canonical_url(
184 getUtility(ILaunchpadRoot), rootsite='code')
185 root_url = root_url.rstrip('/')
186 for row in rows:
187 branch_links = row.findAll('a')
188 self.assertEqual(2, len(branch_links))
189 package_branches_info.append(
190 '%s%s' % (root_url, branch_links[0]['href']))
191 package_branches_info.append(branch_links[0].renderContents())
192 package_branches_info.append(
193 '%s%s' % (root_url, branch_links[1]['href']))
194 package_branches_info.append(branch_links[1].renderContents())
195 expected_branch_info = []
196 for branch in related_package_branches:
197 expected_branch_info.append(
198 canonical_url(branch, rootsite='code'))
199 expected_branch_info.append(branch.displayname)
200 expected_branch_info.append(
201 canonical_url(branch.sourcepackage, rootsite='code'))
202 expected_branch_info.append(branch.sourcepackage.name)
203 self.assertEqual(package_branches_info, expected_branch_info)
204
205 # Check the related series branches.
206 branch_table = soup.find(
207 'table', {'id': 'related-series-branches-listing'})
208 rows = branch_table.tbody.findAll('tr')
209
210 series_branches_info = []
211 for row in rows:
212 branch_links = row.findAll('a')
213 self.assertEqual(2, len(branch_links))
214 series_branches_info.append(
215 '%s%s' % (root_url, branch_links[0]['href']))
216 series_branches_info.append(branch_links[0].renderContents())
217 series_branches_info.append(branch_links[1]['href'])
218 series_branches_info.append(branch_links[1].renderContents())
219 expected_branch_info = []
220 for branch in related_series_branches:
221 expected_branch_info.append(
222 canonical_url(branch, rootsite='code'))
223 expected_branch_info.append(branch.displayname)
224 expected_branch_info.append(canonical_url(branch.owner))
225 expected_branch_info.append(branch.owner.displayname)
226 self.assertEqual(series_branches_info, expected_branch_info)
227
89228
90def get_message_text(browser, index):229def get_message_text(browser, index):
91 """Return the text of a message, specified by index."""230 """Return the text of a message, specified by index."""
@@ -336,8 +475,8 @@
336 browser.getControl('Automatically build each day').click()475 browser.getControl('Automatically build each day').click()
337 browser.getControl('Create Recipe').click()476 browser.getControl('Create Recipe').click()
338 self.assertEqual(477 self.assertEqual(
339 extract_text(find_tags_by_class(browser.contents, 'message')[2]),478 'You must specify at least one series for daily builds.',
340 'You must specify at least one series for daily builds.')479 get_message_text(browser, 2))
341480
342 def test_create_recipe_bad_base_branch(self):481 def test_create_recipe_bad_base_branch(self):
343 # If a user tries to create source package recipe with a bad base482 # If a user tries to create source package recipe with a bad base
@@ -414,6 +553,72 @@
414 get_message_text(browser, 2),553 get_message_text(browser, 2),
415 'Recipe may not refer to private branch: %s' % bzr_identity)554 'Recipe may not refer to private branch: %s' % bzr_identity)
416555
556 def test_new_recipe_with_no_related_branches(self):
557 # The Related Branches section should not appear if there are no
558 # related branches.
559 branch = self.makeBranch()
560 # A new recipe can be created from the branch page.
561 browser = self.getUserBrowser(canonical_url(branch), user=self.chef)
562 # There shouldn't be a related-branches section if there are no
563 # related branches..
564 soup = BeautifulSoup(browser.contents)
565 related_branches = soup.find('fieldset', {'id': 'related-branches'})
566 self.assertIs(related_branches, None)
567
568 def test_new_recipe_with_package_branches(self):
569 # The series branches table should not appear if there are none.
570 (branch, related_series_branches, related_package_branches) = (
571 self.createRelatedBranches(nr_series_branches=0))
572 browser = self.getUserBrowser(
573 canonical_url(branch, view_name='+new-recipe'), user=self.chef)
574 soup = BeautifulSoup(browser.contents)
575 related_branches = soup.find('fieldset', {'id': 'related-branches'})
576 self.assertIsNot(related_branches, None)
577 related_branches = soup.find(
578 'table', {'id': 'related-package-branches-listing'})
579 self.assertIsNot(related_branches, None)
580 related_branches = soup.find(
581 'table', {'id': 'related-series-branches-listing'})
582 self.assertIs(related_branches, None)
583
584 def test_new_recipe_with_series_branches(self):
585 # The package branches table should not appear if there are none.
586 (branch, related_series_branches, related_package_branches) = (
587 self.createRelatedBranches(nr_package_branches=0))
588 browser = self.getUserBrowser(
589 canonical_url(branch, view_name='+new-recipe'), user=self.chef)
590 soup = BeautifulSoup(browser.contents)
591 related_branches = soup.find('fieldset', {'id': 'related-branches'})
592 self.assertIsNot(related_branches, None)
593 related_branches = soup.find(
594 'table', {'id': 'related-series-branches-listing'})
595 self.assertIsNot(related_branches, None)
596 related_branches = soup.find(
597 'table', {'id': 'related-package-branches-listing'})
598 self.assertIs(related_branches, None)
599
600 def test_new_recipe_view_related_branches(self):
601 # The view class should provide the expected related branches for
602 # rendering.
603 (branch, related_series_branches,
604 related_package_branches) = self.createRelatedBranches()
605 with person_logged_in(self.chef):
606 view = create_initialized_view(branch, "+new-recipe")
607 self.assertEqual(
608 related_series_branches, view.related_series_branches)
609 self.assertEqual(
610 related_package_branches, view.related_package_branches)
611
612 def test_new_recipe_with_related_branches(self):
613 # The related branches should be rendered correctly on the page.
614 (branch, related_series_branches,
615 related_package_branches) = self.createRelatedBranches()
616 browser = self.getUserBrowser(
617 canonical_url(branch, view_name='+new-recipe'), user=self.chef)
618 self.checkRelatedBranches(
619 related_series_branches, related_package_branches,
620 browser.contents)
621
417 def test_ppa_selector_not_shown_if_user_has_no_ppas(self):622 def test_ppa_selector_not_shown_if_user_has_no_ppas(self):
418 # If the user creating a recipe has no existing PPAs, the selector623 # If the user creating a recipe has no existing PPAs, the selector
419 # isn't shown, but the field to enter a new PPA name is.624 # isn't shown, but the field to enter a new PPA name is.
@@ -817,6 +1022,81 @@
817 get_message_text(browser, 1),1022 get_message_text(browser, 1),
818 'Recipe may not refer to private branch: %s' % bzr_identity)1023 'Recipe may not refer to private branch: %s' % bzr_identity)
8191024
1025 def test_edit_recipe_with_no_related_branches(self):
1026 # The Related Branches section should not appear if there are no
1027 # related branches.
1028 recipe = self.factory.makeSourcePackageRecipe(owner=self.chef)
1029 browser = self.getUserBrowser(canonical_url(recipe), user=self.chef)
1030 browser.getLink('Edit recipe').click()
1031 # There shouldn't be a related-branches section if there are no
1032 # related branches.
1033 soup = BeautifulSoup(browser.contents)
1034 related_branches = soup.find('fieldset', {'id': 'related-branches'})
1035 self.assertIs(related_branches, None)
1036
1037 def test_edit_recipe_with_package_branches(self):
1038 # The series branches table should not appear if there are none.
1039 with person_logged_in(self.chef):
1040 recipe = self.factory.makeSourcePackageRecipe(owner=self.chef)
1041 self.createRelatedBranches(
1042 base_branch=recipe.base_branch, nr_series_branches=0)
1043 browser = self.getUserBrowser(canonical_url(recipe), user=self.chef)
1044 browser.getLink('Edit recipe').click()
1045 soup = BeautifulSoup(browser.contents)
1046 related_branches = soup.find('fieldset', {'id': 'related-branches'})
1047 self.assertIsNot(related_branches, None)
1048 related_branches = soup.find(
1049 'table', {'id': 'related-package-branches-listing'})
1050 self.assertIsNot(related_branches, None)
1051 related_branches = soup.find(
1052 'table', {'id': 'related-series-branches-listing'})
1053 self.assertIs(related_branches, None)
1054
1055 def test_edit_recipe_with_series_branches(self):
1056 # The package branches table should not appear if there are none.
1057 with person_logged_in(self.chef):
1058 recipe = self.factory.makeSourcePackageRecipe(owner=self.chef)
1059 self.createRelatedBranches(
1060 base_branch=recipe.base_branch, nr_package_branches=0)
1061 browser = self.getUserBrowser(canonical_url(recipe), user=self.chef)
1062 browser.getLink('Edit recipe').click()
1063 soup = BeautifulSoup(browser.contents)
1064 related_branches = soup.find('fieldset', {'id': 'related-branches'})
1065 self.assertIsNot(related_branches, None)
1066 related_branches = soup.find(
1067 'table', {'id': 'related-series-branches-listing'})
1068 self.assertIsNot(related_branches, None)
1069 related_branches = soup.find(
1070 'table', {'id': 'related-package-branches-listing'})
1071 self.assertIs(related_branches, None)
1072
1073 def test_edit_recipe_view_related_branches(self):
1074 # The view class should provide the expected related branches for
1075 # rendering.
1076 with person_logged_in(self.chef):
1077 recipe = self.factory.makeSourcePackageRecipe(owner=self.chef)
1078 (branch, related_series_branches,
1079 related_package_branches) = self.createRelatedBranches(
1080 base_branch=recipe.base_branch)
1081 view = create_initialized_view(recipe, "+edit")
1082 self.assertEqual(
1083 related_series_branches, view.related_series_branches)
1084 self.assertEqual(
1085 related_package_branches, view.related_package_branches)
1086
1087 def test_edit_recipe_with_related_branches(self):
1088 # The related branches should be rendered correctly on the page.
1089 with person_logged_in(self.chef):
1090 recipe = self.factory.makeSourcePackageRecipe(owner=self.chef)
1091 (branch, related_series_branches,
1092 related_package_branches) = self.createRelatedBranches(
1093 base_branch=recipe.base_branch)
1094 browser = self.getUserBrowser(
1095 canonical_url(recipe, view_name='+edit'), user=self.chef)
1096 self.checkRelatedBranches(
1097 related_series_branches, related_package_branches,
1098 browser.contents)
1099
8201100
821class TestSourcePackageRecipeView(TestCaseForRecipe):1101class TestSourcePackageRecipeView(TestCaseForRecipe):
8221102
8231103
=== modified file 'lib/lp/code/templates/sourcepackagerecipe-new.pt'
--- lib/lp/code/templates/sourcepackagerecipe-new.pt 2010-12-06 01:28:24 +0000
+++ lib/lp/code/templates/sourcepackagerecipe-new.pt 2010-12-20 00:01:04 +0000
@@ -104,6 +104,9 @@
104 <tal:widget define="widget nocall:view/widgets/recipe_text">104 <tal:widget define="widget nocall:view/widgets/recipe_text">
105 <metal:block use-macro="context/@@launchpad_form/widget_row" />105 <metal:block use-macro="context/@@launchpad_form/widget_row" />
106 </tal:widget>106 </tal:widget>
107 <tal:widget define="widget nocall:view/widgets/related-branches">
108 <metal:block use-macro="context/@@launchpad_form/widget_row" />
109 </tal:widget>
107110
108 </table>111 </table>
109 </metal:formbody>112 </metal:formbody>
110113
=== added file 'lib/lp/code/templates/sourcepackagerecipe-related-branches.pt'
--- lib/lp/code/templates/sourcepackagerecipe-related-branches.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/code/templates/sourcepackagerecipe-related-branches.pt 2010-12-20 00:01:04 +0000
@@ -0,0 +1,66 @@
1<fieldset id="related-branches"
2 class="collapsible collapsed"
3 tal:define="seriesBranches view/related_series_branches;
4 packageBranches view/related_package_branches"
5 tal:condition="python: seriesBranches or packageBranches">
6 <legend>Related Branches</legend>
7 <table class="extra-options">
8
9 <table class="listing" id="related-package-branches-listing"
10 tal:condition="packageBranches">
11 <thead>
12 <tr>
13 <th>Source Package Branches</th>
14 <th>Source Package</th>
15 </tr>
16 </thead>
17 <tbody>
18 <tr tal:repeat="branch packageBranches">
19 <td width="60%">
20 <a href="#"
21 tal:content="branch/displayname"
22 tal:attributes="href branch/fmt:url">
23 source package branch
24 </a>
25 </td>
26 <td>
27 <a href="#"
28 tal:attributes="href branch/sourcepackage/fmt:url"
29 tal:content="branch/sourcepackagename/name">
30 source package
31 </a>
32 </td>
33 </tr>
34 </tbody>
35 </table>
36
37 <table class="listing" id="related-series-branches-listing"
38 tal:condition="seriesBranches">
39 <thead>
40 <tr>
41 <th>Product Series Branches</th>
42 <th>Owner</th>
43 </tr>
44 </thead>
45 <tbody>
46 <tr tal:repeat="branch seriesBranches">
47 <td width="60%">
48 <a href="#"
49 tal:content="branch/displayname"
50 tal:attributes="href branch/fmt:url">
51 product series branch
52 </a>
53 </td>
54 <td>
55 <a href="#"
56 tal:content="branch/owner/displayname"
57 tal:attributes="href branch/owner/fmt:url">
58 owner
59 </a>
60 </td>
61 </tr>
62 </tbody>
63 </table>
64
65 </table>
66</fieldset>