launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #10301
[Merge] lp:~wgrant/launchpad/branch-informationtype-refactor into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/branch-informationtype-refactor into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/branch-informationtype-refactor/+merge/116805
This branch begins the dismantling of InformationTypeVocabulary, as part of the termination of its short life of implementing information type rules.
We're not using the Proprietary information type in production yet; it has special rules, and will only be enabled for some projects. But BranchNamespace methods return it even for projects where it's disabled, and BranchEditView uses InformationTypeVocabulary's overcomplicated abstraction-violating logic to prevent it from appearing in the UI. I've moved the Proprietary exclusion down into BranchNamespace, so BranchEditView no longer relies on InformationTypeVocabulary to remove it.
I welcome suggestions of a better name for FREE_INFORMATION_TYPES. INFORMATION_TYPES_THAT_ARENT_PROPRIETARY is a bit unwieldy.
--
https://code.launchpad.net/~wgrant/launchpad/branch-informationtype-refactor/+merge/116805
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/branch-informationtype-refactor into lp:launchpad.
=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py 2012-07-17 06:34:59 +0000
+++ lib/lp/code/browser/branch.py 2012-07-26 07:30:31 +0000
@@ -720,6 +720,8 @@
@cachedproperty
def schema(self):
+ info_types = self.getInformationTypesToShow()
+
class BranchEditSchema(Interface):
"""Defines the fields for the edit form.
@@ -738,7 +740,7 @@
])
information_type = copy_field(
IBranch['information_type'], readonly=False,
- vocabulary=InformationTypeVocabulary(self.context))
+ vocabulary=InformationTypeVocabulary(types=info_types))
reviewer = copy_field(IBranch['reviewer'], required=True)
owner = copy_field(IBranch['owner'], readonly=False)
return BranchEditSchema
@@ -1108,21 +1110,6 @@
combined_types.add(self.context.information_type)
return combined_types
- def setUpWidgets(self, context=None):
- super(BranchEditView, self).setUpWidgets()
- if self.form_fields.get('information_type') is not None:
- # Customise the set of shown types.
- types_to_show = self.getInformationTypesToShow()
- # Grab the types 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.
- info_type_vocab = self.widgets['information_type'].vocabulary
- 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
# pseudo project.
=== modified file 'lib/lp/code/model/branchnamespace.py'
--- lib/lp/code/model/branchnamespace.py 2012-07-24 06:39:54 +0000
+++ lib/lp/code/model/branchnamespace.py 2012-07-26 07:30:31 +0000
@@ -48,7 +48,6 @@
from lp.registry.enums import (
BranchSharingPolicy,
InformationType,
- PRIVATE_INFORMATION_TYPES,
PUBLIC_INFORMATION_TYPES,
)
from lp.registry.errors import (
@@ -82,13 +81,16 @@
)
+FREE_PRIVATE_INFORMATION_TYPES = (
+ InformationType.PRIVATESECURITY, InformationType.USERDATA)
+FREE_INFORMATION_TYPES = (
+ PUBLIC_INFORMATION_TYPES + FREE_PRIVATE_INFORMATION_TYPES)
+
POLICY_ALLOWED_TYPES = {
BranchSharingPolicy.PUBLIC: PUBLIC_INFORMATION_TYPES,
- BranchSharingPolicy.PUBLIC_OR_PROPRIETARY:
- PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES,
- BranchSharingPolicy.PROPRIETARY_OR_PUBLIC:
- PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES,
- BranchSharingPolicy.PROPRIETARY: PRIVATE_INFORMATION_TYPES,
+ BranchSharingPolicy.PUBLIC_OR_PROPRIETARY: FREE_INFORMATION_TYPES,
+ BranchSharingPolicy.PROPRIETARY_OR_PUBLIC: FREE_INFORMATION_TYPES,
+ BranchSharingPolicy.PROPRIETARY: FREE_PRIVATE_INFORMATION_TYPES,
}
POLICY_DEFAULT_TYPES = {
@@ -326,7 +328,7 @@
# Private teams get private branches, everyone else gets public ones.
if (self.owner.is_team
and self.owner.visibility == PersonVisibility.PRIVATE):
- return PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES
+ return FREE_INFORMATION_TYPES
else:
return PUBLIC_INFORMATION_TYPES
@@ -454,7 +456,7 @@
if public:
types.extend(PUBLIC_INFORMATION_TYPES)
if private:
- types.extend(PRIVATE_INFORMATION_TYPES)
+ types.extend(FREE_PRIVATE_INFORMATION_TYPES)
return types
def getDefaultInformationType(self):
=== modified file 'lib/lp/code/model/tests/test_branchnamespace.py'
--- lib/lp/code/model/tests/test_branchnamespace.py 2012-07-13 08:29:56 +0000
+++ lib/lp/code/model/tests/test_branchnamespace.py 2012-07-26 07:30:31 +0000
@@ -32,6 +32,8 @@
)
from lp.code.interfaces.branchtarget import IBranchTarget
from lp.code.model.branchnamespace import (
+ FREE_INFORMATION_TYPES,
+ FREE_PRIVATE_INFORMATION_TYPES,
PackageNamespace,
PersonalNamespace,
ProductNamespace,
@@ -39,7 +41,6 @@
from lp.registry.enums import (
BranchSharingPolicy,
InformationType,
- PRIVATE_INFORMATION_TYPES,
PUBLIC_INFORMATION_TYPES,
SharingPermission,
)
@@ -482,8 +483,7 @@
namespace = self.makeProductNamespace(
BranchSharingPolicy.PUBLIC_OR_PROPRIETARY)
self.assertContentEqual(
- PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES,
- namespace.getAllowedInformationTypes())
+ FREE_INFORMATION_TYPES, namespace.getAllowedInformationTypes())
self.assertEqual(
InformationType.PUBLIC, namespace.getDefaultInformationType())
@@ -501,8 +501,7 @@
namespace.product, namespace.owner, namespace.product.owner,
{InformationType.USERDATA: SharingPermission.ALL})
self.assertContentEqual(
- PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES,
- namespace.getAllowedInformationTypes())
+ FREE_INFORMATION_TYPES, namespace.getAllowedInformationTypes())
self.assertEqual(
InformationType.USERDATA, namespace.getDefaultInformationType())
@@ -512,7 +511,7 @@
self.assertContentEqual([], namespace.getAllowedInformationTypes())
self.assertIs(None, namespace.getDefaultInformationType())
- def test_proprietary_grantor(self):
+ def test_proprietary_grantee(self):
namespace = self.makeProductNamespace(
BranchSharingPolicy.PROPRIETARY)
with admin_logged_in():
@@ -520,7 +519,8 @@
namespace.product, namespace.owner, namespace.product.owner,
{InformationType.USERDATA: SharingPermission.ALL})
self.assertContentEqual(
- PRIVATE_INFORMATION_TYPES, namespace.getAllowedInformationTypes())
+ FREE_PRIVATE_INFORMATION_TYPES,
+ namespace.getAllowedInformationTypes())
self.assertEqual(
InformationType.USERDATA, namespace.getDefaultInformationType())
@@ -1051,7 +1051,7 @@
team = self.factory.makeTeam(visibility=PersonVisibility.PRIVATE)
namespace = PersonalNamespace(team)
self.assertContentEqual(
- PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES,
+ FREE_INFORMATION_TYPES,
namespace.getAllowedInformationTypes())
@@ -1110,10 +1110,8 @@
team = self.factory.makeTeam(owner=person)
self.product.setBranchVisibilityTeamPolicy(
team, BranchVisibilityRule.PRIVATE)
- self.assertTypes(
- person, PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES)
- self.assertTypes(
- team, PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES)
+ self.assertTypes(person, FREE_INFORMATION_TYPES)
+ self.assertTypes(team, FREE_INFORMATION_TYPES)
def test_team_member_with_private_only_rule(self):
# If a person is a member of a team that has a PRIVATE_ONLY rule, then
@@ -1124,8 +1122,8 @@
None, BranchVisibilityRule.FORBIDDEN)
self.product.setBranchVisibilityTeamPolicy(
team, BranchVisibilityRule.PRIVATE_ONLY)
- self.assertTypes(person, PRIVATE_INFORMATION_TYPES)
- self.assertTypes(team, PRIVATE_INFORMATION_TYPES)
+ self.assertTypes(person, FREE_PRIVATE_INFORMATION_TYPES)
+ self.assertTypes(team, FREE_PRIVATE_INFORMATION_TYPES)
def test_non_team_member_with_private_rule(self):
# If a person is a not a member of a team that has a privacy rule,
@@ -1146,12 +1144,9 @@
team_1, BranchVisibilityRule.PRIVATE)
self.product.setBranchVisibilityTeamPolicy(
team_2, BranchVisibilityRule.PRIVATE)
- self.assertTypes(
- person, PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES)
- self.assertTypes(
- team_1, PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES)
- self.assertTypes(
- team_2, PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES)
+ self.assertTypes(person, FREE_INFORMATION_TYPES)
+ self.assertTypes(team_1, FREE_INFORMATION_TYPES)
+ self.assertTypes(team_2, FREE_INFORMATION_TYPES)
def test_team_member_with_multiple_differing_private_rules(self):
# If a person is a member of multiple teams that has a privacy rules,
@@ -1165,11 +1160,8 @@
private_team, BranchVisibilityRule.PRIVATE)
self.product.setBranchVisibilityTeamPolicy(
public_team, BranchVisibilityRule.PUBLIC)
- self.assertTypes(
- person, PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES)
- self.assertTypes(
- private_team,
- PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES)
+ self.assertTypes(person, FREE_INFORMATION_TYPES)
+ self.assertTypes(private_team, FREE_INFORMATION_TYPES)
self.assertTypes(public_team, PUBLIC_INFORMATION_TYPES)
=== modified file 'lib/lp/registry/tests/test_information_type_vocabulary.py'
--- lib/lp/registry/tests/test_information_type_vocabulary.py 2012-07-17 03:57:26 +0000
+++ lib/lp/registry/tests/test_information_type_vocabulary.py 2012-07-26 07:30:31 +0000
@@ -51,6 +51,13 @@
for info_type in PUBLIC_INFORMATION_TYPES:
self.assertNotIn(info_type, vocab)
+ def test_vocabulary_items_custom(self):
+ # The vocab can be given a custom set of types to include.
+ vocab = InformationTypeVocabulary(
+ types=[InformationType.PUBLICSECURITY, InformationType.USERDATA])
+ self.assertIn(InformationType.USERDATA, vocab)
+ self.assertNotIn(InformationType.PUBLIC, vocab)
+
def test_getTermByToken(self):
vocab = InformationTypeVocabulary()
self.assertThat(
=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py 2012-07-20 03:15:04 +0000
+++ lib/lp/registry/vocabularies.py 2012-07-26 07:30:31 +0000
@@ -2244,7 +2244,7 @@
implements(IEnumeratedType)
- def __init__(self, context=None, public_only=False, private_only=False):
+ def _calculateTypes(self, context, public_only, private_only):
types = []
if not public_only:
types = [
@@ -2284,6 +2284,12 @@
not context.private_bugs)):
types = [InformationType.PUBLIC,
InformationType.PUBLICSECURITY] + types
+ return types
+
+ def __init__(self, context=None, public_only=False, private_only=False,
+ types=None):
+ if types is None:
+ types = self._calculateTypes(context, public_only, private_only)
terms = []
for type in types:
Follow ups
