launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #11818
[Merge] lp:~abentley/launchpad/spec-creation-info-type into lp:launchpad
Aaron Bentley has proposed merging lp:~abentley/launchpad/spec-creation-info-type into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~abentley/launchpad/spec-creation-info-type/+merge/123821
= Summary =
Allow selecting information type when creating Specification
== Proposed fix ==
When creating a specification for an unknown target, allow any value. (Validation will catch places where an inappropriate value is used.)
When creating a specification for a known target, use the getAlloweedSpecificationInformationTypes for that target.
If only one value is allowed, do not show the choice.
== Pre-implementation notes ==
Discussed hiding when no choice is available with rick_h
== LOC Rationale ==
Part of Private Projects
== Implementation details ==
No changes are visible until blueprints.information_type.enabled is true.
DistroSeries and ProductSeries now explicitly delegate ISpecificationTarget to
distro and product, to simplify implementation.
Distribution always returns PUBLIC as the sole allowed InformationType.
Product always returns the PUBLIC_PROPRIETARY set as the allowed InformationTypes, but this is a stub until Product policies for information types have landed.
As a driveby, removed ProductEditView.private, since Product now provides this directly.
== Tests ==
bin/test specification
== Demo and Q/A ==
Create a specification
- From the root page
- From a sprint (meeting) page
- From a product page
- From a productseries page
- From a distribution page
- From a distroseries pagea
You should not be prompted for an information_type.
Enable blueprints.information_type.enabled
Create a specification
- From the root page
- From a sprint (meeting) page
- From a product page
- From a productseries page
You should be prompted for an information type. The values provided should be "Public", "Proprietary", "Embargoed".
Create a specification
- From a distribution page
- From a distroseries pagea
You should not be prompted for an information_type.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/registry/model/productseries.py
lib/lp/blueprints/interfaces/specificationtarget.py
lib/lp/blueprints/model/specification.py
lib/lp/registry/model/product.py
lib/lp/blueprints/browser/specification.py
lib/lp/registry/model/distroseries.py
lib/lp/blueprints/browser/tests/test_specification.py
lib/lp/registry/model/distribution.py
lib/lp/blueprints/interfaces/specification.py
--
https://code.launchpad.net/~abentley/launchpad/spec-creation-info-type/+merge/123821
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/spec-creation-info-type into lp:launchpad.
=== modified file 'lib/lp/blueprints/browser/specification.py'
--- lib/lp/blueprints/browser/specification.py 2012-09-04 20:24:29 +0000
+++ lib/lp/blueprints/browser/specification.py 2012-09-11 19:43:18 +0000
@@ -116,11 +116,14 @@
from lp.blueprints.interfaces.specificationbranch import ISpecificationBranch
from lp.blueprints.interfaces.sprintspecification import ISprintSpecification
from lp.code.interfaces.branchnamespace import IBranchNamespaceSet
-from lp.registry.enums import PRIVATE_INFORMATION_TYPES
+from lp.registry.enums import (
+ PUBLIC_PROPRIETARY_INFORMATION_TYPES,
+ )
from lp.registry.interfaces.distribution import IDistribution
from lp.registry.interfaces.product import IProduct
from lp.registry.vocabularies import InformationTypeVocabulary
from lp.services.config import config
+from lp.services.features import getFeatureFlag
from lp.services.fields import WorkItemsText
from lp.services.propertycache import cachedproperty
from lp.services.webapp import (
@@ -139,6 +142,9 @@
)
+INFORMATION_TYPE_FLAG = 'blueprints.information_type.enabled'
+
+
class INewSpecification(Interface):
"""A schema for a new specification."""
@@ -207,8 +213,11 @@
page_title = 'Register a blueprint in Launchpad'
label = "Register a new blueprint"
+
custom_widget('specurl', TextWidget, displayWidth=60)
+ custom_widget('information_type', LaunchpadRadioWidgetWithDescription)
+
@action(_('Register Blueprint'), name='register')
def register(self, action, data):
"""Registers a new specification."""
@@ -224,7 +233,8 @@
assignee=data.get('assignee'),
approver=data.get('approver'),
distribution=data.get('distribution'),
- definition_status=data.get('definition_status'))
+ definition_status=data.get('definition_status'),
+ information_type=data.get('information_type'))
# Propose the specification as a series goal, if specified.
series = data.get('series')
if series is not None:
@@ -274,8 +284,27 @@
The context must correspond to a unique specification target.
"""
- schema = Fields(INewSpecification,
- INewSpecificationSprint)
+ @property
+ def info_type_field(self):
+ """An info_type_field for creating a Specification.
+
+ None if the user cannot select different information types or the
+ feature flag is not enabled.
+ """
+ if not getFeatureFlag(INFORMATION_TYPE_FLAG):
+ return None
+ info_types = self.context.getAllowedSpecificationInformationTypes()
+ if len(info_types) < 2:
+ return None
+ return copy_field(ISpecification['information_type'], readonly=False,
+ vocabulary=InformationTypeVocabulary(types=info_types))
+
+ @property
+ def schema(self):
+ fields = Fields(INewSpecification, INewSpecificationSprint)
+ if self.info_type_field is not None:
+ fields = fields + Fields(self.info_type_field)
+ return fields
class NewSpecificationFromDistributionView(NewSpecificationFromTargetView):
@@ -295,9 +324,14 @@
class NewSpecificationFromSeriesView(NewSpecificationFromTargetView):
"""An abstract view for creating a specification from a series."""
- schema = Fields(INewSpecification,
- INewSpecificationSprint,
- INewSpecificationSeriesGoal)
+ @property
+ def schema(self):
+ fields = Fields(INewSpecification,
+ INewSpecificationSprint,
+ INewSpecificationSeriesGoal)
+ if self.info_type_field is not None:
+ fields = fields + Fields(self.info_type_field)
+ return fields
def transform(self, data):
if data['goal']:
@@ -320,13 +354,17 @@
data['product'] = self.context.product
+all_info_type_field = copy_field(ISpecification['information_type'],
+ readonly=False, vocabulary=InformationTypeVocabulary(
+ types=PUBLIC_PROPRIETARY_INFORMATION_TYPES))
+
+
class NewSpecificationFromNonTargetView(NewSpecificationView):
"""An abstract view for creating a specification outside a target context.
The context may not correspond to a unique specification target. Hence
sub-classes must define a schema requiring the user to specify a target.
"""
-
def transform(self, data):
data['distribution'] = IDistribution(data['target'], None)
data['product'] = IProduct(data['target'], None)
@@ -349,22 +387,32 @@
schema = Fields(INewSpecificationProjectTarget,
INewSpecification,
- INewSpecificationSprint)
+ INewSpecificationSprint,
+ all_info_type_field,)
class NewSpecificationFromRootView(NewSpecificationFromNonTargetView):
"""A view for creating a specification from the root of Launchpad."""
- schema = Fields(INewSpecificationTarget,
- INewSpecification,
- INewSpecificationSprint)
+ @property
+ def schema(self):
+ fields = Fields(INewSpecificationTarget,
+ INewSpecification,
+ INewSpecificationSprint)
+ if getFeatureFlag(INFORMATION_TYPE_FLAG):
+ fields = fields + Fields(all_info_type_field)
+ return fields
class NewSpecificationFromSprintView(NewSpecificationFromNonTargetView):
"""A view for creating a specification from a sprint."""
- schema = Fields(INewSpecificationTarget,
- INewSpecification)
+ @property
+ def schema(self):
+ fields = Fields(INewSpecificationTarget, INewSpecification)
+ if getFeatureFlag(INFORMATION_TYPE_FLAG):
+ fields = fields + Fields(all_info_type_field)
+ return fields
def transform(self, data):
super(NewSpecificationFromSprintView, self).transform(data)
@@ -535,6 +583,7 @@
text = 'Link a related branch'
return Link('+linkbranch', text, icon='add')
+ @enabled_with_permission('launchpad.Edit')
def information_type(self):
"""Return the 'Set privacy/security' Link."""
text = 'Change privacy/security'
@@ -559,15 +608,11 @@
@cachedproperty
def privacy_portlet_css(self):
- if self.private:
+ if self.context.private:
return 'portlet private'
else:
return 'portlet public'
- @cachedproperty
- def private(self):
- return self.context.information_type in PRIVATE_INFORMATION_TYPES
-
class SpecificationView(SpecificationSimpleView):
"""Used to render the main view of a specification."""
=== modified file 'lib/lp/blueprints/browser/tests/test_specification.py'
--- lib/lp/blueprints/browser/tests/test_specification.py 2012-09-07 08:07:48 +0000
+++ lib/lp/blueprints/browser/tests/test_specification.py 2012-09-11 19:43:18 +0000
@@ -23,6 +23,7 @@
from lp.app.browser.tales import format_link
from lp.blueprints.browser import specification
+from lp.blueprints.browser.specification import INFORMATION_TYPE_FLAG
from lp.blueprints.enums import SpecificationImplementationStatus
from lp.blueprints.interfaces.specification import (
ISpecification,
@@ -175,6 +176,12 @@
"... Registered by Some Person ... ago ..."))
+def set_blueprint_information_type(test_case, enabled):
+ value = 'true' if enabled else ''
+ fixture = FeatureFixture({INFORMATION_TYPE_FLAG: value})
+ test_case.useFixture(fixture)
+
+
class TestSpecificationInformationType(BrowserTestCase):
layer = DatabaseFunctionalLayer
@@ -184,8 +191,7 @@
def setUp(self):
super(TestSpecificationInformationType, self).setUp()
- self.useFixture(FeatureFixture({'blueprints.information_type.enabled':
- 'true'}))
+ set_blueprint_information_type(self, True)
def assertBrowserMatches(self, matcher):
browser = self.getViewBrowser(self.factory.makeSpecification())
@@ -195,8 +201,7 @@
self.assertBrowserMatches(soupmatchers.HTMLContains(self.portlet_tag))
def test_privacy_portlet_requires_flag(self):
- self.useFixture(FeatureFixture({'blueprints.information_type.enabled':
- ''}))
+ set_blueprint_information_type(self, False)
self.assertBrowserMatches(
Not(soupmatchers.HTMLContains(self.portlet_tag)))
@@ -256,6 +261,114 @@
self.assertEqual(InformationType.PUBLIC, spec.information_type)
+# canonical_url erroneously returns http://blueprints.launchpad.dev/+new
+NEW_SPEC_FROM_ROOT_URL = 'http://blueprints.launchpad.dev/specs/+new'
+
+
+class TestNewSpecificationInformationType(BrowserTestCase):
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestNewSpecificationInformationType, self).setUp()
+ set_blueprint_information_type(self, True)
+ it_field = soupmatchers.Tag('it-field', True,
+ attrs=dict(name='field.information_type'))
+ self.match_it = soupmatchers.HTMLContains(it_field)
+
+ def test_from_root(self):
+ """Information_type is included creating from root."""
+ browser = self.getUserBrowser(NEW_SPEC_FROM_ROOT_URL)
+ self.assertThat(browser.contents, self.match_it)
+
+ def test_from_root_no_flag(self):
+ """Information_type is excluded with no flag."""
+ set_blueprint_information_type(self, False)
+ browser = self.getUserBrowser(NEW_SPEC_FROM_ROOT_URL)
+ self.assertThat(browser.contents, Not(self.match_it))
+
+ def test_from_sprint(self):
+ """Information_type is included creating from a sprint."""
+ sprint = self.factory.makeSprint()
+ browser = self.getViewBrowser(sprint, view_name='+addspec')
+ self.assertThat(browser.contents, self.match_it)
+
+ def test_from_sprint_no_flag(self):
+ """Information_type is excluded with no flag."""
+ set_blueprint_information_type(self, False)
+ sprint = self.factory.makeSprint()
+ browser = self.getViewBrowser(sprint, view_name='+addspec')
+ self.assertThat(browser.contents, Not(self.match_it))
+
+ def submitSpec(self, browser):
+ """Submit a Specification via a browser."""
+ name = self.factory.getUniqueString()
+ browser.getControl('Name').value = name
+ browser.getControl('Title').value = self.factory.getUniqueString()
+ browser.getControl('Summary').value = self.factory.getUniqueString()
+ browser.getControl('Register Blueprint').click()
+ return name
+
+ def createSpec(self, information_type):
+ """Create a specification via a browser."""
+ with person_logged_in(self.user):
+ product = self.factory.makeProduct()
+ browser = self.getViewBrowser(product, view_name='+addspec')
+ control = browser.getControl(information_type.title)
+ if not control.selected:
+ control.click()
+ return product.getSpecification(self.submitSpec(browser))
+
+ def test_from_product(self):
+ """Creating from a product defaults to PUBLIC."""
+ product = self.factory.makeProduct()
+ browser = self.getViewBrowser(product, view_name='+addspec')
+ self.assertThat(browser.contents, self.match_it)
+ spec = product.getSpecification(self.submitSpec(browser))
+ self.assertEqual(spec.information_type, InformationType.PUBLIC)
+
+ def test_supplied_information_types(self):
+ """Creating honours information types."""
+ spec = self.createSpec(InformationType.PUBLIC)
+ self.assertEqual(InformationType.PUBLIC, spec.information_type)
+ spec = self.createSpec(InformationType.PROPRIETARY)
+ self.assertEqual(InformationType.PROPRIETARY, spec.information_type)
+ spec = self.createSpec(InformationType.EMBARGOED)
+ self.assertEqual(InformationType.EMBARGOED, spec.information_type)
+
+ def test_from_product_no_flag(self):
+ """information_type is excluded with no flag."""
+ set_blueprint_information_type(self, False)
+ product = self.factory.makeProduct()
+ browser = self.getViewBrowser(product, view_name='+addspec')
+ self.assertThat(browser.contents, Not(self.match_it))
+
+ def test_from_productseries(self):
+ """Information_type is included creating from productseries."""
+ series = self.factory.makeProductSeries()
+ browser = self.getViewBrowser(series, view_name='+addspec')
+ self.assertThat(browser.contents, self.match_it)
+
+ def test_from_productseries_no_flag(self):
+ """information_type is excluded with no flag."""
+ set_blueprint_information_type(self, False)
+ series = self.factory.makeProductSeries()
+ browser = self.getViewBrowser(series, view_name='+addspec')
+ self.assertThat(browser.contents, Not(self.match_it))
+
+ def test_from_distribution(self):
+ """information_type is excluded creating from distro."""
+ distro = self.factory.makeDistribution()
+ browser = self.getViewBrowser(distro, view_name='+addspec')
+ self.assertThat(browser.contents, Not(self.match_it))
+
+ def test_from_distroseries(self):
+ """information_type is excluded creating from distroseries."""
+ series = self.factory.makeDistroSeries()
+ browser = self.getViewBrowser(series, view_name='+addspec')
+ self.assertThat(browser.contents, Not(self.match_it))
+
+
class TestSpecificationViewPrivateArtifacts(BrowserTestCase):
""" Tests that specifications with private team artifacts can be viewed.
=== modified file 'lib/lp/blueprints/interfaces/specification.py'
--- lib/lp/blueprints/interfaces/specification.py 2012-09-06 10:59:56 +0000
+++ lib/lp/blueprints/interfaces/specification.py 2012-09-11 19:43:18 +0000
@@ -566,10 +566,7 @@
"""
def getAllowedInformationTypes(who):
- """Get a list of acceptable `InformationType`s for this spec.
-
- The intersection of the affected pillars' allowed types is permitted.
- """
+ """Get a list of acceptable `InformationType`s for this spec."""
class ISpecificationEditRestricted(Interface):
=== modified file 'lib/lp/blueprints/interfaces/specificationtarget.py'
--- lib/lp/blueprints/interfaces/specificationtarget.py 2011-12-24 16:54:44 +0000
+++ lib/lp/blueprints/interfaces/specificationtarget.py 2012-09-11 19:43:18 +0000
@@ -110,6 +110,9 @@
or None.
"""
+ def getAllowedSpecificationInformationTypes():
+ """Get the InformationTypes for this targets' specifications."""
+
class ISpecificationGoal(ISpecificationTarget):
"""An interface for those things which can have specifications proposed
=== modified file 'lib/lp/blueprints/model/specification.py'
--- lib/lp/blueprints/model/specification.py 2012-09-10 08:30:42 +0000
+++ lib/lp/blueprints/model/specification.py 2012-09-11 19:43:18 +0000
@@ -70,7 +70,6 @@
InformationType,
PRIVATE_INFORMATION_TYPES,
PUBLIC_INFORMATION_TYPES,
- PUBLIC_PROPRIETARY_INFORMATION_TYPES,
)
from lp.registry.errors import CannotChangeInformationType
from lp.registry.interfaces.distribution import IDistribution
@@ -818,7 +817,8 @@
self.id, self.name, self.target.name)
def getAllowedInformationTypes(self, who):
- return set(PUBLIC_PROPRIETARY_INFORMATION_TYPES)
+ """See `ISpecification`."""
+ return self.target.getAllowedSpecificationInformationTypes()
def transitionToInformationType(self, information_type, who):
"""See ISpecification."""
=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py 2012-09-05 07:16:09 +0000
+++ lib/lp/registry/model/distribution.py 2012-09-11 19:43:18 +0000
@@ -975,6 +975,10 @@
"""See `ISpecificationTarget`."""
return Specification.selectOneBy(distribution=self, name=name)
+ def getAllowedSpecificationInformationTypes(self):
+ """See `ISpecificationTarget`."""
+ return (InformationType.PUBLIC,)
+
def searchQuestions(self, search_text=None,
status=QUESTION_STATUS_DEFAULT_SEARCH,
language=None, sort=None, owner=None,
=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py 2012-09-07 05:13:57 +0000
+++ lib/lp/registry/model/distroseries.py 2012-09-11 19:43:18 +0000
@@ -17,6 +17,7 @@
import logging
import apt_pkg
+from lazr.delegates import delegates
from sqlobject import (
BoolCol,
ForeignKey,
@@ -48,6 +49,7 @@
SpecificationImplementationStatus,
SpecificationSort,
)
+from lp.blueprints.interfaces.specificationtarget import ISpecificationTarget
from lp.blueprints.model.specification import (
HasSpecificationsMixin,
Specification,
@@ -201,6 +203,8 @@
IHasBuildRecords, IHasQueueItems, IServiceUsage,
ISeriesBugTarget)
+ delegates(ISpecificationTarget, 'distribution')
+
_table = 'DistroSeries'
_defaultOrder = ['distribution', 'version']
@@ -908,10 +912,6 @@
results = results.prejoin(['assignee', 'approver', 'drafter'])
return results
- def getSpecification(self, name):
- """See ISpecificationTarget."""
- return self.distribution.getSpecification(name)
-
def getDistroSeriesLanguage(self, language):
"""See `IDistroSeries`."""
return DistroSeriesLanguage.selectOneBy(
=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py 2012-09-06 00:01:38 +0000
+++ lib/lp/registry/model/product.py 2012-09-11 19:43:18 +0000
@@ -122,6 +122,7 @@
FREE_INFORMATION_TYPES,
InformationType,
PRIVATE_INFORMATION_TYPES,
+ PUBLIC_PROPRIETARY_INFORMATION_TYPES,
)
from lp.registry.errors import CommercialSubscribersOnly
from lp.registry.interfaces.accesspolicy import (
@@ -604,6 +605,10 @@
else:
return InformationType.PUBLIC
+ def getAllowedSpecificationInformationTypes(self):
+ """See `ISpecificationTarget`."""
+ return PUBLIC_PROPRIETARY_INFORMATION_TYPES
+
def _ensurePolicies(self, information_types):
# Ensure that the product has access policies for the specified
# information types.
=== modified file 'lib/lp/registry/model/productseries.py'
--- lib/lp/registry/model/productseries.py 2012-08-08 05:36:44 +0000
+++ lib/lp/registry/model/productseries.py 2012-09-11 19:43:18 +0000
@@ -14,6 +14,7 @@
import datetime
+from lazr.delegates import delegates
from sqlobject import (
ForeignKey,
SQLMultipleJoin,
@@ -45,6 +46,7 @@
SpecificationImplementationStatus,
SpecificationSort,
)
+from lp.blueprints.interfaces.specificationtarget import ISpecificationTarget
from lp.blueprints.model.specification import (
HasSpecificationsMixin,
Specification,
@@ -123,6 +125,8 @@
IBugSummaryDimension, IProductSeries, IServiceUsage,
ISeriesBugTarget)
+ delegates(ISpecificationTarget, 'product')
+
_table = 'ProductSeries'
product = ForeignKey(dbName='product', foreignKey='Product', notNull=True)
@@ -451,10 +455,6 @@
from lp.bugs.model.bugsummary import BugSummary
return BugSummary.productseries_id == self.id
- def getSpecification(self, name):
- """See ISpecificationTarget."""
- return self.product.getSpecification(name)
-
def getLatestRelease(self):
"""See `IProductRelease.`"""
try:
Follow ups
