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

[Ian Booth]

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

= 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

[Steve Kowalik (stevenk)]

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

review: Approve (code*)

[Ian Booth (wallyworld)]

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.

[Ian Booth (wallyworld)]

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.

[Tim Penhey (thumper)]

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

[Ian Booth (wallyworld)]

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.

[Tim Penhey (thumper)]

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.

[Ian Booth (wallyworld)]

>>>
>>> 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.

[Ian Booth (wallyworld)]

>
>>> 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.

[Tim Penhey (thumper)]

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

Apart from that, you are good to go.

review: Approve

[Ian Booth (wallyworld)]

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

Done

=== 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 @@
     'SourcePackageRecipeView',
     ]
 
+from operator import attrgetter
 
 from bzrlib.plugins.builder.recipe import (
     ForbiddenInstructionError,
@@ -24,6 +25,9 @@
 from lazr.lifecycle.snapshot import Snapshot
 from lazr.restful.interface import use_template
 from storm.locals import Store
+from z3c.ptcompat import ViewPageTemplateFile
+from zope.app.form.browser.widget import Widget
+from zope.app.form.interfaces import IView
 from zope.component import getUtility
 from zope.event import notify
 from zope.formlib import form
@@ -33,6 +37,7 @@
     providedBy,
     )
 from zope.schema import (
+    Field,
     Choice,
     List,
     Text,
@@ -75,10 +80,12 @@
     )
 from lp.code.errors import (
     BuildAlreadyPending,
+    NoLinkedBranch,
     NoSuchBranch,
     PrivateBranchRecipe,
     TooNewRecipeFormat,
     )
+from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
 from lp.code.interfaces.sourcepackagerecipe import (
     ISourcePackageRecipe,
     ISourcePackageRecipeSource,
@@ -88,6 +95,7 @@
     ISourcePackageRecipeBuildSource,
     )
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.services.propertycache import cachedproperty
 from lp.soyuz.model.archive import Archive
 
 
@@ -360,7 +368,95 @@
             self.setFieldError('recipe_text', str(error))
 
 
-class SourcePackageRecipeAddView(RecipeTextValidatorMixin, LaunchpadFormView):
+class RelatedBranchesWidget(Widget):
+    """A widget to render the related branches for a recipe."""
+    implements(IView)
+
+    __call__ = ViewPageTemplateFile(
+        '../templates/sourcepackagerecipe-related-branches.pt')
+
+    related_package_branches = []
+    related_series_branches = []
+
+    def hasInput(self):
+        return True
+
+    def setRenderedValue(self, value):
+        self.related_package_branches = value['related_package_branches']
+        self.related_series_branches = value['related_series_branches']
+
+
+class RecipeRelatedBranchesMixin(LaunchpadFormView):
+    """A class to find related branches for a recipe's base branch."""
+
+    custom_widget('related-branches', RelatedBranchesWidget)
+
+    def extendFields(self):
+        """See `LaunchpadFormView`.
+
+        Adds a related branches field to the form.
+        """
+        self.form_fields += form.Fields(Field(__name__='related-branches'))
+        self.form_fields['related-branches'].custom_widget = (
+            self.custom_widgets['related-branches'])
+        self.widget_errors['related-branches'] = ''
+
+    def setUpWidgets(self, context=None):
+        # Adds a new related branches widget.
+        super(RecipeRelatedBranchesMixin, self).setUpWidgets(context)
+        self.widgets['related-branches'].display_label = False
+        self.widgets['related-branches'].setRenderedValue(dict(
+                related_package_branches=self.related_package_branches,
+                related_series_branches=self.related_series_branches))
+
+    @cachedproperty
+    def related_series_branches(self):
+        """Find development branches related to the base branch's product."""
+        result = set()
+        branch_to_check = self.getBranch()
+        product = branch_to_check.product
+
+        # We include the development focus branch.
+        dev_focus_branch = ICanHasLinkedBranch(product).branch
+        if dev_focus_branch is not None:
+            result.add(dev_focus_branch)
+
+        # Now any branches for the product's series.
+        for series in product.series:
+            try:
+                branch = ICanHasLinkedBranch(series).branch
+                if branch is not None:
+                    result.add(branch)
+            except NoLinkedBranch:
+                # If there's no branch for a particular series, we don't care.
+                pass
+        # We don't want to include the source branch.
+        result.discard(branch_to_check)
+        return sorted(result, key=attrgetter('unique_name'))
+
+    @cachedproperty
+    def related_package_branches(self):
+        """Find branches for the base branch's product's distro src pkgs."""
+        result = set()
+        branch_to_check = self.getBranch()
+        product = branch_to_check.product
+
+        for sourcepackage in product.distrosourcepackages:
+            try:
+                branch = ICanHasLinkedBranch(sourcepackage).branch
+                if branch is not None:
+                    result.add(branch)
+            except NoLinkedBranch:
+                # If there's no branch for a particular source package,
+                # we don't care.
+                pass
+        # We don't want to include the source branch.
+        result.discard(branch_to_check)
+        return sorted(result, key=attrgetter('unique_name'))
+
+
+class SourcePackageRecipeAddView(RecipeRelatedBranchesMixin,
+                                 RecipeTextValidatorMixin, LaunchpadFormView):
     """View for creating Source Package Recipes."""
 
     title = label = 'Create a new source package recipe'
@@ -390,6 +486,10 @@
         # all confused when we want to create a new PPA.
         archive_widget._displayItemForMissingValue = False
 
+    def getBranch(self):
+        """The branch on which the recipe is built."""
+        return self.context
+
     @property
     def initial_values(self):
         return {
@@ -461,10 +561,15 @@
                     self.setFieldError('ppa_name', error)
 
 
-class SourcePackageRecipeEditView(RecipeTextValidatorMixin,
+class SourcePackageRecipeEditView(RecipeRelatedBranchesMixin,
+                                  RecipeTextValidatorMixin,
                                   LaunchpadEditFormView):
     """View for editing Source Package Recipes."""
 
+    def getBranch(self):
+        """The branch on which the recipe is built."""
+        return self.context.base_branch
+
     @property
     def title(self):
         return 'Edit %s source package recipe' % self.context.name

=== 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 @@
 __metaclass__ = type
 
 
+from BeautifulSoup import BeautifulSoup
 from datetime import (
     datetime,
     timedelta,
     )
+from operator import attrgetter
 from textwrap import dedent
 
 from mechanize import LinkNotFoundError
@@ -28,6 +30,7 @@
     get_radio_button_text_for_field,
     )
 from canonical.launchpad.webapp import canonical_url
+from canonical.launchpad.webapp.interfaces import ILaunchpadRoot
 from canonical.testing.layers import (
     DatabaseFunctionalLayer,
     LaunchpadFunctionalLayer,
@@ -40,15 +43,18 @@
 from lp.code.browser.sourcepackagerecipebuild import (
     SourcePackageRecipeBuildView,
     )
+from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
 from lp.code.interfaces.sourcepackagerecipe import MINIMAL_RECIPE_TEXT
 from lp.code.tests.helpers import recipe_parser_newest_version
 from lp.registry.interfaces.person import TeamSubscriptionPolicy
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.services.propertycache import clear_property_cache
 from lp.soyuz.model.processor import ProcessorFamily
+from lp.testing.views import create_initialized_view
 from lp.testing import (
     ANONYMOUS,
     BrowserTestCase,
+    celebrity_logged_in,
     login,
     person_logged_in,
     )
@@ -86,6 +92,139 @@
             ' Secret Squirrel changes.', branches=[cake_branch],
             daily_build_archive=self.ppa)
 
+    def createRelatedBranches(self, base_branch=None, nr_series_branches=5,
+                              nr_package_branches=5):
+        """Create a recipe base branch and some others associated with it.
+        The other branches are:
+          - series branches: a set of branches associated with product
+            series of the same product as the recipe base branch.
+          - package branches: a set of branches associated with packagesource
+            entities of the same product as the recipe base branch.
+        """
+        related_series_branches = set()
+        related_package_branches = set()
+        if base_branch is None:
+            naked_product = removeSecurityProxy(
+                self.factory.makeProduct(owner=self.chef))
+        else:
+            naked_product = removeSecurityProxy(base_branch.product)
+
+        if nr_series_branches > 0:
+            # Add a development branch
+            naked_product.development_focus.name = 'trunk'
+            devel_branch = self.factory.makeProductBranch(
+                product=naked_product, name='trunk', owner=self.chef)
+            linked_branch = ICanHasLinkedBranch(naked_product)
+            linked_branch.setBranch(devel_branch)
+            related_series_branches.add(devel_branch)
+
+            # Add some product series
+            for x in range(nr_series_branches-1):
+                branch = self.factory.makeBranch(
+                    product=naked_product, owner=self.chef)
+                self.factory.makeProductSeries(
+                    product=naked_product, branch=branch)
+                related_series_branches.add(branch)
+
+        if nr_package_branches > 0:
+            distro = self.factory.makeDistribution()
+            distroseries = self.factory.makeDistroSeries(
+                distribution=distro)
+
+            for x in range(nr_package_branches):
+                sourcepackagename = self.factory.makeSourcePackageName()
+
+                suitesourcepackage = self.factory.makeSuiteSourcePackage(
+                    sourcepackagename=sourcepackagename,
+                    distroseries=distroseries,
+                    pocket=PackagePublishingPocket.RELEASE)
+                naked_sourcepackage = removeSecurityProxy(
+                    suitesourcepackage)
+
+                branch = self.factory.makePackageBranch(
+                    owner=self.chef, sourcepackagename=sourcepackagename,
+                    distroseries=distroseries)
+                linked_branch = ICanHasLinkedBranch(naked_sourcepackage)
+                with celebrity_logged_in('admin'):
+                    linked_branch.setBranch(branch, self.chef)
+
+                series = self.factory.makeProductSeries(
+                    product=naked_product)
+                self.factory.makePackagingLink(
+                    distroseries=distroseries, productseries=series,
+                    sourcepackagename=sourcepackagename)
+                related_package_branches.add(branch)
+
+        if base_branch is None:
+            # Create the 'source' branch ie the base branch of a recipe.
+            base_branch = self.factory.makeProductBranch(
+                                            product=naked_product)
+        return (
+            base_branch,
+            sorted(related_series_branches, key=attrgetter('unique_name')),
+            sorted(related_package_branches, key=attrgetter('unique_name')))
+
+    def checkRelatedBranches(self, related_series_branches,
+                             related_package_branches, browser_contents):
+        """Check that the browser contents contain the correct branch info."""
+        login(ANONYMOUS)
+        soup = BeautifulSoup(browser_contents)
+
+        # The related branches collapsible section needs to be there.
+        related_branches = soup.find('fieldset', {'id': 'related-branches'})
+        self.assertIsNot(related_branches, None)
+
+        # Check the related package branches.
+        branch_table = soup.find(
+            'table', {'id': 'related-package-branches-listing'})
+        rows = branch_table.tbody.findAll('tr')
+
+        package_branches_info = []
+        root_url = canonical_url(
+            getUtility(ILaunchpadRoot), rootsite='code')
+        root_url = root_url.rstrip('/')
+        for row in rows:
+            branch_links = row.findAll('a')
+            self.assertEqual(2, len(branch_links))
+            package_branches_info.append(
+                '%s%s' % (root_url, branch_links[0]['href']))
+            package_branches_info.append(branch_links[0].renderContents())
+            package_branches_info.append(
+                '%s%s' % (root_url, branch_links[1]['href']))
+            package_branches_info.append(branch_links[1].renderContents())
+        expected_branch_info = []
+        for branch in related_package_branches:
+            expected_branch_info.append(
+                canonical_url(branch, rootsite='code'))
+            expected_branch_info.append(branch.displayname)
+            expected_branch_info.append(
+                canonical_url(branch.sourcepackage, rootsite='code'))
+            expected_branch_info.append(branch.sourcepackage.name)
+        self.assertEqual(package_branches_info, expected_branch_info)
+
+        # Check the related series branches.
+        branch_table = soup.find(
+            'table', {'id': 'related-series-branches-listing'})
+        rows = branch_table.tbody.findAll('tr')
+
+        series_branches_info = []
+        for row in rows:
+            branch_links = row.findAll('a')
+            self.assertEqual(2, len(branch_links))
+            series_branches_info.append(
+                '%s%s' % (root_url, branch_links[0]['href']))
+            series_branches_info.append(branch_links[0].renderContents())
+            series_branches_info.append(branch_links[1]['href'])
+            series_branches_info.append(branch_links[1].renderContents())
+        expected_branch_info = []
+        for branch in related_series_branches:
+            expected_branch_info.append(
+                canonical_url(branch, rootsite='code'))
+            expected_branch_info.append(branch.displayname)
+            expected_branch_info.append(canonical_url(branch.owner))
+            expected_branch_info.append(branch.owner.displayname)
+        self.assertEqual(series_branches_info, expected_branch_info)
+
 
 def get_message_text(browser, index):
     """Return the text of a message, specified by index."""
@@ -336,8 +475,8 @@
         browser.getControl('Automatically build each day').click()
         browser.getControl('Create Recipe').click()
         self.assertEqual(
-            extract_text(find_tags_by_class(browser.contents, 'message')[2]),
-            'You must specify at least one series for daily builds.')
+            'You must specify at least one series for daily builds.',
+            get_message_text(browser, 2))
 
     def test_create_recipe_bad_base_branch(self):
         # If a user tries to create source package recipe with a bad base
@@ -414,6 +553,72 @@
             get_message_text(browser, 2),
             'Recipe may not refer to private branch: %s' % bzr_identity)
 
+    def test_new_recipe_with_no_related_branches(self):
+        # The Related Branches section should not appear if there are no
+        # related branches.
+        branch = self.makeBranch()
+        # A new recipe can be created from the branch page.
+        browser = self.getUserBrowser(canonical_url(branch), user=self.chef)
+        # There shouldn't be a related-branches section if there are no
+        # related branches..
+        soup = BeautifulSoup(browser.contents)
+        related_branches = soup.find('fieldset', {'id': 'related-branches'})
+        self.assertIs(related_branches, None)
+
+    def test_new_recipe_with_package_branches(self):
+        # The series branches table should not appear if there are none.
+        (branch, related_series_branches, related_package_branches) = (
+            self.createRelatedBranches(nr_series_branches=0))
+        browser = self.getUserBrowser(
+            canonical_url(branch, view_name='+new-recipe'), user=self.chef)
+        soup = BeautifulSoup(browser.contents)
+        related_branches = soup.find('fieldset', {'id': 'related-branches'})
+        self.assertIsNot(related_branches, None)
+        related_branches = soup.find(
+            'table', {'id': 'related-package-branches-listing'})
+        self.assertIsNot(related_branches, None)
+        related_branches = soup.find(
+            'table', {'id': 'related-series-branches-listing'})
+        self.assertIs(related_branches, None)
+
+    def test_new_recipe_with_series_branches(self):
+        # The package branches table should not appear if there are none.
+        (branch, related_series_branches, related_package_branches) = (
+            self.createRelatedBranches(nr_package_branches=0))
+        browser = self.getUserBrowser(
+            canonical_url(branch, view_name='+new-recipe'), user=self.chef)
+        soup = BeautifulSoup(browser.contents)
+        related_branches = soup.find('fieldset', {'id': 'related-branches'})
+        self.assertIsNot(related_branches, None)
+        related_branches = soup.find(
+            'table', {'id': 'related-series-branches-listing'})
+        self.assertIsNot(related_branches, None)
+        related_branches = soup.find(
+            'table', {'id': 'related-package-branches-listing'})
+        self.assertIs(related_branches, None)
+
+    def test_new_recipe_view_related_branches(self):
+        # The view class should provide the expected related branches for
+        # rendering.
+        (branch, related_series_branches,
+            related_package_branches) = self.createRelatedBranches()
+        with person_logged_in(self.chef):
+            view = create_initialized_view(branch, "+new-recipe")
+            self.assertEqual(
+                related_series_branches, view.related_series_branches)
+            self.assertEqual(
+                related_package_branches, view.related_package_branches)
+
+    def test_new_recipe_with_related_branches(self):
+        # The related branches should be rendered correctly on the page.
+        (branch, related_series_branches,
+            related_package_branches) = self.createRelatedBranches()
+        browser = self.getUserBrowser(
+            canonical_url(branch, view_name='+new-recipe'), user=self.chef)
+        self.checkRelatedBranches(
+            related_series_branches, related_package_branches,
+            browser.contents)
+
     def test_ppa_selector_not_shown_if_user_has_no_ppas(self):
         # If the user creating a recipe has no existing PPAs, the selector
         # isn't shown, but the field to enter a new PPA name is.
@@ -817,6 +1022,81 @@
             get_message_text(browser, 1),
             'Recipe may not refer to private branch: %s' % bzr_identity)
 
+    def test_edit_recipe_with_no_related_branches(self):
+        # The Related Branches section should not appear if there are no
+        # related branches.
+        recipe = self.factory.makeSourcePackageRecipe(owner=self.chef)
+        browser = self.getUserBrowser(canonical_url(recipe), user=self.chef)
+        browser.getLink('Edit recipe').click()
+        # There shouldn't be a related-branches section if there are no
+        # related branches.
+        soup = BeautifulSoup(browser.contents)
+        related_branches = soup.find('fieldset', {'id': 'related-branches'})
+        self.assertIs(related_branches, None)
+
+    def test_edit_recipe_with_package_branches(self):
+        # The series branches table should not appear if there are none.
+        with person_logged_in(self.chef):
+            recipe = self.factory.makeSourcePackageRecipe(owner=self.chef)
+            self.createRelatedBranches(
+                    base_branch=recipe.base_branch, nr_series_branches=0)
+        browser = self.getUserBrowser(canonical_url(recipe), user=self.chef)
+        browser.getLink('Edit recipe').click()
+        soup = BeautifulSoup(browser.contents)
+        related_branches = soup.find('fieldset', {'id': 'related-branches'})
+        self.assertIsNot(related_branches, None)
+        related_branches = soup.find(
+            'table', {'id': 'related-package-branches-listing'})
+        self.assertIsNot(related_branches, None)
+        related_branches = soup.find(
+            'table', {'id': 'related-series-branches-listing'})
+        self.assertIs(related_branches, None)
+
+    def test_edit_recipe_with_series_branches(self):
+        # The package branches table should not appear if there are none.
+        with person_logged_in(self.chef):
+            recipe = self.factory.makeSourcePackageRecipe(owner=self.chef)
+            self.createRelatedBranches(
+                    base_branch=recipe.base_branch, nr_package_branches=0)
+        browser = self.getUserBrowser(canonical_url(recipe), user=self.chef)
+        browser.getLink('Edit recipe').click()
+        soup = BeautifulSoup(browser.contents)
+        related_branches = soup.find('fieldset', {'id': 'related-branches'})
+        self.assertIsNot(related_branches, None)
+        related_branches = soup.find(
+            'table', {'id': 'related-series-branches-listing'})
+        self.assertIsNot(related_branches, None)
+        related_branches = soup.find(
+            'table', {'id': 'related-package-branches-listing'})
+        self.assertIs(related_branches, None)
+
+    def test_edit_recipe_view_related_branches(self):
+        # The view class should provide the expected related branches for
+        # rendering.
+        with person_logged_in(self.chef):
+            recipe = self.factory.makeSourcePackageRecipe(owner=self.chef)
+            (branch, related_series_branches,
+                related_package_branches) = self.createRelatedBranches(
+                    base_branch=recipe.base_branch)
+            view = create_initialized_view(recipe, "+edit")
+            self.assertEqual(
+                related_series_branches, view.related_series_branches)
+            self.assertEqual(
+                related_package_branches, view.related_package_branches)
+
+    def test_edit_recipe_with_related_branches(self):
+        # The related branches should be rendered correctly on the page.
+        with person_logged_in(self.chef):
+            recipe = self.factory.makeSourcePackageRecipe(owner=self.chef)
+            (branch, related_series_branches,
+                related_package_branches) = self.createRelatedBranches(
+                    base_branch=recipe.base_branch)
+        browser = self.getUserBrowser(
+            canonical_url(recipe, view_name='+edit'), user=self.chef)
+        self.checkRelatedBranches(
+            related_series_branches, related_package_branches,
+            browser.contents)
+
 
 class TestSourcePackageRecipeView(TestCaseForRecipe):
 

=== 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 @@
             <tal:widget define="widget nocall:view/widgets/recipe_text">
               <metal:block use-macro="context/@@launchpad_form/widget_row" />
             </tal:widget>
+            <tal:widget define="widget nocall:view/widgets/related-branches">
+              <metal:block use-macro="context/@@launchpad_form/widget_row" />
+            </tal:widget>
 
           </table>
         </metal:formbody>

=== 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 @@
+<fieldset id="related-branches"
+              class="collapsible collapsed"
+              tal:define="seriesBranches view/related_series_branches;
+                          packageBranches view/related_package_branches"
+              tal:condition="python: seriesBranches or packageBranches">
+  <legend>Related Branches</legend>
+  <table class="extra-options">
+
+    <table class="listing" id="related-package-branches-listing"
+        tal:condition="packageBranches">
+      <thead>
+        <tr>
+          <th>Source Package Branches</th>
+          <th>Source Package</th>
+        </tr>
+      </thead>
+      <tbody>
+        <tr tal:repeat="branch packageBranches">
+          <td width="60%">
+            <a href="#"
+               tal:content="branch/displayname"
+               tal:attributes="href branch/fmt:url">
+              source package branch
+            </a>
+          </td>
+          <td>
+            <a href="#"
+               tal:attributes="href branch/sourcepackage/fmt:url"
+               tal:content="branch/sourcepackagename/name">
+              source package
+            </a>
+          </td>
+        </tr>
+      </tbody>
+    </table>
+
+    <table class="listing" id="related-series-branches-listing"
+        tal:condition="seriesBranches">
+      <thead>
+        <tr>
+          <th>Product Series Branches</th>
+          <th>Owner</th>
+        </tr>
+      </thead>
+      <tbody>
+        <tr tal:repeat="branch seriesBranches">
+          <td width="60%">
+            <a href="#"
+               tal:content="branch/displayname"
+               tal:attributes="href branch/fmt:url">
+              product series branch
+            </a>
+          </td>
+          <td>
+            <a href="#"
+               tal:content="branch/owner/displayname"
+               tal:attributes="href branch/owner/fmt:url">
+              owner
+            </a>
+          </td>
+        </tr>
+      </tbody>
+    </table>
+
+  </table>
+</fieldset>