launchpad-reviewers team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Brad Crittenden has proposed merging lp:~bac/launchpad/bug-828572 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #828572 in Launchpad itself: "bugs are marked incomplete_with_response if users or scripts change the status / tags immediately after setting the status"
  https://bugs.launchpad.net/launchpad/+bug/828572

For more details, see:
https://code.launchpad.net/~bac/launchpad/bug-828572/+merge/75263

= Summary =

Changes to a bug task immediately (< 1 minute) after marking it
incomplete should not count as a response.

== Proposed fix ==

Put in time interval tweaks in the query.

== Pre-implementation notes ==

None

== Implementation details ==

As above

== Tests ==

bin/test -vvm lp.bugs -t test_incomplete

== Demo and Q/A ==

Mark bug task incomplete.  Within a minute add a tag.  See that
searches for WITH and WITHOUT RESPONSES work.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/model/tests/test_bugtask.py
  lib/lp/bugs/model/bugtask.py
-- 
https://code.launchpad.net/~bac/launchpad/bug-828572/+merge/75263
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/bug-828572 into lp:launchpad.

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2011-09-12 20:27:32 +0000
+++ lib/lp/bugs/model/bugtask.py	2011-09-13 21:22:25 +0000
@@ -1694,17 +1694,21 @@
                 status_clause = (
                     '(BugTask.status = %s) ' %
                     sqlvalues(BugTaskStatus.INCOMPLETE))
+                # The date_last_message is fudged by one minute for
+                # with_response and without_response to not consider changes
+                # made at about the same time as responses.  See Bug 828572
+                # for more details.
                 if with_response:
                     status_clause += ("""
                         AND (Bug.date_last_message IS NOT NULL
                              AND BugTask.date_incomplete <=
-                                 Bug.date_last_message)
+                                 Bug.date_last_message - interval '1 minute')
                         """)
                 elif without_response:
                     status_clause += ("""
                         AND (Bug.date_last_message IS NULL
                              OR BugTask.date_incomplete >
-                                Bug.date_last_message)
+                                Bug.date_last_message - interval '1 minute')
                         """)
                 else:
                     assert with_response != without_response

=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py	2011-09-12 20:27:32 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py	2011-09-13 21:22:25 +0000
@@ -32,6 +32,7 @@
     BugTaskImportance,
     BugTaskSearchParams,
     BugTaskStatus,
+    BugTaskStatusSearch,
     IBugTaskSet,
     RESOLVED_BUGTASK_STATUSES,
     UNRESOLVED_BUGTASK_STATUSES,
@@ -1008,6 +1009,68 @@
         result = target.searchTasks(None, created_since=date)
         self.assertEqual([task2], list(result))
 
+    def _setupIncompleteBugTask(self):
+        target = self.makeBugTarget()
+        self.login()
+        task = self.factory.makeBugTask(target=target)
+        bug = task.bug
+        task.transitionToStatus(BugTaskStatus.INCOMPLETE, bug.owner)
+        return target, task, bug
+
+    def test_incomplete_with_response_none_found(self):
+        target, task, bug = self._setupIncompleteBugTask()
+        btsp = BugTaskSearchParams(
+            bug.owner,
+            status=BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE)
+        result = target.searchTasks(btsp)
+        self.assertEqual([], list(result))
+
+    def test_incomplete_with_response_ignores_immediate_changes(self):
+        target, task, bug = self._setupIncompleteBugTask()
+        bug.tags = 'escalated'
+        btsp = BugTaskSearchParams(
+            bug.owner,
+            status=BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE)
+        result = target.searchTasks(btsp)
+        self.assertEqual([], list(result))
+
+    def test_incomplete_with_response_added(self):
+        target, task, bug = self._setupIncompleteBugTask()
+        bug.tags = 'escalated'
+        removeSecurityProxy(bug).date_last_message += timedelta(seconds=65)
+        btsp = BugTaskSearchParams(
+            bug.owner,
+            status=BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE)
+        result = target.searchTasks(btsp)
+        self.assertEqual([task], list(result))
+
+    def test_incomplete_without_response_none_found(self):
+        target, task, bug = self._setupIncompleteBugTask()
+        btsp = BugTaskSearchParams(
+            bug.owner,
+            status=BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE)
+        result = target.searchTasks(btsp)
+        self.assertEqual([task], list(result))
+
+    def test_incomplete_without_response_ignores_immediate_change(self):
+        target, task, bug = self._setupIncompleteBugTask()
+        bug.tags = 'escalated'
+        btsp = BugTaskSearchParams(
+            bug.owner,
+            status=BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE)
+        result = target.searchTasks(btsp)
+        self.assertEqual([task], list(result))
+
+    def test_incomplete_without_response_one_added(self):
+        target, task, bug = self._setupIncompleteBugTask()
+        bug.tags = 'escalated'
+        removeSecurityProxy(bug).date_last_message += timedelta(seconds=65)
+        btsp = BugTaskSearchParams(
+            bug.owner,
+            status=BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE)
+        result = target.searchTasks(btsp)
+        self.assertEqual([], list(result))
+
 
 class BugTaskSetSearchTest(TestCaseWithFactory):