launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06492
lp:~linaro-infrastructure/launchpad/workitems-parsing-and-updating into lp:launchpad
Guilherme Salgado has proposed merging lp:~linaro-infrastructure/launchpad/workitems-parsing-and-updating into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~linaro-infrastructure/launchpad/workitems-parsing-and-updating/+merge/94552
This branch adds a new method on ISpecification which will take a list of dictionaries describing work items and update/delete/create work items in the DB to match that. This is going to be used in the new UI for editing work items (lp:~linaro-infrastructure/launchpad/workitems-widget), which will consist of a text area widget identical to the one currently used for the whiteboard.
In lp:~linaro-infrastructure/launchpad/workitems-widget we're implementing a new field class which will parse the work items entered by the user into the list of dicts expected by updateWorkItems()
--
https://code.launchpad.net/~linaro-infrastructure/launchpad/workitems-parsing-and-updating/+merge/94552
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~linaro-infrastructure/launchpad/workitems-parsing-and-updating into lp:launchpad.
=== modified file 'lib/lp/blueprints/interfaces/specification.py'
--- lib/lp/blueprints/interfaces/specification.py 2012-02-09 01:19:00 +0000
+++ lib/lp/blueprints/interfaces/specification.py 2012-02-24 14:50:29 +0000
@@ -41,6 +41,7 @@
Choice,
Datetime,
Int,
+ List,
Text,
TextLine,
)
@@ -63,6 +64,9 @@
IHasSpecifications,
ISpecificationTarget,
)
+from lp.blueprints.interfaces.specificationworkitem import (
+ ISpecificationWorkItem
+ )
from lp.blueprints.interfaces.sprint import ISprint
from lp.bugs.interfaces.buglink import IBugLinkTarget
from lp.code.interfaces.branchlink import IHasLinkedBranches
@@ -289,6 +293,10 @@
date_goal_decided = Attribute("The date the spec was approved "
"or declined as a goal.")
+ work_items = List(
+ description=_("All non-deleted work items for this spec, sorted by "
+ "their 'sequence'"),
+ value_type=Reference(schema=ISpecificationWorkItem), readonly=True)
whiteboard = exported(
Text(title=_('Status Whiteboard'), required=False,
description=_(
@@ -575,6 +583,20 @@
milestone=None):
"""Create a new SpecificationWorkItem."""
+ def updateWorkItems(new_work_items):
+ """Update the existing work items to match the given ones.
+
+ First, for every existing work item that is not present on the new
+ list, mark it as deleted. Then, for every tuple in the given list,
+ lookup an existing work item with the same title and update its
+ status, assignee, milestone and sequence (position on the work-items
+ list). If there's no existing work items with that title, we create a
+ new one.
+
+ :param new_work_items: A list of dictionaries containing the following
+ keys: title, status, assignee and milestone.
+ """
+
def setTarget(target):
"""Set this specification's target.
=== modified file 'lib/lp/blueprints/model/specification.py'
--- lib/lp/blueprints/model/specification.py 2012-02-07 01:43:29 +0000
+++ lib/lp/blueprints/model/specification.py 2012-02-24 14:50:29 +0000
@@ -234,10 +234,73 @@
status=SpecificationWorkItemStatus.TODO, assignee=None,
milestone=None):
"""See ISpecification."""
+ if milestone is not None:
+ assert milestone.target == self.target, (
+ "%s does not belong to this spec's target (%s)" %
+ (milestone.displayname, self.target.name))
return SpecificationWorkItem(
title=title, status=status, specification=self, assignee=assignee,
milestone=milestone, sequence=sequence)
+ @property
+ def work_items(self):
+ """See ISpecification."""
+ return Store.of(self).find(
+ SpecificationWorkItem, specification=self,
+ deleted=False).order_by("sequence")
+
+ def _deleteWorkItemsNotMatching(self, titles):
+ """Delete all work items whose title does not match the given ones.
+
+ Also set the sequence of those deleted work items to -1.
+ """
+ for work_item in self.work_items:
+ if work_item.title not in titles:
+ work_item.deleted = True
+
+ def updateWorkItems(self, new_work_items):
+ """See ISpecification."""
+ # First mark work items with titles that are no longer present as
+ # deleted.
+ self._deleteWorkItemsNotMatching(
+ [wi['title'] for wi in new_work_items])
+ work_items = Store.of(self).find(
+ SpecificationWorkItem, specification=self, deleted=False)
+ work_items = list(work_items.order_by("sequence"))
+ # At this point the list of new_work_items is necessarily the same
+ # size (or longer) than the list of existing ones, so we can just
+ # iterate over it updating the existing items and creating any new
+ # ones.
+ to_insert = []
+ existing_titles = [wi.title for wi in work_items]
+ for i in range(len(new_work_items)):
+ new_wi = new_work_items[i]
+ if new_wi['title'] not in existing_titles:
+ # This is a new work item, so we insert it with 'i' as its
+ # sequence because that's the position it is on the list
+ # entered by the user.
+ to_insert.append((i, new_wi))
+ else:
+ # Get the existing work item with the same title and update
+ # it to match what we have now.
+ existing_wi = work_items[
+ existing_titles.index(new_wi['title'])]
+ # Update the sequence to match its current position on the
+ # list entered by the user.
+ existing_wi.sequence = i
+ existing_wi.status = new_wi['status']
+ existing_wi.assignee = new_wi['assignee']
+ milestone = new_wi['milestone']
+ if milestone is not None:
+ assert milestone.target == self.target, (
+ "%s does not belong to this spec's target (%s)" %
+ (milestone.displayname, self.target.name))
+ existing_wi.milestone = milestone
+
+ for sequence, item in to_insert:
+ self.newWorkItem(item['title'], sequence, item['status'],
+ item['assignee'], item['milestone'])
+
def setTarget(self, target):
"""See ISpecification."""
if IProduct.providedBy(target):
=== modified file 'lib/lp/blueprints/model/tests/test_specification.py'
--- lib/lp/blueprints/model/tests/test_specification.py 2012-02-10 17:32:41 +0000
+++ lib/lp/blueprints/model/tests/test_specification.py 2012-02-24 14:50:29 +0000
@@ -5,7 +5,10 @@
__metaclass__ = type
-from testtools.matchers import Equals
+from testtools.matchers import (
+ Equals,
+ MatchesStructure,
+ )
from lp.app.validators import LaunchpadValidationError
from lp.blueprints.interfaces.specification import ISpecification
@@ -145,6 +148,7 @@
class TestSpecificationWorkItems(TestCaseWithFactory):
+ """Test the Workitem-related methods of ISpecification."""
layer = DatabaseFunctionalLayer
@@ -163,7 +167,7 @@
title = u'new-work-item'
spec = self.factory.makeSpecification()
assignee = self.factory.makePerson()
- milestone = self.factory.makeMilestone()
+ milestone = self.factory.makeMilestone(product=spec.product)
status = SpecificationWorkItemStatus.DONE
login_person(spec.owner)
work_item = spec.newWorkItem(
@@ -174,3 +178,146 @@
self.assertEqual(status, work_item.status)
self.assertEqual(title, work_item.title)
self.assertEqual(milestone, work_item.milestone)
+
+ def test_work_items_property(self):
+ spec = self.factory.makeSpecification()
+ wi1 = self.factory.makeSpecificationWorkItem(
+ specification=spec, sequence=2)
+ wi2 = self.factory.makeSpecificationWorkItem(
+ specification=spec, sequence=1)
+ # This work item won't be included in the results of spec.work_items
+ # because it is deleted.
+ self.factory.makeSpecificationWorkItem(
+ specification=spec, sequence=3, deleted=True)
+ # This work item belongs to a different spec so it won't be returned
+ # by spec.work_items.
+ self.factory.makeSpecificationWorkItem()
+ self.assertEqual([wi2, wi1], list(spec.work_items))
+
+ def test_updateWorkItems_no_existing_items(self):
+ """When there are no existing work items, updateWorkItems will create
+ a new entry for every element in the list given to it.
+ """
+ spec = self.factory.makeSpecification(
+ product=self.factory.makeProduct())
+ milestone = self.factory.makeMilestone(product=spec.product)
+ work_item1_data = dict(
+ title=u'Foo Bar', status=SpecificationWorkItemStatus.DONE,
+ assignee=spec.owner, milestone=None)
+ work_item2_data = dict(
+ title=u'Bar Foo', status=SpecificationWorkItemStatus.TODO,
+ assignee=None, milestone=milestone)
+
+ # We start with no work items.
+ self.assertEquals([], list(spec.work_items))
+
+ login_person(spec.owner)
+ spec.updateWorkItems([work_item1_data, work_item2_data])
+
+ # And after calling updateWorkItems() we have 2 work items.
+ self.assertEqual(2, spec.work_items.count())
+
+ # The data dicts we pass to updateWorkItems() have no sequence because
+ # that's taken from their position on the list, so we update our data
+ # dicts with the sequence we expect our work items to have.
+ work_item1_data['sequence'] = 0
+ work_item2_data['sequence'] = 1
+
+ # Assert that the work items ultimately inserted in the DB are exactly
+ # what we expect them to be.
+ created_wi1, created_wi2 = list(spec.work_items)
+ self.assertThat(
+ created_wi1, MatchesStructure.byEquality(**work_item1_data))
+ self.assertThat(
+ created_wi2, MatchesStructure.byEquality(**work_item2_data))
+
+ def test_updateWorkItems_merges_with_existing_ones(self):
+ spec = self.factory.makeSpecification(
+ product=self.factory.makeProduct())
+ login_person(spec.owner)
+ # Create two work-items in our database.
+ wi1_data = self._createWorkItemAndReturnDataDict(spec)
+ wi2_data = self._createWorkItemAndReturnDataDict(spec)
+ self.assertEqual(2, spec.work_items.count())
+
+ # These are the work items we'll be inserting.
+ new_wi1_data = dict(
+ title=u'Some Title', status=SpecificationWorkItemStatus.TODO,
+ assignee=None, milestone=None)
+ new_wi2_data = dict(
+ title=u'Other title', status=SpecificationWorkItemStatus.TODO,
+ assignee=None, milestone=None)
+
+ # We want to insert the two work items above in the first and third
+ # positions respectively, so the existing ones to be moved around
+ # (e.g. have their sequence updated).
+ work_items = [new_wi1_data, wi1_data, new_wi2_data, wi2_data]
+ spec.updateWorkItems(work_items)
+
+ # Update our data dicts with the sequences we expect the work items in
+ # our DB to have.
+ new_wi1_data['sequence'] = 0
+ wi1_data['sequence'] = 1
+ new_wi2_data['sequence'] = 2
+ wi2_data['sequence'] = 3
+
+ self.assertEqual(4, spec.work_items.count())
+ for data, obj in zip(work_items, list(spec.work_items)):
+ self.assertThat(obj, MatchesStructure.byEquality(**data))
+
+ def test_updateWorkItems_updates_existing_ones(self):
+ spec = self.factory.makeSpecification()
+ login_person(spec.owner)
+ # Create a work-item in our database.
+ wi_data = self._createWorkItemAndReturnDataDict(spec)
+ self.assertEqual(1, spec.work_items.count())
+
+ # This time we're only changing the existing work item; we'll change
+ # its assignee and status.
+ wi_data.update(dict(status=SpecificationWorkItemStatus.DONE,
+ assignee=spec.owner))
+ spec.updateWorkItems([wi_data])
+
+ self.assertEqual(1, spec.work_items.count())
+ self.assertThat(
+ spec.work_items[0], MatchesStructure.byEquality(**wi_data))
+
+ def test_updateWorkItems_deletes_all_if_given_empty_list(self):
+ work_item = self.factory.makeSpecificationWorkItem()
+ spec = work_item.specification
+ self.assertEqual(1, spec.work_items.count())
+ spec.updateWorkItems([])
+ self.assertEqual(0, spec.work_items.count())
+
+ def test_updateWorkItems_marks_removed_ones_as_deleted(self):
+ spec = self.factory.makeSpecification()
+ self._createWorkItemAndReturnDataDict(spec)
+ wi2_data = self._createWorkItemAndReturnDataDict(spec)
+ self.assertEqual(2, spec.work_items.count())
+ login_person(spec.owner)
+
+ # We have two work items in the DB but now we want to update them to
+ # keep just the second one. The first will be deleted and the sequence
+ # of the second will be changed.
+ spec.updateWorkItems([wi2_data])
+ self.assertEqual(1, spec.work_items.count())
+ wi2_data['sequence'] = 0
+ self.assertThat(
+ spec.work_items[0], MatchesStructure.byEquality(**wi2_data))
+
+ def _createWorkItemAndReturnDataDict(self, spec):
+ """Create a new work item for the given spec using the next available
+ sequence number.
+
+ Return a dict with the title, status, assignee, milestone and sequence
+ attributes of the spec.
+ """
+ if spec.work_items.count() == 0:
+ sequence = 0
+ else:
+ sequence = max(wi.sequence for wi in spec.work_items) + 1
+ wi = self.factory.makeSpecificationWorkItem(
+ specification=spec, sequence=sequence)
+ return dict(
+ title=wi.title, status=wi.status, assignee=wi.assignee,
+ milestone=wi.milestone, sequence=sequence)
=== modified file 'lib/lp/registry/model/milestone.py'
--- lib/lp/registry/model/milestone.py 2012-01-16 00:07:45 +0000
+++ lib/lp/registry/model/milestone.py 2012-02-24 14:50:29 +0000
@@ -366,7 +366,7 @@
"""A virtual milestone implementation for project.
The current database schema has no formal concept of milestones related to
- projects. A milestone named `milestone` is considererd to belong to
+ projects. A milestone named `milestone` is considered to belong to
a project if the project contains at least one product with a milestone
of the same name. A project milestone is considered to be active if at
least one product milestone with the same name is active. The
