Launchpad itself

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

  1. dont-update-dupe-bugs-bug-511276
  2. Merge into devel
Proposed by Gavin Panella
Status: Merged
Approved by: Gavin Panella
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~allenap/launchpad/dont-update-dupe-bugs-bug-511276
Merge into: lp:launchpad
Diff against target: 167 lines (+90/-17)
2 files modified
lib/lp/bugs/model/bugwatch.py (+16/-14)
lib/lp/bugs/tests/test_bugwatch.py (+74/-3)
To merge this branch: bzr merge lp:~allenap/launchpad/dont-update-dupe-bugs-bug-511276
Reviewer Review Type Date Requested Status
Abel Deuring (community) code Approve
Review via email: mp+23113@code.launchpad.net

Commit message

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

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Abel Deuring (adeuring) wrote :

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)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/model/bugwatch.py'
2--- lib/lp/bugs/model/bugwatch.py 2010-03-23 12:55:05 +0000
3+++ lib/lp/bugs/model/bugwatch.py 2010-04-09 15:32:31 +0000
4@@ -92,6 +92,20 @@
5 return shortlist(tasks, 10, 100)
6
7 @property
8+ def bugtasks_to_update(self):
9+ """Yield the bug tasks that are eligible for update."""
10+ for bugtask in self.bugtasks:
11+ # We don't update conjoined bug tasks; they must be
12+ # updated through their conjoined masters.
13+ if bugtask._isConjoinedBugTask():
14+ continue
15+ # We don't update tasks of duplicate bugs.
16+ if bugtask.bug.duplicateof is not None:
17+ continue
18+ # Update this one.
19+ yield bugtask
20+
21+ @property
22 def title(self):
23 """See `IBugWatch`."""
24 return "%s #%s" % (self.bugtracker.title, self.remotebug)
25@@ -125,19 +139,12 @@
26 # Sync the object in order to convert the UTC_NOW sql
27 # constant to a datetime value.
28 self.sync()
29-
30- for linked_bugtask in self.bugtasks:
31- # We don't updated conjoined bug tasks; they must be updated
32- # through their conjoined masters.
33- if linked_bugtask._isConjoinedBugTask():
34- continue
35-
36+ for linked_bugtask in self.bugtasks_to_update:
37 old_bugtask = Snapshot(
38 linked_bugtask, providing=providedBy(linked_bugtask))
39 linked_bugtask.transitionToImportance(
40 malone_importance,
41 getUtility(ILaunchpadCelebrities).bug_watch_updater)
42-
43 if linked_bugtask.importance != old_bugtask.importance:
44 event = ObjectModifiedEvent(
45 linked_bugtask, old_bugtask, ['importance'],
46@@ -152,12 +159,7 @@
47 # Sync the object in order to convert the UTC_NOW sql
48 # constant to a datetime value.
49 self.sync()
50- for linked_bugtask in self.bugtasks:
51- # We don't updated conjoined bug tasks; they must be updated
52- # through their conjoined masters.
53- if linked_bugtask._isConjoinedBugTask():
54- continue
55-
56+ for linked_bugtask in self.bugtasks_to_update:
57 old_bugtask = Snapshot(
58 linked_bugtask, providing=providedBy(linked_bugtask))
59 linked_bugtask.transitionToStatus(
60
61=== modified file 'lib/lp/bugs/tests/test_bugwatch.py'
62--- lib/lp/bugs/tests/test_bugwatch.py 2010-03-26 14:49:24 +0000
63+++ lib/lp/bugs/tests/test_bugwatch.py 2010-04-09 15:32:31 +0000
64@@ -14,15 +14,16 @@
65 from urlparse import urlunsplit
66
67 from zope.component import getUtility
68+from zope.security.proxy import removeSecurityProxy
69
70 from canonical.launchpad.ftests import login, ANONYMOUS
71+from canonical.launchpad.scripts.garbo import BugWatchActivityPruner
72 from canonical.launchpad.scripts.logger import QuietFakeLogger
73 from canonical.launchpad.webapp import urlsplit
74-from canonical.launchpad.scripts.garbo import BugWatchActivityPruner
75-from canonical.launchpad.scripts.logger import QuietFakeLogger
76 from canonical.testing import (
77 DatabaseFunctionalLayer, LaunchpadFunctionalLayer, LaunchpadZopelessLayer)
78
79+from lp.bugs.interfaces.bugtask import BugTaskImportance, BugTaskStatus
80 from lp.bugs.interfaces.bugtracker import BugTrackerType, IBugTrackerSet
81 from lp.bugs.interfaces.bugwatch import (
82 BugWatchActivityStatus, IBugWatchSet, NoBugTrackerFound,
83@@ -31,7 +32,7 @@
84 BugWatchScheduler, MAX_SAMPLE_SIZE)
85 from lp.registry.interfaces.person import IPersonSet
86
87-from lp.testing import TestCaseWithFactory
88+from lp.testing import TestCaseWithFactory, login_person
89
90
91 class ExtractBugTrackerAndBugTestBase:
92@@ -335,6 +336,76 @@
93 bug_id = '12345'
94
95
96+class TestBugWatch(TestCaseWithFactory):
97+
98+ layer = LaunchpadZopelessLayer
99+
100+ def test_bugtasks_to_update(self):
101+ # The bugtasks_to_update property should yield the linked bug
102+ # tasks which are not conjoined and for which the bug is not a
103+ # duplicate.
104+ product = self.factory.makeProductNoCommit()
105+ bug = self.factory.makeBug(product=product, owner=product.owner)
106+ product_task = bug.getBugTask(product)
107+ watch = self.factory.makeBugWatch(bug=bug)
108+ product_task.bugwatch = watch
109+ # For a single-task bug the bug task is eligible for update.
110+ self.failUnlessEqual(
111+ [product_task], list(
112+ removeSecurityProxy(watch).bugtasks_to_update))
113+ # If we add a task such that the existing task becomes a
114+ # conjoined slave, only thr master task will be eligible for
115+ # update.
116+ product_series_task = self.factory.makeBugTask(
117+ bug=bug, target=product.development_focus)
118+ product_series_task.bugwatch = watch
119+ self.failUnlessEqual(
120+ [product_series_task], list(
121+ removeSecurityProxy(watch).bugtasks_to_update))
122+ # But once the bug is marked as a duplicate,
123+ # bugtasks_to_update yields nothing.
124+ bug.markAsDuplicate(
125+ self.factory.makeBug(product=product, owner=product.owner))
126+ self.failUnlessEqual(
127+ [], list(removeSecurityProxy(watch).bugtasks_to_update))
128+
129+ def test_updateStatus_with_duplicate_bug(self):
130+ # Calling BugWatch.updateStatus() will not update the status
131+ # of a task that is part of a duplicate bug.
132+ bug = self.factory.makeBug()
133+ bug.markAsDuplicate(self.factory.makeBug())
134+ login_person(bug.owner)
135+ bug_task = bug.default_bugtask
136+ bug_task.bugwatch = self.factory.makeBugWatch()
137+ bug_task_initial_status = bug_task.status
138+ self.failIfEqual(BugTaskStatus.INPROGRESS, bug_task.status)
139+ bug_task.bugwatch.updateStatus('foo', BugTaskStatus.INPROGRESS)
140+ self.failUnlessEqual(bug_task_initial_status, bug_task.status)
141+ # Once the task is no longer linked to a duplicate bug, the
142+ # status will get updated.
143+ bug.markAsDuplicate(None)
144+ bug_task.bugwatch.updateStatus('foo', BugTaskStatus.INPROGRESS)
145+ self.failUnlessEqual(BugTaskStatus.INPROGRESS, bug_task.status)
146+
147+ def test_updateImportance_with_duplicate_bug(self):
148+ # Calling BugWatch.updateImportance() will not update the
149+ # importance of a task that is part of a duplicate bug.
150+ bug = self.factory.makeBug()
151+ bug.markAsDuplicate(self.factory.makeBug())
152+ login_person(bug.owner)
153+ bug_task = bug.default_bugtask
154+ bug_task.bugwatch = self.factory.makeBugWatch()
155+ bug_task_initial_importance = bug_task.importance
156+ self.failIfEqual(BugTaskImportance.HIGH, bug_task.importance)
157+ bug_task.bugwatch.updateImportance('foo', BugTaskImportance.HIGH)
158+ self.failUnlessEqual(bug_task_initial_importance, bug_task.importance)
159+ # Once the task is no longer linked to a duplicate bug, the
160+ # importance will get updated.
161+ bug.markAsDuplicate(None)
162+ bug_task.bugwatch.updateImportance('foo', BugTaskImportance.HIGH)
163+ self.failUnlessEqual(BugTaskImportance.HIGH, bug_task.importance)
164+
165+
166 class TestBugWatchSet(TestCaseWithFactory):
167 """Tests for the bugwatch updating system."""
168