Launchpad itself

Merge lp:~allenap/launchpad/subs-for-bugtask-bug-656194 into lp:launchpad/db-devel

  1. subs-for-bugtask-bug-656194
  2. Merge into db-devel
Proposed by Gavin Panella
Status: Merged
Approved by: Gavin Panella
Approved revision: no longer in the source branch.
Merged at revision: 9888
Proposed branch: lp:~allenap/launchpad/subs-for-bugtask-bug-656194
Merge into: lp:launchpad/db-devel
Diff against target: 294 lines (+70/-83)
3 files modified
lib/lp/registry/interfaces/structuralsubscription.py (+2/-2)
lib/lp/registry/model/structuralsubscription.py (+5/-19)
lib/lp/registry/tests/test_structuralsubscriptiontarget.py (+63/-62)
To merge this branch: bzr merge lp:~allenap/launchpad/subs-for-bugtask-bug-656194
Reviewer Review Type Date Requested Status
Abel Deuring (community) code Approve
Review via email: mp+37835@code.launchpad.net

Description of the change

Switch getSusbcriptionsForBug() to *ForBugTask(). Not much else in it really.

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

(15:46:23) adeuring: allenap: the changes in your branch look good. But what is the reason for the changes? Do you assume that the SQL query for the old method would be too slow?
(15:48:09) allenap: adeuring: It's easier to select the targets elsewhere, and it's makes the resulting code closer to the existing structural subs code, so it's easier to reuse.
(15:48:47) allenap: adeuring: My follow-on branch is now simpler to implement.
(15:49:50) adeuring: allenap: ok. But now you may get a subscription more than once, if somebody is subscribed to two or more bug targets having bug tasks for a given bug
(15:50:27) allenap: adeuring: That turns out to be a problem. So far :)
(15:50:37) allenap: adeuring: That turns out to *not* be a problem. So far :)
(15:52:35) adeuring: allenap: interesting. But anyway, r=me; the possible duplicate results can be treated later

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/registry/interfaces/structuralsubscription.py'
2--- lib/lp/registry/interfaces/structuralsubscription.py 2010-10-05 09:23:22 +0000
3+++ lib/lp/registry/interfaces/structuralsubscription.py 2010-10-08 08:17:42 +0000
4@@ -200,8 +200,8 @@
5 def userHasBugSubscriptions(user):
6 """Is `user` subscribed, directly or via a team, to bug mail?"""
7
8- def getSubscriptionsForBug(bug, level):
9- """Return subscriptions for a given `IBug` at `level`."""
10+ def getSubscriptionsForBugTask(bug, level):
11+ """Return subscriptions for a given `IBugTask` at `level`."""
12
13
14 class IStructuralSubscriptionTargetWrite(Interface):
15
16=== modified file 'lib/lp/registry/model/structuralsubscription.py'
17--- lib/lp/registry/model/structuralsubscription.py 2010-10-05 16:52:02 +0000
18+++ lib/lp/registry/model/structuralsubscription.py 2010-10-08 08:17:42 +0000
19@@ -484,21 +484,8 @@
20 return True
21 return False
22
23- def getSubscriptionsForBug(self, bug, level):
24+ def getSubscriptionsForBugTask(self, bugtask, level):
25 """See `IStructuralSubscriptionTarget`."""
26- if IProjectGroup.providedBy(self):
27- targets = set(self.products)
28- elif IMilestone.providedBy(self):
29- targets = [self.series_target]
30- else:
31- targets = [self]
32-
33- bugtasks = [
34- bugtask for bugtask in bug.bugtasks
35- if bugtask.target in targets]
36-
37- assert len(bugtasks) != 0, repr(self)
38-
39 origin = [
40 StructuralSubscription,
41 LeftJoin(
42@@ -515,7 +502,7 @@
43 BugSubscriptionFilter.id)),
44 ]
45
46- if len(bug.tags) == 0:
47+ if len(bugtask.bug.tags) == 0:
48 tag_conditions = [
49 BugSubscriptionFilter.include_any_tags == False,
50 ]
51@@ -534,13 +521,12 @@
52 # There's no status filter, or there is a status filter
53 # and and it matches.
54 Or(BugSubscriptionFilterStatus.id == None,
55- BugSubscriptionFilterStatus.status.is_in(
56- bugtask.status for bugtask in bugtasks)),
57+ BugSubscriptionFilterStatus.status == bugtask.status),
58 # There's no importance filter, or there is an importance
59 # filter and it matches.
60 Or(BugSubscriptionFilterImportance.id == None,
61- BugSubscriptionFilterImportance.importance.is_in(
62- bugtask.importance for bugtask in bugtasks)),
63+ BugSubscriptionFilterImportance.importance == (
64+ bugtask.importance)),
65 # Any number of conditions relating to tags.
66 *tag_conditions)),
67 ]
68
69=== modified file 'lib/lp/registry/tests/test_structuralsubscriptiontarget.py'
70--- lib/lp/registry/tests/test_structuralsubscriptiontarget.py 2010-10-06 18:53:53 +0000
71+++ lib/lp/registry/tests/test_structuralsubscriptiontarget.py 2010-10-08 08:17:42 +0000
72@@ -173,19 +173,20 @@
73 def makeBugTask(self):
74 return self.factory.makeBugTask(target=self.target)
75
76- def test_getSubscriptionsForBug(self):
77+ def test_getSubscriptionsForBugTask(self):
78 # If no one has a filtered subscription for the given bug, the result
79- # of getSubscriptionsForBug() is the same as for getSubscriptions().
80+ # of getSubscriptionsForBugTask() is the same as for
81+ # getSubscriptions().
82 bugtask = self.makeBugTask()
83 subscriptions = self.target.getSubscriptions(
84 min_bug_notification_level=BugNotificationLevel.NOTHING)
85- subscriptions_for_bug = self.target.getSubscriptionsForBug(
86- bugtask.bug, BugNotificationLevel.NOTHING)
87- self.assertEqual(list(subscriptions), list(subscriptions_for_bug))
88+ subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
89+ bugtask, BugNotificationLevel.NOTHING)
90+ self.assertEqual(list(subscriptions), list(subscriptions_for_bugtask))
91
92- def test_getSubscriptionsForBug_with_filter_on_status(self):
93+ def test_getSubscriptionsForBugTask_with_filter_on_status(self):
94 # If a status filter exists for a subscription, the result of
95- # getSubscriptionsForBug() may be a subset of getSubscriptions().
96+ # getSubscriptionsForBugTask() may be a subset of getSubscriptions().
97 bugtask = self.makeBugTask()
98
99 # Create a new subscription on self.target.
100@@ -195,9 +196,9 @@
101 subscription.bug_notification_level = BugNotificationLevel.COMMENTS
102
103 # Without any filters the subscription is found.
104- subscriptions_for_bug = self.target.getSubscriptionsForBug(
105- bugtask.bug, BugNotificationLevel.NOTHING)
106- self.assertEqual([subscription], list(subscriptions_for_bug))
107+ subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
108+ bugtask, BugNotificationLevel.NOTHING)
109+ self.assertEqual([subscription], list(subscriptions_for_bugtask))
110
111 # Filter the subscription to bugs in the CONFIRMED state.
112 subscription_filter = BugSubscriptionFilter()
113@@ -205,19 +206,19 @@
114 subscription_filter.statuses = [BugTaskStatus.CONFIRMED]
115
116 # With the filter the subscription is not found.
117- subscriptions_for_bug = self.target.getSubscriptionsForBug(
118- bugtask.bug, BugNotificationLevel.NOTHING)
119- self.assertEqual([], list(subscriptions_for_bug))
120+ subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
121+ bugtask, BugNotificationLevel.NOTHING)
122+ self.assertEqual([], list(subscriptions_for_bugtask))
123
124 # If the filter is adjusted, the subscription is found again.
125 subscription_filter.statuses = [bugtask.status]
126- subscriptions_for_bug = self.target.getSubscriptionsForBug(
127- bugtask.bug, BugNotificationLevel.NOTHING)
128- self.assertEqual([subscription], list(subscriptions_for_bug))
129+ subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
130+ bugtask, BugNotificationLevel.NOTHING)
131+ self.assertEqual([subscription], list(subscriptions_for_bugtask))
132
133- def test_getSubscriptionsForBug_with_filter_on_importance(self):
134+ def test_getSubscriptionsForBugTask_with_filter_on_importance(self):
135 # If an importance filter exists for a subscription, the result of
136- # getSubscriptionsForBug() may be a subset of getSubscriptions().
137+ # getSubscriptionsForBugTask() may be a subset of getSubscriptions().
138 bugtask = self.makeBugTask()
139
140 # Create a new subscription on self.target.
141@@ -227,9 +228,9 @@
142 subscription.bug_notification_level = BugNotificationLevel.COMMENTS
143
144 # Without any filters the subscription is found.
145- subscriptions_for_bug = self.target.getSubscriptionsForBug(
146- bugtask.bug, BugNotificationLevel.NOTHING)
147- self.assertEqual([subscription], list(subscriptions_for_bug))
148+ subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
149+ bugtask, BugNotificationLevel.NOTHING)
150+ self.assertEqual([subscription], list(subscriptions_for_bugtask))
151
152 # Filter the subscription to bugs in the CRITICAL state.
153 subscription_filter = BugSubscriptionFilter()
154@@ -237,19 +238,19 @@
155 subscription_filter.importances = [BugTaskImportance.CRITICAL]
156
157 # With the filter the subscription is not found.
158- subscriptions_for_bug = self.target.getSubscriptionsForBug(
159- bugtask.bug, BugNotificationLevel.NOTHING)
160- self.assertEqual([], list(subscriptions_for_bug))
161+ subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
162+ bugtask, BugNotificationLevel.NOTHING)
163+ self.assertEqual([], list(subscriptions_for_bugtask))
164
165 # If the filter is adjusted, the subscription is found again.
166 subscription_filter.importances = [bugtask.importance]
167- subscriptions_for_bug = self.target.getSubscriptionsForBug(
168- bugtask.bug, BugNotificationLevel.NOTHING)
169- self.assertEqual([subscription], list(subscriptions_for_bug))
170+ subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
171+ bugtask, BugNotificationLevel.NOTHING)
172+ self.assertEqual([subscription], list(subscriptions_for_bugtask))
173
174- def test_getSubscriptionsForBug_with_filter_on_level(self):
175+ def test_getSubscriptionsForBugTask_with_filter_on_level(self):
176 # All structural subscriptions have a level for bug notifications
177- # which getSubscriptionsForBug() observes.
178+ # which getSubscriptionsForBugTask() observes.
179 bugtask = self.makeBugTask()
180
181 # Create a new METADATA level subscription on self.target.
182@@ -259,19 +260,19 @@
183 subscription.bug_notification_level = BugNotificationLevel.METADATA
184
185 # The subscription is found when looking for NOTHING or above.
186- subscriptions_for_bug = self.target.getSubscriptionsForBug(
187- bugtask.bug, BugNotificationLevel.NOTHING)
188- self.assertEqual([subscription], list(subscriptions_for_bug))
189+ subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
190+ bugtask, BugNotificationLevel.NOTHING)
191+ self.assertEqual([subscription], list(subscriptions_for_bugtask))
192 # The subscription is found when looking for METADATA or above.
193- subscriptions_for_bug = self.target.getSubscriptionsForBug(
194- bugtask.bug, BugNotificationLevel.METADATA)
195- self.assertEqual([subscription], list(subscriptions_for_bug))
196+ subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
197+ bugtask, BugNotificationLevel.METADATA)
198+ self.assertEqual([subscription], list(subscriptions_for_bugtask))
199 # The subscription is not found when looking for COMMENTS or above.
200- subscriptions_for_bug = self.target.getSubscriptionsForBug(
201- bugtask.bug, BugNotificationLevel.COMMENTS)
202- self.assertEqual([], list(subscriptions_for_bug))
203+ subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
204+ bugtask, BugNotificationLevel.COMMENTS)
205+ self.assertEqual([], list(subscriptions_for_bugtask))
206
207- def test_getSubscriptionsForBug_with_filter_include_any_tags(self):
208+ def test_getSubscriptionsForBugTask_with_filter_include_any_tags(self):
209 # If a subscription filter has include_any_tags, a bug with one or
210 # more tags is matched.
211 bugtask = self.makeBugTask()
212@@ -285,17 +286,17 @@
213 subscription_filter.include_any_tags = True
214
215 # Without any tags the subscription is not found.
216- subscriptions_for_bug = self.target.getSubscriptionsForBug(
217- bugtask.bug, BugNotificationLevel.NOTHING)
218- self.assertEqual([], list(subscriptions_for_bug))
219+ subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
220+ bugtask, BugNotificationLevel.NOTHING)
221+ self.assertEqual([], list(subscriptions_for_bugtask))
222
223 # With any tag the subscription is found.
224 bugtask.bug.tags = ["foo"]
225- subscriptions_for_bug = self.target.getSubscriptionsForBug(
226- bugtask.bug, BugNotificationLevel.NOTHING)
227- self.assertEqual([subscription], list(subscriptions_for_bug))
228+ subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
229+ bugtask, BugNotificationLevel.NOTHING)
230+ self.assertEqual([subscription], list(subscriptions_for_bugtask))
231
232- def test_getSubscriptionsForBug_with_filter_exclude_any_tags(self):
233+ def test_getSubscriptionsForBugTask_with_filter_exclude_any_tags(self):
234 # If a subscription filter has exclude_any_tags, only bugs with no
235 # tags are matched.
236 bugtask = self.makeBugTask()
237@@ -309,17 +310,17 @@
238 subscription_filter.exclude_any_tags = True
239
240 # Without any tags the subscription is found.
241- subscriptions_for_bug = self.target.getSubscriptionsForBug(
242- bugtask.bug, BugNotificationLevel.NOTHING)
243- self.assertEqual([subscription], list(subscriptions_for_bug))
244+ subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
245+ bugtask, BugNotificationLevel.NOTHING)
246+ self.assertEqual([subscription], list(subscriptions_for_bugtask))
247
248 # With any tag the subscription is not found.
249 bugtask.bug.tags = ["foo"]
250- subscriptions_for_bug = self.target.getSubscriptionsForBug(
251- bugtask.bug, BugNotificationLevel.NOTHING)
252- self.assertEqual([], list(subscriptions_for_bug))
253+ subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
254+ bugtask, BugNotificationLevel.NOTHING)
255+ self.assertEqual([], list(subscriptions_for_bugtask))
256
257- def test_getSubscriptionsForBug_with_multiple_filters(self):
258+ def test_getSubscriptionsForBugTask_with_multiple_filters(self):
259 # If multiple filters exist for a subscription, all filters must
260 # match.
261 bugtask = self.makeBugTask()
262@@ -337,23 +338,23 @@
263 subscription_filter.importances = [BugTaskImportance.CRITICAL]
264
265 # With the filter the subscription is not found.
266- subscriptions_for_bug = self.target.getSubscriptionsForBug(
267- bugtask.bug, BugNotificationLevel.NOTHING)
268- self.assertEqual([], list(subscriptions_for_bug))
269+ subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
270+ bugtask, BugNotificationLevel.NOTHING)
271+ self.assertEqual([], list(subscriptions_for_bugtask))
272
273 # If the filter is adjusted to match status but not importance, the
274 # subscription is still not found.
275 subscription_filter.statuses = [bugtask.status]
276- subscriptions_for_bug = self.target.getSubscriptionsForBug(
277- bugtask.bug, BugNotificationLevel.NOTHING)
278- self.assertEqual([], list(subscriptions_for_bug))
279+ subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
280+ bugtask, BugNotificationLevel.NOTHING)
281+ self.assertEqual([], list(subscriptions_for_bugtask))
282
283 # If the filter is adjusted to also match importance, the subscription
284 # is found again.
285 subscription_filter.importances = [bugtask.importance]
286- subscriptions_for_bug = self.target.getSubscriptionsForBug(
287- bugtask.bug, BugNotificationLevel.NOTHING)
288- self.assertEqual([subscription], list(subscriptions_for_bug))
289+ subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
290+ bugtask, BugNotificationLevel.NOTHING)
291+ self.assertEqual([subscription], list(subscriptions_for_bugtask))
292
293
294 class TestStructuralSubscriptionForDistro(

Subscribers

People subscribed via source and target branches

to status/vote changes: