Merge lp:~wgrant/launchpad/bugsummary-v2-apg-app into lp:launchpad
- bugsummary-v2-apg-app
- Merge into devel
Status: | Merged |
---|---|
Approved by: | Curtis Hovey |
Approved revision: | no longer in the source branch. |
Merged at revision: | 15691 |
Proposed branch: | lp:~wgrant/launchpad/bugsummary-v2-apg-app |
Merge into: | lp:launchpad |
Diff against target: |
572 lines (+202/-138) 5 files modified
lib/lp/bugs/doc/bugsummary.txt (+107/-78) lib/lp/bugs/model/bug.py (+13/-21) lib/lp/bugs/model/bugsummary.py (+61/-3) lib/lp/bugs/model/bugtask.py (+11/-21) lib/lp/bugs/model/tests/test_bugsummary.py (+10/-15) |
To merge this branch: | bzr merge lp:~wgrant/launchpad/bugsummary-v2-apg-app |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Curtis Hovey (community) | code | Approve | |
Review via email:
|
Commit message
Teach model and test code about BugSummary.
Description of the change
BugSummary privacy is a little special. Public bugs show up in a single row with viewed_by=None, private bugs in several rows with viewed_
This branch teaches the application code about the new column, removing the last blocker for lp:~wgrant/launchpad/bugsummary-v2-apg-db, which alters the triggers to start setting it. The two production queries and one test query that filter on viewed_by have been adjusted to use a factored out filter method, which knows about access policies. I've verified that they still all perform excellently.
Tests are a bit of an issue, as it can't really be tested until the DB changes are deployed as well. But the query is factored out and well tested.
I also reworked bugsummary.txt a bit. The new column makes the printed tables a bit wide, so I made privacy display optional.
Preview Diff
1 | === modified file 'lib/lp/bugs/doc/bugsummary.txt' | |||
2 | --- lib/lp/bugs/doc/bugsummary.txt 2012-07-11 22:31:52 +0000 | |||
3 | +++ lib/lp/bugs/doc/bugsummary.txt 2012-07-25 08:05:24 +0000 | |||
4 | @@ -19,9 +19,9 @@ | |||
5 | 19 | First we should setup some helpers to use in the examples. These will | 19 | First we should setup some helpers to use in the examples. These will |
6 | 20 | let us dump the BugSummary table in a readable format. | 20 | let us dump the BugSummary table in a readable format. |
7 | 21 | 21 | ||
11 | 22 | --------------------------------------------------------------- | 22 | ---------------------------------------------------------- |
12 | 23 | prod ps dist ds spn tag mile status import pa vis # | 23 | prod ps dist ds spn tag mile status import pa # |
13 | 24 | --------------------------------------------------------------- | 24 | ---------------------------------------------------------- |
14 | 25 | 25 | ||
15 | 26 | The columns are product, productseries, distribution, distroseries, | 26 | The columns are product, productseries, distribution, distroseries, |
16 | 27 | sourcepackagename, tag, milestone, status, importance, has_patch, | 27 | sourcepackagename, tag, milestone, status, importance, has_patch, |
17 | @@ -40,7 +40,18 @@ | |||
18 | 40 | ... return 'x' | 40 | ... return 'x' |
19 | 41 | ... return object_or_none.name | 41 | ... return object_or_none.name |
20 | 42 | 42 | ||
22 | 43 | >>> def print_result(bugsummary_resultset): | 43 | >>> def ap_desc(policy_or_none): |
23 | 44 | ... if policy_or_none is None: | ||
24 | 45 | ... return 'x' | ||
25 | 46 | ... type_names = { | ||
26 | 47 | ... InformationType.PRIVATESECURITY: 'se', | ||
27 | 48 | ... InformationType.USERDATA: 'pr', | ||
28 | 49 | ... InformationType.PROPRIETARY: 'pp', | ||
29 | 50 | ... } | ||
30 | 51 | ... return '%-4s/%-2s' % ( | ||
31 | 52 | ... policy_or_none.pillar.name, type_names[policy_or_none.type]) | ||
32 | 53 | |||
33 | 54 | >>> def print_result(bugsummary_resultset, include_privacy=False): | ||
34 | 44 | ... # First, flush and invalidate the cache so we see the effects | 55 | ... # First, flush and invalidate the cache so we see the effects |
35 | 45 | ... # of the underlying database triggers. Normally you don't want | 56 | ... # of the underlying database triggers. Normally you don't want |
36 | 46 | ... # to bother with this as you are only interested in counts of | 57 | ... # to bother with this as you are only interested in counts of |
37 | @@ -59,18 +70,28 @@ | |||
38 | 59 | ... BugSummary.sourcepackagename_id, BugSummary.tag, | 70 | ... BugSummary.sourcepackagename_id, BugSummary.tag, |
39 | 60 | ... BugSummary.milestone_id, BugSummary.status, | 71 | ... BugSummary.milestone_id, BugSummary.status, |
40 | 61 | ... BugSummary.importance, BugSummary.has_patch, | 72 | ... BugSummary.importance, BugSummary.has_patch, |
42 | 62 | ... BugSummary.viewed_by_id, BugSummary.id) | 73 | ... BugSummary.viewed_by_id, BugSummary.access_policy_id, |
43 | 74 | ... BugSummary.id) | ||
44 | 63 | ... fmt = ( | 75 | ... fmt = ( |
45 | 64 | ... "%-4s %-4s %-4s %-4s %-5s %-3s %-4s " | 76 | ... "%-4s %-4s %-4s %-4s %-5s %-3s %-4s " |
48 | 65 | ... "%-6s %-6s %-2s %-4s %3s") | 77 | ... "%-6s %-6s %-2s") |
49 | 66 | ... header = fmt % ( | 78 | ... titles = ( |
50 | 67 | ... 'prod', 'ps', 'dist', 'ds', 'spn', 'tag', 'mile', | 79 | ... 'prod', 'ps', 'dist', 'ds', 'spn', 'tag', 'mile', |
52 | 68 | ... 'status', 'import', 'pa', 'vis', '#') | 80 | ... 'status', 'import', 'pa') |
53 | 81 | ... if include_privacy: | ||
54 | 82 | ... fmt += ' %-4s %-7s' | ||
55 | 83 | ... titles += ('gra', 'pol') | ||
56 | 84 | ... fmt += ' %3s' | ||
57 | 85 | ... titles += ('#',) | ||
58 | 86 | ... header = fmt % titles | ||
59 | 69 | ... print "-" * len(header) | 87 | ... print "-" * len(header) |
60 | 70 | ... print header | 88 | ... print header |
61 | 71 | ... print "-" * len(header) | 89 | ... print "-" * len(header) |
62 | 72 | ... for bugsummary in ordered_results: | 90 | ... for bugsummary in ordered_results: |
64 | 73 | ... print fmt % ( | 91 | ... if not include_privacy: |
65 | 92 | ... assert bugsummary.viewed_by is None | ||
66 | 93 | ... assert bugsummary.access_policy is None | ||
67 | 94 | ... data = ( | ||
68 | 74 | ... name(bugsummary.product), | 95 | ... name(bugsummary.product), |
69 | 75 | ... name(bugsummary.productseries), | 96 | ... name(bugsummary.productseries), |
70 | 76 | ... name(bugsummary.distribution), | 97 | ... name(bugsummary.distribution), |
71 | @@ -80,9 +101,13 @@ | |||
72 | 80 | ... name(bugsummary.milestone), | 101 | ... name(bugsummary.milestone), |
73 | 81 | ... str(bugsummary.status)[:6], | 102 | ... str(bugsummary.status)[:6], |
74 | 82 | ... str(bugsummary.importance)[:6], | 103 | ... str(bugsummary.importance)[:6], |
78 | 83 | ... str(bugsummary.has_patch)[:1], | 104 | ... str(bugsummary.has_patch)[:1]) |
79 | 84 | ... name(bugsummary.viewed_by), | 105 | ... if include_privacy: |
80 | 85 | ... bugsummary.count) | 106 | ... data += ( |
81 | 107 | ... name(bugsummary.viewed_by), | ||
82 | 108 | ... ap_desc(bugsummary.access_policy), | ||
83 | 109 | ... ) | ||
84 | 110 | ... print fmt % (data + (bugsummary.count,)) | ||
85 | 86 | ... print " " * (len(header) - 4), | 111 | ... print " " * (len(header) - 4), |
86 | 87 | ... print "===" | 112 | ... print "===" |
87 | 88 | ... sum = bugsummary_resultset.sum(BugSummary.count) | 113 | ... sum = bugsummary_resultset.sum(BugSummary.count) |
88 | @@ -90,8 +115,9 @@ | |||
89 | 90 | ... print "%3s" % sum | 115 | ... print "%3s" % sum |
90 | 91 | 116 | ||
91 | 92 | >>> def print_find(*bs_query_args, **bs_query_kw): | 117 | >>> def print_find(*bs_query_args, **bs_query_kw): |
92 | 118 | ... include_privacy = bs_query_kw.pop('include_privacy', False) | ||
93 | 93 | ... resultset = store.find(BugSummary, *bs_query_args, **bs_query_kw) | 119 | ... resultset = store.find(BugSummary, *bs_query_args, **bs_query_kw) |
95 | 94 | ... print_result(resultset) | 120 | ... print_result(resultset, include_privacy=include_privacy) |
96 | 95 | 121 | ||
97 | 96 | 122 | ||
98 | 97 | /!\ A Note About Privacy in These Examples | 123 | /!\ A Note About Privacy in These Examples |
99 | @@ -119,12 +145,12 @@ | |||
100 | 119 | ... BugSummary.tag == None) | 145 | ... BugSummary.tag == None) |
101 | 120 | 146 | ||
102 | 121 | >>> print_result(bug_summaries) | 147 | >>> print_result(bug_summaries) |
109 | 122 | ------------------------------------------------------------ | 148 | ------------------------------------------------------- |
110 | 123 | prod ps dist ds spn tag mile status import pa vis # | 149 | prod ps dist ds spn tag mile status import pa # |
111 | 124 | ------------------------------------------------------------ | 150 | ------------------------------------------------------- |
112 | 125 | pr-a x x x x x x New Undeci F x 1 | 151 | pr-a x x x x x x New Undeci F 1 |
113 | 126 | === | 152 | === |
114 | 127 | 1 | 153 | 1 |
115 | 128 | 154 | ||
116 | 129 | There is one row per tag per combination of product, status and milestone. | 155 | There is one row per tag per combination of product, status and milestone. |
117 | 130 | If we are interested in all bugs targeted to a product regardless of how | 156 | If we are interested in all bugs targeted to a product regardless of how |
118 | @@ -145,13 +171,13 @@ | |||
119 | 145 | ... BugSummary.product == prod_a, | 171 | ... BugSummary.product == prod_a, |
120 | 146 | ... BugSummary.tag == None, | 172 | ... BugSummary.tag == None, |
121 | 147 | ... BugSummary.viewed_by == None) | 173 | ... BugSummary.viewed_by == None) |
129 | 148 | ------------------------------------------------------------ | 174 | ------------------------------------------------------- |
130 | 149 | prod ps dist ds spn tag mile status import pa vis # | 175 | prod ps dist ds spn tag mile status import pa # |
131 | 150 | ------------------------------------------------------------ | 176 | ------------------------------------------------------- |
132 | 151 | pr-a x x x x x x New Undeci F x 2 | 177 | pr-a x x x x x x New Undeci F 2 |
133 | 152 | pr-a x x x x x x Confir Undeci F x 2 | 178 | pr-a x x x x x x Confir Undeci F 2 |
134 | 153 | === | 179 | === |
135 | 154 | 4 | 180 | 4 |
136 | 155 | 181 | ||
137 | 156 | Here are the rows associated with the 't-a' tag. There is 1 Confirmed | 182 | Here are the rows associated with the 't-a' tag. There is 1 Confirmed |
138 | 157 | bug task targetted to the pr-a product who's bug is tagged 't-a'.: | 183 | bug task targetted to the pr-a product who's bug is tagged 't-a'.: |
139 | @@ -160,12 +186,12 @@ | |||
140 | 160 | ... BugSummary.product == prod_a, | 186 | ... BugSummary.product == prod_a, |
141 | 161 | ... BugSummary.tag == u't-a', | 187 | ... BugSummary.tag == u't-a', |
142 | 162 | ... BugSummary.viewed_by == None) | 188 | ... BugSummary.viewed_by == None) |
149 | 163 | ------------------------------------------------------------ | 189 | ------------------------------------------------------- |
150 | 164 | prod ps dist ds spn tag mile status import pa vis # | 190 | prod ps dist ds spn tag mile status import pa # |
151 | 165 | ------------------------------------------------------------ | 191 | ------------------------------------------------------- |
152 | 166 | pr-a x x x x t-a x Confir Undeci F x 1 | 192 | pr-a x x x x t-a x Confir Undeci F 1 |
153 | 167 | === | 193 | === |
154 | 168 | 1 | 194 | 1 |
155 | 169 | 195 | ||
156 | 170 | You will normally want to get the total count counted in the database | 196 | You will normally want to get the total count counted in the database |
157 | 171 | rather than waste transmission time to calculate the rows client side. | 197 | rather than waste transmission time to calculate the rows client side. |
158 | @@ -205,17 +231,17 @@ | |||
159 | 205 | >>> print_find( | 231 | >>> print_find( |
160 | 206 | ... BugSummary.product == prod_a, | 232 | ... BugSummary.product == prod_a, |
161 | 207 | ... BugSummary.viewed_by == None) | 233 | ... BugSummary.viewed_by == None) |
173 | 208 | ------------------------------------------------------------ | 234 | ------------------------------------------------------- |
174 | 209 | prod ps dist ds spn tag mile status import pa vis # | 235 | prod ps dist ds spn tag mile status import pa # |
175 | 210 | ------------------------------------------------------------ | 236 | ------------------------------------------------------- |
176 | 211 | pr-a x x x x t-a x Confir Undeci F x 1 | 237 | pr-a x x x x t-a x Confir Undeci F 1 |
177 | 212 | pr-a x x x x t-b ms-a New Undeci F x 1 | 238 | pr-a x x x x t-b ms-a New Undeci F 1 |
178 | 213 | pr-a x x x x t-c ms-a New Undeci F x 1 | 239 | pr-a x x x x t-c ms-a New Undeci F 1 |
179 | 214 | pr-a x x x x x ms-a New Undeci F x 1 | 240 | pr-a x x x x x ms-a New Undeci F 1 |
180 | 215 | pr-a x x x x x x New Undeci F x 2 | 241 | pr-a x x x x x x New Undeci F 2 |
181 | 216 | pr-a x x x x x x Confir Undeci F x 2 | 242 | pr-a x x x x x x Confir Undeci F 2 |
182 | 217 | === | 243 | === |
183 | 218 | 8 | 244 | 8 |
184 | 219 | 245 | ||
185 | 220 | Number of New bugs not targeted to a milestone. Note the difference | 246 | Number of New bugs not targeted to a milestone. Note the difference |
186 | 221 | between selecting records where tag is None, and where milestone is None: | 247 | between selecting records where tag is None, and where milestone is None: |
187 | @@ -269,13 +295,13 @@ | |||
188 | 269 | ... BugSummary.productseries == productseries_b, | 295 | ... BugSummary.productseries == productseries_b, |
189 | 270 | ... BugSummary.product == prod_b), | 296 | ... BugSummary.product == prod_b), |
190 | 271 | ... BugSummary.viewed_by == None) | 297 | ... BugSummary.viewed_by == None) |
198 | 272 | ------------------------------------------------------------ | 298 | ------------------------------------------------------- |
199 | 273 | prod ps dist ds spn tag mile status import pa vis # | 299 | prod ps dist ds spn tag mile status import pa # |
200 | 274 | ------------------------------------------------------------ | 300 | ------------------------------------------------------- |
201 | 275 | pr-b x x x x x x New Undeci F x 1 | 301 | pr-b x x x x x x New Undeci F 1 |
202 | 276 | x ps-b x x x x x New Undeci F x 1 | 302 | x ps-b x x x x x New Undeci F 1 |
203 | 277 | === | 303 | === |
204 | 278 | 2 | 304 | 2 |
205 | 279 | 305 | ||
206 | 280 | Distribution Bug Counts | 306 | Distribution Bug Counts |
207 | 281 | ----------------------- | 307 | ----------------------- |
208 | @@ -297,14 +323,14 @@ | |||
209 | 297 | >>> print_find( | 323 | >>> print_find( |
210 | 298 | ... BugSummary.distribution == distribution, | 324 | ... BugSummary.distribution == distribution, |
211 | 299 | ... BugSummary.viewed_by == None) | 325 | ... BugSummary.viewed_by == None) |
220 | 300 | ------------------------------------------------------------ | 326 | ------------------------------------------------------- |
221 | 301 | prod ps dist ds spn tag mile status import pa vis # | 327 | prod ps dist ds spn tag mile status import pa # |
222 | 302 | ------------------------------------------------------------ | 328 | ------------------------------------------------------- |
223 | 303 | x x di-a x sp-a x x New Undeci F x 1 | 329 | x x di-a x sp-a x x New Undeci F 1 |
224 | 304 | x x di-a x x x x New Undeci F x 1 | 330 | x x di-a x x x x New Undeci F 1 |
225 | 305 | x x di-a x x x x Confir Undeci F x 1 | 331 | x x di-a x x x x Confir Undeci F 1 |
226 | 306 | === | 332 | === |
227 | 307 | 3 | 333 | 3 |
228 | 308 | 334 | ||
229 | 309 | How many bugs targeted to a distribution? | 335 | How many bugs targeted to a distribution? |
230 | 310 | 336 | ||
231 | @@ -369,12 +395,12 @@ | |||
232 | 369 | >>> print_find( | 395 | >>> print_find( |
233 | 370 | ... BugSummary.distroseries == series_c, | 396 | ... BugSummary.distroseries == series_c, |
234 | 371 | ... BugSummary.viewed_by == None) | 397 | ... BugSummary.viewed_by == None) |
241 | 372 | ------------------------------------------------------------ | 398 | ------------------------------------------------------- |
242 | 373 | prod ps dist ds spn tag mile status import pa vis # | 399 | prod ps dist ds spn tag mile status import pa # |
243 | 374 | ------------------------------------------------------------ | 400 | ------------------------------------------------------- |
244 | 375 | x x x ds-c x x x New Undeci F x 1 | 401 | x x x ds-c x x x New Undeci F 1 |
245 | 376 | === | 402 | === |
246 | 377 | 1 | 403 | 1 |
247 | 378 | 404 | ||
248 | 379 | 405 | ||
249 | 380 | Privacy | 406 | Privacy |
250 | @@ -440,19 +466,19 @@ | |||
251 | 440 | >>> distro_or_series = Or( | 466 | >>> distro_or_series = Or( |
252 | 441 | ... BugSummary.distribution == distro_p, | 467 | ... BugSummary.distribution == distro_p, |
253 | 442 | ... BugSummary.distroseries == series_p) | 468 | ... BugSummary.distroseries == series_p) |
267 | 443 | >>> print_find(distro_or_series) | 469 | >>> print_find(distro_or_series, include_privacy=True) |
268 | 444 | ------------------------------------------------------------ | 470 | -------------------------------------------------------------------- |
269 | 445 | prod ps dist ds spn tag mile status import pa vis # | 471 | prod ps dist ds spn tag mile status import pa gra pol # |
270 | 446 | ------------------------------------------------------------ | 472 | -------------------------------------------------------------------- |
271 | 447 | x x di-p x x x x New Undeci F p-b 1 | 473 | x x di-p x x x x New Undeci F p-b x 1 |
272 | 448 | x x di-p x x x x New Undeci F own 3 | 474 | x x di-p x x x x New Undeci F own x 3 |
273 | 449 | x x di-p x x x x New Undeci F t-a 1 | 475 | x x di-p x x x x New Undeci F t-a x 1 |
274 | 450 | x x di-p x x x x New Undeci F t-c 1 | 476 | x x di-p x x x x New Undeci F t-c x 1 |
275 | 451 | x x di-p x x x x New Undeci F x 1 | 477 | x x di-p x x x x New Undeci F x x 1 |
276 | 452 | x x x ds-p x x x New Undeci F own 1 | 478 | x x x ds-p x x x New Undeci F own x 1 |
277 | 453 | x x x ds-p x x x New Undeci F t-c 1 | 479 | x x x ds-p x x x New Undeci F t-c x 1 |
278 | 454 | === | 480 | === |
279 | 455 | 9 | 481 | 9 |
280 | 456 | 482 | ||
281 | 457 | So how many public bugs are there on the distro? | 483 | So how many public bugs are there on the distro? |
282 | 458 | 484 | ||
283 | @@ -460,12 +486,14 @@ | |||
284 | 460 | ... BugSummary, | 486 | ... BugSummary, |
285 | 461 | ... BugSummary.distribution == distro_p, | 487 | ... BugSummary.distribution == distro_p, |
286 | 462 | ... BugSummary.viewed_by == None, # Public bugs only | 488 | ... BugSummary.viewed_by == None, # Public bugs only |
287 | 489 | ... BugSummary.access_policy == None, # Public bugs only | ||
288 | 463 | ... BugSummary.sourcepackagename == None, | 490 | ... BugSummary.sourcepackagename == None, |
289 | 464 | ... BugSummary.tag == None).sum(BugSummary.count) or 0 | 491 | ... BugSummary.tag == None).sum(BugSummary.count) or 0 |
290 | 465 | 1 | 492 | 1 |
291 | 466 | 493 | ||
292 | 467 | But how many can the owner see? | 494 | But how many can the owner see? |
293 | 468 | 495 | ||
294 | 496 | >>> from storm.expr import And | ||
295 | 469 | >>> join = LeftJoin( | 497 | >>> join = LeftJoin( |
296 | 470 | ... BugSummary, TeamParticipation, | 498 | ... BugSummary, TeamParticipation, |
297 | 471 | ... BugSummary.viewed_by_id == TeamParticipation.teamID) | 499 | ... BugSummary.viewed_by_id == TeamParticipation.teamID) |
298 | @@ -473,7 +501,8 @@ | |||
299 | 473 | ... BugSummary, | 501 | ... BugSummary, |
300 | 474 | ... BugSummary.distribution == distro_p, | 502 | ... BugSummary.distribution == distro_p, |
301 | 475 | ... Or( | 503 | ... Or( |
303 | 476 | ... BugSummary.viewed_by == None, | 504 | ... And(BugSummary.viewed_by == None, |
304 | 505 | ... BugSummary.access_policy == None), | ||
305 | 477 | ... TeamParticipation.person == owner), | 506 | ... TeamParticipation.person == owner), |
306 | 478 | ... BugSummary.sourcepackagename == None, | 507 | ... BugSummary.sourcepackagename == None, |
307 | 479 | ... BugSummary.tag == None).sum(BugSummary.count) or 0 | 508 | ... BugSummary.tag == None).sum(BugSummary.count) or 0 |
308 | 480 | 509 | ||
309 | === modified file 'lib/lp/bugs/model/bug.py' | |||
310 | --- lib/lp/bugs/model/bug.py 2012-07-19 04:40:03 +0000 | |||
311 | +++ lib/lp/bugs/model/bug.py 2012-07-25 08:05:24 +0000 | |||
312 | @@ -227,12 +227,7 @@ | |||
313 | 227 | get_property_cache, | 227 | get_property_cache, |
314 | 228 | ) | 228 | ) |
315 | 229 | from lp.services.webapp.authorization import check_permission | 229 | from lp.services.webapp.authorization import check_permission |
322 | 230 | from lp.services.webapp.interfaces import ( | 230 | from lp.services.webapp.interfaces import ILaunchBag |
317 | 231 | DEFAULT_FLAVOR, | ||
318 | 232 | ILaunchBag, | ||
319 | 233 | IStoreSelector, | ||
320 | 234 | MAIN_STORE, | ||
321 | 235 | ) | ||
323 | 236 | 231 | ||
324 | 237 | 232 | ||
325 | 238 | _bug_tag_query_template = """ | 233 | _bug_tag_query_template = """ |
326 | @@ -291,29 +286,26 @@ | |||
327 | 291 | (and {} returned). | 286 | (and {} returned). |
328 | 292 | """ | 287 | """ |
329 | 293 | # Circular fail. | 288 | # Circular fail. |
331 | 294 | from lp.bugs.model.bugsummary import BugSummary | 289 | from lp.bugs.model.bugsummary import ( |
332 | 290 | BugSummary, | ||
333 | 291 | get_bugsummary_filter_for_user, | ||
334 | 292 | ) | ||
335 | 295 | tags = {} | 293 | tags = {} |
336 | 296 | if include_tags: | 294 | if include_tags: |
337 | 297 | tags = dict((tag, 0) for tag in include_tags) | 295 | tags = dict((tag, 0) for tag in include_tags) |
338 | 298 | store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR) | ||
339 | 299 | admin_team = getUtility(ILaunchpadCelebrities).admin | ||
340 | 300 | if user is not None and not user.inTeam(admin_team): | ||
341 | 301 | store = store.with_(SQL( | ||
342 | 302 | "teams AS (" | ||
343 | 303 | "SELECT team from TeamParticipation WHERE person=?)", (user.id,))) | ||
344 | 304 | where_conditions = [ | 296 | where_conditions = [ |
345 | 305 | BugSummary.status.is_in(UNRESOLVED_BUGTASK_STATUSES), | 297 | BugSummary.status.is_in(UNRESOLVED_BUGTASK_STATUSES), |
346 | 306 | BugSummary.tag != None, | 298 | BugSummary.tag != None, |
347 | 307 | context_condition, | 299 | context_condition, |
348 | 308 | ] | 300 | ] |
357 | 309 | if user is None: | 301 | |
358 | 310 | where_conditions.append(BugSummary.viewed_by_id == None) | 302 | # Apply the privacy filter. |
359 | 311 | elif not user.inTeam(admin_team): | 303 | store = IStore(BugSummary) |
360 | 312 | where_conditions.append( | 304 | user_with, user_where = get_bugsummary_filter_for_user(user) |
361 | 313 | Or( | 305 | if user_with: |
362 | 314 | BugSummary.viewed_by_id == None, | 306 | store = store.with_(user_with) |
363 | 315 | BugSummary.viewed_by_id.is_in(SQL("SELECT team FROM teams")) | 307 | where_conditions.extend(user_where) |
364 | 316 | )) | 308 | |
365 | 317 | sum_count = Sum(BugSummary.count) | 309 | sum_count = Sum(BugSummary.count) |
366 | 318 | tag_count_columns = (BugSummary.tag, sum_count) | 310 | tag_count_columns = (BugSummary.tag, sum_count) |
367 | 319 | 311 | ||
368 | 320 | 312 | ||
369 | === modified file 'lib/lp/bugs/model/bugsummary.py' | |||
370 | --- lib/lp/bugs/model/bugsummary.py 2012-06-25 09:45:56 +0000 | |||
371 | +++ lib/lp/bugs/model/bugsummary.py 2012-07-25 08:05:24 +0000 | |||
372 | @@ -7,16 +7,23 @@ | |||
373 | 7 | __all__ = [ | 7 | __all__ = [ |
374 | 8 | 'BugSummary', | 8 | 'BugSummary', |
375 | 9 | 'CombineBugSummaryConstraint', | 9 | 'CombineBugSummaryConstraint', |
376 | 10 | 'get_bugsummary_filter_for_user', | ||
377 | 10 | ] | 11 | ] |
378 | 11 | 12 | ||
380 | 12 | from storm.locals import ( | 13 | from storm.base import Storm |
381 | 14 | from storm.expr import ( | ||
382 | 13 | And, | 15 | And, |
383 | 16 | Or, | ||
384 | 17 | Select, | ||
385 | 18 | SQL, | ||
386 | 19 | With, | ||
387 | 20 | ) | ||
388 | 21 | from storm.properties import ( | ||
389 | 14 | Bool, | 22 | Bool, |
390 | 15 | Int, | 23 | Int, |
391 | 16 | Reference, | ||
392 | 17 | Storm, | ||
393 | 18 | Unicode, | 24 | Unicode, |
394 | 19 | ) | 25 | ) |
395 | 26 | from storm.references import Reference | ||
396 | 20 | from zope.interface import implements | 27 | from zope.interface import implements |
397 | 21 | from zope.security.proxy import removeSecurityProxy | 28 | from zope.security.proxy import removeSecurityProxy |
398 | 22 | 29 | ||
399 | @@ -29,6 +36,11 @@ | |||
400 | 29 | BugTaskStatus, | 36 | BugTaskStatus, |
401 | 30 | BugTaskStatusSearch, | 37 | BugTaskStatusSearch, |
402 | 31 | ) | 38 | ) |
403 | 39 | from lp.registry.interfaces.role import IPersonRoles | ||
404 | 40 | from lp.registry.model.accesspolicy import ( | ||
405 | 41 | AccessPolicy, | ||
406 | 42 | AccessPolicyGrant, | ||
407 | 43 | ) | ||
408 | 32 | from lp.registry.model.distribution import Distribution | 44 | from lp.registry.model.distribution import Distribution |
409 | 33 | from lp.registry.model.distroseries import DistroSeries | 45 | from lp.registry.model.distroseries import DistroSeries |
410 | 34 | from lp.registry.model.milestone import Milestone | 46 | from lp.registry.model.milestone import Milestone |
411 | @@ -36,6 +48,7 @@ | |||
412 | 36 | from lp.registry.model.product import Product | 48 | from lp.registry.model.product import Product |
413 | 37 | from lp.registry.model.productseries import ProductSeries | 49 | from lp.registry.model.productseries import ProductSeries |
414 | 38 | from lp.registry.model.sourcepackagename import SourcePackageName | 50 | from lp.registry.model.sourcepackagename import SourcePackageName |
415 | 51 | from lp.registry.model.teammembership import TeamParticipation | ||
416 | 39 | from lp.services.database.enumcol import EnumCol | 52 | from lp.services.database.enumcol import EnumCol |
417 | 40 | 53 | ||
418 | 41 | 54 | ||
419 | @@ -76,6 +89,8 @@ | |||
420 | 76 | 89 | ||
421 | 77 | viewed_by_id = Int(name='viewed_by') | 90 | viewed_by_id = Int(name='viewed_by') |
422 | 78 | viewed_by = Reference(viewed_by_id, Person.id) | 91 | viewed_by = Reference(viewed_by_id, Person.id) |
423 | 92 | access_policy_id = Int(name='access_policy') | ||
424 | 93 | access_policy = Reference(access_policy_id, AccessPolicy.id) | ||
425 | 79 | 94 | ||
426 | 80 | has_patch = Bool() | 95 | has_patch = Bool() |
427 | 81 | 96 | ||
428 | @@ -99,3 +114,46 @@ | |||
429 | 99 | def getBugSummaryContextWhereClause(self): | 114 | def getBugSummaryContextWhereClause(self): |
430 | 100 | """See `IBugSummaryDimension`.""" | 115 | """See `IBugSummaryDimension`.""" |
431 | 101 | return And(*self.dimensions) | 116 | return And(*self.dimensions) |
432 | 117 | |||
433 | 118 | |||
434 | 119 | def get_bugsummary_filter_for_user(user): | ||
435 | 120 | """Build a Storm expression to filter BugSummary by visibility. | ||
436 | 121 | |||
437 | 122 | :param user: The user for which visible rows should be calculated. | ||
438 | 123 | :return: (with_clauses, where_clauses) | ||
439 | 124 | """ | ||
440 | 125 | # Admins get to see every bug, everyone else only sees bugs | ||
441 | 126 | # viewable by them-or-their-teams. | ||
442 | 127 | # Note that because admins can see every bug regardless of | ||
443 | 128 | # subscription they will see rather inflated counts. Admins get to | ||
444 | 129 | # deal. | ||
445 | 130 | public_filter = And( | ||
446 | 131 | BugSummary.viewed_by_id == None, | ||
447 | 132 | BugSummary.access_policy_id == None) | ||
448 | 133 | if user is None: | ||
449 | 134 | return [], [public_filter] | ||
450 | 135 | elif IPersonRoles(user).in_admin: | ||
451 | 136 | return [], [] | ||
452 | 137 | else: | ||
453 | 138 | with_clauses = [ | ||
454 | 139 | With( | ||
455 | 140 | 'teams', | ||
456 | 141 | Select( | ||
457 | 142 | TeamParticipation.teamID, tables=[TeamParticipation], | ||
458 | 143 | where=(TeamParticipation.personID == user.id))), | ||
459 | 144 | With( | ||
460 | 145 | 'policies', | ||
461 | 146 | Select( | ||
462 | 147 | AccessPolicyGrant.policy_id, | ||
463 | 148 | tables=[AccessPolicyGrant], | ||
464 | 149 | where=( | ||
465 | 150 | AccessPolicyGrant.grantee_id.is_in( | ||
466 | 151 | SQL("SELECT team FROM teams"))))), | ||
467 | 152 | ] | ||
468 | 153 | where_clauses = [Or( | ||
469 | 154 | public_filter, | ||
470 | 155 | BugSummary.viewed_by_id.is_in( | ||
471 | 156 | SQL("SELECT team FROM teams")), | ||
472 | 157 | BugSummary.access_policy_id.is_in( | ||
473 | 158 | SQL("SELECT policy FROM policies")))] | ||
474 | 159 | return with_clauses, where_clauses | ||
475 | 102 | 160 | ||
476 | === modified file 'lib/lp/bugs/model/bugtask.py' | |||
477 | --- lib/lp/bugs/model/bugtask.py 2012-07-24 10:03:32 +0000 | |||
478 | +++ lib/lp/bugs/model/bugtask.py 2012-07-25 08:05:24 +0000 | |||
479 | @@ -1549,7 +1549,10 @@ | |||
480 | 1549 | def countBugs(self, user, contexts, group_on): | 1549 | def countBugs(self, user, contexts, group_on): |
481 | 1550 | """See `IBugTaskSet`.""" | 1550 | """See `IBugTaskSet`.""" |
482 | 1551 | # Circular fail. | 1551 | # Circular fail. |
484 | 1552 | from lp.bugs.model.bugsummary import BugSummary | 1552 | from lp.bugs.model.bugsummary import ( |
485 | 1553 | BugSummary, | ||
486 | 1554 | get_bugsummary_filter_for_user, | ||
487 | 1555 | ) | ||
488 | 1553 | conditions = [] | 1556 | conditions = [] |
489 | 1554 | # Open bug statuses | 1557 | # Open bug statuses |
490 | 1555 | conditions.append( | 1558 | conditions.append( |
491 | @@ -1577,27 +1580,14 @@ | |||
492 | 1577 | conditions.append(BugSummary.tag == None) | 1580 | conditions.append(BugSummary.tag == None) |
493 | 1578 | else: | 1581 | else: |
494 | 1579 | conditions.append(BugSummary.tag != None) | 1582 | conditions.append(BugSummary.tag != None) |
495 | 1583 | |||
496 | 1584 | # Apply the privacy filter. | ||
497 | 1580 | store = IStore(BugSummary) | 1585 | store = IStore(BugSummary) |
518 | 1581 | admin_team = getUtility(ILaunchpadCelebrities).admin | 1586 | user_with, user_where = get_bugsummary_filter_for_user(user) |
519 | 1582 | if user is not None and not user.inTeam(admin_team): | 1587 | if user_with: |
520 | 1583 | # admins get to see every bug, everyone else only sees bugs | 1588 | store = store.with_(user_with) |
521 | 1584 | # viewable by them-or-their-teams. | 1589 | conditions.extend(user_where) |
522 | 1585 | store = store.with_(SQL( | 1590 | |
503 | 1586 | "teams AS (" | ||
504 | 1587 | "SELECT team from TeamParticipation WHERE person=?)", | ||
505 | 1588 | (user.id,))) | ||
506 | 1589 | # Note that because admins can see every bug regardless of | ||
507 | 1590 | # subscription they will see rather inflated counts. Admins get to | ||
508 | 1591 | # deal. | ||
509 | 1592 | if user is None: | ||
510 | 1593 | conditions.append(BugSummary.viewed_by_id == None) | ||
511 | 1594 | elif not user.inTeam(admin_team): | ||
512 | 1595 | conditions.append( | ||
513 | 1596 | Or( | ||
514 | 1597 | BugSummary.viewed_by_id == None, | ||
515 | 1598 | BugSummary.viewed_by_id.is_in( | ||
516 | 1599 | SQL("SELECT team FROM teams")) | ||
517 | 1600 | )) | ||
523 | 1601 | sum_count = Sum(BugSummary.count) | 1591 | sum_count = Sum(BugSummary.count) |
524 | 1602 | resultset = store.find(group_on + (sum_count,), *conditions) | 1592 | resultset = store.find(group_on + (sum_count,), *conditions) |
525 | 1603 | resultset.group_by(*group_on) | 1593 | resultset.group_by(*group_on) |
526 | 1604 | 1594 | ||
527 | === modified file 'lib/lp/bugs/model/tests/test_bugsummary.py' | |||
528 | --- lib/lp/bugs/model/tests/test_bugsummary.py 2012-05-24 22:37:33 +0000 | |||
529 | +++ lib/lp/bugs/model/tests/test_bugsummary.py 2012-07-25 08:05:24 +0000 | |||
530 | @@ -16,10 +16,12 @@ | |||
531 | 16 | BugTaskStatus, | 16 | BugTaskStatus, |
532 | 17 | ) | 17 | ) |
533 | 18 | from lp.bugs.model.bug import BugTag | 18 | from lp.bugs.model.bug import BugTag |
535 | 19 | from lp.bugs.model.bugsummary import BugSummary | 19 | from lp.bugs.model.bugsummary import ( |
536 | 20 | BugSummary, | ||
537 | 21 | get_bugsummary_filter_for_user, | ||
538 | 22 | ) | ||
539 | 20 | from lp.bugs.model.bugtask import BugTask | 23 | from lp.bugs.model.bugtask import BugTask |
540 | 21 | from lp.registry.enums import InformationType | 24 | from lp.registry.enums import InformationType |
541 | 22 | from lp.registry.model.teammembership import TeamParticipation | ||
542 | 23 | from lp.services.database.lpstorm import IMasterStore | 25 | from lp.services.database.lpstorm import IMasterStore |
543 | 24 | from lp.testing import TestCaseWithFactory | 26 | from lp.testing import TestCaseWithFactory |
544 | 25 | from lp.testing.dbuser import switch_dbuser | 27 | from lp.testing.dbuser import switch_dbuser |
545 | @@ -41,21 +43,14 @@ | |||
546 | 41 | 43 | ||
547 | 42 | def getCount(self, person, **kw_find_expr): | 44 | def getCount(self, person, **kw_find_expr): |
548 | 43 | self._maybe_rollup() | 45 | self._maybe_rollup() |
561 | 44 | 46 | store = self.store | |
562 | 45 | public_summaries = self.store.find( | 47 | user_with, user_where = get_bugsummary_filter_for_user(person) |
563 | 46 | BugSummary, | 48 | if user_with: |
564 | 47 | BugSummary.viewed_by == None, | 49 | store = store.with_(user_with) |
565 | 48 | **kw_find_expr) | 50 | summaries = store.find(BugSummary, *user_where, **kw_find_expr) |
554 | 49 | private_summaries = self.store.find( | ||
555 | 50 | BugSummary, | ||
556 | 51 | BugSummary.viewed_by_id == TeamParticipation.teamID, | ||
557 | 52 | TeamParticipation.person == person, | ||
558 | 53 | **kw_find_expr) | ||
559 | 54 | all_summaries = public_summaries.union(private_summaries, all=True) | ||
560 | 55 | |||
566 | 56 | # Note that if there a 0 records found, sum() returns None, but | 51 | # Note that if there a 0 records found, sum() returns None, but |
567 | 57 | # we prefer to return 0 here. | 52 | # we prefer to return 0 here. |
569 | 58 | return all_summaries.sum(BugSummary.count) or 0 | 53 | return summaries.sum(BugSummary.count) or 0 |
570 | 59 | 54 | ||
571 | 60 | def assertCount(self, count, user=None, **kw_find_expr): | 55 | def assertCount(self, count, user=None, **kw_find_expr): |
572 | 61 | self.assertEqual(count, self.getCount(user, **kw_find_expr)) | 56 | self.assertEqual(count, self.getCount(user, **kw_find_expr)) |
Thank you.