launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/edit-stacked-information-type/+merge/114767

This branch simplifies BranchEditView's handling of the information type of branches stacked on private branches. Previously it replaced the usual radio buttons with a disabled checkbox, and some text describing why it's locked. I've integrated this into the normal information type widget, so if the stacked-on branch is private it'll display only (branch.stacked_on.information_type, branch.information_type), allowing the user to make it consistent by switching to the matching type.

There's no replacement for the warning text describing why the choices are restricted, but the case is uncommon enough that we probably don't care. It's already restricted by branch visibility policies without a corresponding warning.
-- 
https://code.launchpad.net/~wgrant/launchpad/edit-stacked-information-type/+merge/114767
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/edit-stacked-information-type into lp:launchpad.

=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py	2012-07-12 21:56:46 +0000
+++ lib/lp/code/browser/branch.py	2012-07-13 01:13:22 +0000
@@ -1033,34 +1033,7 @@
 
     def setUpFields(self):
         super(BranchEditView, self).setUpFields()
-        # This is to prevent users from converting push/import
-        # branches to pull branches.
         branch = self.context
-        if branch.private:
-            # 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):
-                stacked_info_type = branch.stacked_on.information_type.title
-                private_info = Bool(
-                    __name__="private",
-                    title=_("Branch is %s" % stacked_info_type),
-                    description=_(
-                        "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 = (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"'))
-
         # If the user can administer branches, then they should be able to
         # assign the ownership of the branch to any valid person or team.
         if check_permission('launchpad.Admin', branch):
@@ -1109,27 +1082,33 @@
         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
+
+        # If we're stacked on a private branch, only show that
+        # information type.
+        if self.context.stacked_on and self.context.stacked_on.private:
+            shown_types = set([self.context.stacked_on.information_type])
+        else:
+            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.
         combined_types = set(allowed_types).intersection(shown_types)

=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py	2012-07-12 09:13:16 +0000
+++ lib/lp/code/browser/tests/test_branch.py	2012-07-13 01:13:22 +0000
@@ -985,6 +985,23 @@
         self.assertShownTypes(
             [InformationType.PUBLIC, InformationType.PROPRIETARY], branch)
 
+    def test_stacked_on_private(self):
+        # A branch stacked on a private branch has its choices limited
+        # to the current type and the stacked-on type.
+        product = self.factory.makeProduct()
+        stacked_on_branch = self.factory.makeBranch(
+            product=product, information_type=InformationType.USERDATA)
+        branch = self.factory.makeBranch(
+            product=product, stacked_on=stacked_on_branch,
+            owner=product.owner,
+            information_type=InformationType.EMBARGOEDSECURITY)
+        with admin_logged_in():
+            branch.product.setBranchVisibilityTeamPolicy(
+                branch.owner, BranchVisibilityRule.PRIVATE)
+        self.assertShownTypes(
+            [InformationType.EMBARGOEDSECURITY, InformationType.USERDATA],
+            branch)
+
     def test_private_branch(self):
         # Branches on projects with a private policy can be set to
         # User Data (aka. Private)