Merge lp:~allenap/launchpad/dont-update-dupe-bugs-bug-511276 into lp:launchpad

[Gavin Panella]

Don't update bug task status or importance in duplicate bugs when a watch is updated.

This changes BugWatch.updateStatus() and BugWatch.updateImportance() to not modify the status when the linked bug tasks are part of a duplicate bug. I've done this by factoring out the selection of bug tasks to update into a new property, BugWatch.bugtasks_to_update.

[Abel Deuring (adeuring)]

Nice branch! just one nit pick:

+ # If we add a task such that the existing task becomes a
+ # conjoined slave, only thr master task will be eligible for
+ # update.

s/thr/the/

review: Approve (code)

=== modified file 'lib/lp/bugs/model/bugwatch.py'
--- lib/lp/bugs/model/bugwatch.py	2010-03-23 12:55:05 +0000
+++ lib/lp/bugs/model/bugwatch.py	2010-04-09 15:32:31 +0000
@@ -92,6 +92,20 @@
         return shortlist(tasks, 10, 100)
 
     @property
+    def bugtasks_to_update(self):
+        """Yield the bug tasks that are eligible for update."""
+        for bugtask in self.bugtasks:
+            # We don't update conjoined bug tasks; they must be
+            # updated through their conjoined masters.
+            if bugtask._isConjoinedBugTask():
+                continue
+            # We don't update tasks of duplicate bugs.
+            if bugtask.bug.duplicateof is not None:
+                continue
+            # Update this one.
+            yield bugtask
+
+    @property
     def title(self):
         """See `IBugWatch`."""
         return "%s #%s" % (self.bugtracker.title, self.remotebug)
@@ -125,19 +139,12 @@
             # Sync the object in order to convert the UTC_NOW sql
             # constant to a datetime value.
             self.sync()
-
-        for linked_bugtask in self.bugtasks:
-            # We don't updated conjoined bug tasks; they must be updated
-            # through their conjoined masters.
-            if linked_bugtask._isConjoinedBugTask():
-                continue
-
+        for linked_bugtask in self.bugtasks_to_update:
             old_bugtask = Snapshot(
                 linked_bugtask, providing=providedBy(linked_bugtask))
             linked_bugtask.transitionToImportance(
                 malone_importance,
                 getUtility(ILaunchpadCelebrities).bug_watch_updater)
-
             if linked_bugtask.importance != old_bugtask.importance:
                 event = ObjectModifiedEvent(
                     linked_bugtask, old_bugtask, ['importance'],
@@ -152,12 +159,7 @@
             # Sync the object in order to convert the UTC_NOW sql
             # constant to a datetime value.
             self.sync()
-        for linked_bugtask in self.bugtasks:
-            # We don't updated conjoined bug tasks; they must be updated
-            # through their conjoined masters.
-            if linked_bugtask._isConjoinedBugTask():
-                continue
-
+        for linked_bugtask in self.bugtasks_to_update:
             old_bugtask = Snapshot(
                 linked_bugtask, providing=providedBy(linked_bugtask))
             linked_bugtask.transitionToStatus(

=== modified file 'lib/lp/bugs/tests/test_bugwatch.py'
--- lib/lp/bugs/tests/test_bugwatch.py	2010-03-26 14:49:24 +0000
+++ lib/lp/bugs/tests/test_bugwatch.py	2010-04-09 15:32:31 +0000
@@ -14,15 +14,16 @@
 from urlparse import urlunsplit
 
 from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
 
 from canonical.launchpad.ftests import login, ANONYMOUS
+from canonical.launchpad.scripts.garbo import BugWatchActivityPruner
 from canonical.launchpad.scripts.logger import QuietFakeLogger
 from canonical.launchpad.webapp import urlsplit
-from canonical.launchpad.scripts.garbo import BugWatchActivityPruner
-from canonical.launchpad.scripts.logger import QuietFakeLogger
 from canonical.testing import (
     DatabaseFunctionalLayer, LaunchpadFunctionalLayer, LaunchpadZopelessLayer)
 
+from lp.bugs.interfaces.bugtask import BugTaskImportance, BugTaskStatus
 from lp.bugs.interfaces.bugtracker import BugTrackerType, IBugTrackerSet
 from lp.bugs.interfaces.bugwatch import (
     BugWatchActivityStatus, IBugWatchSet, NoBugTrackerFound,
@@ -31,7 +32,7 @@
     BugWatchScheduler, MAX_SAMPLE_SIZE)
 from lp.registry.interfaces.person import IPersonSet
 
-from lp.testing import TestCaseWithFactory
+from lp.testing import TestCaseWithFactory, login_person
 
 
 class ExtractBugTrackerAndBugTestBase:
@@ -335,6 +336,76 @@
     bug_id = '12345'
 
 
+class TestBugWatch(TestCaseWithFactory):
+
+    layer = LaunchpadZopelessLayer
+
+    def test_bugtasks_to_update(self):
+        # The bugtasks_to_update property should yield the linked bug
+        # tasks which are not conjoined and for which the bug is not a
+        # duplicate.
+        product = self.factory.makeProductNoCommit()
+        bug = self.factory.makeBug(product=product, owner=product.owner)
+        product_task = bug.getBugTask(product)
+        watch = self.factory.makeBugWatch(bug=bug)
+        product_task.bugwatch = watch
+        # For a single-task bug the bug task is eligible for update.
+        self.failUnlessEqual(
+            [product_task], list(
+                removeSecurityProxy(watch).bugtasks_to_update))
+        # If we add a task such that the existing task becomes a
+        # conjoined slave, only thr master task will be eligible for
+        # update.
+        product_series_task = self.factory.makeBugTask(
+            bug=bug, target=product.development_focus)
+        product_series_task.bugwatch = watch
+        self.failUnlessEqual(
+            [product_series_task], list(
+                removeSecurityProxy(watch).bugtasks_to_update))
+        # But once the bug is marked as a duplicate,
+        # bugtasks_to_update yields nothing.
+        bug.markAsDuplicate(
+            self.factory.makeBug(product=product, owner=product.owner))
+        self.failUnlessEqual(
+            [], list(removeSecurityProxy(watch).bugtasks_to_update))
+
+    def test_updateStatus_with_duplicate_bug(self):
+        # Calling BugWatch.updateStatus() will not update the status
+        # of a task that is part of a duplicate bug.
+        bug = self.factory.makeBug()
+        bug.markAsDuplicate(self.factory.makeBug())
+        login_person(bug.owner)
+        bug_task = bug.default_bugtask
+        bug_task.bugwatch = self.factory.makeBugWatch()
+        bug_task_initial_status = bug_task.status
+        self.failIfEqual(BugTaskStatus.INPROGRESS, bug_task.status)
+        bug_task.bugwatch.updateStatus('foo', BugTaskStatus.INPROGRESS)
+        self.failUnlessEqual(bug_task_initial_status, bug_task.status)
+        # Once the task is no longer linked to a duplicate bug, the
+        # status will get updated.
+        bug.markAsDuplicate(None)
+        bug_task.bugwatch.updateStatus('foo', BugTaskStatus.INPROGRESS)
+        self.failUnlessEqual(BugTaskStatus.INPROGRESS, bug_task.status)
+
+    def test_updateImportance_with_duplicate_bug(self):
+        # Calling BugWatch.updateImportance() will not update the
+        # importance of a task that is part of a duplicate bug.
+        bug = self.factory.makeBug()
+        bug.markAsDuplicate(self.factory.makeBug())
+        login_person(bug.owner)
+        bug_task = bug.default_bugtask
+        bug_task.bugwatch = self.factory.makeBugWatch()
+        bug_task_initial_importance = bug_task.importance
+        self.failIfEqual(BugTaskImportance.HIGH, bug_task.importance)
+        bug_task.bugwatch.updateImportance('foo', BugTaskImportance.HIGH)
+        self.failUnlessEqual(bug_task_initial_importance, bug_task.importance)
+        # Once the task is no longer linked to a duplicate bug, the
+        # importance will get updated.
+        bug.markAsDuplicate(None)
+        bug_task.bugwatch.updateImportance('foo', BugTaskImportance.HIGH)
+        self.failUnlessEqual(BugTaskImportance.HIGH, bug_task.importance)
+
+
 class TestBugWatchSet(TestCaseWithFactory):
     """Tests for the bugwatch updating system."""