Merge lp:~rockstar/launchpad/recipe-description into lp:launchpad/db-devel

[Paul Hummer]

This branch adds a description to SourcePackageRecipes - abentley and I
realized that it was missing from the model, and we needed it based on my
mockups and the fact that recipes are very hard to decipher the purpose of
otherwise.

[Brad Crittenden (bac)]

review: Approve (code)

[Stuart Bishop (stub)]

Fine. patch-2207-37-0.sql.

Consider making the new description column NOT NULL if the UI really doesn't make sense without one set.

review: Approve (db)

[Björn Tillenius (bjornt)]

On Tue, Mar 16, 2010 at 07:41:20PM -0000, Paul Hummer wrote:
> === added file 'database/schema/patch-2207-99-0.sql'
> --- database/schema/patch-2207-99-0.sql 1970-01-01 00:00:00 +0000
> +++ database/schema/patch-2207-99-0.sql 2010-03-16 19:41:18 +0000
> @@ -0,0 +1,9 @@
> +-- Copyright 2010 Canonical Ltd. This software is licensed under the
> +-- GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +SET client_min_messages=ERROR;
> +
> +ALTER TABLE SourcePackageRecipe
> +ADD COLUMN description TEXT;
> +
> +INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 99, 0);
>
> === modified file 'lib/lp/code/interfaces/sourcepackagerecipe.py'
> --- lib/lp/code/interfaces/sourcepackagerecipe.py 2010-02-05 14:22:03 +0000
> +++ lib/lp/code/interfaces/sourcepackagerecipe.py 2010-03-16 19:41:18 +0000
> @@ -71,6 +71,10 @@
> constraint=name_validator,
> description=_("The name of this recipe."))
>
> + description = Text(
> + title=_('Description'), required=False,
> + description=_('A short description of the recipe.'))
> +

The interface says that the description can't be None, while the db
patch says that it can. What should it be? Judging from your comments,
it seems that it should be not-None, though.

> builder_recipe = Attribute(
> _("The bzr-builder data structure for the recipe."))
>
>
> === modified file 'lib/lp/code/model/sourcepackagerecipe.py'
> --- lib/lp/code/model/sourcepackagerecipe.py 2010-02-05 15:38:19 +0000
> +++ lib/lp/code/model/sourcepackagerecipe.py 2010-03-16 19:41:18 +0000
> @@ -58,6 +58,7 @@
> sourcepackagename_id, 'SourcePackageName.id')
>
> name = Unicode(allow_none=True)
> + description = StringCol(dbName='description')

StringCol from sqlobject on a storm object? Does this even work? A small
test might be useful :)

    review needsfixing

--
Björn Tillenius | https://launchpad.net/~bjornt

review: Needs Fixing

[Björn Tillenius (bjornt)]

As discussed on IRC, the interface now says that the description can be None, while the db patch says it can. Paul said he would make things consistent (description can't be None).

review: Approve (db)

=== modified file 'database/sampledata/current-dev.sql'
=== modified file 'database/sampledata/current.sql'
=== added file 'database/schema/patch-2207-37-0.sql'
--- database/schema/patch-2207-37-0.sql	1970-01-01 00:00:00 +0000
+++ database/schema/patch-2207-37-0.sql	2010-03-26 17:54:33 +0000
@@ -0,0 +1,9 @@
+-- Copyright 2010 Canonical Ltd.  This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+
+SET client_min_messages=ERROR;
+
+ALTER TABLE SourcePackageRecipe
+ADD COLUMN description TEXT NOT NULL;
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 37, 0);

=== modified file 'lib/lp/code/interfaces/sourcepackagerecipe.py'
--- lib/lp/code/interfaces/sourcepackagerecipe.py	2010-03-25 15:21:54 +0000
+++ lib/lp/code/interfaces/sourcepackagerecipe.py	2010-03-26 17:54:33 +0000
@@ -14,7 +14,7 @@
 from lazr.restful.fields import Reference
 
 from zope.interface import Attribute, Interface
-from zope.schema import Datetime, TextLine
+from zope.schema import Datetime, Text, TextLine
 
 from canonical.launchpad import _
 from canonical.launchpad.validators.name import name_validator
@@ -72,6 +72,10 @@
             constraint=name_validator,
             description=_("The name of this recipe."))
 
+    description = Text(
+        title=_('Description'), required=True,
+        description=_('A short description of the recipe.'))
+
     builder_recipe = Attribute(
         _("The bzr-builder data structure for the recipe."))
 
@@ -98,5 +102,5 @@
     """
 
     def new(registrant, owner, distroseries, sourcepackagename, name,
-            builder_recipe):
+            builder_recipe, description):
         """Create an `ISourcePackageRecipe`."""

=== modified file 'lib/lp/code/model/sourcepackagerecipe.py'
--- lib/lp/code/model/sourcepackagerecipe.py	2010-03-25 15:21:54 +0000
+++ lib/lp/code/model/sourcepackagerecipe.py	2010-03-26 17:54:33 +0000
@@ -58,6 +58,7 @@
         sourcepackagename_id, 'SourcePackageName.id')
 
     name = Unicode(allow_none=True)
+    description = Unicode(allow_none=False)
 
     @property
     def _recipe_data(self):
@@ -85,7 +86,7 @@
 
     @staticmethod
     def new(registrant, owner, distroseries, sourcepackagename, name,
-            builder_recipe):
+            builder_recipe, description):
         """See `ISourcePackageRecipeSource.new`."""
         store = IMasterStore(SourcePackageRecipe)
         sprecipe = SourcePackageRecipe()
@@ -95,6 +96,7 @@
         sprecipe.distroseries = distroseries
         sprecipe.sourcepackagename = sourcepackagename
         sprecipe.name = name
+        sprecipe.description = description
         store.add(sprecipe)
         return sprecipe
 

=== added file 'lib/lp/code/model/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipe.py	1970-01-01 00:00:00 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipe.py	2010-03-26 17:54:33 +0000
@@ -0,0 +1,22 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for SourcePackageRecipes."""
+
+__metaclass__ = type
+
+from lp.testing import (
+    run_with_login, TestCase, TestCaseWithFactory, time_counter)
+from canonical.testing import DatabaseFunctionalLayer, LaunchpadZopelessLayer
+
+
+class TestSourcePackageRecipe(TestCaseWithFactory):
+
+    layer = LaunchpadZopelessLayer
+
+    def test_sourcepackagerecipe_description(self):
+        """Ensure that the SourcePackageRecipe has a proper description."""
+        description = u'The whoozits and whatzits.'
+        source_package_recipe = self.factory.makeSourcePackageRecipe(
+            description=description)
+        self.assertEqual(description, source_package_recipe.description)

=== modified file 'lib/lp/code/tests/test_recipebuilder.py'
--- lib/lp/code/tests/test_recipebuilder.py	2010-02-05 15:26:10 +0000
+++ lib/lp/code/tests/test_recipebuilder.py	2010-03-26 17:54:33 +0000
@@ -59,7 +59,7 @@
         somebranch = self.factory.makeBranch(owner=requester, name="pkg",
             product=self.factory.makeProduct("someapp"))
         recipe = self.factory.makeSourcePackageRecipe(requester, requester,
-             distroseries, spn, u"recept", somebranch)
+             distroseries, spn, u"recept", u"Recipe description", somebranch)
         spb = self.factory.makeSourcePackageRecipeBuild(
             sourcepackage=sourcepackage, recipe=recipe, requester=requester)
         job = spb.makeJob()

=== modified file 'lib/lp/code/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/tests/test_sourcepackagerecipe.py	2010-03-25 15:21:54 +0000
+++ lib/lp/code/tests/test_sourcepackagerecipe.py	2010-03-26 17:54:33 +0000
@@ -51,10 +51,11 @@
         distroseries = self.factory.makeDistroSeries()
         sourcepackagename = self.factory.makeSourcePackageName()
         name = self.factory.getUniqueString(u'recipe-name')
+        description = self.factory.getUniqueString(u'recipe-description')
         return getUtility(ISourcePackageRecipeSource).new(
             registrant=registrant, owner=owner, distroseries=distroseries,
             sourcepackagename=sourcepackagename, name=name,
-            builder_recipe=builder_recipe)
+            description=description, builder_recipe=builder_recipe)
 
     def test_creation(self):
         # The metadata supplied when a SourcePackageRecipe is created is
@@ -64,11 +65,12 @@
         distroseries = self.factory.makeDistroSeries()
         sourcepackagename = self.factory.makeSourcePackageName()
         name = self.factory.getUniqueString(u'recipe-name')
+        description = self.factory.getUniqueString(u'recipe-description')
         builder_recipe = self.factory.makeRecipe()
         recipe = getUtility(ISourcePackageRecipeSource).new(
             registrant=registrant, owner=owner, distroseries=distroseries,
             sourcepackagename=sourcepackagename, name=name,
-            builder_recipe=builder_recipe)
+            description=description, builder_recipe=builder_recipe)
         self.assertEquals(
             (registrant, owner, distroseries, sourcepackagename, name),
             (recipe.registrant, recipe.owner, recipe.distroseries,
@@ -250,10 +252,11 @@
         distroseries = self.factory.makeDistroSeries()
         sourcepackagename = self.factory.makeSourcePackageName()
         name = self.factory.getUniqueString(u'recipe-name')
+        description = self.factory.getUniqueString(u'recipe-description')
         recipe = getUtility(ISourcePackageRecipeSource).new(
             registrant=registrant, owner=owner, distroseries=distroseries,
             sourcepackagename=sourcepackagename, name=name,
-            builder_recipe=builder_recipe)
+            description=description, builder_recipe=builder_recipe)
         return recipe.builder_recipe
 
     def check_base_recipe_branch(self, branch, url, revspec=None,

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-03-25 08:48:14 +0000
+++ lib/lp/testing/factory.py	2010-03-26 17:54:33 +0000
@@ -1761,7 +1761,7 @@
 
     def makeSourcePackageRecipe(self, registrant=None, owner=None,
                                 distroseries=None, sourcepackagename=None,
-                                name=None, *branches):
+                                name=None, description=None, *branches):
         """Make a `SourcePackageRecipe`."""
         if registrant is None:
             registrant = self.makePerson()
@@ -1773,9 +1773,12 @@
             sourcepackagename = self.makeSourcePackageName()
         if name is None:
             name = self.getUniqueString().decode('utf8')
+        if description is None:
+            description = self.getUniqueString().decode('utf8')
         recipe = self.makeRecipe(*branches)
         return getUtility(ISourcePackageRecipeSource).new(
-            registrant, owner, distroseries, sourcepackagename, name, recipe)
+            registrant, owner, distroseries, sourcepackagename, name, recipe,
+            description)
 
     def makeSourcePackageRecipeBuild(self, sourcepackage=None, recipe=None,
                                      requester=None, archive=None,