Code review comment for lp:~jcsackett/launchpad/unknown-blueprints-service-597738

Revision history for this message
j.c.sackett (jcsackett) wrote :

> My other concern is about what has specifications--a SpecificationTarget. 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
> distributionsourcepackage. I think these are also SpediciationTargets, so
> 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.

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

« Back to merge proposal