Merge lp:~linaro-infrastructure/launchpad/workitems-model-classes into lp:launchpad/db-devel

[Mattias Backman]

[r=jcsackett,rharding][bug=933592] Work Items models.

Hi,

This branch adds models for blueprint Work Items.

The topic has been discussed here https://lists.launchpad.net/launchpad-dev/msg08782.html . The gist of it is that we would like Work Items to be proper Launchpad objects to make status.ubuntu.org and status.linaro.org development easier and enable new status pages for teams and individuals.

Thanks,

Mattias

[Richard Harding (rharding)]

Mattias, thanks. We should be using storm vs sqlobject for moving forward. I think you can switch from sqlbase to stormbase and update the columns without too much change.

Aside from that everything looks good.

review: Needs Fixing (code*)

[Mattias Backman (mabac)]

On Thu, Feb 9, 2012 at 7:17 AM, Richard Harding
<email address hidden> wrote:
> Review: Needs Fixing code*
>
> Mattias, thanks. We should be using storm vs sqlobject for moving forward. I think you can switch from sqlbase to stormbase and update the columns without too much change.

Thanks. I'm not entirely sure what needs changing. I have pushed
another revision with my best guess.

>
> Aside from that everything looks good.
> --
> https://code.launchpad.net/~linaro-infrastructure/launchpad/workitems-model-classes/+merge/92174
> Your team Linaro Infrastructure is subscribed to branch lp:~linaro-infrastructure/launchpad/workitems-model-classes.

[Richard Harding (rharding)]

Mattias, so with the change over to using Storm, the model definition needs to be updated to use the Storm column definitions. You can see an example in the sprintattendance.py file there.

    from storm.locals import (
    Bool,
    Int,
    Reference,
    Store,
    )

There's a sample model there that uses StormBase and defines the models and such.

review: Needs Fixing (code*)

[Mattias Backman (mabac)]

We need to fix my typo and add some tests. Guilherme will pick that up tomorrow.

[Guilherme Salgado (salgado)]

Okay, this branch should be good to go now. We were missing a zcml declaration to allow ISpecificationWorkItem on SpecificationWorkItem so I added that and another test to ensure it works.
(I've done this myself because Mattias is not around today)

[Richard Harding (rharding)]

Thanks, this looks much better using the newer storm vs sqlobject code. I appreciate the effort to rework it.

review: Approve (code*)

[j.c.sackett (jcsackett)]

I have only one thing to add to Rick's review, in the diff below. As soon as that change is made we can send this out to land.

> === modified file 'lib/lp/blueprints/enums.py'
> --- lib/lp/blueprints/enums.py 2011-03-02 04:25:06 +0000
> +++ lib/lp/blueprints/enums.py 2012-02-10 17:51:24 +0000
> @@ -511,3 +512,31 @@
> organisers. It has not yet been accepted or declined for the
> agenda.
> """)
> +
> +
> +class SpecificationWorkItemStatus(DBEnumeratedType):
> + TODO = DBItem(0, """
> + TODO
> +
> + A work item that's not done yet.
> + """)
> + DONE = DBItem(1, """
> + DONE
> +
> + A work item that's done.
> + """)
> + POSTPONED = DBItem(2, """
> + POSTPONED
> +
> + A work item that has been postponed.
> + """)
> + INPROGRESS = DBItem(3, """
> + INPROGRESS
> +
> + A work item that is inprogress.
> + """)
> + BLOCKED = DBItem(4, """
> + BLOCKED
> +
> + A work item that is blocked.
> + """)

For enums like this, the first line of the comment describing shouldn't
be in caps. If you look at other enums, the following diff would be better:

=== modified file 'lib/lp/blueprints/enums.py'
--- lib/lp/blueprints/enums.py 2011-03-02 04:25:06 +0000
+++ lib/lp/blueprints/enums.py 2012-02-10 17:51:24 +0000
@@ -511,3 +512,31 @@
         organisers. It has not yet been accepted or declined for the
         agenda.
         """)
+
+
+class SpecificationWorkItemStatus(DBEnumeratedType):
+ TODO = DBItem(0, """
+ Todo
+
+ A work item that's not done yet.
+ """)
+ DONE = DBItem(1, """
+ Done
+
+ A work item that's done.
+ """)
+ POSTPONED = DBItem(2, """
+ Postponed
+
+ A work item that has been postponed.
+ """)
+ INPROGRESS = DBItem(3, """
+ In progress
+
+ A work item that is inprogress.
+ """)
+ BLOCKED = DBItem(4, """
+ Blocked
+
+ A work item that is blocked.
+ """)

review: Needs Fixing

[j.c.sackett (jcsackett)]

Because this branch has taken awhile to get review, I've made making that change as easy as possible. If you just do

`bzr merge lp:~jcsackett/launchpad/quick-fix-for-linaro`

you'll get that change made.

[Mattias Backman (mabac)]

On Tue, Feb 14, 2012 at 6:57 PM, j.c.sackett <email address hidden> wrote:
> Because this branch has taken awhile to get review, I've made making that change as easy as possible. If you just do
>
> `bzr merge lp:~jcsackett/launchpad/quick-fix-for-linaro`
>
> you'll get that change made.

Incredibly nice of you to offer that, I didn't get anything changed
when merging though so I changed it manually, I think it matches your
change. Please have a look at the new revision.

> --
> https://code.launchpad.net/~linaro-infrastructure/launchpad/workitems-model-classes/+merge/92174
> Your team Linaro Infrastructure is subscribed to branch lp:~linaro-infrastructure/launchpad/workitems-model-classes.

[j.c.sackett (jcsackett)]

Alas, I did a bad push. Thus, my help was less than helpful. :-P

Thanks for making those changes.

review: Approve

=== modified file 'lib/lp/blueprints/configure.zcml'
--- lib/lp/blueprints/configure.zcml	2011-12-24 17:49:30 +0000
+++ lib/lp/blueprints/configure.zcml	2012-02-15 21:39:22 +0000
@@ -78,6 +78,12 @@
         set_attributes="whiteboard"/>
   </class>
 
+  <!-- SpecificationWorkItem -->
+  <class class="lp.blueprints.model.specificationworkitem.SpecificationWorkItem">
+    <allow
+        interface="lp.blueprints.interfaces.specificationworkitem.ISpecificationWorkItem"/>
+  </class>
+
   <!-- SpecificationDependency -->
 
   <class class="lp.blueprints.model.specificationdependency.SpecificationDependency">

=== modified file 'lib/lp/blueprints/enums.py'
--- lib/lp/blueprints/enums.py	2011-03-02 04:25:06 +0000
+++ lib/lp/blueprints/enums.py	2012-02-15 21:39:22 +0000
@@ -14,6 +14,7 @@
     'SpecificationPriority',
     'SpecificationSort',
     'SprintSpecificationStatus',
+    'SpecificationWorkItemStatus',
     ]
 
 
@@ -511,3 +512,31 @@
         organisers. It has not yet been accepted or declined for the
         agenda.
         """)
+
+
+class SpecificationWorkItemStatus(DBEnumeratedType):
+    TODO = DBItem(0, """
+        Todo
+
+        A work item that's not done yet.
+        """)
+    DONE = DBItem(1, """
+        Done
+
+        A work item that's done.
+        """)
+    POSTPONED = DBItem(2, """
+        Postponed
+
+        A work item that has been postponed.
+        """)
+    INPROGRESS = DBItem(3, """
+        In progress
+
+        A work item that is inprogress.
+        """)
+    BLOCKED = DBItem(4, """
+        Blocked
+
+        A work item that is blocked.
+        """)

=== modified file 'lib/lp/blueprints/interfaces/specification.py'
--- lib/lp/blueprints/interfaces/specification.py	2012-01-01 02:58:52 +0000
+++ lib/lp/blueprints/interfaces/specification.py	2012-02-15 21:39:22 +0000
@@ -54,6 +54,7 @@
     SpecificationImplementationStatus,
     SpecificationLifecycleStatus,
     SpecificationPriority,
+    SpecificationWorkItemStatus,
     )
 from lp.blueprints.interfaces.specificationsubscription import (
     ISpecificationSubscription,
@@ -569,6 +570,11 @@
     def setImplementationStatus(implementation_status, user):
         """Mutator for implementation_status that calls updateLifeCycle."""
 
+    def newWorkItem(title, sequence,
+                    status=SpecificationWorkItemStatus.TODO, assignee=None,
+                    milestone=None):
+        """Create a new SpecificationWorkItem."""
+
     def setTarget(target):
         """Set this specification's target.
 

=== added file 'lib/lp/blueprints/interfaces/specificationworkitem.py'
--- lib/lp/blueprints/interfaces/specificationworkitem.py	1970-01-01 00:00:00 +0000
+++ lib/lp/blueprints/interfaces/specificationworkitem.py	2012-02-15 21:39:22 +0000
@@ -0,0 +1,76 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""SpecificationWorkItem interfaces."""
+
+__metaclass__ = type
+
+__all__ = [
+    'ISpecificationWorkItem',
+    ]
+
+
+from zope.interface import Interface
+from zope.schema import (
+    Bool,
+    Choice,
+    Datetime,
+    Int,
+    )
+
+from lp import _
+from lp.blueprints.enums import SpecificationWorkItemStatus
+from lp.services.fields import (
+    PublicPersonChoice,
+    Title,
+    )
+
+
+class ISpecificationWorkItem(Interface):
+    """SpecificationWorkItem's public attributes and methods."""
+
+    id = Int(title=_("Database ID"), required=True, readonly=True)
+
+    title = Title(
+        title=_('Title'), required=True, readonly=False,
+        description=_("Work item title."))
+
+    assignee = PublicPersonChoice(
+        title=_('Assignee'), required=False, readonly=False,
+        description=_(
+            "The person responsible for implementing the work item."),
+        vocabulary='ValidPersonOrTeam')
+
+    date_created = Datetime(
+        title=_('Date Created'), required=True, readonly=True)
+
+    milestone = Choice(
+        title=_('Milestone'), required=False, readonly=False,
+        vocabulary='Milestone',
+        description=_(
+            "The milestone to which this work item is targetted. If this "
+            "is not set, then the target is the specification's "
+            "milestone."))
+
+    status = Choice(
+        title=_("Work Item Status"), required=True, readonly=False,
+        default=SpecificationWorkItemStatus.TODO,
+        vocabulary=SpecificationWorkItemStatus,
+        description=_(
+            "The state of progress being made on the actual "
+            "implementation of this work item."))
+
+    specification = Choice(
+        title=_('The specification that the work item is linked to.'),
+        required=True, readonly=True, vocabulary='Specification')
+
+    deleted = Bool(
+        title=_('Is this work item deleted?'),
+        required=True, readonly=False, default=False,
+        description=_("Marks the work item as deleted."))
+
+    sequence = Int(
+        title=_("Work Item Sequence."),
+        required=True, description=_(
+            "The sequence in which the work items are to be displayed in the "
+            "UI."))

=== modified file 'lib/lp/blueprints/model/specification.py'
--- lib/lp/blueprints/model/specification.py	2011-12-30 06:14:56 +0000
+++ lib/lp/blueprints/model/specification.py	2012-02-15 21:39:22 +0000
@@ -45,6 +45,7 @@
     SpecificationLifecycleStatus,
     SpecificationPriority,
     SpecificationSort,
+    SpecificationWorkItemStatus,
     )
 from lp.blueprints.errors import TargetAlreadyHasSpecification
 from lp.blueprints.interfaces.specification import (
@@ -60,6 +61,7 @@
 from lp.blueprints.model.specificationsubscription import (
     SpecificationSubscription,
     )
+from lp.blueprints.model.specificationworkitem import SpecificationWorkItem
 from lp.bugs.interfaces.buglink import IBugLinkTarget
 from lp.bugs.interfaces.bugtask import (
     BugTaskSearchParams,
@@ -228,6 +230,14 @@
             return self.product
         return self.distribution
 
+    def newWorkItem(self, title, sequence,
+                    status=SpecificationWorkItemStatus.TODO, assignee=None,
+                    milestone=None):
+        """See ISpecification."""
+        return SpecificationWorkItem(
+            title=title, status=status, specification=self, assignee=assignee,
+            milestone=milestone, sequence=sequence)
+
     def setTarget(self, target):
         """See ISpecification."""
         if IProduct.providedBy(target):

=== added file 'lib/lp/blueprints/model/specificationworkitem.py'
--- lib/lp/blueprints/model/specificationworkitem.py	1970-01-01 00:00:00 +0000
+++ lib/lp/blueprints/model/specificationworkitem.py	2012-02-15 21:39:22 +0000
@@ -0,0 +1,62 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+__all__ = [
+    'SpecificationWorkItem',
+    ]
+
+from zope.interface import implements
+
+from storm.locals import (
+    Bool,
+    Int,
+    Reference,
+    Unicode,
+    )
+
+from lp.services.database.constants import DEFAULT
+from lp.services.database.datetimecol import UtcDateTimeCol
+from lp.services.database.enumcol import EnumCol
+from lp.services.database.stormbase import StormBase
+
+from lp.blueprints.enums import SpecificationWorkItemStatus
+from lp.blueprints.interfaces.specificationworkitem import (
+    ISpecificationWorkItem,
+    )
+from lp.registry.interfaces.person import validate_public_person
+
+
+class SpecificationWorkItem(StormBase):
+    implements(ISpecificationWorkItem)
+
+    __storm_table__ = 'SpecificationWorkItem'
+
+    id = Int(primary=True)
+    title = Unicode(allow_none=False)
+    specification_id = Int(name='specification')
+    specification = Reference(specification_id, 'Specification.id')
+    assignee_id = Int(name='assignee', validator=validate_public_person)
+    assignee = Reference(assignee_id, 'Person.id')
+    milestone_id = Int(name='milestone')
+    milestone = Reference(milestone_id, 'Milestone.id')
+    status = EnumCol(
+        schema=SpecificationWorkItemStatus,
+        notNull=True, default=SpecificationWorkItemStatus.TODO)
+    date_created = UtcDateTimeCol(notNull=True, default=DEFAULT)
+    sequence = Int(allow_none=False)
+    deleted = Bool(allow_none=False, default=False)
+
+    def __repr__(self):
+        title = self.title.encode('ASCII', 'backslashreplace')
+        return '<SpecificationWorkItem [%s] %s: %s of %s>' % (
+            self.assignee, title, self.status, self.specification)
+
+    def __init__(self, title, status, specification, assignee, milestone,
+                 sequence):
+        self.title=title
+        self.status=status
+        self.specification = specification
+        self.assignee=assignee
+        self.milestone=milestone
+        self.sequence=sequence

=== modified file 'lib/lp/blueprints/model/tests/test_specification.py'
--- lib/lp/blueprints/model/tests/test_specification.py	2012-01-01 02:58:52 +0000
+++ lib/lp/blueprints/model/tests/test_specification.py	2012-02-15 21:39:22 +0000
@@ -9,9 +9,19 @@
 
 from lp.app.validators import LaunchpadValidationError
 from lp.blueprints.interfaces.specification import ISpecification
+from lp.blueprints.interfaces.specificationworkitem import (
+    SpecificationWorkItemStatus,
+    )
+from lp.blueprints.model.specificationworkitem import SpecificationWorkItem
 from lp.services.webapp import canonical_url
-from lp.testing import TestCaseWithFactory
+from lp.testing import (
+    TestCaseWithFactory,
+    ANONYMOUS,
+    login,
+    login_person,
+    )
 from lp.testing.layers import DatabaseFunctionalLayer
+from zope.security.interfaces import Unauthorized
 
 
 class TestSpecificationDependencies(TestCaseWithFactory):
@@ -132,3 +142,35 @@
         self.assertEqual(
             '%s is already registered by <a href="%s">%s</a>.'
             % (u'http://ubuntu.com/foo', url, cleaned_title), str(e))
+
+
+class TestSpecificationWorkItems(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_anonymous_newworkitem_not_allowed(self):
+        spec = self.factory.makeSpecification()
+        login(ANONYMOUS)
+        self.assertRaises(Unauthorized, getattr, spec, 'newWorkItem')
+
+    def test_owner_newworkitem_allowed(self):
+        spec = self.factory.makeSpecification()
+        login_person(spec.owner)
+        work_item = spec.newWorkItem(title=u'new-work-item', sequence=0)
+        self.assertIsInstance(work_item, SpecificationWorkItem)
+
+    def test_newworkitem_uses_passed_arguments(self):
+        title = u'new-work-item'
+        spec = self.factory.makeSpecification()
+        assignee = self.factory.makePerson()
+        milestone = self.factory.makeMilestone()
+        status = SpecificationWorkItemStatus.DONE
+        login_person(spec.owner)
+        work_item = spec.newWorkItem(
+            title=title, assignee=assignee, milestone=milestone,
+            status=status, sequence=0)
+        self.assertEqual(spec, work_item.specification)
+        self.assertEqual(assignee, work_item.assignee)
+        self.assertEqual(status, work_item.status)
+        self.assertEqual(title, work_item.title)
+        self.assertEqual(milestone, work_item.milestone)

=== modified file 'lib/lp/blueprints/tests/test_implements.py'
--- lib/lp/blueprints/tests/test_implements.py	2012-01-01 02:58:52 +0000
+++ lib/lp/blueprints/tests/test_implements.py	2012-02-15 21:39:22 +0000
@@ -9,6 +9,9 @@
     IHasSpecifications,
     ISpecificationTarget,
     )
+from lp.blueprints.interfaces.specificationworkitem import (
+    ISpecificationWorkItem,
+    )
 from lp.testing import TestCaseWithFactory
 from lp.testing.layers import DatabaseFunctionalLayer
 
@@ -65,3 +68,13 @@
     def test_distroseries_implements_ISpecificationTarget(self):
         distroseries = self.factory.makeDistroSeries()
         self.assertProvides(distroseries, ISpecificationTarget)
+
+
+class ImplementsISpecificationWorkItemTests(TestCaseWithFactory):
+    """Test that various objects implement ISpecificationWorkItem."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_specificationworkitem_implements_ISpecificationTarget(self):
+        specificationworkitem = self.factory.makeSpecificationWorkItem()
+        self.assertProvides(specificationworkitem, ISpecificationWorkItem)

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2012-02-14 02:22:59 +0000
+++ lib/lp/testing/factory.py	2012-02-15 21:39:22 +0000
@@ -73,6 +73,7 @@
     NewSpecificationDefinitionStatus,
     SpecificationDefinitionStatus,
     SpecificationPriority,
+    SpecificationWorkItemStatus,
     )
 from lp.blueprints.interfaces.specification import ISpecificationSet
 from lp.blueprints.interfaces.sprint import ISprintSet
@@ -2104,6 +2105,22 @@
 
     makeBlueprint = makeSpecification
 
+    def makeSpecificationWorkItem(self, title=None, specification=None,
+                                  assignee=None, milestone=None, deleted=False,
+                                  status=SpecificationWorkItemStatus.TODO,
+                                  sequence=None):
+        if title is None:
+            title = self.getUniqueString(u'title')
+        if specification is None:
+            specification = self.makeSpecification()
+        if sequence is None:
+            sequence = self.getUniqueInteger()
+        work_item = removeSecurityProxy(specification).newWorkItem(
+            title=title, sequence=sequence, status=status, assignee=assignee,
+            milestone=milestone)
+        work_item.deleted = deleted
+        return work_item
+
     def makeQuestion(self, target=None, title=None,
                      owner=None, description=None, language=None):
         """Create and return a new, arbitrary Question.