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

Proposed by Paul Hummer
Status: Merged
Merged at revision: 10974
Proposed branch: lp:~rockstar/launchpad/edit-recipe-name
Merge into: lp:launchpad
Diff against target: 160 lines (+109/-10)
2 files modified
lib/lp/code/browser/sourcepackagerecipe.py (+26/-9)
lib/lp/code/browser/tests/test_sourcepackagerecipe.py (+83/-1)
To merge this branch: bzr merge lp:~rockstar/launchpad/edit-recipe-name
Reviewer Review Type Date Requested Status
Leonard Richardson (community) code Approve
Review via email: mp+27072@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Leonard Richardson (leonardr) wrote :

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)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
2--- lib/lp/code/browser/sourcepackagerecipe.py 2010-06-08 05:54:36 +0000
3+++ lib/lp/code/browser/sourcepackagerecipe.py 2010-06-08 18:48:36 +0000
4@@ -318,15 +318,6 @@
5 'recipe_text',
6 'The recipe text is not a valid bzr-builder recipe.')
7
8- name = data.get('name', None)
9- if name:
10- SourcePackageRecipeSource = getUtility(ISourcePackageRecipeSource)
11- if SourcePackageRecipeSource.exists(self.user, name):
12- self.setFieldError(
13- 'name',
14- 'There is already a recipe owned by %s with this name.' %
15- self.user.displayname)
16-
17
18 class SourcePackageRecipeAddView(RecipeTextValidatorMixin, LaunchpadFormView):
19 """View for creating Source Package Recipes."""
20@@ -355,6 +346,18 @@
21 data['name'], recipe, data['description'])
22 self.next_url = canonical_url(source_package_recipe)
23
24+ def validate(self, data):
25+ super(SourcePackageRecipeAddView, self).validate(data)
26+ name = data.get('name', None)
27+ owner = data.get('owner', None)
28+ if name and owner:
29+ SourcePackageRecipeSource = getUtility(ISourcePackageRecipeSource)
30+ if SourcePackageRecipeSource.exists(owner, name):
31+ self.setFieldError(
32+ 'name',
33+ 'There is already a recipe owned by %s with this name.' %
34+ owner.displayname)
35+
36
37 class SourcePackageRecipeEditView(RecipeTextValidatorMixin,
38 LaunchpadEditFormView):
39@@ -417,6 +420,20 @@
40 """See `LaunchpadEditFormView`"""
41 return {ISourcePackageAddEditSchema: self.context}
42
43+ def validate(self, data):
44+ super(SourcePackageRecipeEditView, self).validate(data)
45+ name = data.get('name', None)
46+ owner = data.get('owner', None)
47+ if name and owner:
48+ SourcePackageRecipeSource = getUtility(ISourcePackageRecipeSource)
49+ if SourcePackageRecipeSource.exists(owner, name):
50+ recipe = owner.getRecipe(name)
51+ if recipe != self.context:
52+ self.setFieldError(
53+ 'name',
54+ 'There is already a recipe owned by %s with this '
55+ 'name.' % owner.displayname)
56+
57
58 class SourcePackageRecipeDeleteView(LaunchpadFormView):
59
60
61=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
62--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2010-06-08 05:07:35 +0000
63+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2010-06-08 18:48:36 +0000
64@@ -202,7 +202,6 @@
65 owner=self.chef, product=product, name='veggies')
66 meat_branch = self.factory.makeBranch(
67 owner=self.chef, product=product, name='meat')
68- source_package = self.factory.makeSourcePackage()
69 recipe = self.factory.makeSourcePackageRecipe(
70 owner=self.chef, registrant=self.chef,
71 name=u'things', description=u'This is a recipe',
72@@ -241,6 +240,89 @@
73 self.assertTextMatchesExpressionIgnoreWhitespace(
74 pattern, main_text)
75
76+ def test_edit_recipe_already_exists(self):
77+ self.factory.makeDistroSeries(
78+ displayname='Mumbly Midget', name='mumbly',
79+ distribution=self.ppa.distribution)
80+ product = self.factory.makeProduct(
81+ name='ratatouille', displayname='Ratatouille')
82+ veggie_branch = self.factory.makeBranch(
83+ owner=self.chef, product=product, name='veggies')
84+ meat_branch = self.factory.makeBranch(
85+ owner=self.chef, product=product, name='meat')
86+ recipe = self.factory.makeSourcePackageRecipe(
87+ owner=self.chef, registrant=self.chef,
88+ name=u'things', description=u'This is a recipe',
89+ distroseries=self.squirrel, branches=[veggie_branch])
90+ recipe_fings = self.factory.makeSourcePackageRecipe(
91+ owner=self.chef, registrant=self.chef,
92+ name=u'fings', description=u'This is a recipe',
93+ distroseries=self.squirrel, branches=[veggie_branch])
94+
95+ meat_path = meat_branch.bzr_identity
96+
97+ browser = self.getUserBrowser(canonical_url(recipe), user=self.chef)
98+ browser.getLink('Edit recipe').click()
99+ browser.getControl(name='field.name').value = 'fings'
100+ browser.getControl('Description').value = 'This is stuff'
101+ browser.getControl('Recipe text').value = (
102+ MINIMAL_RECIPE_TEXT % meat_path)
103+ browser.getControl('Secret Squirrel').click()
104+ browser.getControl('Mumbly Midget').click()
105+ browser.getControl('Update Recipe').click()
106+
107+ self.assertEqual(
108+ extract_text(find_tags_by_class(browser.contents, 'message')[1]),
109+ 'There is already a recipe owned by Master Chef with this name.')
110+
111+ def test_edit_recipe_but_not_name(self):
112+ self.factory.makeDistroSeries(
113+ displayname='Mumbly Midget', name='mumbly',
114+ distribution=self.ppa.distribution)
115+ product = self.factory.makeProduct(
116+ name='ratatouille', displayname='Ratatouille')
117+ veggie_branch = self.factory.makeBranch(
118+ owner=self.chef, product=product, name='veggies')
119+ meat_branch = self.factory.makeBranch(
120+ owner=self.chef, product=product, name='meat')
121+ recipe = self.factory.makeSourcePackageRecipe(
122+ owner=self.chef, registrant=self.chef,
123+ name=u'things', description=u'This is a recipe',
124+ distroseries=self.squirrel, branches=[veggie_branch])
125+
126+ meat_path = meat_branch.bzr_identity
127+
128+ browser = self.getUserBrowser(canonical_url(recipe), user=self.chef)
129+ browser.getLink('Edit recipe').click()
130+ browser.getControl('Description').value = 'This is stuff'
131+ browser.getControl('Recipe text').value = (
132+ MINIMAL_RECIPE_TEXT % meat_path)
133+ browser.getControl('Secret Squirrel').click()
134+ browser.getControl('Mumbly Midget').click()
135+ browser.getControl('Update Recipe').click()
136+
137+ pattern = """\
138+ Master Chef's things recipe
139+ .*
140+
141+ Description
142+ This is stuff
143+
144+ Recipe information
145+ Owner: Master Chef
146+ Base branch: lp://dev/~chef/ratatouille/meat
147+ Debian version: 1.0
148+ Distribution series: Mumbly Midget
149+ .*
150+
151+ Recipe contents
152+ # bzr-builder format 0.2 deb-version 1.0
153+ lp://dev/~chef/ratatouille/meat"""
154+ main_text = extract_text(find_main_content(browser.contents))
155+ self.assertTextMatchesExpressionIgnoreWhitespace(
156+ pattern, main_text)
157+
158+
159
160 class TestSourcePackageRecipeView(TestCaseForRecipe):
161