launchpad-reviewers team mailing list archive

[James Tunnicliffe <launchpad@xxxxxxxxxxxxx>]

James Tunnicliffe has proposed merging lp:~dooferlad/launchpad/upcoming_view_show_all_work_items into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #998052 in Launchpad itself: "Upcoming Work View now showing all desired work items"
  https://bugs.launchpad.net/launchpad/+bug/998052

For more details, see:
https://code.launchpad.net/~dooferlad/launchpad/upcoming_view_show_all_work_items/+merge/105495

Summary
Bug fix: Show all WIs owned by members of the team and all WIs from BPs assigned to members of the team (even if WI owner is not in the team)

Proposed fix
Slightly modified DB query to get the correct list of work items.

Pre-implementation notes
None

Implementation details
Slightly modified DB query to get the correct list of work items.

LOC Rationale
1 line reduction in live code. Added 1 test. This results in a net increase, but tests the change.

Tests
bin/test -cvt test_workitems_assigned_to_others_working_on_blueprint

Demo and Q/A
https://launchpad.net/~linaro-infrastructure/+upcomingwork shows a list of blueprints. The one entitled "Setup Freescale Click Through" has one work item listed. If you look at the blueprint https://blueprints.launchpad.net/linaro-android/+spec/setup-freescale-click-through you can see work item assigned to other people.

Lint
= Launchpad lint = / 

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/model/person.py
  lib/lp/registry/tests/test_person.py

./lib/lp/registry/model/person.py
    2990: E501 line too long (84 characters)

Above error is not part of my change.
-- 
https://code.launchpad.net/~dooferlad/launchpad/upcoming_view_show_all_work_items/+merge/105495
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~dooferlad/launchpad/upcoming_view_show_all_work_items into lp:launchpad.

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2012-05-04 12:02:44 +0000
+++ lib/lp/registry/model/person.py	2012-05-11 14:49:18 +0000
@@ -1502,8 +1502,7 @@
         query = AND(
             Milestone.dateexpected <= date, Milestone.dateexpected >= today,
             OR(WorkItem.assignee_id.is_in(self.participant_ids),
-               AND(WorkItem.assignee == None,
-                   Specification.assigneeID.is_in(self.participant_ids))))
+               Specification.assigneeID.is_in(self.participant_ids)))
         result = store.using(*origin).find(WorkItem, query)
 
         def eager_load(workitems):

=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py	2012-05-02 05:25:11 +0000
+++ lib/lp/registry/tests/test_person.py	2012-05-11 14:49:18 +0000
@@ -1143,18 +1143,11 @@
         workitem = self.factory.makeSpecificationWorkItem(
             title=u'workitem 1', specification=assigned_spec)
 
-        # Create a workitem with somebody who's not a member of our team as
-        # the assignee. This workitem must not be in the list returned by
-        # getAssignedSpecificationWorkItemsDueBefore().
-        self.factory.makeSpecificationWorkItem(
-            title=u'workitem 2', specification=assigned_spec,
-            assignee=self.factory.makePerson())
-
         # Create a workitem targeted to a milestone too far in the future.
         # This workitem must not be in the list returned by
         # getAssignedSpecificationWorkItemsDueBefore().
         self.factory.makeSpecificationWorkItem(
-            title=u'workitem 3', specification=assigned_spec,
+            title=u'workitem 2', specification=assigned_spec,
             milestone=self.future_milestone)
 
         workitems = self.team.getAssignedSpecificationWorkItemsDueBefore(
@@ -1162,6 +1155,28 @@
 
         self.assertEqual([workitem], list(workitems))
 
+    def test_workitems_assigned_to_others_working_on_blueprint(self):
+        assigned_spec = self.factory.makeSpecification(
+                assignee=self.team.teamowner, milestone=self.current_milestone,
+                product=self.product)
+        # Create a workitem with no explicit assignee/milestone. This way it
+        # will inherit the ones from the spec it belongs to.
+        workitem = self.factory.makeSpecificationWorkItem(
+            title=u'workitem 1', specification=assigned_spec)
+
+        # Create a workitem with somebody who's not a member of our team as
+        # the assignee. This workitem must be in the list returned by
+        # getAssignedSpecificationWorkItemsDueBefore().
+        workitem_for_other_person = self.factory.makeSpecificationWorkItem(
+            title=u'workitem 2', specification=assigned_spec,
+            assignee=self.factory.makePerson())
+
+        workitems = self.team.getAssignedSpecificationWorkItemsDueBefore(
+            self.current_milestone.dateexpected)
+
+        self.assertContentEqual([workitem, workitem_for_other_person],
+                                list(workitems))
+
     def test_skips_workitems_with_milestone_in_the_past(self):
         today = datetime.today().date()
         milestone = self.factory.makeMilestone(