launchpad-reviewers team mailing list archive

[Deryck Hodge <deryck.hodge@xxxxxxxxxxxxx>]

Deryck Hodge has proposed merging lp:~deryck/launchpad/honor-default-for-spec-sharing-policy-1052521 into lp:launchpad.

Commit message:
Ensure that default value for information_type when creating new blueprints honors the default for the give specification sharing policy.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1052521 in Launchpad itself: "Specifications should honor sharing policy defaults when creating specifications"
  https://bugs.launchpad.net/launchpad/+bug/1052521

For more details, see:
https://code.launchpad.net/~deryck/launchpad/honor-default-for-spec-sharing-policy-1052521/+merge/125039

This branch fixes a bug where we were not selecting the correct information type by default when creating a new spec.  This is mostly a set of tests to ensure we get the behavior we want, plus some minor changes to the specification form view.

The first change in the browser code is to set an information_type from the default if no value is present in the form data.  Purely public or purely proprietary projects will not provide an option to set information_type, so this ensures we get the correct value when saving the form.  The other browser code fix is to set initial values with the default, so that this value is selected by default when the form is created.  It's just testing after that.
-- 
https://code.launchpad.net/~deryck/launchpad/honor-default-for-spec-sharing-policy-1052521/+merge/125039
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~deryck/launchpad/honor-default-for-spec-sharing-policy-1052521 into lp:launchpad.

=== modified file 'lib/lp/blueprints/browser/specification.py'
--- lib/lp/blueprints/browser/specification.py	2012-09-13 20:40:34 +0000
+++ lib/lp/blueprints/browser/specification.py	2012-09-19 19:15:28 +0000
@@ -120,7 +120,10 @@
     PUBLIC_PROPRIETARY_INFORMATION_TYPES,
     )
 from lp.registry.interfaces.distribution import IDistribution
-from lp.registry.interfaces.product import IProduct
+from lp.registry.interfaces.product import (
+    IProduct,
+    IProductSeries,
+    )
 from lp.registry.vocabularies import InformationTypeVocabulary
 from lp.services.config import config
 from lp.services.features import getFeatureFlag
@@ -222,6 +225,12 @@
     def register(self, action, data):
         """Registers a new specification."""
         self.transform(data)
+        information_type = data.get('information_type')
+        if information_type is None and (
+            IProduct.providedBy(self.context) or
+            IProductSeries.providedBy(self.context)):
+            information_type = (
+                self.context.getDefaultSpecificationInformationType())
         spec = getUtility(ISpecificationSet).new(
             owner=self.user,
             name=data.get('name'),
@@ -234,7 +243,7 @@
             approver=data.get('approver'),
             distribution=data.get('distribution'),
             definition_status=data.get('definition_status'),
-            information_type=data.get('information_type'))
+            information_type=information_type)
         # Propose the specification as a series goal, if specified.
         series = data.get('series')
         if series is not None:
@@ -277,6 +286,17 @@
         """
         return self._next_url
 
+    @property
+    def initial_values(self):
+        """Set initial values to honor sharing policy default value."""
+        information_type = None
+        if (IProduct.providedBy(self.context) or
+            IProductSeries.providedBy(self.context)):
+            information_type = (
+                self.context.getDefaultSpecificationInformationType())
+        values = {'information_type': information_type}
+        return values
+
 
 class NewSpecificationFromTargetView(NewSpecificationView):
     """An abstract view for creating a specification from a target context.

=== modified file 'lib/lp/blueprints/browser/tests/test_specification.py'
--- lib/lp/blueprints/browser/tests/test_specification.py	2012-09-16 12:52:04 +0000
+++ lib/lp/blueprints/browser/tests/test_specification.py	2012-09-19 19:15:28 +0000
@@ -36,6 +36,10 @@
     SpecificationSharingPolicy,
     )
 from lp.registry.interfaces.person import PersonVisibility
+from lp.registry.interfaces.product import (
+    IProduct,
+    IProductSeries,
+    )
 from lp.services.features.testing import FeatureFixture
 from lp.services.webapp.interfaces import BrowserNotificationLevel
 from lp.services.webapp.publisher import canonical_url
@@ -398,16 +402,6 @@
                 control.click()
             return product.getSpecification(self.submitSpec(browser))
 
-    def test_from_product(self):
-        """Creating from a product defaults to PUBLIC."""
-        product = self.factory.makeProduct(
-            specification_sharing_policy=
-                SpecificationSharingPolicy.PUBLIC_OR_PROPRIETARY)
-        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(
@@ -459,6 +453,128 @@
         self.assertThat(browser.contents, Not(self.match_it))
 
 
+class BaseNewSpecificationInformationTypeDefaultTest(BrowserTestCase):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(BaseNewSpecificationInformationTypeDefaultTest, 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 makeTarget(self, policy, owner=None):
+        raise NotImplementedError('makeTarget')
+
+    def ensurePolicy(self, target, information_type):
+        """Helper to call _ensurePolicies
+
+        Useful because we need to follow to product from
+        ProductSeries to get to _ensurePolicies.
+        """
+        if IProduct.providedBy(target):
+            removeSecurityProxy(target)._ensurePolicies(information_type)
+        elif IProductSeries.providedBy(target):
+            removeSecurityProxy(target.product)._ensurePolicies(
+                information_type)
+
+    def getSpecification(self, target, name):
+        """Helper to get the specification.
+
+        Useful because we need to follow to product from a
+        ProductSeries.
+        """
+        if IProductSeries.providedBy(target):
+            return target.product.getSpecification(name)
+        return target.getSpecification(name)
+
+    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 test_public(self):
+        """Creating from PUBLIC policy allows only PUBLIC."""
+        policy = SpecificationSharingPolicy.PUBLIC
+        target = self.makeTarget(policy)
+        browser = self.getViewBrowser(target, view_name='+addspec')
+        self.assertThat(browser.contents, Not(self.match_it))
+        spec = self.getSpecification(target, self.submitSpec(browser))
+        self.assertEqual(spec.information_type, InformationType.PUBLIC)
+
+    def test_public_or_proprietary(self):
+        """Creating from PUBLIC_OR_PROPRIETARY defaults to PUBLIC."""
+        policy = SpecificationSharingPolicy.PUBLIC_OR_PROPRIETARY
+        target = self.makeTarget(policy)
+        browser = self.getViewBrowser(target, view_name='+addspec')
+        self.assertThat(browser.contents, self.match_it)
+        spec = self.getSpecification(target, self.submitSpec(browser))
+        self.assertEqual(spec.information_type, InformationType.PUBLIC)
+
+    def test_proprietary_or_public(self):
+        """Creating from PROPRIETARY_OR_PUBLIC defaults to PROPRIETARY."""
+        policy = SpecificationSharingPolicy.PROPRIETARY_OR_PUBLIC
+        owner = self.factory.makePerson()
+        target = self.makeTarget(policy, owner=owner)
+        self.ensurePolicy(target, [InformationType.PROPRIETARY])
+        browser = self.getViewBrowser(
+            target, view_name='+addspec', user=owner)
+        self.assertThat(browser.contents, self.match_it)
+        spec = self.getSpecification(target, self.submitSpec(browser))
+        self.assertEqual(spec.information_type, InformationType.PROPRIETARY)
+
+    def test_proprietary(self):
+        """PROPRIETARY only allows proprietary when creating blueprints."""
+        policy = SpecificationSharingPolicy.PROPRIETARY
+        owner = self.factory.makePerson()
+        target = self.makeTarget(policy, owner=owner)
+        self.ensurePolicy(target, [InformationType.PROPRIETARY])
+        browser = self.getViewBrowser(
+            target, view_name='+addspec', user=owner)
+        self.assertThat(browser.contents, Not(self.match_it))
+        spec = self.getSpecification(target, self.submitSpec(browser))
+        self.assertEqual(spec.information_type, InformationType.PROPRIETARY)
+
+    def test_embargoed_or_proprietary(self):
+        """Creating from EMBARGOED_OR_PROPRIETARY defaults to embargoed."""
+        policy = SpecificationSharingPolicy.EMBARGOED_OR_PROPRIETARY
+        owner = self.factory.makePerson()
+        target = self.makeTarget(policy, owner=owner)
+        self.ensurePolicy(target, [InformationType.EMBARGOED])
+        browser = self.getViewBrowser(
+            target, view_name='+addspec', user=owner)
+        self.assertThat(browser.contents, self.match_it)
+        spec = self.getSpecification(target, self.submitSpec(browser))
+        self.assertEqual(spec.information_type, InformationType.EMBARGOED)
+
+
+
+class TestNewSpecificationDefaultInformationTypeProduct(
+    BaseNewSpecificationInformationTypeDefaultTest):
+
+    def makeTarget(self, policy, owner=None):
+        if owner is None:
+            owner = self.factory.makePerson()
+        return self.factory.makeProduct(
+            owner=owner, specification_sharing_policy=policy)
+
+
+class TestNewSpecificationDefaultInformationTypeProductSeries(
+    BaseNewSpecificationInformationTypeDefaultTest):
+
+    def makeTarget(self, policy, owner=None):
+        if owner is None:
+            owner = self.factory.makePerson()
+        product = self.factory.makeProduct(
+            owner=owner, specification_sharing_policy=policy)
+        return self.factory.makeProductSeries(product=product)
+
+
 class TestSpecificationViewPrivateArtifacts(BrowserTestCase):
     """ Tests that specifications with private team artifacts can be viewed.