launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #11452
[Merge] lp:~wgrant/launchpad/bug-restrict-type into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/bug-restrict-type into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1039631 in Launchpad itself: "bug.transitionToInformationType does not respect BugSharingPolicy"
https://bugs.launchpad.net/launchpad/+bug/1039631
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-restrict-type/+merge/121989
This branch prevents bugs from transitioning to an information type that's illegal in their targets, or affecting a new target that disallows their information type.
I replaced the exceptions BranchCannotChangeInformationType and BugCannotBePrivate with a generic CannotChangeInformationType. transitionToInformationType checks directly that the new type is permitted, and createBug/createTask/transitionToTarget all use validate_target to confirm that the type is permitted in the new target.
I also extended the Proprietary single-pillar checks to Embargoed, as they were intended to have been from the start.
--
https://code.launchpad.net/~wgrant/launchpad/bug-restrict-type/+merge/121989
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-restrict-type into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2012-08-29 06:24:05 +0000
+++ lib/lp/bugs/browser/bugtask.py 2012-08-30 06:06:19 +0000
@@ -222,7 +222,10 @@
from lp.bugs.model.bugtasksearch import orderby_expression
from lp.code.interfaces.branchcollection import IAllBranches
from lp.layers import FeedsLayer
-from lp.registry.enums import InformationType
+from lp.registry.enums import (
+ InformationType,
+ PROPRIETARY_INFORMATION_TYPES,
+ )
from lp.registry.interfaces.distribution import (
IDistribution,
IDistributionSet,
@@ -699,7 +702,6 @@
cancel_url = canonical_url(self.context)
return cancel_url
-
@cachedproperty
def is_duplicate_active(self):
active = True
@@ -3892,18 +3894,12 @@
def canAddProjectTask(self):
"""Can a new bug task on a project be added to this bug?
- If a bug has any bug tasks already, were it to be private, it cannot
- be marked as also affecting any other project, so return False.
-
- Note: this check is currently only relevant if a bug is private.
- Eventually, even public bugs will have this restriction too. So what
- happens now is that this API is used by the tales to add a class
- called 'disallow-private' to the Also Affects Project link. A css rule
- is used to hide the link when body.private is True.
-
+ If a bug has any bug tasks already, were it to be Proprietary or
+ Embargoed, it cannot be marked as also affecting any other
+ project, so return False.
"""
bug = self.context
- if bug.information_type != InformationType.PROPRIETARY:
+ if bug.information_type not in PROPRIETARY_INFORMATION_TYPES:
return True
return len(bug.bugtasks) == 0
@@ -3911,20 +3907,14 @@
"""Can a new bug task on a src pkg be added to this bug?
If a bug has any existing bug tasks on a project, were it to
- be private, then it cannot be marked as affecting a package,
- so return False.
+ be Proprietary or Embargoed, then it cannot be marked as
+ affecting a package, so return False.
A task on a given package may still be illegal to add, but
this will be caught when bug.addTask() is attempted.
-
- Note: this check is currently only relevant if a bug is private.
- Eventually, even public bugs will have this restriction too. So what
- happens now is that this API is used by the tales to add a class
- called 'disallow-private' to the Also Affects Package link. A css rule
- is used to hide the link when body.private is True.
"""
bug = self.context
- if bug.information_type != InformationType.PROPRIETARY:
+ if bug.information_type not in PROPRIETARY_INFORMATION_TYPES:
return True
for pillar in bug.affected_pillars:
if IProduct.providedBy(pillar):
=== modified file 'lib/lp/bugs/errors.py'
--- lib/lp/bugs/errors.py 2012-08-08 04:48:40 +0000
+++ lib/lp/bugs/errors.py 2012-08-30 06:06:19 +0000
@@ -5,7 +5,6 @@
__metaclass__ = type
__all__ = [
- 'BugCannotBePrivate',
'InvalidDuplicateValue',
]
@@ -19,8 +18,3 @@
@error_status(httplib.EXPECTATION_FAILED)
class InvalidDuplicateValue(LaunchpadValidationError):
"""A bug cannot be set as the duplicate of another."""
-
-
-@error_status(httplib.BAD_REQUEST)
-class BugCannotBePrivate(Exception):
- """The bug is not allowed to be private."""
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2012-08-24 05:09:51 +0000
+++ lib/lp/bugs/model/bug.py 2012-08-30 06:06:19 +0000
@@ -100,10 +100,7 @@
UnsubscribedFromBug,
)
from lp.bugs.enums import BugNotificationLevel
-from lp.bugs.errors import (
- BugCannotBePrivate,
- InvalidDuplicateValue,
- )
+from lp.bugs.errors import InvalidDuplicateValue
from lp.bugs.interfaces.bug import (
IBug,
IBugBecameQuestionEvent,
@@ -160,8 +157,10 @@
from lp.registry.enums import (
InformationType,
PRIVATE_INFORMATION_TYPES,
+ PROPRIETARY_INFORMATION_TYPES,
SECURITY_INFORMATION_TYPES,
)
+from lp.registry.errors import CannotChangeInformationType
from lp.registry.interfaces.accesspolicy import (
IAccessArtifactGrantSource,
IAccessArtifactSource,
@@ -1710,10 +1709,12 @@
"""See `IBug`."""
if self.information_type == information_type:
return False
- if (information_type == InformationType.PROPRIETARY and
- len(self.affected_pillars) > 1):
- raise BugCannotBePrivate(
- "Multi-pillar bugs cannot be proprietary.")
+ if information_type not in self.getAllowedInformationTypes(who):
+ raise CannotChangeInformationType("Forbidden by project policy.")
+ if (information_type in PROPRIETARY_INFORMATION_TYPES
+ and len(self.affected_pillars) > 1):
+ raise CannotChangeInformationType(
+ "Proprietary bugs can only affect one project.")
if information_type in PRIVATE_INFORMATION_TYPES:
self.who_made_private = who
self.date_made_private = UTC_NOW
=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py 2012-08-29 06:24:05 +0000
+++ lib/lp/bugs/model/bugtask.py 2012-08-30 06:06:19 +0000
@@ -94,7 +94,7 @@
)
from lp.bugs.interfaces.bugtasksearch import BugTaskSearchParams
from lp.registry.enums import (
- InformationType,
+ PROPRIETARY_INFORMATION_TYPES,
PUBLIC_INFORMATION_TYPES,
)
from lp.registry.interfaces.distribution import (
@@ -337,7 +337,13 @@
except NotFoundError as e:
raise IllegalTarget(e[0])
- if bug.information_type == InformationType.PROPRIETARY:
+ legal_types = target.pillar.getAllowedBugInformationTypes()
+ if bug.information_type not in legal_types:
+ raise IllegalTarget(
+ "%s doesn't allow %s bugs." % (
+ target.pillar.bugtargetdisplayname, bug.information_type.title))
+
+ if bug.information_type in PROPRIETARY_INFORMATION_TYPES:
# Perhaps we are replacing the one and only existing bugtask, in
# which case that's ok.
if retarget_existing and len(bug.bugtasks) <= 1:
@@ -348,7 +354,7 @@
raise IllegalTarget(
"This proprietary bug already affects %s. "
"Proprietary bugs cannot affect multiple projects."
- % bug.default_bugtask.target.bugtargetdisplayname)
+ % bug.default_bugtask.target.pillar.bugtargetdisplayname)
def validate_new_target(bug, target):
=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
--- lib/lp/bugs/model/tests/test_bug.py 2012-08-28 01:35:08 +0000
+++ lib/lp/bugs/model/tests/test_bug.py 2012-08-30 06:06:19 +0000
@@ -20,7 +20,6 @@
BugNotificationLevel,
BugNotificationStatus,
)
-from lp.bugs.errors import BugCannotBePrivate
from lp.bugs.interfaces.bugnotification import IBugNotificationSet
from lp.bugs.interfaces.bugtask import BugTaskStatus
from lp.bugs.mail.bugnotificationrecipients import BugNotificationRecipients
@@ -28,7 +27,11 @@
BugNotification,
BugSubscriptionInfo,
)
-from lp.registry.enums import InformationType
+from lp.registry.enums import (
+ BugSharingPolicy,
+ InformationType,
+ )
+from lp.registry.errors import CannotChangeInformationType
from lp.registry.interfaces.accesspolicy import (
IAccessArtifactSource,
IAccessPolicyArtifactSource,
@@ -753,12 +756,17 @@
def test_multipillar_proprietary_bugs_disallowed(self):
# A multi-pillar bug cannot be made proprietary.
- bug = self.factory.makeBug()
- product = self.factory.makeProduct()
- self.factory.makeBugTask(bug=bug, target=product)
+ p1 = self.factory.makeProduct(
+ bug_sharing_policy=BugSharingPolicy.PUBLIC_OR_PROPRIETARY)
+ p2 = self.factory.makeProduct(
+ bug_sharing_policy=BugSharingPolicy.PUBLIC_OR_PROPRIETARY)
+ bug = self.factory.makeBug(target=p1)
+ self.factory.makeBugTask(bug=bug, target=p2)
login_person(bug.owner)
- self.assertRaises(
- BugCannotBePrivate, bug.transitionToInformationType,
+ self.assertRaisesWithContent(
+ CannotChangeInformationType,
+ "Proprietary bugs can only affect one project.",
+ bug.transitionToInformationType,
InformationType.PROPRIETARY, bug.owner)
bug.transitionToInformationType(InformationType.USERDATA, bug.owner)
self.assertTrue(bug.private)
@@ -869,6 +877,29 @@
InformationType.PRIVATESECURITY, InformationType.USERDATA],
self.factory.makeBug().getAllowedInformationTypes(None))
+ def test_transitionToInformationType_respects_allowed_proprietary(self):
+ # transitionToInformationType rejects types that aren't allowed
+ # for the bug.
+ product = self.factory.makeProduct()
+ with person_logged_in(product.owner):
+ bug = self.factory.makeBug(target=product)
+ self.assertRaisesWithContent(
+ CannotChangeInformationType, "Forbidden by project policy.",
+ bug.transitionToInformationType,
+ InformationType.PROPRIETARY, bug.owner)
+
+ def test_transitionToInformationType_respects_allowed_public(self):
+ # transitionToInformationType rejects types that aren't allowed
+ # for the bug.
+ product = self.factory.makeProduct(
+ bug_sharing_policy=BugSharingPolicy.PROPRIETARY)
+ with person_logged_in(product.owner):
+ bug = self.factory.makeBug(target=product)
+ self.assertRaisesWithContent(
+ CannotChangeInformationType, "Forbidden by project policy.",
+ bug.transitionToInformationType,
+ InformationType.PUBLIC, bug.owner)
+
class TestBugPrivateAndSecurityRelatedUpdatesPrivateProject(
TestBugPrivateAndSecurityRelatedUpdatesMixin, TestCaseWithFactory):
=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py 2012-08-29 06:24:05 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py 2012-08-30 06:06:19 +0000
@@ -56,6 +56,7 @@
)
from lp.bugs.tests.bug import create_old_bug
from lp.registry.enums import (
+ BugSharingPolicy,
InformationType,
TeamMembershipPolicy,
)
@@ -3082,6 +3083,24 @@
% (dsp.sourcepackagename.name, dsp.distribution.displayname),
validate_target, task.bug, dsp)
+ def test_illegal_information_type_disallowed(self):
+ # The bug's current information_type must be permitted by the
+ # new target.
+ free_prod = self.factory.makeProduct()
+ other_free_prod = self.factory.makeProduct()
+ commercial_prod = self.factory.makeProduct(
+ bug_sharing_policy=BugSharingPolicy.PROPRIETARY)
+ bug = self.factory.makeBug(target=free_prod)
+
+ # The new bug is Public, which is legal on the other free product.
+ self.assertIs(None, validate_target(bug, other_free_prod))
+
+ # But not on the proprietary-only product.
+ self.assertRaisesWithContent(
+ IllegalTarget,
+ "%s doesn't allow Public bugs." % commercial_prod.displayname,
+ validate_target, bug, commercial_prod)
+
class TestValidateNewTarget(TestCaseWithFactory, ValidateTargetMixin):
=== modified file 'lib/lp/code/errors.py'
--- lib/lp/code/errors.py 2012-07-11 09:36:13 +0000
+++ lib/lp/code/errors.py 2012-08-30 06:06:19 +0000
@@ -8,7 +8,6 @@
'AlreadyLatestFormat',
'BadBranchMergeProposalSearchContext',
'BadStateTransition',
- 'BranchCannotChangeInformationType',
'BranchCreationException',
'BranchCreationForbidden',
'BranchCreatorNotMemberOfOwnerTeam',
@@ -146,10 +145,6 @@
"""
-class BranchCannotChangeInformationType(Exception):
- """The information type of this branch cannot be changed."""
-
-
class InvalidBranchException(Exception):
"""Base exception for an error resolving a branch for a component.
=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py 2012-08-29 02:40:36 +0000
+++ lib/lp/code/model/branch.py 2012-08-30 06:06:19 +0000
@@ -80,7 +80,6 @@
)
from lp.code.errors import (
AlreadyLatestFormat,
- BranchCannotChangeInformationType,
BranchMergeProposalExists,
BranchTargetError,
BranchTypeError,
@@ -137,6 +136,7 @@
PRIVATE_INFORMATION_TYPES,
PUBLIC_INFORMATION_TYPES,
)
+from lp.registry.errors import CannotChangeInformationType
from lp.registry.interfaces.accesspolicy import (
IAccessArtifactGrantSource,
IAccessArtifactSource,
@@ -249,10 +249,10 @@
if (self.stacked_on
and self.stacked_on.information_type in PRIVATE_INFORMATION_TYPES
and information_type in PUBLIC_INFORMATION_TYPES):
- raise BranchCannotChangeInformationType()
+ raise CannotChangeInformationType("Must match stacked-on branch.")
if (verify_policy
and information_type not in self.getAllowedInformationTypes(who)):
- raise BranchCannotChangeInformationType()
+ raise CannotChangeInformationType("Forbidden by project policy.")
self.information_type = information_type
self._reconcileAccess()
if information_type in PRIVATE_INFORMATION_TYPES and self.subscribers:
=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py 2012-08-29 03:19:38 +0000
+++ lib/lp/code/model/tests/test_branch.py 2012-08-30 06:06:19 +0000
@@ -54,7 +54,6 @@
)
from lp.code.errors import (
AlreadyLatestFormat,
- BranchCannotChangeInformationType,
BranchCreatorNotMemberOfOwnerTeam,
BranchCreatorNotOwner,
BranchTargetError,
@@ -120,6 +119,7 @@
PUBLIC_INFORMATION_TYPES,
TeamMembershipPolicy,
)
+from lp.registry.errors import CannotChangeInformationType
from lp.registry.interfaces.accesspolicy import (
IAccessArtifactSource,
IAccessPolicyArtifactSource,
@@ -2480,13 +2480,12 @@
def test_public_to_private_not_allowed(self):
# If there are no privacy policies allowing private branches, then
- # BranchCannotChangeInformationType is rasied.
+ # CannotChangeInformationType is raised.
product = self.factory.makeLegacyProduct()
branch = self.factory.makeBranch(product=product)
self.assertRaises(
- BranchCannotChangeInformationType,
- branch.setPrivate,
- True, branch.owner)
+ CannotChangeInformationType,
+ branch.setPrivate, True, branch.owner)
def test_public_to_private_for_admins(self):
# Admins can override the default behaviour and make any public branch
@@ -2522,8 +2521,7 @@
def test_private_to_public_not_allowed(self):
# If the namespace policy does not allow public branches, attempting
- # to change the branch to be public raises
- # BranchCannotChangeInformationType.
+ # to change the branch to be public raises CannotChangeInformationType.
product = self.factory.makeLegacyProduct()
branch = self.factory.makeBranch(
product=product,
@@ -2533,9 +2531,8 @@
branch.product.setBranchVisibilityTeamPolicy(
branch.owner, BranchVisibilityRule.PRIVATE_ONLY)
self.assertRaises(
- BranchCannotChangeInformationType,
- branch.setPrivate,
- False, branch.owner)
+ CannotChangeInformationType,
+ branch.setPrivate, False, branch.owner)
def test_cannot_transition_with_private_stacked_on(self):
# If a public branch is stacked on a private branch, it can not
@@ -2544,7 +2541,7 @@
information_type=InformationType.USERDATA)
branch = self.factory.makeBranch(stacked_on=stacked_on)
self.assertRaises(
- BranchCannotChangeInformationType,
+ CannotChangeInformationType,
branch.transitionToInformationType, InformationType.PUBLIC,
branch.owner)
=== modified file 'lib/lp/registry/enums.py'
--- lib/lp/registry/enums.py 2012-08-20 13:26:21 +0000
+++ lib/lp/registry/enums.py 2012-08-30 06:06:19 +0000
@@ -18,6 +18,7 @@
'PersonTransferJobType',
'PersonVisibility',
'PRIVATE_INFORMATION_TYPES',
+ 'PROPRIETARY_INFORMATION_TYPES',
'PUBLIC_INFORMATION_TYPES',
'ProductJobType',
'SECURITY_INFORMATION_TYPES',
@@ -97,6 +98,9 @@
FREE_INFORMATION_TYPES = (
PUBLIC_INFORMATION_TYPES + FREE_PRIVATE_INFORMATION_TYPES)
+PROPRIETARY_INFORMATION_TYPES = (
+ InformationType.PROPRIETARY, InformationType.EMBARGOED)
+
class SharingPermission(DBEnumeratedType):
"""Sharing permission.
=== modified file 'lib/lp/registry/errors.py'
--- lib/lp/registry/errors.py 2012-08-13 20:17:56 +0000
+++ lib/lp/registry/errors.py 2012-08-30 06:06:19 +0000
@@ -5,6 +5,7 @@
__all__ = [
'DistroSeriesDifferenceError',
'NotADerivedSeriesError',
+ 'CannotChangeInformationType',
'CannotDeleteCommercialSubscription',
'CannotTransitionToCountryMirror',
'CommercialSubscribersOnly',
@@ -190,3 +191,8 @@
class CannotDeleteCommercialSubscription(Exception):
"""Raised when a commercial subscription cannot be deleted."""
+
+
+@error_status(httplib.BAD_REQUEST)
+class CannotChangeInformationType(Exception):
+ """The information type cannot be changed."""
Follow ups
