← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~wallyworld/launchpad/make-private-branches-527900 into lp:launchpad

 

Ian Booth has proposed merging lp:~wallyworld/launchpad/make-private-branches-527900 into lp:launchpad.

Requested reviews:
  Curtis Hovey (sinzui)
Related bugs:
  Bug #527900 in Launchpad itself: "Users should be able to make private branches of public projects"
  https://bugs.launchpad.net/launchpad/+bug/527900

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/make-private-branches-527900/+merge/113532

== Implementation ==

This branch correctly sets up the Information Type field on the branch edit page so that authorised users can make branches private. Previously, the information type field was simply made visible/hidden just like the old privacy checkbox and this is now wrong for the new sharing world order. Along the way, an issue was found with the InformationTypeVocabulary, and a new search term needed to be added to bugtask search.

1. InformationTypeVocabulary

The vocab was including PROPRIETARY incorrectly in the list of terms. It correctly excluded PROPRIETARY if the feature flag was not set or if the context was multi-pillar bug. But it included PROPRIETARY if the context was none or if the context was a project without a commercial subscription. Code was added to include PROPRIETARY iff:
- context is not none
- context is a project with a current commercial subscription
- context is a single pillar bug with target being a project with a current commercial subscription
- context is a branch targetted to a project with a current commercial subscription

2. Bugtask Search

Support for searching for bugtasks with only specified information_types was added.

3. Branch edit view

The view itself was fixed up so that the correct information type choices are displayed. If the branch is private and cannot be made public for example, only private information type choices are given. If the branch is stacked on a private branch, the information type is fixed to be the same as that of the stacked on branch and the user cannot change it. A suitable message is shown. Part of the work involved fixing up how the form fields were put together so that the ordering was correct in all circumstances

Previously, methods on the branch visibility policy were used to see if a branch could be public or private. BVP is going away. So new methods were added to IBranch: canBePrivate() and canBePublic(). These new methods make the old BVP calls and add to those the new rules for allowing people to make a branch private/pubic.

The only place where BVP is now used from what I can tell is in model.Branch itself to check that transitionToInformationType can proceed, so it should be easy to rip out BVP entirely and just call the branch instance's canBePublic() and canBePrivate() methods.

== Tests ==

1. Add/fix tests for InformationTypeVocabulary change
2. Add tests for bugtask search change
3. Add/fix tests for branch edit view changes
4. Add tests for new branch canBePrivate and canBePublic methods

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/interfaces/bugtask.py
  lib/lp/bugs/model/bugtasksearch.py
  lib/lp/bugs/tests/test_bugtask_search.py
  lib/lp/code/browser/branch.py
  lib/lp/code/browser/tests/test_branch.py
  lib/lp/code/interfaces/branch.py
  lib/lp/code/model/branch.py
  lib/lp/code/model/tests/test_branch.py
  lib/lp/registry/vocabularies.py
  lib/lp/registry/tests/test_information_type_vocabulary.py
-- 
https://code.launchpad.net/~wallyworld/launchpad/make-private-branches-527900/+merge/113532
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'lib/lp/bugs/interfaces/bugtask.py'
--- lib/lp/bugs/interfaces/bugtask.py	2012-06-14 21:50:59 +0000
+++ lib/lp/bugs/interfaces/bugtask.py	2012-07-06 03:42:22 +0000
@@ -47,6 +47,7 @@
     'valid_remote_bug_url',
     ]
 
+import collections
 import httplib
 
 from lazr.enum import (
@@ -1178,7 +1179,8 @@
                  structural_subscriber=None, modified_since=None,
                  created_since=None, exclude_conjoined_tasks=False, cve=None,
                  upstream_target=None, milestone_dateexpected_before=None,
-                 milestone_dateexpected_after=None, created_before=None):
+                 milestone_dateexpected_after=None, created_before=None,
+                 information_types=None):
 
         self.bug = bug
         self.searchtext = searchtext
@@ -1232,6 +1234,10 @@
         self.upstream_target = upstream_target
         self.milestone_dateexpected_before = milestone_dateexpected_before
         self.milestone_dateexpected_after = milestone_dateexpected_after
+        if isinstance(information_types, collections.Iterable):
+            self.information_types = information_types
+        else:
+            self.information_types = (information_types,)
 
     def setProduct(self, product):
         """Set the upstream context on which to filter the search."""

=== modified file 'lib/lp/bugs/model/bugtasksearch.py'
--- lib/lp/bugs/model/bugtasksearch.py	2012-06-18 01:55:31 +0000
+++ lib/lp/bugs/model/bugtasksearch.py	2012-07-06 03:42:22 +0000
@@ -734,6 +734,10 @@
         extra_clauses.append(
             BugTaskFlat.datecreated < params.created_before)
 
+    if params.information_types:
+        extra_clauses.append(
+            BugTaskFlat.information_type.is_in(params.information_types))
+
     query = And(extra_clauses)
 
     if not decorators:

=== modified file 'lib/lp/bugs/tests/test_bugtask_search.py'
--- lib/lp/bugs/tests/test_bugtask_search.py	2012-06-15 00:36:50 +0000
+++ lib/lp/bugs/tests/test_bugtask_search.py	2012-07-06 03:42:22 +0000
@@ -24,6 +24,7 @@
     )
 from lp.bugs.model.bugsummary import BugSummary
 from lp.bugs.model.bugtasksearch import _process_order_by
+from lp.registry.enums import InformationType
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.distributionsourcepackage import (
     IDistributionSourcePackage,
@@ -409,6 +410,20 @@
             user=None, importance=BugTaskImportance.MEDIUM)
         self.assertSearchFinds(params, [])
 
+    def test_filter_by_information_types(self):
+        # Search results can be filtered by information_type.
+        with person_logged_in(self.owner):
+            self.bugtasks[2].bug.transitionToInformationType(
+                InformationType.EMBARGOEDSECURITY, self.owner)
+        params = self.getBugTaskSearchParams(
+            user=self.owner,
+            information_types=InformationType.EMBARGOEDSECURITY)
+        self.assertSearchFinds(params, [self.bugtasks[2]])
+        params = self.getBugTaskSearchParams(
+            user=self.owner,
+            information_types=InformationType.UNEMBARGOEDSECURITY)
+        self.assertSearchFinds(params, [])
+
     def test_omit_duplicate_bugs(self):
         # Duplicate bugs can optionally be excluded from search results.
         # The default behaviour is to include duplicates.

=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py	2012-06-29 08:40:05 +0000
+++ lib/lp/code/browser/branch.py	2012-07-06 03:42:22 +0000
@@ -116,13 +116,14 @@
 from lp.code.interfaces.branch import (
     IBranch,
     IBranchSet,
-    user_has_special_branch_access,
     )
 from lp.code.interfaces.branchcollection import IAllBranches
 from lp.code.interfaces.branchmergeproposal import IBranchMergeProposal
-from lp.code.interfaces.branchnamespace import IBranchNamespacePolicy
 from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference
-from lp.registry.enums import PRIVATE_INFORMATION_TYPES
+from lp.registry.enums import (
+    PRIVATE_INFORMATION_TYPES,
+    PUBLIC_INFORMATION_TYPES,
+    )
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.productseries import IProductSeries
 from lp.registry.vocabularies import (
@@ -738,7 +739,7 @@
                 ])
             information_type = copy_field(
                 IBranch['information_type'], readonly=False,
-                vocabulary=InformationTypeVocabulary())
+                vocabulary=InformationTypeVocabulary(self.context))
             reviewer = copy_field(IBranch['reviewer'], required=True)
             owner = copy_field(IBranch['owner'], readonly=False)
         return BranchEditSchema
@@ -1030,42 +1031,30 @@
         # This is to prevent users from converting push/import
         # branches to pull branches.
         branch = self.context
-        if branch.branch_type in (BranchType.HOSTED, BranchType.IMPORTED):
-            self.form_fields = self.form_fields.omit('url')
-
-        policy = IBranchNamespacePolicy(branch.namespace)
         if branch.private:
-            # If the branch is private, and can be public, show the field.
-            show_private_field = policy.canBranchesBePublic()
-
-            # If this branch is stacked on a private branch, disable the
-            # field.
+            # If this branch is stacked on a private branch, render some text
+            # to inform the user the information type cannot be changed.
             if (branch.stacked_on and branch.stacked_on.information_type in
                 PRIVATE_INFORMATION_TYPES):
-                show_private_field = False
+                stacked_info_type = branch.stacked_on.information_type.title
                 private_info = Bool(
                     __name__="private",
-                    title=_("Branch is confidential"),
+                    title=_("Branch is %s" % stacked_info_type),
                     description=_(
-                        "This branch is confidential because it is stacked "
-                        "on a private branch."))
+                        "This branch is %(info_type)s because it is "
+                        "stacked on a %(info_type)s branch." % {
+                            'info_type': stacked_info_type}))
                 private_info_field = form.Fields(
                     private_info, render_context=self.render_context)
-                self.form_fields = self.form_fields.omit('private')
-                self.form_fields = private_info_field + self.form_fields
+                self.form_fields = (private_info_field
+                    + self.form_fields.omit('information_type'))
+                new_field_names = self.field_names
+                index = new_field_names.index('information_type')
+                new_field_names[index] = 'private'
+                self.form_fields = self.form_fields.select(*new_field_names)
                 self.form_fields['private'].custom_widget = (
                     CustomWidgetFactory(
                         CheckBoxWidget, extra='disabled="disabled"'))
-        else:
-            # If the branch is public, and can be made private, show the
-            # field.  Users with special access rights to branches can set
-            # public branches as private.
-            show_private_field = (
-                policy.canBranchesBePrivate() or
-                user_has_special_branch_access(self.user))
-
-        if not show_private_field:
-            self.form_fields = self.form_fields.omit('information_type')
 
         # If the user can administer branches, then they should be able to
         # assign the ownership of the branch to any valid person or team.
@@ -1103,6 +1092,40 @@
                 self.form_fields = self.form_fields.omit('owner')
                 self.form_fields = new_owner_field + self.form_fields
 
+        if branch.branch_type in (BranchType.HOSTED, BranchType.IMPORTED):
+            self.form_fields = self.form_fields.omit('url')
+
+    def setUpWidgets(self, context=None):
+        super(BranchEditView, self).setUpWidgets()
+        branch = self.context
+
+        if self.form_fields.get('information_type') is not None:
+            # The vocab uses feature flags to control what is displayed so we
+            # need to pull info_types from the vocab to use to make the subset
+            # of what we show the user.
+            info_type_vocab = self.widgets['information_type'].vocabulary
+            public_types = [
+                    info_type
+                    for info_type in info_type_vocab
+                    if info_type.value in PUBLIC_INFORMATION_TYPES]
+            private_types = [
+                    info_type
+                    for info_type in info_type_vocab
+                    if info_type.value in PRIVATE_INFORMATION_TYPES]
+
+            allowed_information_types = []
+            if branch.private:
+                if branch.canBePublic():
+                    allowed_information_types.extend(public_types)
+                allowed_information_types.extend(private_types)
+            else:
+                allowed_information_types.extend(public_types)
+                if branch.canBePrivate(self.user):
+                    allowed_information_types.extend(private_types)
+
+            self.widgets['information_type'].vocabulary = (
+                SimpleVocabulary(allowed_information_types))
+
     def validate(self, data):
         # Check that we're not moving a team branch to the +junk
         # pseudo project.

=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py	2012-06-22 05:52:17 +0000
+++ lib/lp/code/browser/tests/test_branch.py	2012-07-06 03:42:22 +0000
@@ -36,7 +36,10 @@
     BranchVisibilityRule,
     )
 from lp.registry.enums import InformationType
-from lp.registry.interfaces.person import PersonVisibility
+from lp.registry.interfaces.person import (
+    PersonVisibility,
+    TeamSubscriptionPolicy,
+    )
 from lp.services.config import config
 from lp.services.database.constants import UTC_NOW
 from lp.services.features.testing import FeatureFixture
@@ -922,7 +925,9 @@
         # The vocabulary that Branch:+edit uses for the information_type
         # has been correctly created.
         person = self.factory.makePerson()
-        branch = self.factory.makeProductBranch(owner=person)
+        product = self.factory.makeProduct()
+        self.factory.makeCommercialSubscription(product)
+        branch = self.factory.makeProductBranch(product=product, owner=person)
         feature_flag = {
             'disclosure.proprietary_information_type.disabled': 'on'}
         admins = getUtility(ILaunchpadCelebrities).admin
@@ -946,7 +951,33 @@
             browser = self.getUserBrowser(
                 canonical_url(branch) + '/+edit', user=owner)
         self.assertRaises(
-            LookupError, browser.getControl, "Keep branch confidential")
+            LookupError, browser.getControl, "Information Type")
+
+    def test_authorised_user_can_change_branch_to_private(self):
+        # An authorised user can make the information type private.
+        team_owner = self.factory.makePerson()
+        user = self.factory.makePerson()
+        team = self.factory.makeTeam(
+            owner=team_owner,
+            visibility=PersonVisibility.PRIVATE,
+            subscription_policy=TeamSubscriptionPolicy.RESTRICTED)
+        with person_logged_in(team_owner):
+            team.addMember(user, team_owner)
+        with person_logged_in(user):
+            branch = self.factory.makeBranch(owner=team)
+            browser = self.getUserBrowser(
+                canonical_url(branch) + '/+edit', user=user)
+        self.assertIsNotNone(browser.getControl, "Embargoed Security")
+
+    def test_unauthorised_user_cannot_change_branch_to_private(self):
+        # An authorised user can make the information type private.
+        user = self.factory.makePerson()
+        with person_logged_in(user):
+            branch = self.factory.makeBranch(owner=user)
+            browser = self.getUserBrowser(
+                canonical_url(branch) + '/+edit', user=user)
+        self.assertRaises(
+            LookupError, browser.getControl, "Embargoed Security")
 
 
 class TestBranchUpgradeView(TestCaseWithFactory):

=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py	2012-05-24 04:21:10 +0000
+++ lib/lp/code/interfaces/branch.py	2012-07-06 03:42:22 +0000
@@ -968,6 +968,25 @@
     def visibleByUser(user):
         """Can the specified user see this branch?"""
 
+    def canBePublic(user):
+        """Can this branch be public?
+
+        A branch can be made public if:
+        - the branch has a visibility policy which allows it
+        - the user is an admin or bzr expert
+        """
+
+    def canBePrivate(user):
+        """Can this branch be private?
+
+        A branch can be made private if:
+        - the branch has a visibility policy which allows it
+        - the user is an admin or bzr expert
+        - the branch is owned by a private team
+          (The branch is already implicitly private)
+        - the branch is linked to a private bug the user can access
+        """
+
 
 class IBranchEditableAttributes(Interface):
     """IBranch attributes that can be edited.

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2012-06-27 22:31:38 +0000
+++ lib/lp/code/model/branch.py	2012-07-06 03:42:22 +0000
@@ -142,6 +142,7 @@
     IAccessArtifactSource,
     )
 from lp.registry.interfaces.person import (
+    PersonVisibility,
     validate_person,
     validate_public_person,
     )
@@ -1286,6 +1287,26 @@
                     user, checked_branches)
         return can_access
 
+    def canBePublic(self, user):
+        """See `IBranch`."""
+        policy = IBranchNamespacePolicy(self.namespace)
+        return (policy.canBranchesBePublic() or
+                user_has_special_branch_access(user))
+
+    def canBePrivate(self, user):
+        """See `IBranch`."""
+        policy = IBranchNamespacePolicy(self.namespace)
+        # Do the easy checks first.
+        if (policy.canBranchesBePrivate() or
+                user_has_special_branch_access(user) or
+                user.visibility == PersonVisibility.PRIVATE):
+            return True
+        params = BugTaskSearchParams(
+            user=user, linked_branches=self.id,
+            information_types=PRIVATE_INFORMATION_TYPES)
+        bug_ids = getUtility(IBugTaskSet).searchBugIds(params)
+        return bug_ids.count() > 0
+
     @property
     def recipes(self):
         """See `IHasRecipes`."""

=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py	2012-06-11 10:25:46 +0000
+++ lib/lp/code/model/tests/test_branch.py	2012-07-06 03:42:22 +0000
@@ -110,7 +110,11 @@
 from lp.code.tests.helpers import add_revision_to_branch
 from lp.codehosting.safe_open import BadUrl
 from lp.codehosting.vfs.branchfs import get_real_branch_path
-from lp.registry.enums import InformationType
+from lp.registry.enums import (
+    InformationType,
+    PRIVATE_INFORMATION_TYPES,
+    PUBLIC_INFORMATION_TYPES,
+    )
 from lp.registry.interfaces.accesspolicy import (
     IAccessArtifactSource,
     IAccessPolicyArtifactSource,
@@ -2504,6 +2508,106 @@
             get_policies_for_artifact(branch)[0].type)
 
 
+class TestBranchCanBePrivate(TestCaseWithFactory):
+    """Test IBranch.canBePrivate."""
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        # Use an admin user as we aren't checking edit permissions here.
+        TestCaseWithFactory.setUp(self, 'admin@xxxxxxxxxxxxx')
+
+    def test_arbitary_branch(self):
+        # By default branches cannot be private.
+        branch = self.factory.makeBranch()
+        self.assertFalse(branch.canBePrivate(branch.owner))
+
+    def test_admin(self):
+        # Admins can make a branch private.
+        branch = self.factory.makeBranch()
+        admin = getUtility(ILaunchpadCelebrities).admin
+        self.assertTrue(branch.canBePrivate(admin))
+
+    def test_visibility_policy_private(self):
+        # Users with a suitable  visibility policy can make a branch private.
+        team = self.factory.makeTeam(
+            subscription_policy=TeamSubscriptionPolicy.RESTRICTED)
+        product = self.factory.makeProduct()
+        product.setBranchVisibilityTeamPolicy(
+            team, BranchVisibilityRule.PRIVATE)
+        branch = self.factory.makeBranch(product=product, owner=team)
+        self.assertTrue(branch.canBePrivate(team))
+
+    def test_private_owner(self):
+        # Private team owners can make a branch private.
+        team = self.factory.makeTeam(
+            visibility=PersonVisibility.PRIVATE,
+            subscription_policy=TeamSubscriptionPolicy.RESTRICTED)
+        branch = self.factory.makeBranch(owner=team)
+        self.assertTrue(branch.canBePrivate(team))
+
+    def test_linked_private_bug(self):
+        # Users with access to linked private bugs can make a branch private.
+        for info_type in PRIVATE_INFORMATION_TYPES:
+            user = self.factory.makePerson()
+            bug = self.factory.makeBug(
+                owner=user, information_type=info_type)
+            branch = self.factory.makeBranch()
+            removeSecurityProxy(bug).linkBranch(branch, user)
+            self.assertTrue(branch.canBePrivate(user))
+
+    def test_linked_public_bug(self):
+        # Users with access to linked public bugs cannot make a branch private.
+        for info_type in PUBLIC_INFORMATION_TYPES:
+            user = self.factory.makePerson()
+            bug = self.factory.makeBug(
+                owner=user, information_type=info_type)
+            branch = self.factory.makeBranch()
+            removeSecurityProxy(bug).linkBranch(branch, user)
+            self.assertFalse(branch.canBePrivate(user))
+
+
+class TestBranchCanBePublic(TestCaseWithFactory):
+    """Test IBranch.canBePublic."""
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        # Use an admin user as we aren't checking edit permissions here.
+        TestCaseWithFactory.setUp(self, 'admin@xxxxxxxxxxxxx')
+
+    def test_arbitary_branch(self):
+        # By default branches can be private.
+        branch = self.factory.makeBranch(
+            information_type=InformationType.USERDATA)
+        self.assertTrue(branch.canBePublic(branch.owner))
+
+    def test_admin(self):
+        # Admins can make a branch public.
+        team = self.factory.makeTeam(
+            subscription_policy=TeamSubscriptionPolicy.RESTRICTED)
+        product = self.factory.makeProduct()
+        product.setBranchVisibilityTeamPolicy(
+            team, BranchVisibilityRule.PRIVATE_ONLY)
+        branch = self.factory.makeBranch(
+            product=product, owner=team,
+            information_type=InformationType.USERDATA)
+        admin = getUtility(ILaunchpadCelebrities).admin
+        self.assertTrue(branch.canBePublic(admin))
+
+    def test_visibility_policy_public_not_allowed(self):
+        # Branches cannot be public if the visibility policy forbids it.
+        team = self.factory.makeTeam(
+            subscription_policy=TeamSubscriptionPolicy.RESTRICTED)
+        product = self.factory.makeProduct()
+        product.setBranchVisibilityTeamPolicy(
+            team, BranchVisibilityRule.PRIVATE_ONLY)
+        branch = self.factory.makeBranch(
+            product=product, owner=team,
+            information_type=InformationType.USERDATA)
+        self.assertFalse(branch.canBePublic(team))
+
+
 class TestBranchCommitsForDays(TestCaseWithFactory):
     """Tests for `Branch.commitsForDays`."""
 

=== modified file 'lib/lp/registry/tests/test_information_type_vocabulary.py'
--- lib/lp/registry/tests/test_information_type_vocabulary.py	2012-07-03 06:06:13 +0000
+++ lib/lp/registry/tests/test_information_type_vocabulary.py	2012-07-06 03:42:22 +0000
@@ -24,13 +24,16 @@
     layer = DatabaseFunctionalLayer
 
     def test_vocabulary_items(self):
-        vocab = InformationTypeVocabulary()
+        product = self.factory.makeProduct()
+        self.factory.makeCommercialSubscription(product)
+        vocab = InformationTypeVocabulary(product)
         for info_type in InformationType:
             self.assertIn(info_type.value, vocab)
 
     def test_vocabulary_items_project(self):
         # The vocab has all info types for a project without private_bugs set.
         product = self.factory.makeProduct()
+        self.factory.makeCommercialSubscription(product)
         vocab = InformationTypeVocabulary(product)
         for info_type in InformationType:
             self.assertIn(info_type.value, vocab)
@@ -38,6 +41,7 @@
     def test_vocabulary_items_private_bugs_project(self):
         # The vocab has private info types for a project with private_bugs set.
         product = self.factory.makeProduct(private_bugs=True)
+        self.factory.makeCommercialSubscription(product)
         vocab = InformationTypeVocabulary(product)
         for info_type in PRIVATE_INFORMATION_TYPES:
             self.assertIn(info_type, vocab)
@@ -57,13 +61,25 @@
     def test_proprietary_disabled(self):
         feature_flag = {
             'disclosure.proprietary_information_type.disabled': 'on'}
+        product = self.factory.makeProduct()
+        self.factory.makeCommercialSubscription(product)
         with FeatureFixture(feature_flag):
-            vocab = InformationTypeVocabulary()
+            vocab = InformationTypeVocabulary(product)
             self.assertRaises(
                 LookupError, vocab.getTermByToken, 'PROPRIETARY')
 
+    def test_proprietary_disabled_for_non_commercial_projects(self):
+        # Only projects with commercial subscriptions have PROPRIETARY.
+        product = self.factory.makeProduct()
+        vocab = InformationTypeVocabulary(product)
+        self.assertRaises(
+            LookupError, vocab.getTermByToken, 'PROPRIETARY')
+
     def test_proprietary_enabled(self):
-        vocab = InformationTypeVocabulary()
+        # Only projects with commercial subscriptions have PROPRIETARY.
+        product = self.factory.makeProduct()
+        self.factory.makeCommercialSubscription(product)
+        vocab = InformationTypeVocabulary(product)
         term = vocab.getTermByToken('PROPRIETARY')
         self.assertEqual('Proprietary', term.title)
 
@@ -98,7 +114,9 @@
 
     def test_multi_task_bugs(self):
         # Multi-task bugs are allowed to be PROPRIETARY.
-        bug = self.factory.makeBug()
+        product = self.factory.makeProduct()
+        self.factory.makeCommercialSubscription(product)
+        bug = self.factory.makeBug(product=product)
         self.factory.makeBugTask(bug=bug) # Uses the same pillar.
         vocab = InformationTypeVocabulary(bug)
         term = vocab.getTermByToken('PROPRIETARY')

=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py	2012-07-03 06:06:13 +0000
+++ lib/lp/registry/vocabularies.py	2012-07-06 03:42:22 +0000
@@ -105,6 +105,7 @@
 from lp.blueprints.interfaces.specification import ISpecification
 from lp.bugs.interfaces.bug import IBug
 from lp.bugs.interfaces.bugtask import IBugTask
+from lp.code.interfaces.branch import IBranch
 from lp.registry.enums import InformationType
 from lp.registry.interfaces.accesspolicy import IAccessPolicySource
 from lp.registry.interfaces.distribution import (
@@ -2251,9 +2252,22 @@
             'disclosure.proprietary_information_type.disabled'))
         show_userdata_as_private = bool(getFeatureFlag(
             'disclosure.display_userdata_as_private.enabled'))
-        if not proprietary_disabled and not (IBug.providedBy(context) and
-            len(context.affected_pillars) > 1):
+        # Proprietary is allowed for:
+        # - single pillar bugs where the target has a current commercial
+        #   subscription
+        # - branches for a project with a current commercial subscription
+        # - projects with current commercial subscriptions
+        subscription_context = context
+        if IBug.providedBy(context) and len(context.affected_pillars) == 1:
+            subscription_context = context.affected_pillars[0]
+        if IBranch.providedBy(context):
+            subscription_context = context.target.context
+        has_commercial_subscription = (
+            IProduct.providedBy(subscription_context) and
+            subscription_context.has_current_commercial_subscription)
+        if not proprietary_disabled and has_commercial_subscription:
             types.append(InformationType.PROPRIETARY)
+        # Disallow public items for projects with private bugs.
         if (context is None or
             not IProduct.providedBy(context) or
             not context.private_bugs):


Follow ups