Merge lp:~stevenk/launchpad/no-more-o-n-queries-bug-dupes into lp:launchpad
- no-more-o-n-queries-bug-dupes
- Merge into devel
Status: | Merged | ||||||||
---|---|---|---|---|---|---|---|---|---|
Merged at revision: | 16140 | ||||||||
Proposed branch: | lp:~stevenk/launchpad/no-more-o-n-queries-bug-dupes | ||||||||
Merge into: | lp:launchpad | ||||||||
Diff against target: |
345 lines (+88/-90) 5 files modified
lib/lp/bugs/browser/tests/test_bug_views.py (+25/-10) lib/lp/bugs/browser/tests/test_bugtask.py (+1/-1) lib/lp/bugs/model/bug.py (+25/-22) lib/lp/bugs/model/personsubscriptioninfo.py (+36/-56) lib/lp/bugs/model/tests/test_personsubscriptioninfo.py (+1/-1) |
||||||||
To merge this branch: | bzr merge lp:~stevenk/launchpad/no-more-o-n-queries-bug-dupes | ||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
William Grant | code | Approve | |
Review via email: mp+129355@code.launchpad.net |
Commit message
Rewrite the heavy-lifting queries in IBug.getSubscri
Description of the change
Rewrite the heavy-lifting queries in IBug.getSubscri
Do a small number of whitespace function, and also rewrite IPersonSubscrip
Preview Diff
1 | === modified file 'lib/lp/bugs/browser/tests/test_bug_views.py' | |||
2 | --- lib/lp/bugs/browser/tests/test_bug_views.py 2012-09-28 16:47:42 +0000 | |||
3 | +++ lib/lp/bugs/browser/tests/test_bug_views.py 2012-10-12 11:59:29 +0000 | |||
4 | @@ -269,8 +269,7 @@ | |||
5 | 269 | self.assertFalse(self.bug.isSubscribed(person)) | 269 | self.assertFalse(self.bug.isSubscribed(person)) |
6 | 270 | self.assertFalse(self.bug.isMuted(person)) | 270 | self.assertFalse(self.bug.isMuted(person)) |
7 | 271 | self.assertFalse( | 271 | self.assertFalse( |
10 | 272 | self.bug.personIsAlsoNotifiedSubscriber( | 272 | self.bug.personIsAlsoNotifiedSubscriber(person)) |
9 | 273 | person)) | ||
11 | 274 | view = create_initialized_view( | 273 | view = create_initialized_view( |
12 | 275 | self.bug, name="+portlet-subscription") | 274 | self.bug, name="+portlet-subscription") |
13 | 276 | self.assertFalse(view.user_should_see_mute_link) | 275 | self.assertFalse(view.user_should_see_mute_link) |
14 | @@ -305,14 +304,31 @@ | |||
15 | 305 | self.assertTrue(self.bug.isMuted(person)) | 304 | self.assertTrue(self.bug.isMuted(person)) |
16 | 306 | view = create_initialized_view( | 305 | view = create_initialized_view( |
17 | 307 | self.bug, name="+portlet-subscription") | 306 | self.bug, name="+portlet-subscription") |
20 | 308 | self.assertTrue(view.user_should_see_mute_link, | 307 | self.assertTrue( |
21 | 309 | "User should see mute link.") | 308 | view.user_should_see_mute_link, "User should see mute link.") |
22 | 310 | contents = view.render() | 309 | contents = view.render() |
25 | 311 | self.assertTrue('mute_subscription' in contents, | 310 | self.assertTrue( |
26 | 312 | "'mute_subscription' not in contents.") | 311 | 'mute_subscription' in contents, |
27 | 312 | "'mute_subscription' not in contents.") | ||
28 | 313 | self.assertFalse( | 313 | self.assertFalse( |
31 | 314 | self._hasCSSClass( | 314 | self._hasCSSClass(contents, 'mute-link-container', 'hidden')) |
32 | 315 | contents, 'mute-link-container', 'hidden')) | 315 | |
33 | 316 | def test_bug_portlet_subscription_query_count(self): | ||
34 | 317 | # Bug:+portlet-subscription doesn't make O(n) queries based on the | ||
35 | 318 | # number of duplicate bugs. | ||
36 | 319 | user = self.factory.makePerson() | ||
37 | 320 | bug = self.factory.makeBug() | ||
38 | 321 | for n in range(20): | ||
39 | 322 | dupe = self.factory.makeBug() | ||
40 | 323 | removeSecurityProxy(dupe)._markAsDuplicate(bug) | ||
41 | 324 | removeSecurityProxy(dupe).subscribe(user, dupe.owner) | ||
42 | 325 | Store.of(bug).invalidate() | ||
43 | 326 | with person_logged_in(user): | ||
44 | 327 | with StormStatementRecorder() as recorder: | ||
45 | 328 | view = create_initialized_view( | ||
46 | 329 | bug, name='+portlet-subscription', principal=user) | ||
47 | 330 | view.render() | ||
48 | 331 | self.assertThat(recorder, HasQueryCount(Equals(21))) | ||
49 | 316 | 332 | ||
50 | 317 | 333 | ||
51 | 318 | class TestBugSecrecyViews(TestCaseWithFactory): | 334 | class TestBugSecrecyViews(TestCaseWithFactory): |
52 | @@ -328,12 +344,11 @@ | |||
53 | 328 | if bug is None: | 344 | if bug is None: |
54 | 329 | bug = self.factory.makeBug() | 345 | bug = self.factory.makeBug() |
55 | 330 | with person_logged_in(person): | 346 | with person_logged_in(person): |
57 | 331 | view = create_initialized_view( | 347 | return create_initialized_view( |
58 | 332 | bug.default_bugtask, name='+secrecy', form={ | 348 | bug.default_bugtask, name='+secrecy', form={ |
59 | 333 | 'field.information_type': 'USERDATA', | 349 | 'field.information_type': 'USERDATA', |
60 | 334 | 'field.actions.change': 'Change', | 350 | 'field.actions.change': 'Change', |
61 | 335 | }, request=request) | 351 | }, request=request) |
62 | 336 | return view | ||
63 | 337 | 352 | ||
64 | 338 | def test_notification_shown_if_marking_private_and_not_subscribed(self): | 353 | def test_notification_shown_if_marking_private_and_not_subscribed(self): |
65 | 339 | # If a user who is not subscribed to a bug marks that bug as | 354 | # If a user who is not subscribed to a bug marks that bug as |
66 | 340 | 355 | ||
67 | === modified file 'lib/lp/bugs/browser/tests/test_bugtask.py' | |||
68 | --- lib/lp/bugs/browser/tests/test_bugtask.py 2012-10-10 02:45:36 +0000 | |||
69 | +++ lib/lp/bugs/browser/tests/test_bugtask.py 2012-10-12 11:59:29 +0000 | |||
70 | @@ -156,7 +156,7 @@ | |||
71 | 156 | def test_rendered_query_counts_constant_with_attachments(self): | 156 | def test_rendered_query_counts_constant_with_attachments(self): |
72 | 157 | with celebrity_logged_in('admin'): | 157 | with celebrity_logged_in('admin'): |
73 | 158 | browses_under_limit = BrowsesWithQueryLimit( | 158 | browses_under_limit = BrowsesWithQueryLimit( |
75 | 159 | 95, self.factory.makePerson()) | 159 | 96, self.factory.makePerson()) |
76 | 160 | 160 | ||
77 | 161 | # First test with a single attachment. | 161 | # First test with a single attachment. |
78 | 162 | task = self.factory.makeBugTask() | 162 | task = self.factory.makeBugTask() |
79 | 163 | 163 | ||
80 | === modified file 'lib/lp/bugs/model/bug.py' | |||
81 | --- lib/lp/bugs/model/bug.py 2012-10-11 04:21:07 +0000 | |||
82 | +++ lib/lp/bugs/model/bug.py 2012-10-12 11:59:29 +0000 | |||
83 | @@ -13,6 +13,7 @@ | |||
84 | 13 | 'BugSet', | 13 | 'BugSet', |
85 | 14 | 'BugTag', | 14 | 'BugTag', |
86 | 15 | 'FileBugData', | 15 | 'FileBugData', |
87 | 16 | 'generate_subscription_with', | ||
88 | 16 | 'get_also_notified_subscribers', | 17 | 'get_also_notified_subscribers', |
89 | 17 | 'get_bug_tags_open_count', | 18 | 'get_bug_tags_open_count', |
90 | 18 | ] | 19 | ] |
91 | @@ -54,6 +55,7 @@ | |||
92 | 54 | SQL, | 55 | SQL, |
93 | 55 | Sum, | 56 | Sum, |
94 | 56 | Union, | 57 | Union, |
95 | 58 | With, | ||
96 | 57 | ) | 59 | ) |
97 | 58 | from storm.info import ClassAlias | 60 | from storm.info import ClassAlias |
98 | 59 | from storm.locals import ( | 61 | from storm.locals import ( |
99 | @@ -1056,36 +1058,21 @@ | |||
100 | 1056 | else: | 1058 | else: |
101 | 1057 | self._subscriber_dups_cache.add(subscriber) | 1059 | self._subscriber_dups_cache.add(subscriber) |
102 | 1058 | return subscriber | 1060 | return subscriber |
104 | 1059 | return DecoratedResultSet(Store.of(self).find( | 1061 | with_statement = generate_subscription_with(self, person) |
105 | 1062 | store = Store.of(self).with_(with_statement) | ||
106 | 1063 | return DecoratedResultSet(store.find( | ||
107 | 1060 | # Return people and subscriptions | 1064 | # Return people and subscriptions |
108 | 1061 | (Person, BugSubscription), | 1065 | (Person, BugSubscription), |
128 | 1062 | # For this bug or its duplicates | 1066 | BugSubscription.id.is_in( |
129 | 1063 | Or( | 1067 | SQL('SELECT bugsubscriptions.id FROM bugsubscriptions')), |
130 | 1064 | Bug.id == self.id, | 1068 | Person.id == BugSubscription.person_id, |
112 | 1065 | Bug.duplicateof == self.id), | ||
113 | 1066 | # Get subscriptions for these bugs | ||
114 | 1067 | BugSubscription.bug_id == Bug.id, | ||
115 | 1068 | # Filter by subscriptions to any team person is in. | ||
116 | 1069 | # Note that teamparticipation includes self-participation entries | ||
117 | 1070 | # (person X is in the team X) | ||
118 | 1071 | TeamParticipation.person == person.id, | ||
119 | 1072 | # XXX: Storm fails to compile this, so manually done. | ||
120 | 1073 | # bug=https://bugs.launchpad.net/storm/+bug/627137 | ||
121 | 1074 | # RBC 20100831 | ||
122 | 1075 | SQL("""TeamParticipation.team = BugSubscription.person"""), | ||
123 | 1076 | # Join in the Person rows we want | ||
124 | 1077 | # XXX: Storm fails to compile this, so manually done. | ||
125 | 1078 | # bug=https://bugs.launchpad.net/storm/+bug/627137 | ||
126 | 1079 | # RBC 20100831 | ||
127 | 1080 | SQL("""Person.id = TeamParticipation.team"""), | ||
131 | 1081 | ).order_by(Person.name).config( | 1069 | ).order_by(Person.name).config( |
132 | 1082 | distinct=(Person.name, BugSubscription.person_id)), | 1070 | distinct=(Person.name, BugSubscription.person_id)), |
133 | 1083 | cache_subscriber, pre_iter_hook=cache_unsubscribed) | 1071 | cache_subscriber, pre_iter_hook=cache_unsubscribed) |
134 | 1084 | 1072 | ||
135 | 1085 | def getSubscriptionForPerson(self, person): | 1073 | def getSubscriptionForPerson(self, person): |
136 | 1086 | """See `IBug`.""" | 1074 | """See `IBug`.""" |
139 | 1087 | store = Store.of(self) | 1075 | return Store.of(self).find( |
138 | 1088 | return store.find( | ||
140 | 1089 | BugSubscription, | 1076 | BugSubscription, |
141 | 1090 | BugSubscription.person == person, | 1077 | BugSubscription.person == person, |
142 | 1091 | BugSubscription.bug == self).one() | 1078 | BugSubscription.bug == self).one() |
143 | @@ -2830,3 +2817,19 @@ | |||
144 | 2830 | date_created = DateTime( | 2817 | date_created = DateTime( |
145 | 2831 | "date_created", allow_none=False, default=UTC_NOW, | 2818 | "date_created", allow_none=False, default=UTC_NOW, |
146 | 2832 | tzinfo=pytz.UTC) | 2819 | tzinfo=pytz.UTC) |
147 | 2820 | |||
148 | 2821 | |||
149 | 2822 | def generate_subscription_with(bug, person): | ||
150 | 2823 | return [ | ||
151 | 2824 | With('all_bugsubscriptions', Select( | ||
152 | 2825 | (BugSubscription.id, BugSubscription.person_id), | ||
153 | 2826 | tables=[ | ||
154 | 2827 | BugSubscription, Join(Bug, Bug.id == BugSubscription.bug_id)], | ||
155 | 2828 | where=Or(Bug.id == bug.id, Bug.duplicateofID == bug.id))), | ||
156 | 2829 | With('bugsubscriptions', Select( | ||
157 | 2830 | SQL('all_bugsubscriptions.id'), | ||
158 | 2831 | tables=[ | ||
159 | 2832 | SQL('all_bugsubscriptions'), | ||
160 | 2833 | Join(TeamParticipation, TeamParticipation.teamID == SQL( | ||
161 | 2834 | 'all_bugsubscriptions.person'))], | ||
162 | 2835 | where=[TeamParticipation.personID == person.id]))] | ||
163 | 2833 | 2836 | ||
164 | === modified file 'lib/lp/bugs/model/personsubscriptioninfo.py' | |||
165 | --- lib/lp/bugs/model/personsubscriptioninfo.py 2012-08-22 01:41:50 +0000 | |||
166 | +++ lib/lp/bugs/model/personsubscriptioninfo.py 2012-10-12 11:59:29 +0000 | |||
167 | @@ -1,4 +1,4 @@ | |||
169 | 1 | # Copyright 2011 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2011-2012 Canonical Ltd. This software is licensed under the |
170 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
171 | 3 | 3 | ||
172 | 4 | __metaclass__ = type | 4 | __metaclass__ = type |
173 | @@ -6,8 +6,9 @@ | |||
174 | 6 | 'PersonSubscriptions', | 6 | 'PersonSubscriptions', |
175 | 7 | ] | 7 | ] |
176 | 8 | 8 | ||
178 | 9 | from storm.expr import Or | 9 | from storm.expr import SQL |
179 | 10 | from storm.store import Store | 10 | from storm.store import Store |
180 | 11 | from zope.component import getUtility | ||
181 | 11 | from zope.interface import implements | 12 | from zope.interface import implements |
182 | 12 | from zope.proxy import sameProxiedObjects | 13 | from zope.proxy import sameProxiedObjects |
183 | 13 | 14 | ||
184 | @@ -19,17 +20,17 @@ | |||
185 | 19 | IVirtualSubscriptionInfo, | 20 | IVirtualSubscriptionInfo, |
186 | 20 | IVirtualSubscriptionInfoCollection, | 21 | IVirtualSubscriptionInfoCollection, |
187 | 21 | ) | 22 | ) |
188 | 22 | from lp.bugs.interfaces.structuralsubscription import ( | ||
189 | 23 | IStructuralSubscriptionTargetHelper, | ||
190 | 24 | ) | ||
191 | 25 | from lp.bugs.model.bug import ( | 23 | from lp.bugs.model.bug import ( |
192 | 26 | Bug, | 24 | Bug, |
193 | 27 | BugMute, | 25 | BugMute, |
194 | 26 | generate_subscription_with, | ||
195 | 28 | ) | 27 | ) |
196 | 29 | from lp.bugs.model.bugsubscription import BugSubscription | 28 | from lp.bugs.model.bugsubscription import BugSubscription |
198 | 30 | from lp.registry.interfaces.sourcepackage import ISourcePackage | 29 | from lp.registry.interfaces.person import IPersonSet |
199 | 30 | from lp.registry.model.distribution import Distribution | ||
200 | 31 | from lp.registry.model.person import Person | 31 | from lp.registry.model.person import Person |
202 | 32 | from lp.registry.model.teammembership import TeamParticipation | 32 | from lp.registry.model.product import Product |
203 | 33 | from lp.services.database.bulk import load_related | ||
204 | 33 | 34 | ||
205 | 34 | 35 | ||
206 | 35 | class RealSubscriptionInfo: | 36 | class RealSubscriptionInfo: |
207 | @@ -96,8 +97,8 @@ | |||
208 | 96 | person, administrated_team_ids) | 97 | person, administrated_team_ids) |
209 | 97 | self._principal_pillar_to_info = {} | 98 | self._principal_pillar_to_info = {} |
210 | 98 | 99 | ||
213 | 99 | def _add_item_to_collection(self, | 100 | def _add_item_to_collection(self, collection, principal, bug, pillar, |
214 | 100 | collection, principal, bug, pillar, task): | 101 | task): |
215 | 101 | key = (principal, pillar) | 102 | key = (principal, pillar) |
216 | 102 | info = self._principal_pillar_to_info.get(key) | 103 | info = self._principal_pillar_to_info.get(key) |
217 | 103 | if info is None: | 104 | if info is None: |
218 | @@ -118,8 +119,8 @@ | |||
219 | 118 | person, administrated_team_ids) | 119 | person, administrated_team_ids) |
220 | 119 | self._principal_bug_to_infos = {} | 120 | self._principal_bug_to_infos = {} |
221 | 120 | 121 | ||
224 | 121 | def _add_item_to_collection(self, collection, principal, | 122 | def _add_item_to_collection(self, collection, principal, bug, |
225 | 122 | bug, subscription): | 123 | subscription): |
226 | 123 | info = RealSubscriptionInfo(principal, bug, subscription) | 124 | info = RealSubscriptionInfo(principal, bug, subscription) |
227 | 124 | key = (principal, bug) | 125 | key = (principal, bug) |
228 | 125 | infos = self._principal_bug_to_infos.get(key) | 126 | infos = self._principal_bug_to_infos.get(key) |
229 | @@ -158,34 +159,17 @@ | |||
230 | 158 | """See `IPersonSubscriptions`.""" | 159 | """See `IPersonSubscriptions`.""" |
231 | 159 | self.loadSubscriptionsFor(self.person, self.bug) | 160 | self.loadSubscriptionsFor(self.person, self.bug) |
232 | 160 | 161 | ||
233 | 161 | def _getTaskPillar(self, bugtask): | ||
234 | 162 | """Return a pillar for a given BugTask.""" | ||
235 | 163 | # There is no adaptor for ISourcePackage. Perhaps there | ||
236 | 164 | # should be since the data model doesn't seem to prohibit it. | ||
237 | 165 | # For now, we simply work around the problem. It Would Be Nice If | ||
238 | 166 | # there were a reliable generic way of getting the pillar for any | ||
239 | 167 | # bugtarget, but we are not going to tackle that right now. | ||
240 | 168 | if ISourcePackage.providedBy(bugtask.target): | ||
241 | 169 | pillar = IStructuralSubscriptionTargetHelper( | ||
242 | 170 | bugtask.target.distribution_sourcepackage).pillar | ||
243 | 171 | else: | ||
244 | 172 | pillar = IStructuralSubscriptionTargetHelper( | ||
245 | 173 | bugtask.target).pillar | ||
246 | 174 | return pillar | ||
247 | 175 | |||
248 | 176 | def _getDirectAndDuplicateSubscriptions(self, person, bug): | 162 | def _getDirectAndDuplicateSubscriptions(self, person, bug): |
249 | 177 | # Fetch all information for direct and duplicate | 163 | # Fetch all information for direct and duplicate |
250 | 178 | # subscriptions (including indirect through team | 164 | # subscriptions (including indirect through team |
251 | 179 | # membership) in a single query. | 165 | # membership) in a single query. |
255 | 180 | store = Store.of(person) | 166 | with_statement = generate_subscription_with(bug, person) |
256 | 181 | bug_id_options = [Bug.id == bug.id, Bug.duplicateofID == bug.id] | 167 | info = Store.of(person).with_(with_statement).find( |
254 | 182 | info = store.find( | ||
257 | 183 | (BugSubscription, Bug, Person), | 168 | (BugSubscription, Bug, Person), |
263 | 184 | BugSubscription.bug == Bug.id, | 169 | BugSubscription.id.is_in( |
264 | 185 | BugSubscription.person == Person.id, | 170 | SQL('SELECT bugsubscriptions.id FROM bugsubscriptions')), |
265 | 186 | Or(*bug_id_options), | 171 | Person.id == BugSubscription.person_id, |
266 | 187 | TeamParticipation.personID == person.id, | 172 | Bug.id == BugSubscription.bug_id) |
262 | 188 | TeamParticipation.teamID == Person.id) | ||
267 | 189 | 173 | ||
268 | 190 | direct = RealSubscriptionInfoCollection( | 174 | direct = RealSubscriptionInfoCollection( |
269 | 191 | self.person, self.administrated_team_ids) | 175 | self.person, self.administrated_team_ids) |
270 | @@ -202,30 +186,27 @@ | |||
271 | 202 | collection = direct | 186 | collection = direct |
272 | 203 | collection.add( | 187 | collection.add( |
273 | 204 | subscriber, subscribed_bug, subscription) | 188 | subscriber, subscribed_bug, subscription) |
274 | 189 | # Preload bug owners, then all pillars. | ||
275 | 190 | list(getUtility(IPersonSet).getPrecachedPersonsFromIDs( | ||
276 | 191 | [bug.ownerID for bug in bugs])) | ||
277 | 192 | all_tasks = [task for task in bug.bugtasks for bug in bugs] | ||
278 | 193 | load_related(Product, all_tasks, ['productID']) | ||
279 | 194 | load_related(Distribution, all_tasks, ['distributionID']) | ||
280 | 205 | for bug in bugs: | 195 | for bug in bugs: |
281 | 206 | # indicate the reporter and bug_supervisor | 196 | # indicate the reporter and bug_supervisor |
282 | 207 | duplicates.annotateReporter(bug, bug.owner) | 197 | duplicates.annotateReporter(bug, bug.owner) |
283 | 208 | direct.annotateReporter(bug, bug.owner) | 198 | direct.annotateReporter(bug, bug.owner) |
291 | 209 | for task in bug.bugtasks: | 199 | for task in all_tasks: |
292 | 210 | # Get bug_supervisor. | 200 | # Get bug_supervisor. |
293 | 211 | pillar = self._getTaskPillar(task) | 201 | duplicates.annotateBugTaskResponsibilities( |
294 | 212 | duplicates.annotateBugTaskResponsibilities( | 202 | task, task.pillar, task.pillar.bug_supervisor) |
295 | 213 | task, pillar, pillar.bug_supervisor) | 203 | direct.annotateBugTaskResponsibilities( |
296 | 214 | direct.annotateBugTaskResponsibilities( | 204 | task, task.pillar, task.pillar.bug_supervisor) |
290 | 215 | task, pillar, pillar.bug_supervisor) | ||
297 | 216 | return (direct, duplicates) | 205 | return (direct, duplicates) |
298 | 217 | 206 | ||
299 | 218 | def _isMuted(self, person, bug): | 207 | def _isMuted(self, person, bug): |
310 | 219 | store = Store.of(person) | 208 | return not Store.of(person).find( |
311 | 220 | mutes = store.find( | 209 | BugMute, bug=bug, person=person).is_empty() |
302 | 221 | BugMute, | ||
303 | 222 | BugMute.bug == bug, | ||
304 | 223 | BugMute.person == person) | ||
305 | 224 | is_muted = mutes.one() | ||
306 | 225 | if is_muted is None: | ||
307 | 226 | return False | ||
308 | 227 | else: | ||
309 | 228 | return True | ||
312 | 229 | 210 | ||
313 | 230 | def loadSubscriptionsFor(self, person, bug): | 211 | def loadSubscriptionsFor(self, person, bug): |
314 | 231 | self.person = person | 212 | self.person = person |
315 | @@ -246,13 +227,12 @@ | |||
316 | 246 | as_assignee = VirtualSubscriptionInfoCollection( | 227 | as_assignee = VirtualSubscriptionInfoCollection( |
317 | 247 | self.person, self.administrated_team_ids) | 228 | self.person, self.administrated_team_ids) |
318 | 248 | for bugtask in bug.bugtasks: | 229 | for bugtask in bug.bugtasks: |
323 | 249 | pillar = self._getTaskPillar(bugtask) | 230 | owner = bugtask.pillar.owner |
324 | 250 | owner = pillar.owner | 231 | if person.inTeam(owner) and bugtask.pillar.bug_supervisor is None: |
325 | 251 | if person.inTeam(owner) and pillar.bug_supervisor is None: | 232 | as_owner.add(owner, bug, bugtask.pillar, bugtask) |
322 | 252 | as_owner.add(owner, bug, pillar, bugtask) | ||
326 | 253 | assignee = bugtask.assignee | 233 | assignee = bugtask.assignee |
327 | 254 | if person.inTeam(assignee): | 234 | if person.inTeam(assignee): |
329 | 255 | as_assignee.add(assignee, bug, pillar, bugtask) | 235 | as_assignee.add(assignee, bug, bugtask.pillar, bugtask) |
330 | 256 | self.count = 0 | 236 | self.count = 0 |
331 | 257 | for name, collection in ( | 237 | for name, collection in ( |
332 | 258 | ('direct', direct), ('from_duplicate', from_duplicate), | 238 | ('direct', direct), ('from_duplicate', from_duplicate), |
333 | 259 | 239 | ||
334 | === modified file 'lib/lp/bugs/model/tests/test_personsubscriptioninfo.py' | |||
335 | --- lib/lp/bugs/model/tests/test_personsubscriptioninfo.py 2012-08-21 04:04:47 +0000 | |||
336 | +++ lib/lp/bugs/model/tests/test_personsubscriptioninfo.py 2012-10-12 11:59:29 +0000 | |||
337 | @@ -502,7 +502,7 @@ | |||
338 | 502 | with StormStatementRecorder() as recorder: | 502 | with StormStatementRecorder() as recorder: |
339 | 503 | self.subscriptions.reload() | 503 | self.subscriptions.reload() |
340 | 504 | # This should produce a very small number of queries. | 504 | # This should produce a very small number of queries. |
342 | 505 | self.assertThat(recorder, HasQueryCount(LessThan(5))) | 505 | self.assertThat(recorder, HasQueryCount(LessThan(6))) |
343 | 506 | count_with_one_subscribed_duplicate = recorder.count | 506 | count_with_one_subscribed_duplicate = recorder.count |
344 | 507 | # It should have the correct result. | 507 | # It should have the correct result. |
345 | 508 | self.assertCollectionsAreEmpty(except_='from_duplicate') | 508 | self.assertCollectionsAreEmpty(except_='from_duplicate') |
8 - # XXX 2010-10-05 gmb bug=655597: getSubscribersF orPerson( self.user) )
9 - # This line of code keeps the view's query count down,
10 - # possibly using witchcraft. It should be rewritten to be
11 - # useful or removed in favour of making other queries more
12 - # efficient. The witchcraft is because the subscribers are accessed
13 - # in the initial page load, so the data is actually used.
14 - if self.user is not None:
15 - list(bug.
I'd suggest restoring this for now. We know the rest of the changes are probably safe, but about this we're just guessing. It's not detrimental to leave, since we deferred the plan to redesign the method.
153 tzinfo=pytz.UTC) subscription_ with(bug, person):
154 +
155 +def generate_
Looks like a missing newline.
155 +def generate_ subscription_ with(bug, person): bugsubscription s', Select( n.id, BugSubscription .person_ id), [BugSubscriptio n, Join( .bug_id) ], iptions' , Select( bugsubscription s.id'), bugsubscription s'), Join( on.teamID == SQL( ptions. person' ))], TeamParticipati on.personID == person.id]))]
156 + return [
157 + With('all_
158 + (BugSubscriptio
159 + tables=
160 + Bug, Bug.id == BugSubscription
161 + where=Or(Bug.id == bug.id, Bug.duplicateofID == bug.id))),
162 + With('bugsubscr
163 + SQL('all_
164 + tables=[
165 + SQL('all_
166 + TeamParticipation, TeamParticipati
167 + 'all_bugsubscri
168 + where=[
Some of the wrapping here is a bit off.
274 + pillar = self._getTaskPi llar(task)
Can you just use task.pillar instead?
292 + is_muted = Store.of( person) .find(
293 + BugMute, BugMute.bug == bug, BugMute.person == person).one()
294 + return is_muted is not None
This could even be 'return not Store.of( person) .find(BugMute, bug=bug, person= person) .is_empty( )'