← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~wgrant/launchpad/improve-branch-edit-type into lp:launchpad

 

William Grant has proposed merging lp:~wgrant/launchpad/improve-branch-edit-type into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/improve-branch-edit-type/+merge/114591

This branch reworks BranchEditView's information type widget to support the new sharing model a little more easily. It used to choose types to show based on Branch.canBe{Public,Private}, which doesn't work well if we want to use a different set (eg. private projects only support Proprietary).

I adjusted Branch.transitionToInformationType to use the information types allowed by the branch's namespace, or all the information types if the user is a Launchpad admin. BranchEditView.getInformationTypesToShow restricts this further, hiding uninteresting types to avoid confusion, and in a sort of attempt to prevent projects from using private branches for nefarious purposes without paying us. These obscure types are only shown in the UI if there's a linked bug of one of the obscure types, similar to the old behaviour in Branch.canBePrivate.

I've dropped the commercial subscription check from Branch.canBePrivate for now, as it's new, not yet enabled, and will be replaced by a branch configuration option shortly.

There's still some awfulness in BranchEditView around restricting type changes for stacked branches, but I plan to disentangle that in a followup.
-- 
https://code.launchpad.net/~wgrant/launchpad/improve-branch-edit-type/+merge/114591
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/improve-branch-edit-type into lp:launchpad.
=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py	2012-07-10 10:58:27 +0000
+++ lib/lp/code/browser/branch.py	2012-07-12 09:33:20 +0000
@@ -91,7 +91,11 @@
 from lp.blueprints.interfaces.specificationbranch import ISpecificationBranch
 from lp.bugs.interfaces.bug import IBugSet
 from lp.bugs.interfaces.bugbranch import IBugBranch
-from lp.bugs.interfaces.bugtask import UNRESOLVED_BUGTASK_STATUSES
+from lp.bugs.interfaces.bugtask import (
+    BugTaskSearchParams,
+    IBugTaskSet,
+    UNRESOLVED_BUGTASK_STATUSES,
+    )
 from lp.code.browser.branchmergeproposal import (
     latest_proposals_for_each_branch,
     )
@@ -121,6 +125,7 @@
 from lp.code.interfaces.branchmergeproposal import IBranchMergeProposal
 from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference
 from lp.registry.enums import (
+    InformationType,
     PRIVATE_INFORMATION_TYPES,
     PUBLIC_INFORMATION_TYPES,
     )
@@ -1095,36 +1100,57 @@
         if branch.branch_type in (BranchType.HOSTED, BranchType.IMPORTED):
             self.form_fields = self.form_fields.omit('url')
 
+    def getInformationTypesToShow(self):
+        """Get the information types to display on the edit form.
+
+        We display a highly customised set of information types:
+        anything allowed by the namespace, plus the current type,
+        except some of the obscure types unless there's a linked
+        bug with an obscure type.
+        """
+        allowed_types = self.context.getAllowedInformationTypes(self.user)
+        shown_types = (
+            InformationType.PUBLIC,
+            InformationType.USERDATA,
+            InformationType.PROPRIETARY,
+            )
+
+        # We only show Embargoed Security and Unembargoed Security
+        # if the branch is linked to a bug with one of those types,
+        # as they're confusing and not generally useful otherwise.
+        # Once Proprietary is fully deployed, User Data should be
+        # added here.
+        hidden_types = (
+            InformationType.UNEMBARGOEDSECURITY,
+            InformationType.EMBARGOEDSECURITY,
+            )
+        if set(allowed_types).intersection(hidden_types):
+            params = BugTaskSearchParams(
+                user=self.user, linked_branches=self.context.id,
+                information_type=hidden_types)
+            if getUtility(IBugTaskSet).searchBugIds(params).count() > 0:
+                shown_types += hidden_types
+
+        # Now take the intersection of the allowed and shown types,
+        # plus the current type, and grab them from the vocab if
+        # they exist.
+        # 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. This is mostly to hide Proprietary
+        # while it's disabled.
+        combined_types = set(allowed_types).intersection(shown_types)
+        combined_types.add(self.context.information_type)
+        return combined_types
+
     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.
+            # Customise the set of shown types.
+            types_to_show = self.getInformationTypesToShow()
             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(self.user):
-                    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))
+            self.widgets['information_type'].vocabulary = SimpleVocabulary(
+                [info_type for info_type in info_type_vocab
+                 if info_type.value in types_to_show])
 
     def validate(self, data):
         # Check that we're not moving a team branch to the +junk

=== modified file 'lib/lp/code/browser/branchlisting.py'
--- lib/lp/code/browser/branchlisting.py	2012-03-07 00:58:37 +0000
+++ lib/lp/code/browser/branchlisting.py	2012-07-12 09:33:20 +0000
@@ -99,6 +99,10 @@
     ProductDownloadFileMixin,
     SortSeriesMixin,
     )
+from lp.registry.enums import (
+    InformationType,
+    PRIVATE_INFORMATION_TYPES,
+    )
 from lp.registry.interfaces.person import (
     IPerson,
     IPersonSet,
@@ -749,17 +753,32 @@
         self.form_fields = form.Fields(*fields)
         super(BranchListingView, self).setUpWidgets(context)
 
-    @property
-    def new_branches_are_private(self):
-        """Are new branches by the user private."""
+    @cachedproperty
+    def default_information_type(self):
+        """The default information type for new branches."""
         if self.user is None:
-            return False
+            return None
         target = IBranchTarget(self.context)
         if target is None:
             return False
         namespace = target.getNamespace(self.user)
-        policy = IBranchNamespacePolicy(namespace)
-        return policy.areNewBranchesPrivate()
+        return IBranchNamespacePolicy(namespace).getDefaultInformationType()
+
+    @property
+    def default_information_type_title(self):
+        """The title of the default information type for new branches."""
+        information_type = self.default_information_type
+        if information_type is None:
+            return None
+        if (information_type == InformationType.USERDATA and
+            getFeatureFlag('disclosure.display_userdata_as_private.enabled')):
+            return 'Private'
+        return information_type.title
+
+    @property
+    def default_information_type_is_private(self):
+        """The title of the default information type for new branches."""
+        return self.default_information_type in PRIVATE_INFORMATION_TYPES
 
 
 class NoContextBranchListingView(BranchListingView):

=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py	2012-07-09 04:14:09 +0000
+++ lib/lp/code/browser/tests/test_branch.py	2012-07-12 09:33:20 +0000
@@ -47,6 +47,7 @@
 from lp.services.webapp.publisher import canonical_url
 from lp.services.webapp.servers import LaunchpadTestRequest
 from lp.testing import (
+    admin_logged_in,
     BrowserTestCase,
     login,
     login_person,
@@ -915,32 +916,11 @@
         admin = admins.teamowner
         browser = self.getUserBrowser(
             canonical_url(branch) + '/+edit', user=admin)
-        browser.getControl("Embargoed Security").click()
+        browser.getControl("User Data").click()
         browser.getControl("Change Branch").click()
         with person_logged_in(person):
             self.assertEqual(
-                InformationType.EMBARGOEDSECURITY, branch.information_type)
-
-    def test_proprietary_in_ui_vocabulary_commercial_projects(self):
-        # Commercial projects can have information type Proprietary.
-        owner = self.factory.makePerson()
-        product = self.factory.makeProduct()
-        self.factory.makeCommercialSubscription(product)
-        branch = self.factory.makeProductBranch(product=product, owner=owner)
-        with person_logged_in(owner):
-            browser = self.getUserBrowser(
-                canonical_url(branch) + '/+edit', user=owner)
-            self.assertIsNotNone(browser.getControl("Proprietary"))
-
-    def test_proprietary_not_in_ui_vocabulary_normal_projects(self):
-        # Non-commercial projects can not have information type Proprietary.
-        owner = self.factory.makePerson()
-        product = self.factory.makeProduct()
-        branch = self.factory.makeProductBranch(product=product, owner=owner)
-        with person_logged_in(owner):
-            browser = self.getUserBrowser(
-                canonical_url(branch) + '/+edit', user=owner)
-            self.assertRaises(LookupError, browser.getControl, "Proprietary")
+                InformationType.USERDATA, branch.information_type)
 
     def test_can_not_change_privacy_of_stacked_on_private(self):
         # The privacy field is not shown if the branch is stacked on a
@@ -958,41 +938,83 @@
         self.assertRaises(
             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 unauthorised user cannot 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")
-
-    def test_branch_for_commercial_project(self):
-        # A branch for a commercial project can be private.
-        product = self.factory.makeProduct()
-        self.factory.makeCommercialSubscription(product)
-        branch = self.factory.makeProductBranch(product=product)
-        with person_logged_in(branch.owner):
-            browser = self.getUserBrowser(
-                canonical_url(branch) + '/+edit', user=branch.owner)
-        self.assertIsNotNone(browser.getControl, "Embargoed Security")
+
+class TestBranchEditViewInformationTypes(TestCaseWithFactory):
+    """Tests for BranchEditView.getInformationTypesToShow."""
+
+    layer = DatabaseFunctionalLayer
+
+    def assertShownTypes(self, types, branch, user=None):
+        if user is None:
+            user = removeSecurityProxy(branch).owner
+        with person_logged_in(user):
+            view = create_initialized_view(branch, '+edit', user=user)
+            self.assertContentEqual(types, view.getInformationTypesToShow())
+
+    def test_public_branch(self):
+        # A normal public branch on a public project can only be public.
+        # We don't show information types like Unembargoed Security
+        # unless there's a linked branch of that type, as they're not
+        # useful or unconfusing otherwise.
+        # The model doesn't enforce this, so it's just a UI thing.
+        branch = self.factory.makeBranch(
+            information_type=InformationType.PUBLIC)
+        self.assertShownTypes([InformationType.PUBLIC], branch)
+
+    def test_public_branch_with_security_bug(self):
+        # A public branch can be set to Unembargoed Security if it has a
+        # linked Unembargoed Security bug. The project policy doesn't
+        # allow private branches, so Embargoed Security and User Data
+        # are unavailable.
+        branch = self.factory.makeBranch(
+            information_type=InformationType.PUBLIC)
+        bug = self.factory.makeBug(
+            information_type=InformationType.UNEMBARGOEDSECURITY)
+        with admin_logged_in():
+            branch.linkBug(bug, branch.owner)
+        self.assertShownTypes(
+            [InformationType.PUBLIC, InformationType.UNEMBARGOEDSECURITY],
+            branch)
+
+    def test_branch_with_disallowed_type(self):
+        # We don't force branches with a disallowed type (eg. Proprietary on a
+        # non-commercial project) to change, so the current type is
+        # shown.
+        branch = self.factory.makeBranch(
+            information_type=InformationType.PROPRIETARY)
+        self.assertShownTypes(
+            [InformationType.PUBLIC, InformationType.PROPRIETARY], branch)
+
+    def test_private_branch(self):
+        # Branches on projects with a private policy can be set to
+        # User Data (aka. Private)
+        branch = self.factory.makeBranch(
+            information_type=InformationType.PUBLIC)
+        with admin_logged_in():
+            branch.product.setBranchVisibilityTeamPolicy(
+                branch.owner, BranchVisibilityRule.PRIVATE)
+        self.assertShownTypes(
+            [InformationType.PUBLIC, InformationType.USERDATA,
+             InformationType.PROPRIETARY], branch)
+
+    def test_private_branch_with_security_bug(self):
+        # Branches on projects that allow private branches can use the
+        # Embargoed Security information type if they have a security
+        # bug linked.
+        branch = self.factory.makeBranch(
+            information_type=InformationType.PUBLIC)
+        with admin_logged_in():
+            branch.product.setBranchVisibilityTeamPolicy(
+                branch.owner, BranchVisibilityRule.PRIVATE)
+        bug = self.factory.makeBug(
+            information_type=InformationType.UNEMBARGOEDSECURITY)
+        with admin_logged_in():
+            branch.linkBug(bug, branch.owner)
+        self.assertShownTypes(
+            [InformationType.PUBLIC, InformationType.UNEMBARGOEDSECURITY,
+             InformationType.EMBARGOEDSECURITY, InformationType.USERDATA,
+             InformationType.PROPRIETARY],
+            branch)
 
 
 class TestBranchUpgradeView(TestCaseWithFactory):

=== modified file 'lib/lp/code/browser/tests/test_product.py'
--- lib/lp/code/browser/tests/test_product.py	2012-07-05 04:59:52 +0000
+++ lib/lp/code/browser/tests/test_product.py	2012-07-12 09:33:20 +0000
@@ -22,6 +22,7 @@
 from lp.code.interfaces.revision import IRevisionSet
 from lp.code.publisher import CodeLayer
 from lp.registry.enums import InformationType
+from lp.services.features.testing import FeatureFixture
 from lp.services.webapp import canonical_url
 from lp.testing import (
     ANONYMOUS,
@@ -360,11 +361,15 @@
         product.development_focus.branch = branch
         product.setBranchVisibilityTeamPolicy(
             team, BranchVisibilityRule.PRIVATE)
-        view = create_initialized_view(
-            product, '+code-index', rootsite='code', principal=product.owner)
-        text = extract_text(find_tag_by_id(view.render(), 'privacy'))
-        expected = ("New branches you create for %(name)s are private "
-                    "initially.*" % dict(name=product.displayname))
+        with FeatureFixture(
+                {'disclosure.display_userdata_as_private.enabled': 'true'}):
+            view = create_initialized_view(
+                product, '+code-index', rootsite='code',
+                principal=product.owner)
+            text = extract_text(find_tag_by_id(view.render(), 'privacy'))
+        expected = (
+            "New branches for %(name)s are Private.*"
+            % dict(name=product.displayname))
         self.assertTextMatchesExpressionIgnoreWhitespace(expected, text)
 
     def test_is_public(self):
@@ -374,8 +379,9 @@
         product.development_focus.branch = branch
         browser = self.getUserBrowser(canonical_url(product, rootsite='code'))
         text = extract_text(find_tag_by_id(browser.contents, 'privacy'))
-        expected = ("New branches you create for %(name)s are public "
-                    "initially.*" % dict(name=product.displayname))
+        expected = (
+            "New branches for %(name)s are Public.*"
+            % dict(name=product.displayname))
         self.assertTextMatchesExpressionIgnoreWhitespace(expected, text)
 
 

=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py	2012-07-06 03:33:59 +0000
+++ lib/lp/code/interfaces/branch.py	2012-07-12 09:33:20 +0000
@@ -968,23 +968,11 @@
     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
+    def getAllowedInformationTypes(user):
+        """Get a list of acceptable `InformationType`s for this branch.
+
+        If the user is a Launchpad admin, any type is acceptable. Otherwise
+        the `IBranchNamespace` is consulted.
         """
 
 

=== modified file 'lib/lp/code/interfaces/branchnamespace.py'
--- lib/lp/code/interfaces/branchnamespace.py	2012-07-11 22:16:47 +0000
+++ lib/lp/code/interfaces/branchnamespace.py	2012-07-12 09:33:20 +0000
@@ -116,12 +116,10 @@
         :return: A sequence of `InformationType`s.
         """
 
-    def areNewBranchesPrivate():
-        """Are new branches in this namespace private?
-
-        No check is made about whether or not a user can create branches.
-
-        :return: A Boolean value.
+    def getDefaultInformationType():
+        """Get the default information type for branches in this namespace.
+
+        :return: An `InformationType`.
         """
 
     def validateRegistrant(registrant):

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2012-07-11 09:43:04 +0000
+++ lib/lp/code/model/branch.py	2012-07-12 09:33:20 +0000
@@ -237,6 +237,17 @@
             information_type = InformationType.PUBLIC
         return self.transitionToInformationType(information_type, user)
 
+    def getAllowedInformationTypes(self, who):
+        """See `IBranch`."""
+        if user_has_special_branch_access(who):
+            # Until sharing settles down, admins can set any type.
+            types = set(PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES)
+        else:
+            # Otherwise the permitted types are defined by the namespace.
+            policy = IBranchNamespacePolicy(self.namespace)
+            types = set(policy.getAllowedInformationTypes())
+        return types
+
     def transitionToInformationType(self, information_type, who,
                                     verify_policy=True):
         """See `IBranch`."""
@@ -246,11 +257,9 @@
             and self.stacked_on.information_type in PRIVATE_INFORMATION_TYPES
             and information_type in PUBLIC_INFORMATION_TYPES):
             raise BranchCannotChangeInformationType()
-        # Only check the privacy policy if the user is not special.
-        if verify_policy and not user_has_special_branch_access(who):
-            policy = IBranchNamespacePolicy(self.namespace)
-            if information_type not in policy.getAllowedInformationTypes():
-                raise BranchCannotChangeInformationType()
+        if (verify_policy
+            and information_type not in self.getAllowedInformationTypes(who)):
+            raise BranchCannotChangeInformationType()
         self.information_type = information_type
         self._reconcileAccess()
         if information_type in PRIVATE_INFORMATION_TYPES:
@@ -1292,33 +1301,6 @@
                     user, checked_branches)
         return can_access
 
-    def canBePublic(self, user):
-        """See `IBranch`."""
-        policy = IBranchNamespacePolicy(self.namespace)
-        return InformationType.PUBLIC in policy.getAllowedInformationTypes()
-
-    def canBePrivate(self, user):
-        """See `IBranch`."""
-        policy = IBranchNamespacePolicy(self.namespace)
-        # Do the easy checks first.
-        policy_allows = (
-            InformationType.USERDATA in policy.getAllowedInformationTypes())
-        if (policy_allows or
-                user_has_special_branch_access(user) or
-                user.visibility == PersonVisibility.PRIVATE):
-            return True
-        # Branches linked to commercial projects can be private.
-        target = self.target.context
-        if (IProduct.providedBy(target) and
-            target.has_current_commercial_subscription):
-            return True
-        # Branches linked to private bugs can be private.
-        params = BugTaskSearchParams(
-            user=user, linked_branches=self.id,
-            information_type=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/branchnamespace.py'
--- lib/lp/code/model/branchnamespace.py	2012-07-11 22:16:47 +0000
+++ lib/lp/code/model/branchnamespace.py	2012-07-12 09:33:20 +0000
@@ -110,12 +110,7 @@
             distroseries = sourcepackage.distroseries
             sourcepackagename = sourcepackage.sourcepackagename
 
-        # If branches can be private, make them private initially.
-        private = self.areNewBranchesPrivate()
-        if private:
-            information_type = InformationType.USERDATA
-        else:
-            information_type = InformationType.PUBLIC
+        information_type = self.getDefaultInformationType()
 
         branch = Branch(
             registrant=registrant, name=name, owner=self.owner,
@@ -268,9 +263,11 @@
         """See `IBranchNamespace`."""
         raise NotImplementedError
 
-    def areNewBranchesPrivate(self):
+    def getDefaultInformationType(self):
         """See `IBranchNamespace`."""
-        return InformationType.USERDATA in self.getAllowedInformationTypes()
+        if InformationType.USERDATA in self.getAllowedInformationTypes():
+            return InformationType.USERDATA
+        return InformationType.PUBLIC
 
     def getPrivacySubscriber(self):
         """See `IBranchNamespace`."""

=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py	2012-07-11 09:36:13 +0000
+++ lib/lp/code/model/tests/test_branch.py	2012-07-12 09:33:20 +0000
@@ -76,7 +76,10 @@
 from lp.code.interfaces.branchmergeproposal import (
     BRANCH_MERGE_PROPOSAL_FINAL_STATES as FINAL_STATES,
     )
-from lp.code.interfaces.branchnamespace import IBranchNamespaceSet
+from lp.code.interfaces.branchnamespace import (
+    IBranchNamespacePolicy,
+    IBranchNamespaceSet,
+    )
 from lp.code.interfaces.branchrevision import IBranchRevision
 from lp.code.interfaces.codehosting import branch_id_alias
 from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
@@ -2388,6 +2391,38 @@
         self.assertEqual([], get_policies_for_artifact(branch))
 
 
+class TestBranchGetAllowedInformationTypes(TestCaseWithFactory):
+    """Test Branch.getAllowedInformationTypes."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_normal_user_sees_namespace_types(self):
+        # An unprivileged user sees the types allowed by the namespace.
+        branch = self.factory.makeBranch()
+        policy = IBranchNamespacePolicy(branch.namespace)
+        self.assertContentEqual(
+            policy.getAllowedInformationTypes(),
+            branch.getAllowedInformationTypes(branch.owner))
+        self.assertNotIn(
+            InformationType.PROPRIETARY,
+            branch.getAllowedInformationTypes(branch.owner))
+
+    def test_admin_sees_namespace_types(self):
+        # An admin sees all the types, since they occasionally need to
+        # override the namespace rules. This is hopefully temporary, and
+        # can go away once the new sharing rules (granting
+        # non-commercial projects limited use of private branches) are
+        # deployed.
+        branch = self.factory.makeBranch()
+        admin = self.factory.makeAdministrator()
+        self.assertContentEqual(
+            PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES,
+            branch.getAllowedInformationTypes(admin))
+        self.assertIn(
+            InformationType.PROPRIETARY,
+            branch.getAllowedInformationTypes(admin))
+
+
 class TestBranchSetPrivate(TestCaseWithFactory):
     """Test IBranch.setPrivate."""
 
@@ -2507,101 +2542,6 @@
             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_commercial_project(self):
-        # Branches linked to commercial projects can be private.
-        product = self.factory.makeProduct()
-        self.factory.makeCommercialSubscription(product)
-        branch = self.factory.makeProductBranch(product=product)
-        user = self.factory.makePerson()
-        self.assertTrue(branch.canBePrivate(user))
-
-    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_arbitrary_branch(self):
-        # By default branches can be public.
-        branch = self.factory.makeBranch(
-            information_type=InformationType.USERDATA)
-        self.assertTrue(branch.canBePublic(branch.owner))
-
-    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/code/templates/product-branches.pt'
--- lib/lp/code/templates/product-branches.pt	2012-03-10 13:48:37 +0000
+++ lib/lp/code/templates/product-branches.pt	2012-07-12 09:33:20 +0000
@@ -19,19 +19,20 @@
     <div id="branch-portlet"
          tal:condition="not: context/codehosting_usage/enumvalue:UNKNOWN">
       <div id="privacy"
-           tal:define="are_private view/new_branches_are_private"
-           tal:attributes="class python: are_private and 'first portlet private' or 'first portlet public'">
-
-        <p tal:condition="not:view/new_branches_are_private" id="privacy-text">
-          New branches you create for <tal:name replace="context/displayname"/>
-          are <strong>public</strong> initially.
-        </p>
-
-        <p tal:condition="view/new_branches_are_private" id="privacy-text">
-          New branches you create for <tal:name replace="context/displayname"/>
-          are <strong>private</strong> initially.
-        </p>
-
+           tal:define="private_class python: 'private' if view.default_information_type_is_private else 'public'"
+           tal:attributes="class string:first portlet ${private_class}">
+        <span tal:condition="not: view/default_information_type"
+           id="privacy-text">
+          You can't create new branches for
+          <tal:name replace="context/displayname"/>.
+        </span>
+
+        <span tal:condition="view/default_information_type"
+           tal:attributes="class string:sprite ${private_class}"
+           id="privacy-text">
+          New branches for <tal:name replace="context/displayname"/> are
+          <strong tal:content="view/default_information_type_title" />.
+        </span>
       </div>
 
       <div id="involvement" class="portlet"


Follow ups