Hi Jon, Thanks for working on this branch. I was unaware of the added complexity of the whole template/page/portlet mess. I like the work you've done here modulo the on-going discussion with Gary about a better approach to the template swapping. I'm marking the MP 'needs information' so I can track the change you come up with, but the branch is very close. You mentioned lint being confused by the dependency on another branch. Even so, please ensure these are not for real: ./lib/lp/blueprints/browser/specificationtarget.py 251: E301 expected 1 blank line, found 0 254: E301 expected 1 blank line, found 0 ./lib/lp/blueprints/templates/unknown-specs.pt 38: Line has trailing whitespace. ./lib/lp/registry/doc/distribution.txt 582: source exceeds 78 characters. 584: source exceeds 78 characters. > === modified file 'lib/lp/blueprints/browser/specificationtarget.py' > --- lib/lp/blueprints/browser/specificationtarget.py 2010-08-24 10:45:57 +0000 > +++ lib/lp/blueprints/browser/specificationtarget.py 2010-09-10 14:48:00 +0000 > @@ -15,6 +15,7 @@ > > from operator import itemgetter > > +from z3c.ptcompat import ViewPageTemplateFile > from zope.component import queryMultiAdapter > > from canonical.config import config > @@ -32,6 +33,8 @@ > Link, > ) > from canonical.lazr.utils import smartquote > +from lp.app.enums import service_uses_launchpad > +from lp.app.interfaces.launchpad import IServiceUsage > from lp.blueprints.interfaces.specification import ( > SpecificationFilter, > SpecificationSort, > @@ -133,6 +136,50 @@ > is_sprint = False > has_drivers = False > > + # Templates for the various conditions of blueprints: > + # * On Launchpad > + # * External > + # * Disabled > + # * Unknown > + default_template = ViewPageTemplateFile( > + '../templates/hasspecifications-specs.pt') > + not_launchpad_template = ViewPageTemplateFile( > + '../templates/unknown-specs.pt') > + > + @property > + def template(self): > + # If the template has been defined in the zcml, use that, as this > + # isn't the base service page for blueprints. > + if hasattr(self, 'index'): As we discussed on IRC, you should use 'safe_hasattr' from canonical.lazr.utils. Also, as I suggested please check around and see if there is a more obvious technique for this. Sadly I cannot offer any other solution. > + return super(HasSpecificationsView, self).template > + > + # Sprints and Persons don't have a usage enum for blueprints, so we > + # have to fallback to the default. > + if (ISprint.providedBy(self.context) > + or IPerson.providedBy(self.context)): > + return self.default_template > + > + # ProjectGroups are a special case, as their products may be a > + # combination of usage settings. Use the default, since it handles > + # fetching anything that's set for ProjectGroups, and it's not correct > + # to say anything about the ProjectGroup's usage. > + if IProjectGroup.providedBy(self.context): > + return self.default_template > + > + # If specifications exist, ignore the usage enum. > + if self.has_any_specifications: > + return self.default_template If the enum is wrong, how will the user get here? The tab will be disabled, so we assume they have a direct URL? > + # Otherwise, determine usage and provide the correct template. > + service_usage = IServiceUsage(self.context) > + if service_uses_launchpad(service_usage.blueprints_usage): > + return self.default_template > + else: > + return self.not_launchpad_template > === modified file 'lib/lp/blueprints/browser/tests/test_specificationtarget.py' > --- lib/lp/blueprints/browser/tests/test_specificationtarget.py 2010-08-20 20:31:18 +0000 > +++ lib/lp/blueprints/browser/tests/test_specificationtarget.py 2010-09-10 14:48:00 +0000 > @@ -1,15 +1,18 @@ > -# Copyright 2009 Canonical Ltd. This software is licensed under the > +# Copyright 2009-2010 Canonical Ltd. This software is licensed under the > # GNU Affero General Public License version 3 (see the file LICENSE). > > __metaclass__ = type > > import unittest > > +from zope.security.proxy import removeSecurityProxy > + > from canonical.testing.layers import DatabaseFunctionalLayer > from lp.blueprints.interfaces.specificationtarget import ( > IHasSpecifications, > ISpecificationTarget, > ) > +from lp.app.enums import ServiceUsage > from lp.blueprints.publisher import BlueprintsLayer > from lp.testing import ( > login_person, > @@ -50,7 +53,7 @@ > self.verify_view(context, 'sprints/%s' % context.name) > > > -class TestHasSpecificationsView(TestCaseWithFactory): > +class TestHasSpecificationsViewInvolvement(TestCaseWithFactory): > """Test specification menus links.""" > layer = DatabaseFunctionalLayer > > @@ -68,6 +71,8 @@ > > def test_specificationtarget(self): > context = self.factory.makeProduct(name='almond') > + naked_product = removeSecurityProxy(context) > + naked_product.blueprints_usage = ServiceUsage.LAUNCHPAD > self.verify_involvment(context) You didn't introduce it, but please fix the typo of s/verify_involment/verify_involvement/ > > def test_adaptable_to_specificationtarget(self): > @@ -87,6 +92,63 @@ > '
' in view()) > > > +class TestHasSpecificationsTemplates(TestCaseWithFactory): > + """Tests the selection of templates based on blueprints usage.""" > + > + layer = DatabaseFunctionalLayer > + > + def setUp(self): > + super(TestHasSpecificationsTemplates, self).setUp() > + self.user = self.factory.makePerson() > + self.product = self.factory.makeProduct() > + self.naked_product = removeSecurityProxy(self.product) > + login_person(self.user) While I'm not a prude, I must ask why all of the nakedness here? Couldn't you just make self.user be the owner of the product and proceed fully clothed? > + > + def test_not_configured(self): > + self.naked_product.blueprints_usage = ServiceUsage.UNKNOWN > + view = create_view( > + self.product, > + '+specs', > + layer=BlueprintsLayer, > + principal=self.user) > + self.assertEqual( > + view.not_launchpad_template.filename, > + view.template.filename) > + > + def test_external(self): > + self.naked_product.blueprints_usage = ServiceUsage.EXTERNAL > + view = create_view( > + self.product, > + '+specs', > + layer=BlueprintsLayer, > + principal=self.user) > + self.assertEqual( > + view.not_launchpad_template.filename, > + view.template.filename) > + > + def test_not_applicable(self): > + self.naked_product.blueprints_usage = ServiceUsage.NOT_APPLICABLE > + view = create_view( > + self.product, > + '+specs', > + layer=BlueprintsLayer, > + principal=self.user) > + self.assertEqual( > + view.not_launchpad_template.filename, > + view.template.filename) > + > + def test_on_launchpad(self): > + self.naked_product.blueprints_usage = ServiceUsage.LAUNCHPAD > + view = create_view( > + self.product, > + '+specs', > + layer=BlueprintsLayer, > + principal=self.user) > + self.assertEqual( > + view.default_template.filename, > + view.template.filename) > + > + > def test_suite(): > suite = unittest.TestSuite() > suite.addTest(unittest.TestLoader().loadTestsFromName(__name__)) > === added file 'lib/lp/blueprints/templates/unknown-specs.pt' > --- lib/lp/blueprints/templates/unknown-specs.pt 1970-01-01 00:00:00 +0000 > +++ lib/lp/blueprints/templates/unknown-specs.pt 2010-09-10 14:48:00 +0000 > @@ -0,0 +1,59 @@ > + + xmlns="http://www.w3.org/1999/xhtml" > + xmlns:tal="http://xml.zope.org/namespaces/tal" > + xmlns:metal="http://xml.zope.org/namespaces/metal" > + xmlns:i18n="http://xml.zope.org/namespaces/i18n" > + metal:use-macro="view/macro:page/main_side" > + i18n:domain="launchpad" > +> > + > + > + > +
+ tal:define="specs view/specs; > + has_any_specs view/has_any_specifications"> > + > +
> + does not use Launchpad > + for specification tracking. > +
Why use 'specifications' just the once? The page says 'blueprints' six times and it is unclear that specifications and blueprints are the same from the context. Should we be more frank: tracks specifications somewhere besides Launchpad, but we don't know where. You should check with the project maintainers for the location. I'm just throwing that out for thought. I'm not sure I even like the idea. > + > +
> + does not track > + specifications. > +
> + > +
> + > + Launchpad does not know how > + uses specifications. > + > +
> + > +
> +

has a wiki, which > + may be used for specifications.

> + > +

> + > + wiki > + > +

> + > +
> + > + > +
> + > + Thanks for the additional drive-by fixes.