launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #09709
[Merge] lp:~wallyworld/launchpad/proprietary-information-type-933782 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/proprietary-information-type-933782 into lp:launchpad with lp:~wallyworld/launchpad/make-private-branches-527900 as a prerequisite.
Requested reviews:
Curtis Hovey (sinzui)
Related bugs:
Bug #933782 in Launchpad itself: "'Proprietary' information type is only valid for projects with commercial subscriptions"
https://bugs.launchpad.net/launchpad/+bug/933782
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/proprietary-information-type-933782/+merge/113890
== Implementation ==
This branch follows on from the previous one which allowed branches to be marked private under controlled circumstances.
Proprietary is an information type which is restricted for use with commercial projects. But the vocab and code didn't know this, so a feature flag was needed to hide this information type as an option. This mp teaches the code how to properly use Proprietary so the feature flag can be removed.
There are 2 main changes:
1. InformationTypeVocabulary
If the context is a product with a current commercial subscription, include Proprietary in the results, else not.
2. When a product has a voucher redeemed to create a commercial subscription, then an AccessPolicy needs to be created for Proprietary. A little refactoring was done to make this logic handle existing policies etc.
The main places which need to be aware of, and use, the new vocab:
- branch edit page
- bug edit page
- sharing page
The sharing page required no changes, since the service just iterates across all access policies for the pillar. The bug and branch pages just needed to ensure the data mode for their information type portlets was correctly initialised so that Proprietary was included.
Also, if a bug or branch is already proprietary, the vocab needs to contain Proprietary even if the project's subscription has lapsed, since the bug/branch still needs to be editable.
Next step: a garbo job to create Proprietary access policies for all current commercial projects.
== Tests ==
Add tests:
test_bug_views - test that the +secrecy view contains Proprietary for commercial projects
test_bugtarget_filebug - the +filebug forms contain Proprietary when filing a bug for commercial projects
test_bugview - the JSON request cache contains the correct information types for commercial / non commercial projects
test_branch - the branch edit view contains Proprietary for commercial projects
test_product - when a commercial subscription is added to a product, a Proprietary access policy is created
test_information_type_vocabulary - the vocabulary contains Proprietary in the correct circumstances
== Lint ==
Lint free, but the merge of devel has confused lint.sh
--
https://code.launchpad.net/~wallyworld/launchpad/proprietary-information-type-933782/+merge/113890
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py 2012-07-03 07:06:48 +0000
+++ lib/lp/bugs/browser/bug.py 2012-07-09 11:40:42 +0000
@@ -826,7 +826,7 @@
class information_type_schema(Interface):
information_type_field = copy_field(
IBug['information_type'], readonly=False,
- vocabulary=InformationTypeVocabulary())
+ vocabulary=InformationTypeVocabulary(self.context))
return information_type_schema
@property
=== modified file 'lib/lp/bugs/browser/tests/test_bug_views.py'
--- lib/lp/bugs/browser/tests/test_bug_views.py 2012-07-03 07:06:48 +0000
+++ lib/lp/bugs/browser/tests/test_bug_views.py 2012-07-09 11:40:42 +0000
@@ -402,7 +402,6 @@
# Test that the view creates the vocabulary correctly.
bug = self.factory.makeBug()
feature_flags = {
- 'disclosure.proprietary_information_type.disabled': 'on',
'disclosure.display_userdata_as_private.enabled': 'on'}
with FeatureFixture(feature_flags):
with person_logged_in(bug.owner):
@@ -413,7 +412,20 @@
soup = BeautifulSoup(html)
self.assertEqual(u'Private', soup.find('label', text="Private"))
self.assertIs(None, soup.find('label', text="User Data"))
- self.assertIs(None, soup.find('label', text="Proprietary"))
+
+ def test_information_type_vocabulary_commercial_project(self):
+ # Test that the view creates the vocabulary correctly for commercial
+ # projects.
+ product = self.factory.makeProduct()
+ self.factory.makeCommercialSubscription(product)
+ bug = self.factory.makeBug(product=product)
+ with person_logged_in(bug.owner):
+ view = create_initialized_view(
+ bug.default_bugtask, name='+secrecy',
+ principal=bug.owner)
+ html = view.render()
+ soup = BeautifulSoup(html)
+ self.assertIsNot(None, soup.find('label', text="Proprietary"))
class TestBugTextViewPrivateTeams(TestCaseWithFactory):
=== modified file 'lib/lp/bugs/browser/tests/test_bugtarget_filebug.py'
--- lib/lp/bugs/browser/tests/test_bugtarget_filebug.py 2012-07-05 00:49:00 +0000
+++ lib/lp/bugs/browser/tests/test_bugtarget_filebug.py 2012-07-09 11:40:42 +0000
@@ -29,6 +29,7 @@
PRIVATE_INFORMATION_TYPES,
PUBLIC_INFORMATION_TYPES,
)
+from lp.registry.vocabularies import InformationTypeVocabulary
from lp.services.features.testing import FeatureFixture
from lp.services.webapp.servers import LaunchpadTestRequest
from lp.testing import (
@@ -396,7 +397,6 @@
# The vocabulary for information_type when filing a bug is created
# correctly when 'User Data' is to be replaced by 'Private'.
feature_flags = {
- 'disclosure.proprietary_information_type.disabled': 'on',
'disclosure.display_userdata_as_private.enabled': 'on'}
product = self.factory.makeProduct(official_malone=True)
with FeatureFixture(feature_flags):
@@ -409,6 +409,29 @@
self.assertIsNone(soup.find('label', text="User Data"))
self.assertIsNone(soup.find('label', text="Proprietary"))
+ def test_filebug_information_type_commercial_projects(self):
+ # The vocabulary for information_type when filing a bug is created
+ # correctly when 'User Data' is to be replaced by 'Private'.
+ product = self.factory.makeProduct(official_malone=True)
+ self.factory.makeCommercialSubscription(product)
+ with person_logged_in(product.owner):
+ view = create_initialized_view(
+ product, '+filebug', principal=product.owner)
+ html = view.render()
+ soup = BeautifulSoup(html)
+ self.assertIsNotNone(soup.find('label', text="Proprietary"))
+
+ def test_filebug_information_type_normal_projects(self):
+ # The vocabulary for information_type when filing a bug is created
+ # correctly when 'User Data' is to be replaced by 'Private'.
+ product = self.factory.makeProduct(official_malone=True)
+ with person_logged_in(product.owner):
+ view = create_initialized_view(
+ product, '+filebug', principal=product.owner)
+ html = view.render()
+ soup = BeautifulSoup(html)
+ self.assertIsNone(soup.find('label', text="Proprietary"))
+
def test_filebug_information_type_vocabulary(self):
# The vocabulary for information_type when filing a bug is created
# correctly.
@@ -418,7 +441,7 @@
product, '+filebug', principal=product.owner)
html = view.render()
soup = BeautifulSoup(html)
- for info_type in InformationType:
+ for info_type in InformationTypeVocabulary(product):
self.assertIsNotNone(soup.find('label', text=info_type.title))
def test_filebug_view_renders_info_type_widget(self):
@@ -444,7 +467,7 @@
product, '+filebug', principal=product.owner)
html = view.render()
soup = BeautifulSoup(html)
- for info_type in PRIVATE_INFORMATION_TYPES:
+ for info_type in InformationTypeVocabulary(private_only=True):
self.assertIsNotNone(soup.find('label', text=info_type.title))
for info_type in PUBLIC_INFORMATION_TYPES:
self.assertIsNone(soup.find('label', text=info_type.title))
@@ -650,10 +673,7 @@
self.assertEqual(
bugtask_importance_data, cache['bugtask_importance_data'])
bugtask_info_type_data = []
- information_types = list(PRIVATE_INFORMATION_TYPES)
- if not private_only:
- information_types.extend(PUBLIC_INFORMATION_TYPES)
- for item in information_types:
+ for item in InformationTypeVocabulary(private_only=private_only):
new_item = {'name': item.title, 'value': item.name,
'description': item.description,
'description_css_class': 'choice-description'}
=== modified file 'lib/lp/bugs/browser/tests/test_bugview.py'
--- lib/lp/bugs/browser/tests/test_bugview.py 2012-07-03 07:06:48 +0000
+++ lib/lp/bugs/browser/tests/test_bugview.py 2012-07-09 11:40:42 +0000
@@ -90,20 +90,39 @@
'private information.',
view.information_type_description)
- def test_proprietary_hidden(self):
- # When the proprietary_information_type.disabled feature flag is
- # enabled, it isn't in the JSON request cache.
- feature_flag = {
- 'disclosure.proprietary_information_type.disabled': 'on'}
- with FeatureFixture(feature_flag):
- view = BugView(self.bug, LaunchpadTestRequest())
- view.initialize()
- cache = IJSONRequestCache(view.request)
- expected = [
- InformationType.PUBLIC.name,
- InformationType.UNEMBARGOEDSECURITY.name,
- InformationType.EMBARGOEDSECURITY.name,
- InformationType.USERDATA.name]
- self.assertContentEqual(expected, [
- type['value']
- for type in cache.objects['information_type_data']])
+ def test_proprietary_excluded_for_normal_projects(self):
+ # The Proprietary information type isn't in the JSON request cache for
+ # projects without commercial subscriptions.
+ # projects with commercial subscriptions.
+ product = self.factory.makeProduct(official_malone=True)
+ bug = self.factory.makeBug(product=product)
+ view = BugView(bug, LaunchpadTestRequest())
+ view.initialize()
+ cache = IJSONRequestCache(view.request)
+ expected = [
+ InformationType.PUBLIC.name,
+ InformationType.UNEMBARGOEDSECURITY.name,
+ InformationType.EMBARGOEDSECURITY.name,
+ InformationType.USERDATA.name]
+ self.assertContentEqual(expected, [
+ type['value']
+ for type in cache.objects['information_types']])
+
+ def test_proprietary_included_for_commercial_projects(self):
+ # The Proprietary information type is in the JSON request cache for
+ # projects with commercial subscriptions.
+ product = self.factory.makeProduct(official_malone=True)
+ self.factory.makeCommercialSubscription(product)
+ bug = self.factory.makeBug(product=product)
+ view = BugView(bug, LaunchpadTestRequest())
+ view.initialize()
+ cache = IJSONRequestCache(view.request)
+ expected = [
+ InformationType.PUBLIC.name,
+ InformationType.UNEMBARGOEDSECURITY.name,
+ InformationType.EMBARGOEDSECURITY.name,
+ InformationType.USERDATA.name,
+ InformationType.PROPRIETARY.name]
+ self.assertContentEqual(expected, [
+ type['value']
+ for type in cache.objects['information_types']])
=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py 2012-07-09 11:40:40 +0000
+++ lib/lp/code/browser/tests/test_branch.py 2012-07-09 11:40:42 +0000
@@ -907,8 +907,8 @@
self.assertEquals(team, branch.owner)
def test_information_type_in_ui(self):
- # The information_type of a branch can be changed via the UI when
- # the feature flag is enabled.
+ # The information_type of a branch can be changed via the UI by an
+ # authorised user.
person = self.factory.makePerson()
branch = self.factory.makeProductBranch(owner=person)
admins = getUtility(ILaunchpadCelebrities).admin
@@ -921,20 +921,25 @@
self.assertEqual(
InformationType.EMBARGOEDSECURITY, branch.information_type)
- def test_information_type_in_ui_vocabulary(self):
- # The vocabulary that Branch:+edit uses for the information_type
- # has been correctly created.
- person = self.factory.makePerson()
+ 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=person)
- feature_flag = {
- 'disclosure.proprietary_information_type.disabled': 'on'}
- admins = getUtility(ILaunchpadCelebrities).admin
- admin = admins.teamowner
- with FeatureFixture(feature_flag):
- browser = self.getUserBrowser(
- canonical_url(branch) + '/+edit', user=admin)
+ 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")
def test_can_not_change_privacy_of_stacked_on_private(self):
=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py 2012-07-06 19:00:25 +0000
+++ lib/lp/registry/model/product.py 2012-07-09 11:40:42 +0000
@@ -557,6 +557,24 @@
self.checkPrivateBugsTransitionAllowed(private_bugs, user)
self.private_bugs = private_bugs
+ def _ensurePolicies(self, information_types):
+ # Ensure that the product has access policies for the specified
+ # information types.
+ aps = getUtility(IAccessPolicySource)
+ existing_policies = aps.findByPillar([self])
+ existing_types = set([
+ access_policy.type for access_policy in existing_policies])
+ # Create the missing policies.
+ required_types = set(information_types).difference(existing_types)
+ policies = itertools.product((self,), required_types)
+ policies = getUtility(IAccessPolicySource).create(policies)
+
+ # Add the maintainer to the policies.
+ grants = []
+ for p in policies:
+ grants.append((p, self.owner, self.owner))
+ getUtility(IAccessPolicyGrantSource).grant(grants)
+
@cachedproperty
def commercial_subscription(self):
return CommercialSubscription.selectOneBy(product=self)
@@ -627,6 +645,10 @@
self.commercial_subscription.registrant = registrant
self.commercial_subscription.purchaser = purchaser
+ # The product now has a commercial subscription, so we need to ensure
+ # it has a Proprietary access policy.
+ self._ensurePolicies([InformationType.PROPRIETARY])
+
@property
def qualifies_for_free_hosting(self):
"""See `IProduct`."""
@@ -1515,17 +1537,8 @@
product.development_focus = trunk
# Add default AccessPolicies.
- policies = itertools.product(
- (product,), (InformationType.USERDATA,
+ product._ensurePolicies((InformationType.USERDATA,
InformationType.EMBARGOEDSECURITY))
- policies = getUtility(IAccessPolicySource).create(policies)
-
- # Add the maintainer to the default policies.
- grants = []
- for p in policies:
- grants.append((p, owner, owner))
- getUtility(IAccessPolicyGrantSource).grant(grants)
-
return product
def forReview(self, search_text=None, active=None,
=== modified file 'lib/lp/registry/tests/test_information_type_vocabulary.py'
--- lib/lp/registry/tests/test_information_type_vocabulary.py 2012-07-09 11:40:40 +0000
+++ lib/lp/registry/tests/test_information_type_vocabulary.py 2012-07-09 11:40:42 +0000
@@ -2,6 +2,8 @@
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test the Distribution Source Package vocabulary."""
+from zope.security.proxy import removeSecurityProxy
+from lp.testing._login import person_logged_in
__metaclass__ = type
@@ -58,16 +60,6 @@
title='Public',
description=InformationType.PUBLIC.description))
- 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(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()
@@ -75,7 +67,7 @@
self.assertRaises(
LookupError, vocab.getTermByToken, 'PROPRIETARY')
- def test_proprietary_enabled(self):
+ def test_proprietary_enabled_for_commercial_projects(self):
# Only projects with commercial subscriptions have PROPRIETARY.
product = self.factory.makeProduct()
self.factory.makeCommercialSubscription(product)
@@ -83,6 +75,17 @@
term = vocab.getTermByToken('PROPRIETARY')
self.assertEqual('Proprietary', term.title)
+ def test_proprietary_enabled_for_contexts_already_proprietary(self):
+ # The vocabulary has PROPRIETARY for contexts which are already
+ # proprietary.
+ owner = self.factory.makePerson()
+ bug = self.factory.makeBug(
+ owner=owner, information_type=InformationType.PROPRIETARY)
+ with person_logged_in(owner):
+ vocab = InformationTypeVocabulary(bug)
+ term = vocab.getTermByToken('PROPRIETARY')
+ self.assertEqual('Proprietary', term.title)
+
def test_display_userdata_as_private(self):
feature_flag = {
'disclosure.display_userdata_as_private.enabled': 'on'}
=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py 2012-06-27 14:37:14 +0000
+++ lib/lp/registry/tests/test_product.py 2012-07-09 11:40:42 +0000
@@ -374,6 +374,33 @@
grantees = set([grant.grantee for grant in grants])
self.assertEqual(expected_grantess, grantees)
+ def test_redeemSubscription_creates_proprietary_policy(self):
+ # Creating a commercial subscription for a product also creates a
+ # Proprietary policy.
+ product = self.factory.makeProduct()
+ product.redeemSubscriptionVoucher(
+ 'hello', product.owner, product.owner, 1)
+
+ ap = getUtility(IAccessPolicySource).findByPillar((product,))
+ expected = [
+ InformationType.USERDATA, InformationType.EMBARGOEDSECURITY,
+ InformationType.PROPRIETARY]
+ self.assertContentEqual(expected, [policy.type for policy in ap])
+
+ def test_redeemSubscription_grants_maintainer_access(self):
+ # Creating a commercial subscription for a product creates an access
+ # grant on the Proprietary policy for the maintainer.
+ product = self.factory.makeProduct()
+ product.redeemSubscriptionVoucher(
+ 'hello', product.owner, product.owner, 1)
+
+ policies = getUtility(IAccessPolicySource).find(
+ [(product, InformationType.PROPRIETARY)])
+ grants = getUtility(IAccessPolicyGrantSource).findByPolicy(policies)
+ expected_grantess = set([product.owner])
+ grantees = set([grant.grantee for grant in grants])
+ self.assertEqual(expected_grantess, grantees)
+
class TestProductFiles(TestCase):
"""Tests for downloadable product files."""
=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py 2012-07-09 11:40:40 +0000
+++ lib/lp/registry/vocabularies.py 2012-07-09 11:40:42 +0000
@@ -2244,33 +2244,41 @@
implements(IEnumeratedType)
- def __init__(self, context=None):
- types = [
- InformationType.EMBARGOEDSECURITY,
- InformationType.USERDATA]
- proprietary_disabled = bool(getFeatureFlag(
- 'disclosure.proprietary_information_type.disabled'))
- show_userdata_as_private = bool(getFeatureFlag(
- 'disclosure.display_userdata_as_private.enabled'))
- # 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)
+ def __init__(self, context=None, public_only=False, private_only=False):
+ types = []
+ if not public_only:
+ types = [
+ InformationType.EMBARGOEDSECURITY,
+ InformationType.USERDATA]
+ show_userdata_as_private = bool(getFeatureFlag(
+ 'disclosure.display_userdata_as_private.enabled'))
+ # 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
+ # - contexts which already have an information type set to
+ # proprietary
+ subscription_context = context
+ if IBug.providedBy(context) and len(context.affected_pillars) == 1:
+ subscription_context = context.affected_pillars[0]
+ elif (IBugTask.providedBy(context)
+ and len(context.bug.affected_pillars) == 1):
+ subscription_context = context.pillar
+ elif IBranch.providedBy(context):
+ subscription_context = context.target.context
+ has_commercial_subscription = (
+ IProduct.providedBy(subscription_context) and
+ subscription_context.has_current_commercial_subscription)
+ already_proprietary = (
+ safe_hasattr(context, 'information_type')
+ and context.information_type == InformationType.PROPRIETARY)
+ if has_commercial_subscription or already_proprietary:
+ types.append(InformationType.PROPRIETARY)
# Disallow public items for projects with private bugs.
- if (context is None or
+ if (not private_only and (context is None or
not IProduct.providedBy(context) or
- not context.private_bugs):
+ not context.private_bugs)):
types = [InformationType.PUBLIC,
InformationType.UNEMBARGOEDSECURITY] + types
=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py 2012-06-26 10:19:37 +0000
+++ lib/lp/services/features/flags.py 2012-07-09 11:40:42 +0000
@@ -314,12 +314,6 @@
'',
'',
''),
- ('disclosure.proprietary_information_type.disabled',
- 'boolean',
- 'If true, disables the PROPRIETARY information_type for bugs.',
- '',
- '',
- ''),
('disclosure.display_userdata_as_private.enabled',
'boolean',
'If true, displays the USERDATA information_type as Private.',
Follow ups
