Launchpad itself

Merge lp:~thumper/launchpad/allow-admin-to-set-branch-privacy into lp:launchpad

  1. allow-admin-to-set-branch-privacy
  2. Merge into devel
Proposed by Tim Penhey
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: not available
Merged at revision: 9959
Proposed branch: lp:~thumper/launchpad/allow-admin-to-set-branch-privacy
Merge into: lp:launchpad
Diff against target: 254 lines (+71/-28)
6 files modified
lib/lp/code/browser/branch.py (+8/-4)
lib/lp/code/interfaces/branch.py (+2/-1)
lib/lp/code/model/branch.py (+8/-6)
lib/lp/code/model/tests/test_branch.py (+15/-6)
lib/lp/code/model/tests/test_branchmergeproposals.py (+3/-3)
lib/lp/code/stories/branches/xx-branch-edit-privacy.txt (+35/-8)
To merge this branch: bzr merge lp:~thumper/launchpad/allow-admin-to-set-branch-privacy
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle Approve
Review via email: mp+15259@code.launchpad.net

Commit message

Allow admins to make public branches private.

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

Allow an admin (or bzr expert) to make a public branch private.

tests:
  TestBranchSetPrivate
  xx-branch-edit-privacy

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :
Download full text (4.6 KiB)

Hi Tim,

I think this branch is basically OK and will make Tom hate us at least
8% less :-)

> === modified file 'lib/lp/code/model/branch.py'
> --- lib/lp/code/model/branch.py 2009-11-20 15:58:09 +0000
> +++ lib/lp/code/model/branch.py 2009-11-26 02:41:00 +0000
> @@ -98,16 +98,18 @@
>
> private = BoolCol(default=False, notNull=True)
>
> - def setPrivate(self, private):
> + def setPrivate(self, private, user):
> """See `IBranch`."""
> if private == self.private:
> return
> - policy = IBranchNamespacePolicy(self.namespace)
> + # Only check the privacy if the user is not special.

I think "privacy policy" is clearer than just "privacy" here.

> + if (not user_has_special_branch_access(user)):
> + policy = IBranchNamespacePolicy(self.namespace)
>
> - if private and not policy.canBranchesBePrivate():
> - raise BranchCannotBePrivate()
> - if not private and not policy.canBranchesBePublic():
> - raise BranchCannotBePublic()
> + if private and not policy.canBranchesBePrivate():
> + raise BranchCannotBePrivate()
> + if not private and not policy.canBranchesBePublic():
> + raise BranchCannotBePublic()
> self.private = private
>
> registrant = ForeignKey(

> === modified file 'lib/lp/code/stories/branches/xx-branch-edit-privacy.txt'
> --- lib/lp/code/stories/branches/xx-branch-edit-privacy.txt 2009-05-06 15:13:39 +0000
> +++ lib/lp/code/stories/branches/xx-branch-edit-privacy.txt 2009-11-26 02:41:00 +0000
> @@ -1,4 +1,5 @@
> -= Changing branch privacy =
> +Changing branch privacy
> +=======================
>
> Which users can have private branches is controlled by
> the branch visibility policy (see doc/branch-visibility-policy.txt).
> @@ -9,22 +10,46 @@
> Once the branch is created, the user is able to change a public branch
> to private or a private branch to public in certain circumstances.
>
> -
> -== Branches with no visibility policy ==
> + >>> login('<email address hidden>')
> + >>> eric = factory.makePerson(
> + ... name='eric', displayname='Eric the Viking',
> + ... <email address hidden>', password='test')
> + >>> fooix = factory.makeProduct(
> + ... name='fooix', displayname='Fooix', owner=eric)
> + >>> trunk = factory.makeProductBranch(
> + ... owner=eric, product=fooix, name='trunk')
> + >>> logout()

Yay for killing off sampledata usage.

> +Branches with no visibility policy
> +----------------------------------
>
> If there is no policy set, then the user is not able to change the visiblity
> of the branch.
>
> - >>> browser = setupBrowser(auth='Basic <email address hidden>:test')
> - >>> browser.open('http://code.launchpad.dev/~name12/firefox/main')
> + >>> browser = setupBrowser(auth='Basic <email address hidden>:test')
> + >>> browser.open('http://code.launchpad.dev/~eric/fooix/trunk')
> >>> browser.getLink('Change branch details').click()
> >>> browser.getControl('Keep branch confidential')
> Traceback (most recent call last):
> ...
> LookupError: label 'Keep branch confidential'
>
> -
>...

Read more...

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/browser/branch.py'
2--- lib/lp/code/browser/branch.py 2009-11-14 22:44:10 +0000
3+++ lib/lp/code/browser/branch.py 2009-11-27 03:28:11 +0000
4@@ -81,7 +81,8 @@
5 from lp.code.enums import (
6 BranchLifecycleStatus, BranchType, UICreatableBranchType)
7 from lp.code.interfaces.branch import (
8- BranchCreationForbidden, BranchExists, IBranch)
9+ BranchCreationForbidden, BranchExists, IBranch,
10+ user_has_special_branch_access)
11 from lp.code.interfaces.branchmergeproposal import InvalidBranchMergeProposal
12 from lp.code.interfaces.branchtarget import IBranchTarget
13 from lp.code.interfaces.codeimport import CodeImportReviewStatus
14@@ -651,7 +652,7 @@
15 private = data.pop('private')
16 if private != self.context.private:
17 # We only want to show notifications if it actually changed.
18- self.context.setPrivate(private)
19+ self.context.setPrivate(private, self.user)
20 changed = True
21 if private:
22 self.request.response.addNotification(
23@@ -892,8 +893,11 @@
24 show_private_field = policy.canBranchesBePublic()
25 else:
26 # If the branch is public, and can be made private, show the
27- # field.
28- show_private_field = policy.canBranchesBePrivate()
29+ # field. Users with special access rights to branches can set
30+ # public branches as private.
31+ show_private_field = (
32+ policy.canBranchesBePrivate() or
33+ user_has_special_branch_access(self.user))
34
35 if not show_private_field:
36 self.form_fields = self.form_fields.omit('private')
37
38=== modified file 'lib/lp/code/interfaces/branch.py'
39--- lib/lp/code/interfaces/branch.py 2009-11-20 15:58:09 +0000
40+++ lib/lp/code/interfaces/branch.py 2009-11-27 03:28:11 +0000
41@@ -399,10 +399,11 @@
42 description=_(
43 "Make this branch visible only to its subscribers.")))
44
45+ @call_with(user=REQUEST_USER)
46 @operation_parameters(
47 private=Bool(title=_("Keep branch confidential")))
48 @export_write_operation()
49- def setPrivate(private):
50+ def setPrivate(private, user):
51 """Set the branch privacy for this branch."""
52
53 # People attributes
54
55=== modified file 'lib/lp/code/model/branch.py'
56--- lib/lp/code/model/branch.py 2009-11-20 15:58:09 +0000
57+++ lib/lp/code/model/branch.py 2009-11-27 03:28:11 +0000
58@@ -98,16 +98,18 @@
59
60 private = BoolCol(default=False, notNull=True)
61
62- def setPrivate(self, private):
63+ def setPrivate(self, private, user):
64 """See `IBranch`."""
65 if private == self.private:
66 return
67- policy = IBranchNamespacePolicy(self.namespace)
68+ # Only check the privacy policy if the user is not special.
69+ if (not user_has_special_branch_access(user)):
70+ policy = IBranchNamespacePolicy(self.namespace)
71
72- if private and not policy.canBranchesBePrivate():
73- raise BranchCannotBePrivate()
74- if not private and not policy.canBranchesBePublic():
75- raise BranchCannotBePublic()
76+ if private and not policy.canBranchesBePrivate():
77+ raise BranchCannotBePrivate()
78+ if not private and not policy.canBranchesBePublic():
79+ raise BranchCannotBePublic()
80 self.private = private
81
82 registrant = ForeignKey(
83
84=== modified file 'lib/lp/code/model/tests/test_branch.py'
85--- lib/lp/code/model/tests/test_branch.py 2009-11-13 10:46:52 +0000
86+++ lib/lp/code/model/tests/test_branch.py 2009-11-27 03:28:11 +0000
87@@ -1512,7 +1512,7 @@
88 # Setting a public branch to be public is a no-op.
89 branch = self.factory.makeProductBranch()
90 self.assertFalse(branch.private)
91- branch.setPrivate(False)
92+ branch.setPrivate(False, branch.owner)
93 self.assertFalse(branch.private)
94
95 def test_public_to_private_allowed(self):
96@@ -1521,7 +1521,7 @@
97 branch = self.factory.makeProductBranch()
98 branch.product.setBranchVisibilityTeamPolicy(
99 branch.owner, BranchVisibilityRule.PRIVATE)
100- branch.setPrivate(True)
101+ branch.setPrivate(True, branch.owner)
102 self.assertTrue(branch.private)
103
104 def test_public_to_private_not_allowed(self):
105@@ -1531,20 +1531,29 @@
106 self.assertRaises(
107 BranchCannotBePrivate,
108 branch.setPrivate,
109- True)
110+ True, branch.owner)
111+
112+ def test_public_to_private_for_admins(self):
113+ # Admins can override the default behaviour and make any public branch
114+ # private.
115+ branch = self.factory.makeProductBranch()
116+ # Grab a random admin, the teamowner is good enough here.
117+ admins = getUtility(ILaunchpadCelebrities).admin
118+ branch.setPrivate(True, admins.teamowner)
119+ self.assertTrue(branch.private)
120
121 def test_private_to_private(self):
122 # Setting a private branch to be private is a no-op.
123 branch = self.factory.makeProductBranch(private=True)
124 self.assertTrue(branch.private)
125- branch.setPrivate(True)
126+ branch.setPrivate(True, branch.owner)
127 self.assertTrue(branch.private)
128
129 def test_private_to_public_allowed(self):
130 # If the namespace policy allows public branches, then changing from
131 # private to public is allowed.
132 branch = self.factory.makeProductBranch(private=True)
133- branch.setPrivate(False)
134+ branch.setPrivate(False, branch.owner)
135 self.assertFalse(branch.private)
136
137 def test_private_to_public_not_allowed(self):
138@@ -1558,7 +1567,7 @@
139 self.assertRaises(
140 BranchCannotBePublic,
141 branch.setPrivate,
142- False)
143+ False, branch.owner)
144
145
146 class TestBranchCommitsForDays(TestCaseWithFactory):
147
148=== modified file 'lib/lp/code/model/tests/test_branchmergeproposals.py'
149--- lib/lp/code/model/tests/test_branchmergeproposals.py 2009-11-03 21:05:14 +0000
150+++ lib/lp/code/model/tests/test_branchmergeproposals.py 2009-11-27 03:28:11 +0000
151@@ -84,7 +84,7 @@
152 login_person(branch.owner)
153 branch.product.setBranchVisibilityTeamPolicy(
154 branch.owner, BranchVisibilityRule.PRIVATE)
155- branch.setPrivate(True)
156+ branch.setPrivate(True, branch.owner)
157
158 def test_private(self):
159 """Private flag should be True if True for any involved branch."""
160@@ -92,10 +92,10 @@
161 self.assertFalse(bmp.private)
162 self.setPrivate(bmp.source_branch)
163 self.assertTrue(bmp.private)
164- bmp.source_branch.setPrivate(False)
165+ bmp.source_branch.setPrivate(False, bmp.source_branch.owner)
166 self.setPrivate(bmp.target_branch)
167 self.assertTrue(bmp.private)
168- bmp.target_branch.setPrivate(False)
169+ bmp.target_branch.setPrivate(False, bmp.target_branch.owner)
170 removeSecurityProxy(bmp).prerequisite_branch = (
171 self.factory.makeBranch(product=bmp.source_branch.product))
172 self.setPrivate(bmp.prerequisite_branch)
173
174=== modified file 'lib/lp/code/stories/branches/xx-branch-edit-privacy.txt'
175--- lib/lp/code/stories/branches/xx-branch-edit-privacy.txt 2009-05-06 15:13:39 +0000
176+++ lib/lp/code/stories/branches/xx-branch-edit-privacy.txt 2009-11-27 03:28:11 +0000
177@@ -1,4 +1,5 @@
178-= Changing branch privacy =
179+Changing branch privacy
180+=======================
181
182 Which users can have private branches is controlled by
183 the branch visibility policy (see doc/branch-visibility-policy.txt).
184@@ -9,22 +10,46 @@
185 Once the branch is created, the user is able to change a public branch
186 to private or a private branch to public in certain circumstances.
187
188-
189-== Branches with no visibility policy ==
190+ >>> login('admin@canonical.com')
191+ >>> eric = factory.makePerson(
192+ ... name='eric', displayname='Eric the Viking',
193+ ... email='eric@example.com', password='test')
194+ >>> fooix = factory.makeProduct(
195+ ... name='fooix', displayname='Fooix', owner=eric)
196+ >>> trunk = factory.makeProductBranch(
197+ ... owner=eric, product=fooix, name='trunk')
198+ >>> logout()
199+
200+
201+Branches with no visibility policy
202+----------------------------------
203
204 If there is no policy set, then the user is not able to change the visiblity
205 of the branch.
206
207- >>> browser = setupBrowser(auth='Basic test@canonical.com:test')
208- >>> browser.open('http://code.launchpad.dev/~name12/firefox/main')
209+ >>> browser = setupBrowser(auth='Basic eric@example.com:test')
210+ >>> browser.open('http://code.launchpad.dev/~eric/fooix/trunk')
211 >>> browser.getLink('Change branch details').click()
212 >>> browser.getControl('Keep branch confidential')
213 Traceback (most recent call last):
214 ...
215 LookupError: label 'Keep branch confidential'
216
217-
218-== Private branches allowed ==
219+However an admin can.
220+
221+ >>> admin_browser.open('http://code.launchpad.dev/~eric/fooix/trunk')
222+ >>> admin_browser.getLink('Change branch details').click()
223+ >>> admin_browser.getControl('Keep branch confidential').click()
224+ >>> admin_browser.getControl('Change Branch').click()
225+
226+The branch is now private, so it assumes the standard private presentation.
227+
228+ >>> print_tag_with_id(admin_browser.contents, 'privacy')
229+ This branch is private
230+
231+
232+Private branches allowed
233+------------------------
234
235 Allow the Warty Security Team to have private firefox branches.
236
237@@ -40,6 +65,7 @@
238 Now Sample Person, who is a member of the Warty Security Team should be
239 able to mark the branch as private.
240
241+ >>> browser = setupBrowser(auth='Basic test@canonical.com:test')
242 >>> browser.open('http://code.launchpad.dev/~name12/firefox/main')
243 >>> browser.getLink('Change branch details').click()
244 >>> browser.getControl('Keep branch confidential').selected = True
245@@ -71,7 +97,8 @@
246 The branch is now publicly accessible.
247
248
249-== Private Only branches ==
250+Private Only branches
251+---------------------
252
253 If the policy specifies private only, then the user is able to make
254 a public branch private, but not vice versa.