launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/more-branchnamespace into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/more-branchnamespace/+merge/114563

Dispose of BranchNamespace.areNewBranchesPrivate. It's replaced by getDefaultInformationType.

As a side-effect this now says "You can't create new branches" if you can't create new branches, rather than saying that branches will be public. The three states look like <http://people.canonical.com/~wgrant/launchpad/privacies.png>.
-- 
https://code.launchpad.net/~wgrant/launchpad/more-branchnamespace/+merge/114563
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/more-branchnamespace into lp:launchpad.

=== 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 05:13:22 +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_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 05:13:22 +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/branchnamespace.py'
--- lib/lp/code/interfaces/branchnamespace.py	2012-07-11 22:16:47 +0000
+++ lib/lp/code/interfaces/branchnamespace.py	2012-07-12 05:13:22 +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/branchnamespace.py'
--- lib/lp/code/model/branchnamespace.py	2012-07-11 22:16:47 +0000
+++ lib/lp/code/model/branchnamespace.py	2012-07-12 05:13:22 +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/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 05:13:22 +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"