Merge lp:~rockstar/launchpad/no-recipe-dupes into lp:launchpad

Proposed by Paul Hummer
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merge reported by: Paul Hummer
Merged at revision: not available
Proposed branch: lp:~rockstar/launchpad/no-recipe-dupes
Merge into: lp:launchpad
Diff against target: 111 lines (+60/-0)
5 files modified
lib/lp/code/browser/sourcepackagerecipe.py (+7/-0)
lib/lp/code/browser/tests/test_sourcepackagerecipe.py (+25/-0)
lib/lp/code/interfaces/sourcepackagerecipe.py (+3/-0)
lib/lp/code/model/sourcepackagerecipe.py (+13/-0)
lib/lp/code/model/tests/test_sourcepackagerecipe.py (+12/-0)
To merge this branch: bzr merge lp:~rockstar/launchpad/no-recipe-dupes
Reviewer Review Type Date Requested Status
Curtis Hovey (community) rc + code Approve
Review via email: mp+26212@code.launchpad.net

Description of the change

This branch makes it so that the UI can't create a recipe owned by the same person with the same name. There's a database patch to go along with this.

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

Thanks for reworking the implementation and providing the extra test.

review: Approve

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-05-18 20:29:30 +0000
3+++ lib/lp/code/browser/sourcepackagerecipe.py 2010-05-27 20:23:27 +0000
4@@ -321,6 +321,13 @@
5 'recipe_text',
6 'The recipe text is not a valid bzr-builder recipe.')
7
8+ if getUtility(ISourcePackageRecipeSource).exists(
9+ self.user, data['name']):
10+ self.setFieldError(
11+ 'name',
12+ 'There is already a recipe owned by %s with this name.' %
13+ self.user.displayname)
14+
15
16 class SourcePackageRecipeAddView(RecipeTextValidatorMixin, LaunchpadFormView):
17 """View for creating Source Package Recipes."""
18
19=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
20--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2010-05-26 12:11:31 +0000
21+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2010-05-27 20:23:27 +0000
22@@ -127,6 +127,31 @@
23 extract_text(find_tags_by_class(browser.contents, 'message')[1]),
24 'The recipe text is not a valid bzr-builder recipe.')
25
26+ def test_create_dupe_recipe(self):
27+ # You shouldn't be able to create a duplicate recipe owned by the same
28+ # person with the same name.
29+ recipe = self.factory.makeSourcePackageRecipe(owner=self.chef)
30+
31+ product = self.factory.makeProduct(
32+ name='ratatouille', displayname='Ratatouille')
33+ branch = self.factory.makeBranch(
34+ owner=self.chef, product=product, name='veggies')
35+ self.factory.makeSourcePackage(sourcepackagename='ratatouille')
36+
37+ # A new recipe can be created from the branch page.
38+ browser = self.getUserBrowser(canonical_url(branch), user=self.chef)
39+ browser.getLink('Create packaging recipe').click()
40+
41+ browser.getControl(name='field.name').value = recipe.name
42+ browser.getControl('Description').value = 'Make some food!'
43+ browser.getControl('Source Package Name').value = 'ratatouille'
44+ browser.getControl('Secret Squirrel').click()
45+ browser.getControl('Create Recipe').click()
46+
47+ self.assertEqual(
48+ extract_text(find_tags_by_class(browser.contents, 'message')[1]),
49+ 'There is already a recipe owned by Master Chef with this name.')
50+
51
52 class TestSourcePackageRecipeEditView(TestCaseForRecipe):
53 """Test the editing behaviour of a source package recipe."""
54
55=== modified file 'lib/lp/code/interfaces/sourcepackagerecipe.py'
56--- lib/lp/code/interfaces/sourcepackagerecipe.py 2010-05-18 20:29:30 +0000
57+++ lib/lp/code/interfaces/sourcepackagerecipe.py 2010-05-27 20:23:27 +0000
58@@ -186,3 +186,6 @@
59 def new(registrant, owner, distroseries, sourcepackagename, name,
60 builder_recipe, description):
61 """Create an `ISourcePackageRecipe`."""
62+
63+ def exists(owner, name):
64+ """Check to see if a recipe by the same name and owner exists."""
65
66=== modified file 'lib/lp/code/model/sourcepackagerecipe.py'
67--- lib/lp/code/model/sourcepackagerecipe.py 2010-05-26 19:23:20 +0000
68+++ lib/lp/code/model/sourcepackagerecipe.py 2010-05-27 20:23:27 +0000
69@@ -134,6 +134,19 @@
70 store.add(sprecipe)
71 return sprecipe
72
73+ @staticmethod
74+ def exists(owner, name):
75+ """See `ISourcePackageRecipeSource.new`."""
76+ store = IMasterStore(SourcePackageRecipe)
77+ recipe = store.find(
78+ SourcePackageRecipe,
79+ SourcePackageRecipe.owner == owner,
80+ SourcePackageRecipe.name == name).one()
81+ if recipe:
82+ return True
83+ else:
84+ return False
85+
86 def destroySelf(self):
87 store = Store.of(self)
88 self.distroseries.clear()
89
90=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipe.py'
91--- lib/lp/code/model/tests/test_sourcepackagerecipe.py 2010-05-18 19:14:16 +0000
92+++ lib/lp/code/model/tests/test_sourcepackagerecipe.py 2010-05-27 20:23:27 +0000
93@@ -85,6 +85,18 @@
94 (recipe.registrant, recipe.owner, set(recipe.distroseries),
95 recipe.sourcepackagename, recipe.name))
96
97+ def test_exists(self):
98+ # Test ISourcePackageRecipeSource.exists
99+ recipe = self.factory.makeSourcePackageRecipe()
100+
101+ self.assertTrue(
102+ getUtility(ISourcePackageRecipeSource).exists(
103+ recipe.owner, recipe.name))
104+
105+ self.assertFalse(
106+ getUtility(ISourcePackageRecipeSource).exists(
107+ recipe.owner, u'daily'))
108+
109 def test_source_implements_interface(self):
110 # The SourcePackageRecipe class implements ISourcePackageRecipeSource.
111 self.assertProvides(