launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/complete-incomplete into lp:launchpad.

Commit message:
Fix BugTaskSet.createManyTasks to map Incomplete to its storage values.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1576857 in Launchpad itself: "Bug tasks can be created with bare unsearchable Incomplete status"
  https://bugs.launchpad.net/launchpad/+bug/1576857

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/complete-incomplete/+merge/293859

Fix BugTaskSet.createManyTasks to map Incomplete to its storage values.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/complete-incomplete into lp:launchpad.

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2015-09-28 12:25:52 +0000
+++ lib/lp/bugs/model/bugtask.py	2016-05-05 08:25:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Classes that implement IBugTask and its related interfaces."""
@@ -302,6 +302,19 @@
         return value
 
 
+def map_status_for_storage(bug, status, when=None):
+    if status == BugTaskStatus.INCOMPLETE:
+        # We store INCOMPLETE as INCOMPLETE_WITHOUT_RESPONSE so that it
+        # can be queried on efficiently.
+        if (when is None or bug.date_last_message is None or
+            when > bug.date_last_message):
+            return BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE
+        else:
+            return BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE
+    else:
+        return status
+
+
 def validate_assignee(self, attr, value):
     value = validate_conjoined_attribute(self, attr, value)
     # Check if this person is valid and not None.
@@ -868,15 +881,7 @@
                 "Only Bug Supervisors may change status to %s." % (
                     new_status.title,))
 
-        if new_status == BugTaskStatus.INCOMPLETE:
-            # We store INCOMPLETE as INCOMPLETE_WITHOUT_RESPONSE so that it
-            # can be queried on efficiently.
-            if (when is None or self.bug.date_last_message is None or
-                when > self.bug.date_last_message):
-                new_status = BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE
-            else:
-                new_status = BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE
-
+        new_status = map_status_for_storage(self.bug, new_status, when=when)
         self._setStatusDateProperties(self.status, new_status, when=when)
 
     def _setStatusDateProperties(self, old_status, new_status, when=None):
@@ -1573,6 +1578,7 @@
         """See `IBugTaskSet`."""
         if status is None:
             status = IBugTask['status'].default
+        status = map_status_for_storage(bug, status)
         if importance is None:
             importance = IBugTask['importance'].default
         target_keys = []

=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py	2016-01-26 15:47:37 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py	2016-05-05 08:25:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2014 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -224,6 +224,21 @@
         self.assertContentEqual(
             expected_policies, get_policies_for_artifact(bug))
 
+    def test_incomplete_mapped_for_storage(self):
+        """A task is never actually "Incomplete" in the DB.
+
+        Incomplete is automatically mapped to (with response) or
+        (without response) as appropriate.
+        """
+        task = getUtility(IBugTaskSet).createTask(
+            self.factory.makeBug(), self.factory.makePerson(),
+            self.factory.makeProduct(),
+            status=BugTaskStatus.INCOMPLETE)
+        self.assertEqual(
+            BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE, task._status)
+        self.assertEqual(
+            BugTaskStatus.INCOMPLETE, task.status)
+
 
 class TestBugTaskCreationPackageComponent(TestCaseWithFactory):
     """IBugTask contains a convenience method to look up archive component

=== modified file 'lib/lp/bugs/tests/test_bug_messages.py'
--- lib/lp/bugs/tests/test_bug_messages.py	2012-09-18 18:36:09 +0000
+++ lib/lp/bugs/tests/test_bug_messages.py	2016-05-05 08:25:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Webservice unit tests related to Launchpad Bugs."""
@@ -8,10 +8,15 @@
 from zope.component import getUtility
 
 from lp.app.enums import InformationType
+from lp.bugs.interfaces.bugtask import (
+    BugTaskStatus,
+    BugTaskStatusSearch,
+    )
 from lp.registry.interfaces.accesspolicy import IAccessPolicySource
 from lp.testing import (
     login,
     login_celebrity,
+    person_logged_in,
     TestCaseWithFactory,
     )
 from lp.testing.layers import DatabaseFunctionalLayer
@@ -77,3 +82,48 @@
         self.factory.makeAccessPolicyGrant(
             policy=policy, grantor=product.owner, grantee=person)
         self.assertTrue(bug.userCanSetCommentVisibility(person))
+
+
+class TestBugLinkMessageSetsIncompleteStatus(TestCaseWithFactory):
+
+    """Test that Bug.linkMessage updates "Incomplete (without response)" bugs.
+
+    They should transition from "Incomplete (without response)" to
+    "Incomplete (with response)".
+    """
+
+    layer = DatabaseFunctionalLayer
+
+    def test_new_untouched(self):
+        bugtask = self.factory.makeBugTask(status=BugTaskStatus.NEW)
+        with person_logged_in(bugtask.owner):
+            bugtask.bug.linkMessage(self.factory.makeMessage())
+        self.assertEqual(BugTaskStatus.NEW, bugtask.status)
+
+    def test_incomplete_with_response_untouched(self):
+        bugtask = self.factory.makeBugTask(
+            status=BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE)
+        self.assertEqual(
+            BugTaskStatus.INCOMPLETE, bugtask.status)
+        self.assertEqual(
+            BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE, bugtask._status)
+        with person_logged_in(bugtask.owner):
+            bugtask.bug.linkMessage(self.factory.makeMessage())
+        self.assertEqual(
+            BugTaskStatus.INCOMPLETE, bugtask.status)
+        self.assertEqual(
+            BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE, bugtask._status)
+
+    def test_incomplete_without_response_updated(self):
+        bugtask = self.factory.makeBugTask(
+            status=BugTaskStatus.INCOMPLETE)
+        self.assertEqual(
+            BugTaskStatus.INCOMPLETE, bugtask.status)
+        self.assertEqual(
+            BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE, bugtask._status)
+        with person_logged_in(bugtask.owner):
+            bugtask.bug.linkMessage(self.factory.makeMessage())
+        self.assertEqual(
+            BugTaskStatus.INCOMPLETE, bugtask.status)
+        self.assertEqual(
+            BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE, bugtask._status)

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2016-04-28 02:25:46 +0000
+++ lib/lp/testing/factory.py	2016-05-05 08:25:25 +0000
@@ -83,7 +83,10 @@
     CreateBugParams,
     IBugSet,
     )
-from lp.bugs.interfaces.bugtask import BugTaskStatus
+from lp.bugs.interfaces.bugtask import (
+    BugTaskStatus,
+    IBugTaskSet,
+    )
 from lp.bugs.interfaces.bugtracker import (
     BugTrackerType,
     IBugTrackerSet,
@@ -1875,7 +1878,8 @@
         removeSecurityProxy(bug).clearBugNotificationRecipientsCache()
         return bug
 
-    def makeBugTask(self, bug=None, target=None, owner=None, publish=True):
+    def makeBugTask(self, bug=None, target=None, owner=None, publish=True,
+                    status=None):
         """Create and return a bug task.
 
         If the bug is already targeted to the given target, the existing
@@ -1944,8 +1948,8 @@
 
         if owner is None:
             owner = self.makePerson()
-        task = removeSecurityProxy(bug).addTask(
-            owner, removeSecurityProxy(target))
+        task = getUtility(IBugTaskSet).createTask(
+            bug, owner, target, status=status)
         removeSecurityProxy(bug).clearBugNotificationRecipientsCache()
         return task