Code review comment for lp:~jcsackett/launchpad/unknown-blueprints-service-597738
- unknown-blueprints-service-597738
- Merge into devel
Revision history for this message
j.c.sackett (jcsackett) wrote : | # |
1 | === modified file 'lib/lp/blueprints/browser/specificationtarget.py' |
2 | --- lib/lp/blueprints/browser/specificationtarget.py 2010-09-14 15:50:16 +0000 |
3 | +++ lib/lp/blueprints/browser/specificationtarget.py 2010-09-14 19:02:37 +0000 |
4 | @@ -138,6 +138,7 @@ |
5 | is_project = False |
6 | is_series = False |
7 | is_sprint = False |
8 | + has_wiki = False |
9 | has_drivers = False |
10 | |
11 | # Templates for the various conditions of blueprints: |
12 | @@ -153,9 +154,9 @@ |
13 | @property |
14 | def template(self): |
15 | # Check for the magical "index" added by the browser:page template |
16 | - # machinery. If it exists this is actually the |
17 | + # machinery. If it exists this is actually the |
18 | # zope.app.pagetemplate.simpleviewclass.simple class that is magically |
19 | - # mixed in by the browser:page zcml directive the template defined in |
20 | + # mixed in by the browser:page zcml directive the template defined in |
21 | # the directive should be used. |
22 | if safe_hasattr(self, 'index'): |
23 | return super(HasSpecificationsView, self).template |
24 | @@ -167,11 +168,15 @@ |
25 | return self.default_template |
26 | |
27 | # ProjectGroups are a special case, as their products may be a |
28 | - # combination of usage settings. Use the default, since it handles |
29 | - # fetching anything that's set for ProjectGroups, and it's not correct |
30 | - # to say anything about the ProjectGroup's usage. |
31 | + # combination of usage settings. To deal with this, check all |
32 | + # products, and if a product has LAUNCHPAD for the enum, the |
33 | + # group does; if not, assume external. |
34 | if IProjectGroup.providedBy(self.context): |
35 | - return self.default_template |
36 | + for product in self.context.products: |
37 | + if service_uses_launchpad(product.blueprints_usage): |
38 | + return self.default_template |
39 | + else: |
40 | + return self.not_launchpad_template |
41 | |
42 | # Otherwise, determine usage and provide the correct template. |
43 | service_usage = IServiceUsage(self.context) |
44 | @@ -188,14 +193,19 @@ |
45 | def initialize(self): |
46 | if IPerson.providedBy(self.context): |
47 | self.is_person = True |
48 | - elif (IDistribution.providedBy(self.context) or |
49 | - IProduct.providedBy(self.context)): |
50 | - self.is_target = True |
51 | - self.is_pillar = True |
52 | + elif IDistribution.providedBy(self.context): |
53 | + self.is_target = True |
54 | + self.is_pillar = True |
55 | + self.show_series = True |
56 | + elif IProduct.providedBy(self.context): |
57 | + self.is_target = True |
58 | + self.is_pillar = True |
59 | + self.has_wiki = True |
60 | self.show_series = True |
61 | elif IProjectGroup.providedBy(self.context): |
62 | self.is_project = True |
63 | self.is_pillar = True |
64 | + self.has_wiki = True |
65 | self.show_target = True |
66 | self.show_series = True |
67 | elif IProjectGroupSeries.providedBy(self.context): |
68 | @@ -221,7 +231,8 @@ |
69 | |
70 | @property |
71 | def can_configure_blueprints(self): |
72 | - """Can the user configure blueprints for the `ISpecificationTarget`.""" |
73 | + """Can the user configure blueprints for the `ISpecificationTarget`. |
74 | + """ |
75 | target = self.context |
76 | if IProduct.providedBy(target) or IDistribution.providedBy(target): |
77 | return check_permission('launchpad.Edit', self.context) |
78 | |
79 | === modified file 'lib/lp/blueprints/browser/tests/test_specificationtarget.py' |
80 | --- lib/lp/blueprints/browser/tests/test_specificationtarget.py 2010-09-14 15:50:16 +0000 |
81 | +++ lib/lp/blueprints/browser/tests/test_specificationtarget.py 2010-09-14 19:01:23 +0000 |
82 | @@ -13,6 +13,7 @@ |
83 | ISpecificationTarget, |
84 | ) |
85 | from lp.app.enums import ServiceUsage |
86 | +from lp.blueprints.browser.specificationtarget import HasSpecificationsView |
87 | from lp.blueprints.publisher import BlueprintsLayer |
88 | from lp.testing import ( |
89 | login_person, |
90 | @@ -103,53 +104,65 @@ |
91 | def setUp(self): |
92 | super(TestHasSpecificationsTemplates, self).setUp() |
93 | self.user = self.factory.makePerson() |
94 | - self.product = self.factory.makeProduct() |
95 | - self.naked_product = removeSecurityProxy(self.product) |
96 | login_person(self.user) |
97 | |
98 | - def test_not_configured(self): |
99 | - self.naked_product.blueprints_usage = ServiceUsage.UNKNOWN |
100 | - view = create_view( |
101 | - self.product, |
102 | - '+specs', |
103 | - layer=BlueprintsLayer, |
104 | - principal=self.user) |
105 | - self.assertEqual( |
106 | - view.not_launchpad_template.filename, |
107 | - view.template.filename) |
108 | - |
109 | - def test_external(self): |
110 | - self.naked_product.blueprints_usage = ServiceUsage.EXTERNAL |
111 | - view = create_view( |
112 | - self.product, |
113 | - '+specs', |
114 | - layer=BlueprintsLayer, |
115 | - principal=self.user) |
116 | - self.assertEqual( |
117 | - view.not_launchpad_template.filename, |
118 | - view.template.filename) |
119 | - |
120 | - def test_not_applicable(self): |
121 | - self.naked_product.blueprints_usage = ServiceUsage.NOT_APPLICABLE |
122 | - view = create_view( |
123 | - self.product, |
124 | - '+specs', |
125 | - layer=BlueprintsLayer, |
126 | - principal=self.user) |
127 | - self.assertEqual( |
128 | - view.not_launchpad_template.filename, |
129 | - view.template.filename) |
130 | - |
131 | - def test_on_launchpad(self): |
132 | - self.naked_product.blueprints_usage = ServiceUsage.LAUNCHPAD |
133 | - view = create_view( |
134 | - self.product, |
135 | - '+specs', |
136 | - layer=BlueprintsLayer, |
137 | - principal=self.user) |
138 | - self.assertEqual( |
139 | - view.default_template.filename, |
140 | - view.template.filename) |
141 | + def _test_templates_for_configuration(self, target, context=None): |
142 | + if context is None: |
143 | + context = target |
144 | + naked_target = removeSecurityProxy(target) |
145 | + test_configurations = [ |
146 | + ServiceUsage.UNKNOWN, |
147 | + ServiceUsage.EXTERNAL, |
148 | + ServiceUsage.NOT_APPLICABLE, |
149 | + ServiceUsage.LAUNCHPAD, |
150 | + ] |
151 | + correct_templates = [ |
152 | + HasSpecificationsView.not_launchpad_template.filename, |
153 | + HasSpecificationsView.not_launchpad_template.filename, |
154 | + HasSpecificationsView.not_launchpad_template.filename, |
155 | + HasSpecificationsView.default_template.filename, |
156 | + ] |
157 | + used_templates = list() |
158 | + for config in test_configurations: |
159 | + naked_target.blueprints_usage = config |
160 | + view = create_view( |
161 | + context, |
162 | + '+specs', |
163 | + layer=BlueprintsLayer, |
164 | + principal=self.user) |
165 | + used_templates.append(view.template.filename) |
166 | + self.assertEqual(correct_templates, used_templates) |
167 | + |
168 | + def test_product(self): |
169 | + product = self.factory.makeProduct() |
170 | + self._test_templates_for_configuration(product) |
171 | + |
172 | + def test_product_series(self): |
173 | + product = self.factory.makeProduct() |
174 | + product_series = self.factory.makeProductSeries(product=product) |
175 | + self._test_templates_for_configuration( |
176 | + target=product, |
177 | + context=product_series) |
178 | + |
179 | + def test_distribution(self): |
180 | + distribution = self.factory.makeDistribution() |
181 | + self._test_templates_for_configuration(distribution) |
182 | + |
183 | + def test_distroseries(self): |
184 | + distribution = self.factory.makeDistribution() |
185 | + distro_series = self.factory.makeDistroSeries( |
186 | + distribution=distribution) |
187 | + self._test_templates_for_configuration( |
188 | + target=distribution, |
189 | + context=distro_series) |
190 | + |
191 | + def test_projectgroup(self): |
192 | + project = self.factory.makeProject() |
193 | + product1 = self.factory.makeProduct(project=project) |
194 | + product2 = self.factory.makeProduct(project=project) |
195 | + self._test_templates_for_configuration( |
196 | + target=product1, |
197 | + context=project) |
198 | |
199 | |
200 | class TestHasSpecificationsConfiguration(TestCaseWithFactory): |
201 | @@ -167,7 +180,7 @@ |
202 | view = create_initialized_view(product, '+specs') |
203 | self.assertEqual(True, view.can_configure_blueprints) |
204 | |
205 | - def test_cannot_configure_blueprints_distribution_no_edit_permission(self): |
206 | + def test_cant_configure_blueprints_distribution_no_edit_permission(self): |
207 | distribution = self.factory.makeDistribution() |
208 | view = create_initialized_view(distribution, '+specs') |
209 | self.assertEqual(False, view.can_configure_blueprints) |
210 | @@ -178,12 +191,13 @@ |
211 | view = create_initialized_view(distribution, '+specs') |
212 | self.assertEqual(True, view.can_configure_blueprints) |
213 | |
214 | - def test_cannot_configure_blueprints_projectgroup_with_edit_permission(self): |
215 | + def test_cannot_configure_blueprints_projectgroup(self): |
216 | project_group = self.factory.makeProject() |
217 | login_person(project_group.owner) |
218 | view = create_initialized_view(project_group, '+specs') |
219 | self.assertEqual(False, view.can_configure_blueprints) |
220 | |
221 | + |
222 | def test_suite(): |
223 | suite = unittest.TestSuite() |
224 | suite.addTest(unittest.TestLoader().loadTestsFromName(__name__)) |
225 | |
226 | === modified file 'lib/lp/blueprints/templates/unknown-specs.pt' |
227 | --- lib/lp/blueprints/templates/unknown-specs.pt 2010-09-14 16:03:51 +0000 |
228 | +++ lib/lp/blueprints/templates/unknown-specs.pt 2010-09-14 19:07:36 +0000 |
229 | @@ -9,29 +9,38 @@ |
230 | |
231 | <body> |
232 | |
233 | -<div metal:fill-slot="main" |
234 | - tal:define="specs view/specs; |
235 | - has_any_specs view/has_any_specifications; |
236 | - blueprints_usage view/context/blueprints_usage"> |
237 | +<div metal:fill-slot="main"> |
238 | <div class="top-portlet"> |
239 | <div id="specs-unknown"> |
240 | <strong> |
241 | - <p tal:condition="blueprints_usage/enumvalue:EXTERNAL"> |
242 | - <tal:project replace="view/context/displayname" /> does not use Launchpad |
243 | - for planning or documentation. |
244 | - </p> |
245 | - <p tal:condition="blueprints_usage/enumvalue:NOT_APPLICABLE"> |
246 | - <tal:project replace="view/context/displayname" /> does not track |
247 | - use feature planning or documentation. |
248 | - </p> |
249 | - <p tal:condition="blueprints_usage/enumvalue:UNKNOWN"> |
250 | - Launchpad does not know how |
251 | - <tal:project replace="view/context/displayname" /> tracks feature |
252 | - planning or documentation. |
253 | - </p> |
254 | + <div tal:omit-tag tal:condition="view/is_project"> |
255 | + <p> |
256 | + Launchpad does not know how |
257 | + <tal:project replace="view/context/displayname" /> tracks feature |
258 | + planning or documentation. |
259 | + </p> |
260 | + </div> |
261 | + <div tal:omit-tag tal:condition="not:view/is_project"> |
262 | + <div tal:omit-tag tal:define="blueprints_usage view/context/blueprints_usage"> |
263 | + <p tal:condition="blueprints_usage/enumvalue:EXTERNAL"> |
264 | + <tal:project replace="view/context/displayname" /> does not use Launchpad |
265 | + for planning or documentation. |
266 | + </p> |
267 | + <p tal:condition="blueprints_usage/enumvalue:NOT_APPLICABLE"> |
268 | + <tal:project replace="view/context/displayname" /> does not track |
269 | + use feature planning or documentation. |
270 | + </p> |
271 | + <p tal:condition="blueprints_usage/enumvalue:UNKNOWN"> |
272 | + Launchpad does not know how |
273 | + <tal:project replace="view/context/displayname" /> tracks feature |
274 | + planning or documentation. |
275 | + </p> |
276 | + </div> |
277 | + </div> |
278 | </strong> |
279 | |
280 | - <p id="wiki-fallback" |
281 | + <div tal:omit-tag tal:condition="view/has_wiki"> |
282 | + <p id="wiki-fallback" |
283 | tal:define="wiki view/context/wikiurl" |
284 | tal:condition="wiki"> |
285 | <tal:project replace="view/context/displayname" /> has a wiki, which |
286 | @@ -40,8 +49,9 @@ |
287 | <tal:project replace="view/context/displayname" /> wiki |
288 | </a> |
289 | </p> |
290 | + </div> |
291 | </div> |
292 | - |
293 | + |
294 | <p id="configure-support" |
295 | tal:condition="view/can_configure_blueprints"> |
296 | Launchpad Blueprints allow your project to track feature planning and |
297 | |
298 | === modified file 'lib/lp/registry/browser/distribution.py' |
299 | --- lib/lp/registry/browser/distribution.py 2010-09-13 12:09:30 +0000 |
300 | +++ lib/lp/registry/browser/distribution.py 2010-09-14 19:01:42 +0000 |
301 | @@ -342,6 +342,7 @@ |
302 | 'announcements', |
303 | 'ppas', |
304 | 'configure_answers', |
305 | + 'configure_blueprints', |
306 | ] |
307 | |
308 | @enabled_with_permission('launchpad.Edit') |
309 | @@ -453,6 +454,12 @@ |
310 | summary = 'Allow users to ask questions on this project' |
311 | return Link('+edit', text, summary, icon='edit') |
312 | |
313 | + @enabled_with_permission('launchpad.Edit') |
314 | + def configure_blueprints(self): |
315 | + text = 'Configure blueprints' |
316 | + summary = 'Enable tracking of specifications and meetings' |
317 | + return Link('+edit', text, summary, icon='edit') |
318 | + |
319 | |
320 | class DerivativeDistributionOverviewMenu(DistributionOverviewMenu): |
321 |
> My other concern is about what has specifications--a SpecificationTa rget. I
> reviewed the bugs branch a few days ago and saw only IProduct was supports
> well.
Valid; I've done some digging and have some explanations below.
> A BugTarget may be a product, projectgroup, distribution, and rcepackage. I think these are also SpediciationTar gets, so
> distributionsou
> everyone needs to visit an example of each to ensure the message is correct
> and the actions that can be takes are presented. eg, a project group's use of
> an app is implicit in the settings of its products.
So, I think products are now handled right.
Project groups are tricky--right now I have it defaulting to the old style, so that any blueprints made for any project will be shown. Given how the earlier question of whether or not to continue showing blueprints if has_any_ specifications was True, I think it is appropriate to only turn this on if at least one product has LAUNCHPAD as its usage setting.
Distributions were mostly handled right; the issue with say, Kubuntu was the configure_ blueprints link, which I fixed.
DSPs aren't valid for blueprints; it's always NOT_APPLICABLE and the links are all disabled. You can't even get to it by going to blueprints. launchpad. net for the DSP in question; it takes you to the overview. (see https:/ /blueprints. edge.launchpad. net/ubuntu/ +source/ firefox or https:/ /blueprints. launchpad. dev/ubuntu/ +source/ mozilla- firefox)
Attached is a diff of this round of changes.