Launchpad itself

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

  1. edit-recipe-name
  2. Merge into devel
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
=== 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 @@
318 'recipe_text',318 'recipe_text',
319 'The recipe text is not a valid bzr-builder recipe.')319 'The recipe text is not a valid bzr-builder recipe.')
320320
321 name = data.get('name', None)
322 if name:
323 SourcePackageRecipeSource = getUtility(ISourcePackageRecipeSource)
324 if SourcePackageRecipeSource.exists(self.user, name):
325 self.setFieldError(
326 'name',
327 'There is already a recipe owned by %s with this name.' %
328 self.user.displayname)
329
330321
331class SourcePackageRecipeAddView(RecipeTextValidatorMixin, LaunchpadFormView):322class SourcePackageRecipeAddView(RecipeTextValidatorMixin, LaunchpadFormView):
332 """View for creating Source Package Recipes."""323 """View for creating Source Package Recipes."""
@@ -355,6 +346,18 @@
355 data['name'], recipe, data['description'])346 data['name'], recipe, data['description'])
356 self.next_url = canonical_url(source_package_recipe)347 self.next_url = canonical_url(source_package_recipe)
357348
349 def validate(self, data):
350 super(SourcePackageRecipeAddView, self).validate(data)
351 name = data.get('name', None)
352 owner = data.get('owner', None)
353 if name and owner:
354 SourcePackageRecipeSource = getUtility(ISourcePackageRecipeSource)
355 if SourcePackageRecipeSource.exists(owner, name):
356 self.setFieldError(
357 'name',
358 'There is already a recipe owned by %s with this name.' %
359 owner.displayname)
360
358361
359class SourcePackageRecipeEditView(RecipeTextValidatorMixin,362class SourcePackageRecipeEditView(RecipeTextValidatorMixin,
360 LaunchpadEditFormView):363 LaunchpadEditFormView):
@@ -417,6 +420,20 @@
417 """See `LaunchpadEditFormView`"""420 """See `LaunchpadEditFormView`"""
418 return {ISourcePackageAddEditSchema: self.context}421 return {ISourcePackageAddEditSchema: self.context}
419422
423 def validate(self, data):
424 super(SourcePackageRecipeEditView, self).validate(data)
425 name = data.get('name', None)
426 owner = data.get('owner', None)
427 if name and owner:
428 SourcePackageRecipeSource = getUtility(ISourcePackageRecipeSource)
429 if SourcePackageRecipeSource.exists(owner, name):
430 recipe = owner.getRecipe(name)
431 if recipe != self.context:
432 self.setFieldError(
433 'name',
434 'There is already a recipe owned by %s with this '
435 'name.' % owner.displayname)
436
420437
421class SourcePackageRecipeDeleteView(LaunchpadFormView):438class SourcePackageRecipeDeleteView(LaunchpadFormView):
422439
423440
=== 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 @@
202 owner=self.chef, product=product, name='veggies')202 owner=self.chef, product=product, name='veggies')
203 meat_branch = self.factory.makeBranch(203 meat_branch = self.factory.makeBranch(
204 owner=self.chef, product=product, name='meat')204 owner=self.chef, product=product, name='meat')
205 source_package = self.factory.makeSourcePackage()
206 recipe = self.factory.makeSourcePackageRecipe(205 recipe = self.factory.makeSourcePackageRecipe(
207 owner=self.chef, registrant=self.chef,206 owner=self.chef, registrant=self.chef,
208 name=u'things', description=u'This is a recipe',207 name=u'things', description=u'This is a recipe',
@@ -241,6 +240,89 @@
241 self.assertTextMatchesExpressionIgnoreWhitespace(240 self.assertTextMatchesExpressionIgnoreWhitespace(
242 pattern, main_text)241 pattern, main_text)
243242
243 def test_edit_recipe_already_exists(self):
244 self.factory.makeDistroSeries(
245 displayname='Mumbly Midget', name='mumbly',
246 distribution=self.ppa.distribution)
247 product = self.factory.makeProduct(
248 name='ratatouille', displayname='Ratatouille')
249 veggie_branch = self.factory.makeBranch(
250 owner=self.chef, product=product, name='veggies')
251 meat_branch = self.factory.makeBranch(
252 owner=self.chef, product=product, name='meat')
253 recipe = self.factory.makeSourcePackageRecipe(
254 owner=self.chef, registrant=self.chef,
255 name=u'things', description=u'This is a recipe',
256 distroseries=self.squirrel, branches=[veggie_branch])
257 recipe_fings = self.factory.makeSourcePackageRecipe(
258 owner=self.chef, registrant=self.chef,
259 name=u'fings', description=u'This is a recipe',
260 distroseries=self.squirrel, branches=[veggie_branch])
261
262 meat_path = meat_branch.bzr_identity
263
264 browser = self.getUserBrowser(canonical_url(recipe), user=self.chef)
265 browser.getLink('Edit recipe').click()
266 browser.getControl(name='field.name').value = 'fings'
267 browser.getControl('Description').value = 'This is stuff'
268 browser.getControl('Recipe text').value = (
269 MINIMAL_RECIPE_TEXT % meat_path)
270 browser.getControl('Secret Squirrel').click()
271 browser.getControl('Mumbly Midget').click()
272 browser.getControl('Update Recipe').click()
273
274 self.assertEqual(
275 extract_text(find_tags_by_class(browser.contents, 'message')[1]),
276 'There is already a recipe owned by Master Chef with this name.')
277
278 def test_edit_recipe_but_not_name(self):
279 self.factory.makeDistroSeries(
280 displayname='Mumbly Midget', name='mumbly',
281 distribution=self.ppa.distribution)
282 product = self.factory.makeProduct(
283 name='ratatouille', displayname='Ratatouille')
284 veggie_branch = self.factory.makeBranch(
285 owner=self.chef, product=product, name='veggies')
286 meat_branch = self.factory.makeBranch(
287 owner=self.chef, product=product, name='meat')
288 recipe = self.factory.makeSourcePackageRecipe(
289 owner=self.chef, registrant=self.chef,
290 name=u'things', description=u'This is a recipe',
291 distroseries=self.squirrel, branches=[veggie_branch])
292
293 meat_path = meat_branch.bzr_identity
294
295 browser = self.getUserBrowser(canonical_url(recipe), user=self.chef)
296 browser.getLink('Edit recipe').click()
297 browser.getControl('Description').value = 'This is stuff'
298 browser.getControl('Recipe text').value = (
299 MINIMAL_RECIPE_TEXT % meat_path)
300 browser.getControl('Secret Squirrel').click()
301 browser.getControl('Mumbly Midget').click()
302 browser.getControl('Update Recipe').click()
303
304 pattern = """\
305 Master Chef's things recipe
306 .*
307
308 Description
309 This is stuff
310
311 Recipe information
312 Owner: Master Chef
313 Base branch: lp://dev/~chef/ratatouille/meat
314 Debian version: 1.0
315 Distribution series: Mumbly Midget
316 .*
317
318 Recipe contents
319 # bzr-builder format 0.2 deb-version 1.0
320 lp://dev/~chef/ratatouille/meat"""
321 main_text = extract_text(find_main_content(browser.contents))
322 self.assertTextMatchesExpressionIgnoreWhitespace(
323 pattern, main_text)
324
325
244326
245class TestSourcePackageRecipeView(TestCaseForRecipe):327class TestSourcePackageRecipeView(TestCaseForRecipe):
246328