Merge lp:~abentley/launchpad/spec-creation-info-type into lp:launchpad

[Aaron Bentley]

Allow specifying information_type when creating Blueprint.

= 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

[Deryck Hodge (deryck)]

Looks great. Good test coverage. Thanks!

review: Approve

=== 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: