Merge lp:~rockstar/launchpad/edit-recipe-name into lp:launchpad

[Paul Hummer]

This branch fixes bug #591271. The validation method checks to see if a recipe exists with the same owner and the same name. This was fine for new recipes, but broke editing recipes. So I had it check to see if there was an existing recipe AND see if that existing recipe was the current one.

[Leonard Richardson (leonardr)]

I don't see a test case for trying to rename an existing recipe to the name of another existing recipe. Other than that, this looks good.

review: Approve (code)

=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
--- lib/lp/code/browser/sourcepackagerecipe.py	2010-06-08 05:54:36 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py	2010-06-08 18:48:36 +0000
@@ -318,15 +318,6 @@
                 'recipe_text',
                 'The recipe text is not a valid bzr-builder recipe.')
 
-        name = data.get('name', None)
-        if name:
-            SourcePackageRecipeSource = getUtility(ISourcePackageRecipeSource)
-            if SourcePackageRecipeSource.exists(self.user, name):
-                self.setFieldError(
-                    'name',
-                    'There is already a recipe owned by %s with this name.' %
-                        self.user.displayname)
-
 
 class SourcePackageRecipeAddView(RecipeTextValidatorMixin, LaunchpadFormView):
     """View for creating Source Package Recipes."""
@@ -355,6 +346,18 @@
             data['name'], recipe, data['description'])
         self.next_url = canonical_url(source_package_recipe)
 
+    def validate(self, data):
+        super(SourcePackageRecipeAddView, self).validate(data)
+        name = data.get('name', None)
+        owner = data.get('owner', None)
+        if name and owner:
+            SourcePackageRecipeSource = getUtility(ISourcePackageRecipeSource)
+            if SourcePackageRecipeSource.exists(owner, name):
+                self.setFieldError(
+                    'name',
+                    'There is already a recipe owned by %s with this name.' %
+                        owner.displayname)
+
 
 class SourcePackageRecipeEditView(RecipeTextValidatorMixin,
                                   LaunchpadEditFormView):
@@ -417,6 +420,20 @@
         """See `LaunchpadEditFormView`"""
         return {ISourcePackageAddEditSchema: self.context}
 
+    def validate(self, data):
+        super(SourcePackageRecipeEditView, self).validate(data)
+        name = data.get('name', None)
+        owner = data.get('owner', None)
+        if name and owner:
+            SourcePackageRecipeSource = getUtility(ISourcePackageRecipeSource)
+            if SourcePackageRecipeSource.exists(owner, name):
+                recipe = owner.getRecipe(name)
+                if recipe != self.context:
+                    self.setFieldError(
+                        'name',
+                        'There is already a recipe owned by %s with this '
+                        'name.' % owner.displayname)
+
 
 class SourcePackageRecipeDeleteView(LaunchpadFormView):
 

=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2010-06-08 05:07:35 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2010-06-08 18:48:36 +0000
@@ -202,7 +202,6 @@
             owner=self.chef, product=product, name='veggies')
         meat_branch = self.factory.makeBranch(
             owner=self.chef, product=product, name='meat')
-        source_package = self.factory.makeSourcePackage()
         recipe = self.factory.makeSourcePackageRecipe(
             owner=self.chef, registrant=self.chef,
             name=u'things', description=u'This is a recipe',
@@ -241,6 +240,89 @@
         self.assertTextMatchesExpressionIgnoreWhitespace(
             pattern, main_text)
 
+    def test_edit_recipe_already_exists(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])
+        recipe_fings = self.factory.makeSourcePackageRecipe(
+            owner=self.chef, registrant=self.chef,
+            name=u'fings', description=u'This is a recipe',
+            distroseries=self.squirrel, branches=[veggie_branch])
+
+        meat_path = meat_branch.bzr_identity
+
+        browser = self.getUserBrowser(canonical_url(recipe), user=self.chef)
+        browser.getLink('Edit recipe').click()
+        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()
+
+        self.assertEqual(
+            extract_text(find_tags_by_class(browser.contents, 'message')[1]),
+            'There is already a recipe owned by Master Chef with this name.')
+
+    def test_edit_recipe_but_not_name(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])
+
+        meat_path = meat_branch.bzr_identity
+
+        browser = self.getUserBrowser(canonical_url(recipe), user=self.chef)
+        browser.getLink('Edit recipe').click()
+        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 things recipe
+            .*
+
+            Description
+            This is stuff
+
+            Recipe information
+            Owner: Master Chef
+            Base branch: lp://dev/~chef/ratatouille/meat
+            Debian version: 1.0
+            Distribution series: Mumbly Midget
+            .*
+
+            Recipe contents
+            # bzr-builder format 0.2 deb-version 1.0
+            lp://dev/~chef/ratatouille/meat"""
+        main_text = extract_text(find_main_content(browser.contents))
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            pattern, main_text)
+
+
 
 class TestSourcePackageRecipeView(TestCaseForRecipe):