launchpad-reviewers team mailing list archive

[Paul Hummer <paul@xxxxxxxxxxxxxxxxxxxx>]

Paul Hummer has proposed merging lp:~rockstar/launchpad/fix-reassign-ownership-of-recipes into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #667886 Editing a source package recipe can accidentally re-assign ownership
  https://bugs.launchpad.net/bugs/667886


This branch removes some widgets that aren't compatible with launchpad.Admin in the SourcePackageRecipe edit flow.  It changes the owner widget to be a lazr-js person picker in the cases where it's an admin editing the recipe.
-- 
https://code.launchpad.net/~rockstar/launchpad/fix-reassign-ownership-of-recipes/+merge/39590
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rockstar/launchpad/fix-reassign-ownership-of-recipes into lp:launchpad/devel.

=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
--- lib/lp/code/browser/sourcepackagerecipe.py	2010-10-25 13:16:10 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py	2010-10-29 00:29:48 +0000
@@ -26,6 +26,7 @@
 from storm.locals import Store
 from zope.component import getUtility
 from zope.event import notify
+from zope.formlib import form
 from zope.interface import (
     implements,
     Interface,
@@ -54,6 +55,7 @@
     stepthrough,
     structured,
     )
+from canonical.launchpad.webapp.authorization import check_permission
 from canonical.launchpad.webapp.breadcrumb import Breadcrumb
 from canonical.widgets.itemswidgets import LabeledMultiCheckBoxWidget
 from lp.code.errors import (
@@ -380,6 +382,26 @@
     schema = ISourcePackageAddEditSchema
     custom_widget('distros', LabeledMultiCheckBoxWidget)
 
+    def setUpFields(self):
+        super(SourcePackageRecipeEditView, self).setUpFields()
+
+        if check_permission('launchpad.Admin', self.context):
+            # Exclude the PPA archive dropdown.
+            self.form_fields = self.form_fields.omit('daily_build_archive')
+
+            owner_field = self.schema['owner']
+            any_owner_choice = Choice(
+                __name__='owner', title=owner_field.title,
+                description=(u"As an administrator you are able to reassign"
+                             u" this branch to any person or team."),
+                required=True, vocabulary='ValidPersonOrTeam')
+            any_owner_field = form.Fields(
+                any_owner_choice, render_context=self.render_context)
+            # Replace the normal owner field with a more permissive vocab.
+            self.form_fields = self.form_fields.omit('owner')
+            self.form_fields = any_owner_field + self.form_fields
+
+
     @property
     def initial_values(self):
         return {

=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2010-10-26 15:47:24 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2010-10-29 00:29:48 +0000
@@ -16,9 +16,11 @@
 from mechanize import LinkNotFoundError
 from pytz import utc
 import transaction
+from zope.component import getUtility
 from zope.security.interfaces import Unauthorized
 from zope.security.proxy import removeSecurityProxy
 
+from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.testing.pages import (
     extract_text,
     find_main_content,
@@ -420,6 +422,66 @@
         self.assertTextMatchesExpressionIgnoreWhitespace(
             pattern, main_text)
 
+    def test_admin_edit(self):
+        self.factory.makeDistroSeries(
+            displayname='Mumbly Midget', name='mumbly',
+            distribution=self.ppa.distribution)
+        product = self.factory.makeProduct(
+            name='ratatouille', displayname='Ratatouille')
+        veggie_branch = self.factory.makeBranch(
+            owner=self.chef, product=product, name='veggies')
+        meat_branch = self.factory.makeBranch(
+            owner=self.chef, product=product, name='meat')
+        recipe = self.factory.makeSourcePackageRecipe(
+            owner=self.chef, registrant=self.chef,
+            name=u'things', description=u'This is a recipe',
+            distroseries=self.squirrel, branches=[veggie_branch],
+            daily_build_archive=self.ppa)
+
+        meat_path = meat_branch.bzr_identity
+        expert = getUtility(ILaunchpadCelebrities).admin.teamowner
+
+        browser = self.getUserBrowser(canonical_url(recipe), user=expert)
+        browser.getLink('Edit recipe').click()
+
+        # There shouldn't be a daily build archive property.
+        self.assertRaises(
+            LookupError,
+            browser.getControl,
+            name='field.daily_build_archive')
+
+        browser.getControl(name='field.name').value = 'fings'
+        browser.getControl('Description').value = 'This is stuff'
+        browser.getControl('Recipe text').value = (
+            MINIMAL_RECIPE_TEXT % meat_path)
+        browser.getControl('Secret Squirrel').click()
+        browser.getControl('Mumbly Midget').click()
+        browser.getControl('Update Recipe').click()
+
+        pattern = """\
+            Master Chef's fings recipe
+            .*
+
+            Description
+            This is stuff
+
+            Recipe information
+            Build schedule: Built on request
+            Owner: Master Chef
+            Base branch: lp://dev/~chef/ratatouille/meat
+            Debian version: 0\+\{revno\}
+            Daily build archive:
+            Secret PPA
+            Distribution series: Mumbly Midget
+            .*
+
+            Recipe contents
+            # bzr-builder format 0.2 deb-version 0\+\{revno\}
+            lp://dev/~chef/ratatouille/meat"""
+        main_text = extract_text(find_main_content(browser.contents))
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            pattern, main_text)
+
     def test_edit_recipe_forbidden_instruction(self):
         self.factory.makeDistroSeries(
             displayname='Mumbly Midget', name='mumbly',