launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #14511
[Merge] lp:~abentley/launchpad/transitive-confidential into lp:launchpad
Aaron Bentley has proposed merging lp:~abentley/launchpad/transitive-confidential into lp:launchpad.
Requested reviews:
Abel Deuring (adeuring)
Aaron Bentley (abentley)
Related bugs:
Bug #1079785 in Launchpad itself: "public artifacts are permitted on confidential projects"
https://bugs.launchpad.net/launchpad/+bug/1079785
For more details, see:
https://code.launchpad.net/~abentley/launchpad/transitive-confidential/+merge/135926
= Summary =
Fix bug #1079785: public artifacts are permitted on confidential projects
== Proposed fix ==
Move checks from view to model, raise exceptions, include private non-proprietary types
== Pre-implementation notes ==
None
== LOC Rationale ==
Reduces LOC
== Implementation details ==
Revert 16258 and migrate code from getPublicWarning. Use early exit to separate on-change code from on-creation code.
Tweak to check for USERDATA and PRIVATESECURITY bugs.
Remove getPublicWarning and related code and tests.
== Tests ==
bin/test -t test_change_info_type_proprietary_check_artifacts -t test_change_info_type_proprietary_check_translations
== Demo and Q/A ==
Create a public product with other/proprietary license. Create a public branch, bug, blueprint. Enable translations. You should not be able to change its information type to proprietary until you've made all artifacts private and disabled translations.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/registry/browser/product.py
lib/lp/registry/browser/tests/test_product.py
lib/lp/registry/model/product.py
lib/lp/registry/tests/test_product.py
--
https://code.launchpad.net/~abentley/launchpad/transitive-confidential/+merge/135926
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py 2012-11-15 16:03:56 +0000
+++ lib/lp/registry/browser/product.py 2012-11-23 15:16:34 +0000
@@ -105,7 +105,6 @@
InformationType,
PROPRIETARY_INFORMATION_TYPES,
PUBLIC_PROPRIETARY_INFORMATION_TYPES,
- PUBLIC_INFORMATION_TYPES,
ServiceUsage,
)
from lp.app.errors import NotFoundError
@@ -129,9 +128,6 @@
from lp.blueprints.browser.specificationtarget import (
HasSpecificationsMenuMixin,
)
-from lp.blueprints.enums import (
- SpecificationFilter,
- )
from lp.bugs.browser.bugtask import (
BugTargetTraversalMixin,
get_buglisting_search_filter_url,
@@ -1450,47 +1446,10 @@
@action("Change", name='change')
def change_action(self, action, data):
- old_info_type = self.context.information_type
try:
self.updateContextFromData(data)
except CannotChangeInformationType as e:
self.setFieldError('information_type', str(e))
- warning = self.getPublicWarning(old_info_type)
- if warning is not None:
- self.request.response.addWarningNotification(warning)
-
- def getPublicWarning(self, old_info_type):
- """Get warning if public artifacts might expose the product.
-
- :param old_info_type: The former information type of the product.
- """
- if (old_info_type != InformationType.PUBLIC or
- self.context.information_type == InformationType.PUBLIC):
- return None
- public_bugs = self.context.searchTasks(None,
- information_type=PUBLIC_INFORMATION_TYPES,
- omit_duplicates=False, omit_targeted=False)
- public_artifacts = []
- if not public_bugs.is_empty():
- public_artifacts.append('bugs')
- # Default returns all public branches.
- public_branches = self.context.getBranches()
- if not public_branches.is_empty():
- public_artifacts.append('branches')
- # All specs located by an ALL search are public.
- public_specs = self.context.specifications(
- None, filter=[SpecificationFilter.ALL])
- if not public_specs.is_empty():
- public_artifacts.append('blueprints')
- if len(public_artifacts) == 0:
- return None
- elif len(public_artifacts) < 3:
- list_phrase = ' and '.join(public_artifacts)
- else:
- list_phrase = 'bugs, branches, and blueprints'
- return ('Some %s are public. This project will not be fully '
- '%s until they have been changed.' %
- (list_phrase, self.context.information_type.title))
class ProductValidationMixin:
=== modified file 'lib/lp/registry/browser/tests/test_product.py'
--- lib/lp/registry/browser/tests/test_product.py 2012-11-15 16:03:56 +0000
+++ lib/lp/registry/browser/tests/test_product.py 2012-11-23 15:16:34 +0000
@@ -25,7 +25,6 @@
from lp.app.enums import (
InformationType,
PROPRIETARY_INFORMATION_TYPES,
- PUBLIC_PROPRIETARY_INFORMATION_TYPES,
ServiceUsage,
)
from lp.registry.browser.product import (
@@ -553,118 +552,6 @@
self.assertEqual(
InformationType.PUBLIC, updated_product.information_type)
- def test_warning_on_public_artifacts(self):
- # When changing to proprietary, any public artifacts will trigger a
- # warning.
- self.useFixture(FeatureFixture(
- {u'disclosure.private_projects.enabled': u'on'}))
- owner = self.factory.makePerson()
- product = self.factory.makeProduct(owner=owner)
- bug = self.factory.makeBug(target=product)
-
- def make_proprietary():
- with person_logged_in(None):
- browser = self.getViewBrowser(product, '+edit',
- user=product.owner)
- info_type = browser.getControl(name='field.information_type')
- info_type.value = ['PROPRIETARY']
- browser.getControl('Change').click()
- with person_logged_in(owner):
- product.information_type = InformationType.PUBLIC
- return browser
- browser = make_proprietary()
-
- def assertWarning(browser, text):
- warning_tag = Tag('warning', 'div', text=text,
- attrs={'class': 'warning message'})
- self.assertThat(browser.contents, HTMLContains(warning_tag))
- assertWarning(
- browser, 'Some bugs are public. This project will not be fully '
- 'Proprietary until they have been changed.')
- browser = make_proprietary()
- with person_logged_in(bug.owner):
- bug.transitionToInformationType(InformationType.PROPRIETARY,
- bug.owner)
- browser = make_proprietary()
- self.assertThat(browser.contents, Not(HTMLContains(
- Tag('warning', 'div', attrs={'class': 'warning message'}))))
-
- def test_getPublicWarning_artifacts(self):
- # The warning indicates what kinds of artifacts are public, in
- # grammatical English.
- owner = self.factory.makePerson()
- product = self.factory.makeProduct(owner=owner)
- bug = self.factory.makeBug(target=product)
- view = create_initialized_view(product, '+edit')
- with person_logged_in(owner):
- product.information_type = InformationType.PROPRIETARY
- warning = view.getPublicWarning(InformationType.PUBLIC)
- self.assertEqual(
- 'Some bugs are public. This project will not be fully '
- 'Proprietary until they have been changed.', warning)
- spec = self.factory.makeSpecification(product=product)
- warning = view.getPublicWarning(InformationType.PUBLIC)
- self.assertEqual(
- 'Some bugs and blueprints are public. This project will not be '
- 'fully Proprietary until they have been changed.', warning)
- branch = self.factory.makeBranch(product=product, owner=owner,
- information_type=InformationType.PUBLIC)
- warning = view.getPublicWarning(InformationType.PUBLIC)
- self.assertEqual(
- 'Some bugs, branches, and blueprints are public. This project '
- 'will not be fully Proprietary until they have been changed.',
- warning)
- with person_logged_in(owner):
- bug.transitionToInformationType(
- InformationType.PROPRIETARY, owner)
- warning = view.getPublicWarning(InformationType.PUBLIC)
- self.assertEqual(
- 'Some branches and blueprints are public. This project '
- 'will not be fully Proprietary until they have been changed.',
- warning)
- with person_logged_in(owner):
- branch.transitionToInformationType(
- InformationType.PROPRIETARY, owner)
- warning = view.getPublicWarning(InformationType.PUBLIC)
- self.assertEqual(
- 'Some blueprints are public. This project will not be fully '
- 'Proprietary until they have been changed.', warning)
- with person_logged_in(owner):
- spec.transitionToInformationType(
- InformationType.PROPRIETARY, owner)
- self.assertIs(None, view.getPublicWarning(InformationType.PUBLIC))
-
- def test_getPublicWarning_embargoed(self):
- # The message reflects an Embargoed product type.
- owner = self.factory.makePerson()
- product = self.factory.makeProduct(owner=owner)
- self.factory.makeBug(target=product)
- with person_logged_in(owner):
- product.information_type = InformationType.EMBARGOED
- view = create_initialized_view(product, '+edit')
- warning = view.getPublicWarning(InformationType.PUBLIC)
- self.assertEqual(
- 'Some bugs are public. This project will not be fully '
- 'Embargoed until they have been changed.', warning)
-
- def test_getPublicWarning_transition(self):
- # The warning is emitted only when transitioning from PUBLIC to a
- # proprietary type.
- owner = self.factory.makePerson()
- product = self.factory.makeProduct(owner=owner)
- self.factory.makeBug(target=product)
- view = create_initialized_view(product, '+edit')
- for from_type in PUBLIC_PROPRIETARY_INFORMATION_TYPES:
- for to_type in PUBLIC_PROPRIETARY_INFORMATION_TYPES:
- with person_logged_in(owner):
- product.information_type = to_type
- warning = view.getPublicWarning(from_type)
- if (to_type in PROPRIETARY_INFORMATION_TYPES and
- from_type == InformationType.PUBLIC):
- self.assertIsNot(None, warning)
- else:
- self.assertIs(None, warning)
-
class ProductSetReviewLicensesViewTestCase(TestCaseWithFactory):
"""Tests the ProductSetReviewLicensesView."""
=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py 2012-11-19 18:23:06 +0000
+++ lib/lp/registry/model/product.py 2012-11-23 15:16:34 +0000
@@ -111,6 +111,7 @@
BUG_POLICY_DEFAULT_TYPES,
)
from lp.bugs.interfaces.bugtaskfilter import OrderedBugTask
+from lp.bugs.model.bug import Bug
from lp.bugs.model.bugtarget import (
BugTargetBase,
OfficialBugTagTargetMixin,
@@ -460,27 +461,53 @@
if value in PROPRIETARY_INFORMATION_TYPES:
if self.answers_usage == ServiceUsage.LAUNCHPAD:
raise CannotChangeInformationType('Answers is enabled.')
+ if self._SO_creating or value not in PROPRIETARY_INFORMATION_TYPES:
+ return value
+ # Additional checks when transitioning an existing product to a
+ # proprietary type
+ # All specs located by an ALL search are public.
+ public_specs = self.specifications(
+ None, filter=[SpecificationFilter.ALL])
+ if not public_specs.is_empty():
+ # Unlike bugs and branches, specifications cannot be USERDATA or a
+ # security type.
+ raise CannotChangeInformationType(
+ 'Some blueprints are public.')
+ non_proprietary_bugs = Store.of(self).find(Bug,
+ Not(Bug.information_type.is_in(PROPRIETARY_INFORMATION_TYPES)),
+ BugTask.bug == Bug.id, BugTask.product == self.id)
+ if not non_proprietary_bugs.is_empty():
+ raise CannotChangeInformationType(
+ 'Some bugs are neither proprietary nor embargoed.')
+ # Default returns all public branches.
+ non_proprietary_branches = Store.of(self).find(
+ Branch, Branch.product == self.id,
+ Not(Branch.information_type.is_in(PROPRIETARY_INFORMATION_TYPES))
+ )
+ if not non_proprietary_branches.is_empty():
+ raise CannotChangeInformationType(
+ 'Some branches are neither proprietary nor embargoed.')
+ if not self.packagings.is_empty():
+ raise CannotChangeInformationType('Some series are packaged.')
+ if self.translations_usage == ServiceUsage.LAUNCHPAD:
+ raise CannotChangeInformationType('Translations are enabled.')
# Proprietary check works only after creation, because during
# creation, has_commercial_subscription cannot give the right value
# and triggers an inappropriate DB flush.
# If you're changing the license, and setting a PROPRIETARY
- # information type, yet you don't have a subscription you get one when
- # the license is set.
+ # information type, yet you don't have a subscription, you get one
+ # when the license is set.
+
+ # Create the complimentary commercial subscription for the product.
+ self._ensure_complimentary_subscription()
# If you have a commercial subscription, but it's not current, you
# cannot set the information type to a PROPRIETARY type.
- if not self._SO_creating and value in PROPRIETARY_INFORMATION_TYPES:
- if not self.packagings.is_empty():
- raise CannotChangeInformationType('Some series are packaged.')
- # Create the complimentary commercial subscription for the product.
- self._ensure_complimentary_subscription()
-
- if not self.has_current_commercial_subscription:
- raise CommercialSubscribersOnly(
- 'A valid commercial subscription is required for private'
- ' Projects.')
-
+ if not self.has_current_commercial_subscription:
+ raise CommercialSubscribersOnly(
+ 'A valid commercial subscription is required for private'
+ ' Projects.')
return value
_information_type = EnumCol(
=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py 2012-11-19 06:26:32 +0000
+++ lib/lp/registry/tests/test_product.py 2012-11-23 15:16:34 +0000
@@ -391,6 +391,63 @@
expected = [InformationType.USERDATA, InformationType.PRIVATESECURITY]
self.assertContentEqual(expected, [policy.type for policy in aps])
+ def test_change_info_type_proprietary_check_artifacts(self):
+ # Cannot change product information_type if any artifacts are public.
+ spec_policy = SpecificationSharingPolicy.PUBLIC_OR_PROPRIETARY
+ product = self.factory.makeProduct(
+ licenses=[License.OTHER_PROPRIETARY],
+ specification_sharing_policy=spec_policy,
+ bug_sharing_policy=BugSharingPolicy.PUBLIC_OR_PROPRIETARY,
+ branch_sharing_policy=BranchSharingPolicy.PUBLIC_OR_PROPRIETARY,
+ )
+ self.useContext(person_logged_in(product.owner))
+ spec = self.factory.makeSpecification(product=product)
+ for info_type in PROPRIETARY_INFORMATION_TYPES:
+ with ExpectedException(
+ CannotChangeInformationType,
+ 'Some blueprints are public.'):
+ product.information_type = info_type
+ spec.transitionToInformationType(InformationType.PROPRIETARY,
+ product.owner)
+ bug = self.factory.makeBug(target=product)
+ for bug_info_type in FREE_INFORMATION_TYPES:
+ bug.transitionToInformationType(bug_info_type, product.owner)
+ for info_type in PROPRIETARY_INFORMATION_TYPES:
+ with ExpectedException(
+ CannotChangeInformationType,
+ 'Some bugs are neither proprietary nor embargoed.'):
+ product.information_type = info_type
+ bug.transitionToInformationType(InformationType.PROPRIETARY,
+ product.owner)
+ branch = self.factory.makeBranch(product=product)
+ for branch_info_type in FREE_INFORMATION_TYPES:
+ branch.transitionToInformationType(branch_info_type,
+ product.owner)
+ for info_type in PROPRIETARY_INFORMATION_TYPES:
+ with ExpectedException(
+ CannotChangeInformationType,
+ 'Some branches are neither proprietary nor embargoed.'):
+ product.information_type = info_type
+ branch.transitionToInformationType(InformationType.PROPRIETARY,
+ product.owner)
+ for info_type in PROPRIETARY_INFORMATION_TYPES:
+ product.information_type = info_type
+
+ def test_change_info_type_proprietary_check_translations(self):
+ product = self.factory.makeProduct(
+ licenses=[License.OTHER_PROPRIETARY])
+ with person_logged_in(product.owner):
+ for usage in ServiceUsage:
+ product.translations_usage = usage.value
+ for info_type in PROPRIETARY_INFORMATION_TYPES:
+ if product.translations_usage == ServiceUsage.LAUNCHPAD:
+ with ExpectedException(
+ CannotChangeInformationType,
+ 'Translations are enabled.'):
+ product.information_type = info_type
+ else:
+ product.information_type = info_type
+
def test_change_info_type_proprietary_sets_policies(self):
# Changing information type from public to proprietary sets the
# appropriate policies
Follow ups
