Merge lp://staging/~abentley/launchpad/bmp-iprivacy into lp://staging/launchpad

Proposed by Aaron Bentley
Status: Merged
Approved by: Gavin Panella
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp://staging/~abentley/launchpad/bmp-iprivacy
Merge into: lp://staging/launchpad
Diff against target: None lines
To merge this branch: bzr merge lp://staging/~abentley/launchpad/bmp-iprivacy
Reviewer Review Type Date Requested Status
Gavin Panella (community) code Approve
Review via email: mp+11442@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

= Summary =
Implement IPrivacy for BranchMergeProposal

== Proposed fix ==
Privacy for branch merge proposals is determined by the involved
branches: the source, target and optional dependent branch. If any are
private, the merge proposal is private.

== Pre-implementation notes ==
Pre-implementation was with thumper

== Implementation details ==
I also added a test that BranchMergeProposal implements
IBranchMergeProposal, as a drive-by.

== Tests ==
bin/test test_branchmergeproposals -t Privacy

== Demo and Q/A ==
Create a private branch. Propose it for merging. Note that the merge
proposal has the standard privacy bars.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/code/configure.zcml
  lib/lp/code/interfaces/branchmergeproposal.py
  lib/lp/code/model/branchmergeproposal.py
  lib/lp/code/model/tests/test_branchmergeproposals.py

== Pylint notices ==

lib/lp/code/interfaces/branchmergeproposal.py
    27: [F0401] Unable to import 'lazr.lifecycle.event' (No module named
lifecycle)
    42: [F0401] Unable to import 'lazr.restful.fields' (No module named
restful)
    43: [F0401] Unable to import 'lazr.restful.declarations' (No module
named restful)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkqnyGgACgkQ0F+nu1YWqI2mIACaAv9Y1uhElpNOOFhOgoiT/aFY
V0gAn24bUSNXGGRCZxoWzYoL0VBhDHjM
=T23r
-----END PGP SIGNATURE-----

Revision history for this message
Gavin Panella (allenap) wrote :

> 115 +class TestBranchMergeProposalPrivacy(TestCaseWithFactory):
> 116 + """Ensure that BranchMergeProposal implements its interface."""

I think you need to change this to say "... implements IPrivacy" or something like that. Otherwise, nice branch.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/configure.zcml'
2--- lib/lp/code/configure.zcml 2009-09-04 13:10:32 +0000
3+++ lib/lp/code/configure.zcml 2009-09-09 15:16:45 +0000
4@@ -189,6 +189,7 @@
5 dependent_branch
6 whiteboard
7 queue_status
8+ private
9 reviewer
10 reviewed_revision_id
11 queuer
12
13=== modified file 'lib/lp/code/interfaces/branchmergeproposal.py'
14--- lib/lp/code/interfaces/branchmergeproposal.py 2009-09-09 02:18:01 +0000
15+++ lib/lp/code/interfaces/branchmergeproposal.py 2009-09-09 15:16:45 +0000
16@@ -28,11 +28,11 @@
17 from zope.event import notify
18 from zope.interface import Attribute, Interface
19 from zope.schema import (
20- Bytes, Choice, Datetime, Int, Object, Text, TextLine)
21+ Bytes, Bool, Choice, Datetime, Int, Object, Text, TextLine)
22
23 from canonical.launchpad import _
24 from canonical.launchpad.fields import PublicPersonChoice, Summary, Whiteboard
25-from canonical.launchpad.interfaces import IBug
26+from canonical.launchpad.interfaces import IBug, IPrivacy
27 from lp.code.enums import BranchMergeProposalStatus, CodeReviewVote
28 from lp.code.interfaces.branch import IBranch
29 from lp.registry.interfaces.person import IPerson
30@@ -85,7 +85,7 @@
31 )
32
33
34-class IBranchMergeProposal(Interface):
35+class IBranchMergeProposal(IPrivacy):
36 """Branch merge proposals show intent of landing one branch on another."""
37
38 export_as_webservice_entry()
39@@ -121,6 +121,15 @@
40 "If this is the same as the target branch, then "
41 "leave this field blank.")))
42
43+ # This is redefined from IPrivacy.private because the attribute is
44+ # read-only. The value is determined by the involved branches.
45+ private = exported(
46+ Bool(
47+ title=_("Proposal is confidential"), required=False,
48+ readonly=True, default=False,
49+ description=_(
50+ "If True, this proposal is visible only to subscribers.")))
51+
52 whiteboard = Whiteboard(
53 title=_('Whiteboard'), required=False,
54 description=_('Notes about the merge.'))
55
56=== modified file 'lib/lp/code/model/branchmergeproposal.py'
57--- lib/lp/code/model/branchmergeproposal.py 2009-09-09 02:18:01 +0000
58+++ lib/lp/code/model/branchmergeproposal.py 2009-09-09 15:16:45 +0000
59@@ -117,6 +117,13 @@
60 enum=BranchMergeProposalStatus, notNull=True,
61 default=BranchMergeProposalStatus.WORK_IN_PROGRESS)
62
63+ @property
64+ def private(self):
65+ return (
66+ self.source_branch.private or self.target_branch.private or
67+ (self.dependent_branch is not None and
68+ self.dependent_branch.private))
69+
70 reviewer = ForeignKey(
71 dbName='reviewer', foreignKey='Person',
72 storm_validator=validate_public_person, notNull=False,
73
74=== modified file 'lib/lp/code/model/tests/test_branchmergeproposals.py'
75--- lib/lp/code/model/tests/test_branchmergeproposals.py 2009-09-09 02:18:01 +0000
76+++ lib/lp/code/model/tests/test_branchmergeproposals.py 2009-09-09 15:16:45 +0000
77@@ -24,6 +24,7 @@
78 DatabaseFunctionalLayer, LaunchpadFunctionalLayer, LaunchpadZopelessLayer)
79 from lazr.lifecycle.event import ObjectModifiedEvent
80
81+from canonical.launchpad.interfaces import IPrivacy
82 from canonical.launchpad.interfaces.message import IMessageJob
83 from canonical.launchpad.webapp.testing import verifyObject
84 from lp.code.event.branchmergeproposal import (
85@@ -33,11 +34,12 @@
86 ANONYMOUS, import_secret_test_key, login, syncUpdate)
87 from lp.code.enums import (
88 BranchMergeProposalStatus, BranchSubscriptionNotificationLevel,
89- BranchType, CodeReviewNotificationLevel, CodeReviewVote)
90+ BranchType, CodeReviewNotificationLevel, CodeReviewVote,
91+ BranchVisibilityRule)
92 from lp.code.interfaces.branchmergeproposal import (
93 BadStateTransition,
94 BRANCH_MERGE_PROPOSAL_FINAL_STATES as FINAL_STATES,
95- IBranchMergeProposalGetter, IBranchMergeProposalJob,
96+ IBranchMergeProposal, IBranchMergeProposalGetter, IBranchMergeProposalJob,
97 ICreateMergeProposalJob, ICreateMergeProposalJobSource,
98 IMergeProposalCreatedJob, notify_modified, WrongBranchMergeProposal)
99 from lp.code.model.branchmergeproposaljob import (
100@@ -56,6 +58,51 @@
101 from lp.testing.mail_helpers import pop_notifications
102
103
104+class TestBranchMergeProposalInterface(TestCaseWithFactory):
105+ """Ensure that BranchMergeProposal implements its interface."""
106+
107+ layer = DatabaseFunctionalLayer
108+
109+ def test_BranchMergeProposal_implements_interface(self):
110+ """Ensure that BranchMergeProposal implements its interface."""
111+ bmp = self.factory.makeBranchMergeProposal()
112+ verifyObject(IBranchMergeProposal, bmp)
113+
114+
115+class TestBranchMergeProposalPrivacy(TestCaseWithFactory):
116+ """Ensure that BranchMergeProposal implements its interface."""
117+
118+ layer = DatabaseFunctionalLayer
119+
120+ def test_BranchMergeProposal_implements_interface(self):
121+ """Ensure that BranchMergeProposal implements privacy."""
122+ bmp = self.factory.makeBranchMergeProposal()
123+ verifyObject(IPrivacy, bmp)
124+
125+ @staticmethod
126+ def setPrivate(branch):
127+ """Force a branch to be private."""
128+ login_person(branch.owner)
129+ branch.product.setBranchVisibilityTeamPolicy(
130+ branch.owner, BranchVisibilityRule.PRIVATE)
131+ branch.setPrivate(True)
132+
133+ def test_private(self):
134+ """Private flag should be True if True for any involved branch."""
135+ bmp = self.factory.makeBranchMergeProposal()
136+ self.assertFalse(bmp.private)
137+ self.setPrivate(bmp.source_branch)
138+ self.assertTrue(bmp.private)
139+ bmp.source_branch.setPrivate(False)
140+ self.setPrivate(bmp.target_branch)
141+ self.assertTrue(bmp.private)
142+ bmp.target_branch.setPrivate(False)
143+ removeSecurityProxy(bmp).dependent_branch = self.factory.makeBranch(
144+ product=bmp.source_branch.product)
145+ self.setPrivate(bmp.dependent_branch)
146+ self.assertTrue(bmp.private)
147+
148+
149 class TestBranchMergeProposalTransitions(TestCaseWithFactory):
150 """Test the state transitions of branch merge proposals."""
151