Merge lp:~rockstar/launchpad/no-recipes-on-private-branches into lp:launchpad

[Paul Hummer]

This branch fixes bug #612567 - It doesn't show the "Create recipe" link on private branches.

[Curtis Hovey (sinzui)]

I would have used
    login_person(self.chef)
or removed the need for logout using
    with person_logged_in(self.chef):

Also you changed the menu, but test the template; I would have tested the view. I was puzzled and saw in the templates:

    <span
      tal:define="link context_menu/create_recipe"
      tal:condition="link/enabled"
      tal:replace="structure link/render"
      />

That is odd, because I think we have a safty net in the formatters for this that ensure a disabled link is never rendered:

    <a tal:replace="context_menu/create_recipe/fmt:link" />

I can see why you tested the template, but should it have an independent rule?

review: Needs Information

[Curtis Hovey (sinzui)]

We spoke on IRC and you explained that we want to switch the link to a JS link and that is not compatible with fmt:link. And since it will be JS, the tests will be in the browser/windmill.

You decided to change the login() call in test setup.

review: Approve (code)

=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py	2010-08-04 04:07:21 +0000
+++ lib/lp/code/browser/branch.py	2010-08-16 20:42:46 +0000
@@ -319,7 +319,10 @@
             '+upgrade', 'Upgrade this branch', icon='edit', enabled=enabled)
 
     def create_recipe(self):
-        enabled = config.build_from_branch.enabled
+        if not self.context.private and config.build_from_branch.enabled:
+            enabled = True
+        else:
+            enabled = False
         text = 'Create packaging recipe'
         return Link('+new-recipe', text, enabled=enabled, icon='add')
 
@@ -564,8 +567,8 @@
     def show_merge_links(self):
         """Return whether or not merge proposal links should be shown.
 
-        Merge proposal links should not be shown if there is only one branch in
-        a non-final state.
+        Merge proposal links should not be shown if there is only one branch
+        in a non-final state.
         """
         if not self.context.target.supports_merge_proposals:
             return False

=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2010-08-09 17:14:10 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2010-08-16 20:42:46 +0000
@@ -136,6 +136,18 @@
         self.assertTextMatchesExpressionIgnoreWhitespace(
             pattern, main_text)
 
+    def test_create_new_recipe_private_branch(self):
+        # Recipes can't be created on private branches.
+        with person_logged_in(self.chef):
+            branch = self.factory.makeBranch(private=True, owner=self.chef)
+            branch_url = canonical_url(branch)
+
+        browser = self.getUserBrowser(branch_url, user=self.chef)
+        self.assertRaises(
+            LinkNotFoundError,
+            browser.getLink,
+            'Create packaging recipe')
+
     def test_create_new_recipe_users_teams_as_owner_options(self):
         # Teams that the user is in are options for the recipe owner.
         self.factory.makeTeam(