Merge lp:~abentley/launchpad/transitive-confidential into lp:launchpad

[Aaron Bentley]

Forbid public artifacts on confidential products.

= Summary =
Fix bug #1079785: public artifacts are permitted on confidential projects

== Proposed fix ==
Move checks from view to model and raise exceptions. Add shared bug check.

== Pre-implementation notes ==
None

== LOC Rationale ==
Reduces LOC

== Implementation details ==
Revert 16258 and migrate code from getPublicWarning. Add check for shared bugs.

Remove getPublicWarning and related code and tests.

== Tests ==
bin/test -t test_change_info_type_proprietary_check_artifacts -t test_change_info_type_check_shared

== Demo and Q/A ==
Create a public product with other/proprietary license. Create a public branch, bug, blueprint. Create a private shared bug. You should not be able to change its information type to proprietary until you've made all artifacts private and removed the extra bugtask from the shared bug.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/browser/product.py
  lib/lp/registry/browser/tests/test_product.py
  lib/lp/registry/model/product.py
  lib/lp/registry/tests/test_product.py

[Richard Harding (rharding)]

review: Approve

[Aaron Bentley (abentley)]

Further discussion with Curtis reveals that only Proprietary and Embargoed artefacts should be permitted when transitioning product to proprietary/embargoed. See bug.

review: Needs Fixing

=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py	2012-11-15 16:03:56 +0000
+++ lib/lp/registry/browser/product.py	2012-11-23 14:31:20 +0000
@@ -105,7 +105,6 @@
     InformationType,
     PROPRIETARY_INFORMATION_TYPES,
     PUBLIC_PROPRIETARY_INFORMATION_TYPES,
-    PUBLIC_INFORMATION_TYPES,
     ServiceUsage,
     )
 from lp.app.errors import NotFoundError
@@ -129,9 +128,6 @@
 from lp.blueprints.browser.specificationtarget import (
     HasSpecificationsMenuMixin,
     )
-from lp.blueprints.enums import (
-    SpecificationFilter,
-    )
 from lp.bugs.browser.bugtask import (
     BugTargetTraversalMixin,
     get_buglisting_search_filter_url,
@@ -1450,47 +1446,10 @@
 
     @action("Change", name='change')
     def change_action(self, action, data):
-        old_info_type = self.context.information_type
         try:
             self.updateContextFromData(data)
         except CannotChangeInformationType as e:
             self.setFieldError('information_type', str(e))
-        warning = self.getPublicWarning(old_info_type)
-        if warning is not None:
-            self.request.response.addWarningNotification(warning)
-
-    def getPublicWarning(self, old_info_type):
-        """Get warning if public artifacts might expose the product.
-
-        :param old_info_type: The former information type of the product.
-        """
-        if (old_info_type != InformationType.PUBLIC or
-            self.context.information_type == InformationType.PUBLIC):
-            return None
-        public_bugs = self.context.searchTasks(None,
-            information_type=PUBLIC_INFORMATION_TYPES,
-            omit_duplicates=False, omit_targeted=False)
-        public_artifacts = []
-        if not public_bugs.is_empty():
-            public_artifacts.append('bugs')
-        # Default returns all public branches.
-        public_branches = self.context.getBranches()
-        if not public_branches.is_empty():
-            public_artifacts.append('branches')
-        # All specs located by an ALL search are public.
-        public_specs = self.context.specifications(
-            None, filter=[SpecificationFilter.ALL])
-        if not public_specs.is_empty():
-            public_artifacts.append('blueprints')
-        if len(public_artifacts) == 0:
-            return None
-        elif len(public_artifacts) < 3:
-            list_phrase = ' and '.join(public_artifacts)
-        else:
-            list_phrase = 'bugs, branches, and blueprints'
-        return ('Some %s are public.  This project will not be fully '
-                '%s until they have been changed.' %
-                (list_phrase, self.context.information_type.title))
 
 
 class ProductValidationMixin:

=== modified file 'lib/lp/registry/browser/tests/test_product.py'
--- lib/lp/registry/browser/tests/test_product.py	2012-11-15 16:03:56 +0000
+++ lib/lp/registry/browser/tests/test_product.py	2012-11-23 14:31:20 +0000
@@ -25,7 +25,6 @@
 from lp.app.enums import (
     InformationType,
     PROPRIETARY_INFORMATION_TYPES,
-    PUBLIC_PROPRIETARY_INFORMATION_TYPES,
     ServiceUsage,
     )
 from lp.registry.browser.product import (
@@ -553,118 +552,6 @@
             self.assertEqual(
                 InformationType.PUBLIC, updated_product.information_type)
 
-    def test_warning_on_public_artifacts(self):
-        # When changing to proprietary, any public artifacts will trigger a
-        # warning.
-        self.useFixture(FeatureFixture(
-            {u'disclosure.private_projects.enabled': u'on'}))
-        owner = self.factory.makePerson()
-        product = self.factory.makeProduct(owner=owner)
-        bug = self.factory.makeBug(target=product)
-
-        def make_proprietary():
-            with person_logged_in(None):
-                browser = self.getViewBrowser(product, '+edit',
-                    user=product.owner)
-                info_type = browser.getControl(name='field.information_type')
-                info_type.value = ['PROPRIETARY']
-                browser.getControl('Change').click()
-            with person_logged_in(owner):
-                product.information_type = InformationType.PUBLIC
-            return browser
-        browser = make_proprietary()
-
-        def assertWarning(browser, text):
-            warning_tag = Tag('warning', 'div', text=text,
-                              attrs={'class': 'warning message'})
-            self.assertThat(browser.contents, HTMLContains(warning_tag))
-        assertWarning(
-            browser, 'Some bugs are public.  This project will not be fully '
-            'Proprietary until they have been changed.')
-        browser = make_proprietary()
-        with person_logged_in(bug.owner):
-            bug.transitionToInformationType(InformationType.PROPRIETARY,
-                                            bug.owner)
-        browser = make_proprietary()
-        self.assertThat(browser.contents, Not(HTMLContains(
-            Tag('warning', 'div', attrs={'class': 'warning message'}))))
-
-    def test_getPublicWarning_artifacts(self):
-        # The warning indicates what kinds of artifacts are public, in
-        # grammatical English.
-        owner = self.factory.makePerson()
-        product = self.factory.makeProduct(owner=owner)
-        bug = self.factory.makeBug(target=product)
-        view = create_initialized_view(product, '+edit')
-        with person_logged_in(owner):
-            product.information_type = InformationType.PROPRIETARY
-        warning = view.getPublicWarning(InformationType.PUBLIC)
-        self.assertEqual(
-            'Some bugs are public.  This project will not be fully '
-            'Proprietary until they have been changed.', warning)
-        spec = self.factory.makeSpecification(product=product)
-        warning = view.getPublicWarning(InformationType.PUBLIC)
-        self.assertEqual(
-            'Some bugs and blueprints are public.  This project will not be '
-            'fully Proprietary until they have been changed.', warning)
-        branch = self.factory.makeBranch(product=product, owner=owner,
-                information_type=InformationType.PUBLIC)
-        warning = view.getPublicWarning(InformationType.PUBLIC)
-        self.assertEqual(
-            'Some bugs, branches, and blueprints are public.  This project '
-            'will not be fully Proprietary until they have been changed.',
-            warning)
-        with person_logged_in(owner):
-            bug.transitionToInformationType(
-                InformationType.PROPRIETARY, owner)
-        warning = view.getPublicWarning(InformationType.PUBLIC)
-        self.assertEqual(
-            'Some branches and blueprints are public.  This project '
-            'will not be fully Proprietary until they have been changed.',
-            warning)
-        with person_logged_in(owner):
-            branch.transitionToInformationType(
-                InformationType.PROPRIETARY, owner)
-        warning = view.getPublicWarning(InformationType.PUBLIC)
-        self.assertEqual(
-            'Some blueprints are public.  This project will not be fully '
-            'Proprietary until they have been changed.', warning)
-        with person_logged_in(owner):
-            spec.transitionToInformationType(
-                InformationType.PROPRIETARY, owner)
-        self.assertIs(None, view.getPublicWarning(InformationType.PUBLIC))
-
-    def test_getPublicWarning_embargoed(self):
-        # The message reflects an Embargoed product type.
-        owner = self.factory.makePerson()
-        product = self.factory.makeProduct(owner=owner)
-        self.factory.makeBug(target=product)
-        with person_logged_in(owner):
-            product.information_type = InformationType.EMBARGOED
-            view = create_initialized_view(product, '+edit')
-        warning = view.getPublicWarning(InformationType.PUBLIC)
-        self.assertEqual(
-            'Some bugs are public.  This project will not be fully '
-            'Embargoed until they have been changed.', warning)
-
-    def test_getPublicWarning_transition(self):
-        # The warning is emitted only when transitioning from PUBLIC to a
-        # proprietary type.
-        owner = self.factory.makePerson()
-        product = self.factory.makeProduct(owner=owner)
-        self.factory.makeBug(target=product)
-        view = create_initialized_view(product, '+edit')
-        for from_type in PUBLIC_PROPRIETARY_INFORMATION_TYPES:
-            for to_type in PUBLIC_PROPRIETARY_INFORMATION_TYPES:
-                with person_logged_in(owner):
-                    product.information_type = to_type
-                warning = view.getPublicWarning(from_type)
-                if (to_type in PROPRIETARY_INFORMATION_TYPES and
-                    from_type == InformationType.PUBLIC):
-                    self.assertIsNot(None, warning)
-                else:
-                    self.assertIs(None, warning)
-
 
 class ProductSetReviewLicensesViewTestCase(TestCaseWithFactory):
     """Tests the ProductSetReviewLicensesView."""

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2012-11-19 18:23:06 +0000
+++ lib/lp/registry/model/product.py	2012-11-23 14:31:20 +0000
@@ -111,6 +111,7 @@
     BUG_POLICY_DEFAULT_TYPES,
     )
 from lp.bugs.interfaces.bugtaskfilter import OrderedBugTask
+from lp.bugs.model.bug import Bug
 from lp.bugs.model.bugtarget import (
     BugTargetBase,
     OfficialBugTagTargetMixin,
@@ -460,27 +461,53 @@
         if value in PROPRIETARY_INFORMATION_TYPES:
             if self.answers_usage == ServiceUsage.LAUNCHPAD:
                 raise CannotChangeInformationType('Answers is enabled.')
+        if self._SO_creating or value not in PROPRIETARY_INFORMATION_TYPES:
+            return value
+        # Additional checks when transitioning an existing product to a
+        # proprietary type
+        # All specs located by an ALL search are public.
+        public_specs = self.specifications(
+            None, filter=[SpecificationFilter.ALL])
+        if not public_specs.is_empty():
+            # Unlike bugs and branches, specifications cannot be USERDATA or a
+            # security type.
+            raise CannotChangeInformationType(
+                'Some blueprints are public.')
+        non_proprietary_bugs = Store.of(self).find(Bug,
+            Not(Bug.information_type.is_in(PROPRIETARY_INFORMATION_TYPES)),
+            BugTask.bug == Bug.id, BugTask.product == self.id)
+        if not non_proprietary_bugs.is_empty():
+            raise CannotChangeInformationType(
+                'Some bugs are neither proprietary nor embargoed.')
+        # Default returns all public branches.
+        non_proprietary_branches = Store.of(self).find(
+            Branch, Branch.product == self.id,
+            Not(Branch.information_type.is_in(PROPRIETARY_INFORMATION_TYPES))
+        )
+        if not non_proprietary_branches.is_empty():
+            raise CannotChangeInformationType(
+                'Some branches are neither proprietary nor embargoed.')
+        if not self.packagings.is_empty():
+            raise CannotChangeInformationType('Some series are packaged.')
+        if self.translations_usage == ServiceUsage.LAUNCHPAD:
+            raise CannotChangeInformationType('Translations are enabled.')
         # Proprietary check works only after creation, because during
         # creation, has_commercial_subscription cannot give the right value
         # and triggers an inappropriate DB flush.
 
         # If you're changing the license, and setting a PROPRIETARY
-        # information type, yet you don't have a subscription you get one when
-        # the license is set.
+        # information type, yet you don't have a subscription, you get one
+        # when the license is set.
+
+        # Create the complimentary commercial subscription for the product.
+        self._ensure_complimentary_subscription()
 
         # If you have a commercial subscription, but it's not current, you
         # cannot set the information type to a PROPRIETARY type.
-        if not self._SO_creating and value in PROPRIETARY_INFORMATION_TYPES:
-            if not self.packagings.is_empty():
-                raise CannotChangeInformationType('Some series are packaged.')
-            # Create the complimentary commercial subscription for the product.
-            self._ensure_complimentary_subscription()
-
-            if not self.has_current_commercial_subscription:
-                raise CommercialSubscribersOnly(
-                    'A valid commercial subscription is required for private'
-                    ' Projects.')
-
+        if not self.has_current_commercial_subscription:
+            raise CommercialSubscribersOnly(
+                'A valid commercial subscription is required for private'
+                ' Projects.')
         return value
 
     _information_type = EnumCol(

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py	2012-11-19 06:26:32 +0000
+++ lib/lp/registry/tests/test_product.py	2012-11-23 14:31:20 +0000
@@ -391,6 +391,63 @@
         expected = [InformationType.USERDATA, InformationType.PRIVATESECURITY]
         self.assertContentEqual(expected, [policy.type for policy in aps])
 
+    def test_change_info_type_proprietary_check_artifacts(self):
+        # Cannot change product information_type if any artifacts are public.
+        spec_policy = SpecificationSharingPolicy.PUBLIC_OR_PROPRIETARY
+        product = self.factory.makeProduct(
+            licenses=[License.OTHER_PROPRIETARY],
+            specification_sharing_policy=spec_policy,
+            bug_sharing_policy=BugSharingPolicy.PUBLIC_OR_PROPRIETARY,
+            branch_sharing_policy=BranchSharingPolicy.PUBLIC_OR_PROPRIETARY,
+        )
+        self.useContext(person_logged_in(product.owner))
+        spec = self.factory.makeSpecification(product=product)
+        for info_type in PROPRIETARY_INFORMATION_TYPES:
+            with ExpectedException(
+                CannotChangeInformationType,
+                'Some blueprints are public.'):
+                product.information_type = info_type
+        spec.transitionToInformationType(InformationType.PROPRIETARY,
+                                         product.owner)
+        bug = self.factory.makeBug(target=product)
+        for bug_info_type in FREE_INFORMATION_TYPES:
+            bug.transitionToInformationType(bug_info_type, product.owner)
+            for info_type in PROPRIETARY_INFORMATION_TYPES:
+                with ExpectedException(
+                    CannotChangeInformationType,
+                    'Some bugs are neither proprietary nor embargoed.'):
+                    product.information_type = info_type
+        bug.transitionToInformationType(InformationType.PROPRIETARY,
+                                        product.owner)
+        branch = self.factory.makeBranch(product=product)
+        for branch_info_type in FREE_INFORMATION_TYPES:
+            branch.transitionToInformationType(branch_info_type,
+                                               product.owner)
+            for info_type in PROPRIETARY_INFORMATION_TYPES:
+                with ExpectedException(
+                    CannotChangeInformationType,
+                    'Some branches are neither proprietary nor embargoed.'):
+                    product.information_type = info_type
+        branch.transitionToInformationType(InformationType.PROPRIETARY,
+                                           product.owner)
+        for info_type in PROPRIETARY_INFORMATION_TYPES:
+            product.information_type = info_type
+
+    def test_change_info_type_proprietary_check_translations(self):
+        product = self.factory.makeProduct(
+            licenses=[License.OTHER_PROPRIETARY])
+        with person_logged_in(product.owner):
+            for usage in ServiceUsage:
+                product.translations_usage = usage.value
+                for info_type in PROPRIETARY_INFORMATION_TYPES:
+                    if product.translations_usage == ServiceUsage.LAUNCHPAD:
+                        with ExpectedException(
+                            CannotChangeInformationType,
+                            'Translations are enabled.'):
+                            product.information_type = info_type
+                    else:
+                        product.information_type = info_type
+
     def test_change_info_type_proprietary_sets_policies(self):
         # Changing information type from public to proprietary sets the
         # appropriate policies