Launchpad itself

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

  1. no-recipes-on-private-branches
  2. Merge into devel
Proposed by Paul Hummer
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 11362
Proposed branch: lp:~rockstar/launchpad/no-recipes-on-private-branches
Merge into: lp:launchpad
Diff against target: 49 lines (+18/-3)
2 files modified
lib/lp/code/browser/branch.py (+6/-3)
lib/lp/code/browser/tests/test_sourcepackagerecipe.py (+12/-0)
To merge this branch: bzr merge lp:~rockstar/launchpad/no-recipes-on-private-branches
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Review via email: mp+32799@code.launchpad.net

Description of the change

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

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

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
Revision history for this message
Curtis Hovey (sinzui) wrote :

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)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
319 '+upgrade', 'Upgrade this branch', icon='edit', enabled=enabled)319 '+upgrade', 'Upgrade this branch', icon='edit', enabled=enabled)
320320
321 def create_recipe(self):321 def create_recipe(self):
322 enabled = config.build_from_branch.enabled322 if not self.context.private and config.build_from_branch.enabled:
323 enabled = True
324 else:
325 enabled = False
323 text = 'Create packaging recipe'326 text = 'Create packaging recipe'
324 return Link('+new-recipe', text, enabled=enabled, icon='add')327 return Link('+new-recipe', text, enabled=enabled, icon='add')
325328
@@ -564,8 +567,8 @@
564 def show_merge_links(self):567 def show_merge_links(self):
565 """Return whether or not merge proposal links should be shown.568 """Return whether or not merge proposal links should be shown.
566569
567 Merge proposal links should not be shown if there is only one branch in570 Merge proposal links should not be shown if there is only one branch
568 a non-final state.571 in a non-final state.
569 """572 """
570 if not self.context.target.supports_merge_proposals:573 if not self.context.target.supports_merge_proposals:
571 return False574 return False
572575
=== 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 @@
136 self.assertTextMatchesExpressionIgnoreWhitespace(136 self.assertTextMatchesExpressionIgnoreWhitespace(
137 pattern, main_text)137 pattern, main_text)
138138
139 def test_create_new_recipe_private_branch(self):
140 # Recipes can't be created on private branches.
141 with person_logged_in(self.chef):
142 branch = self.factory.makeBranch(private=True, owner=self.chef)
143 branch_url = canonical_url(branch)
144
145 browser = self.getUserBrowser(branch_url, user=self.chef)
146 self.assertRaises(
147 LinkNotFoundError,
148 browser.getLink,
149 'Create packaging recipe')
150
139 def test_create_new_recipe_users_teams_as_owner_options(self):151 def test_create_new_recipe_users_teams_as_owner_options(self):
140 # Teams that the user is in are options for the recipe owner.152 # Teams that the user is in are options for the recipe owner.
141 self.factory.makeTeam(153 self.factory.makeTeam(