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
=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py 2009-11-14 22:44:10 +0000
+++ lib/lp/code/browser/branch.py 2009-11-27 03:28:11 +0000
@@ -81,7 +81,8 @@
81from lp.code.enums import (81from lp.code.enums import (
82 BranchLifecycleStatus, BranchType, UICreatableBranchType)82 BranchLifecycleStatus, BranchType, UICreatableBranchType)
83from lp.code.interfaces.branch import (83from lp.code.interfaces.branch import (
84 BranchCreationForbidden, BranchExists, IBranch)84 BranchCreationForbidden, BranchExists, IBranch,
85 user_has_special_branch_access)
85from lp.code.interfaces.branchmergeproposal import InvalidBranchMergeProposal86from lp.code.interfaces.branchmergeproposal import InvalidBranchMergeProposal
86from lp.code.interfaces.branchtarget import IBranchTarget87from lp.code.interfaces.branchtarget import IBranchTarget
87from lp.code.interfaces.codeimport import CodeImportReviewStatus88from lp.code.interfaces.codeimport import CodeImportReviewStatus
@@ -651,7 +652,7 @@
651 private = data.pop('private')652 private = data.pop('private')
652 if private != self.context.private:653 if private != self.context.private:
653 # We only want to show notifications if it actually changed.654 # We only want to show notifications if it actually changed.
654 self.context.setPrivate(private)655 self.context.setPrivate(private, self.user)
655 changed = True656 changed = True
656 if private:657 if private:
657 self.request.response.addNotification(658 self.request.response.addNotification(
@@ -892,8 +893,11 @@
892 show_private_field = policy.canBranchesBePublic()893 show_private_field = policy.canBranchesBePublic()
893 else:894 else:
894 # If the branch is public, and can be made private, show the895 # If the branch is public, and can be made private, show the
895 # field.896 # field. Users with special access rights to branches can set
896 show_private_field = policy.canBranchesBePrivate()897 # public branches as private.
898 show_private_field = (
899 policy.canBranchesBePrivate() or
900 user_has_special_branch_access(self.user))
897901
898 if not show_private_field:902 if not show_private_field:
899 self.form_fields = self.form_fields.omit('private')903 self.form_fields = self.form_fields.omit('private')
900904
=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py 2009-11-20 15:58:09 +0000
+++ lib/lp/code/interfaces/branch.py 2009-11-27 03:28:11 +0000
@@ -399,10 +399,11 @@
399 description=_(399 description=_(
400 "Make this branch visible only to its subscribers.")))400 "Make this branch visible only to its subscribers.")))
401401
402 @call_with(user=REQUEST_USER)
402 @operation_parameters(403 @operation_parameters(
403 private=Bool(title=_("Keep branch confidential")))404 private=Bool(title=_("Keep branch confidential")))
404 @export_write_operation()405 @export_write_operation()
405 def setPrivate(private):406 def setPrivate(private, user):
406 """Set the branch privacy for this branch."""407 """Set the branch privacy for this branch."""
407408
408 # People attributes409 # People attributes
409410
=== 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-27 03:28:11 +0000
@@ -98,16 +98,18 @@
9898
99 private = BoolCol(default=False, notNull=True)99 private = BoolCol(default=False, notNull=True)
100100
101 def setPrivate(self, private):101 def setPrivate(self, private, user):
102 """See `IBranch`."""102 """See `IBranch`."""
103 if private == self.private:103 if private == self.private:
104 return104 return
105 policy = IBranchNamespacePolicy(self.namespace)105 # Only check the privacy policy if the user is not special.
106 if (not user_has_special_branch_access(user)):
107 policy = IBranchNamespacePolicy(self.namespace)
106108
107 if private and not policy.canBranchesBePrivate():109 if private and not policy.canBranchesBePrivate():
108 raise BranchCannotBePrivate()110 raise BranchCannotBePrivate()
109 if not private and not policy.canBranchesBePublic():111 if not private and not policy.canBranchesBePublic():
110 raise BranchCannotBePublic()112 raise BranchCannotBePublic()
111 self.private = private113 self.private = private
112114
113 registrant = ForeignKey(115 registrant = ForeignKey(
114116
=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py 2009-11-13 10:46:52 +0000
+++ lib/lp/code/model/tests/test_branch.py 2009-11-27 03:28:11 +0000
@@ -1512,7 +1512,7 @@
1512 # Setting a public branch to be public is a no-op.1512 # Setting a public branch to be public is a no-op.
1513 branch = self.factory.makeProductBranch()1513 branch = self.factory.makeProductBranch()
1514 self.assertFalse(branch.private)1514 self.assertFalse(branch.private)
1515 branch.setPrivate(False)1515 branch.setPrivate(False, branch.owner)
1516 self.assertFalse(branch.private)1516 self.assertFalse(branch.private)
15171517
1518 def test_public_to_private_allowed(self):1518 def test_public_to_private_allowed(self):
@@ -1521,7 +1521,7 @@
1521 branch = self.factory.makeProductBranch()1521 branch = self.factory.makeProductBranch()
1522 branch.product.setBranchVisibilityTeamPolicy(1522 branch.product.setBranchVisibilityTeamPolicy(
1523 branch.owner, BranchVisibilityRule.PRIVATE)1523 branch.owner, BranchVisibilityRule.PRIVATE)
1524 branch.setPrivate(True)1524 branch.setPrivate(True, branch.owner)
1525 self.assertTrue(branch.private)1525 self.assertTrue(branch.private)
15261526
1527 def test_public_to_private_not_allowed(self):1527 def test_public_to_private_not_allowed(self):
@@ -1531,20 +1531,29 @@
1531 self.assertRaises(1531 self.assertRaises(
1532 BranchCannotBePrivate,1532 BranchCannotBePrivate,
1533 branch.setPrivate,1533 branch.setPrivate,
1534 True)1534 True, branch.owner)
1535
1536 def test_public_to_private_for_admins(self):
1537 # Admins can override the default behaviour and make any public branch
1538 # private.
1539 branch = self.factory.makeProductBranch()
1540 # Grab a random admin, the teamowner is good enough here.
1541 admins = getUtility(ILaunchpadCelebrities).admin
1542 branch.setPrivate(True, admins.teamowner)
1543 self.assertTrue(branch.private)
15351544
1536 def test_private_to_private(self):1545 def test_private_to_private(self):
1537 # Setting a private branch to be private is a no-op.1546 # Setting a private branch to be private is a no-op.
1538 branch = self.factory.makeProductBranch(private=True)1547 branch = self.factory.makeProductBranch(private=True)
1539 self.assertTrue(branch.private)1548 self.assertTrue(branch.private)
1540 branch.setPrivate(True)1549 branch.setPrivate(True, branch.owner)
1541 self.assertTrue(branch.private)1550 self.assertTrue(branch.private)
15421551
1543 def test_private_to_public_allowed(self):1552 def test_private_to_public_allowed(self):
1544 # If the namespace policy allows public branches, then changing from1553 # If the namespace policy allows public branches, then changing from
1545 # private to public is allowed.1554 # private to public is allowed.
1546 branch = self.factory.makeProductBranch(private=True)1555 branch = self.factory.makeProductBranch(private=True)
1547 branch.setPrivate(False)1556 branch.setPrivate(False, branch.owner)
1548 self.assertFalse(branch.private)1557 self.assertFalse(branch.private)
15491558
1550 def test_private_to_public_not_allowed(self):1559 def test_private_to_public_not_allowed(self):
@@ -1558,7 +1567,7 @@
1558 self.assertRaises(1567 self.assertRaises(
1559 BranchCannotBePublic,1568 BranchCannotBePublic,
1560 branch.setPrivate,1569 branch.setPrivate,
1561 False)1570 False, branch.owner)
15621571
15631572
1564class TestBranchCommitsForDays(TestCaseWithFactory):1573class TestBranchCommitsForDays(TestCaseWithFactory):
15651574
=== modified file 'lib/lp/code/model/tests/test_branchmergeproposals.py'
--- lib/lp/code/model/tests/test_branchmergeproposals.py 2009-11-03 21:05:14 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposals.py 2009-11-27 03:28:11 +0000
@@ -84,7 +84,7 @@
84 login_person(branch.owner)84 login_person(branch.owner)
85 branch.product.setBranchVisibilityTeamPolicy(85 branch.product.setBranchVisibilityTeamPolicy(
86 branch.owner, BranchVisibilityRule.PRIVATE)86 branch.owner, BranchVisibilityRule.PRIVATE)
87 branch.setPrivate(True)87 branch.setPrivate(True, branch.owner)
8888
89 def test_private(self):89 def test_private(self):
90 """Private flag should be True if True for any involved branch."""90 """Private flag should be True if True for any involved branch."""
@@ -92,10 +92,10 @@
92 self.assertFalse(bmp.private)92 self.assertFalse(bmp.private)
93 self.setPrivate(bmp.source_branch)93 self.setPrivate(bmp.source_branch)
94 self.assertTrue(bmp.private)94 self.assertTrue(bmp.private)
95 bmp.source_branch.setPrivate(False)95 bmp.source_branch.setPrivate(False, bmp.source_branch.owner)
96 self.setPrivate(bmp.target_branch)96 self.setPrivate(bmp.target_branch)
97 self.assertTrue(bmp.private)97 self.assertTrue(bmp.private)
98 bmp.target_branch.setPrivate(False)98 bmp.target_branch.setPrivate(False, bmp.target_branch.owner)
99 removeSecurityProxy(bmp).prerequisite_branch = (99 removeSecurityProxy(bmp).prerequisite_branch = (
100 self.factory.makeBranch(product=bmp.source_branch.product))100 self.factory.makeBranch(product=bmp.source_branch.product))
101 self.setPrivate(bmp.prerequisite_branch)101 self.setPrivate(bmp.prerequisite_branch)
102102
=== 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-27 03:28:11 +0000
@@ -1,4 +1,5 @@
1= Changing branch privacy =1Changing branch privacy
2=======================
23
3Which users can have private branches is controlled by4Which users can have private branches is controlled by
4the branch visibility policy (see doc/branch-visibility-policy.txt).5the branch visibility policy (see doc/branch-visibility-policy.txt).
@@ -9,22 +10,46 @@
9Once the branch is created, the user is able to change a public branch10Once the branch is created, the user is able to change a public branch
10to private or a private branch to public in certain circumstances.11to private or a private branch to public in certain circumstances.
1112
1213 >>> login('admin@canonical.com')
13== Branches with no visibility policy ==14 >>> eric = factory.makePerson(
15 ... name='eric', displayname='Eric the Viking',
16 ... email='eric@example.com', password='test')
17 >>> fooix = factory.makeProduct(
18 ... name='fooix', displayname='Fooix', owner=eric)
19 >>> trunk = factory.makeProductBranch(
20 ... owner=eric, product=fooix, name='trunk')
21 >>> logout()
22
23
24Branches with no visibility policy
25----------------------------------
1426
15If there is no policy set, then the user is not able to change the visiblity27If there is no policy set, then the user is not able to change the visiblity
16of the branch.28of the branch.
1729
18 >>> browser = setupBrowser(auth='Basic test@canonical.com:test')30 >>> browser = setupBrowser(auth='Basic eric@example.com:test')
19 >>> browser.open('http://code.launchpad.dev/~name12/firefox/main')31 >>> browser.open('http://code.launchpad.dev/~eric/fooix/trunk')
20 >>> browser.getLink('Change branch details').click()32 >>> browser.getLink('Change branch details').click()
21 >>> browser.getControl('Keep branch confidential')33 >>> browser.getControl('Keep branch confidential')
22 Traceback (most recent call last):34 Traceback (most recent call last):
23 ...35 ...
24 LookupError: label 'Keep branch confidential'36 LookupError: label 'Keep branch confidential'
2537
2638However an admin can.
27== Private branches allowed ==39
40 >>> admin_browser.open('http://code.launchpad.dev/~eric/fooix/trunk')
41 >>> admin_browser.getLink('Change branch details').click()
42 >>> admin_browser.getControl('Keep branch confidential').click()
43 >>> admin_browser.getControl('Change Branch').click()
44
45The branch is now private, so it assumes the standard private presentation.
46
47 >>> print_tag_with_id(admin_browser.contents, 'privacy')
48 This branch is private
49
50
51Private branches allowed
52------------------------
2853
29Allow the Warty Security Team to have private firefox branches.54Allow the Warty Security Team to have private firefox branches.
3055
@@ -40,6 +65,7 @@
40Now Sample Person, who is a member of the Warty Security Team should be65Now Sample Person, who is a member of the Warty Security Team should be
41able to mark the branch as private.66able to mark the branch as private.
4267
68 >>> browser = setupBrowser(auth='Basic test@canonical.com:test')
43 >>> browser.open('http://code.launchpad.dev/~name12/firefox/main')69 >>> browser.open('http://code.launchpad.dev/~name12/firefox/main')
44 >>> browser.getLink('Change branch details').click()70 >>> browser.getLink('Change branch details').click()
45 >>> browser.getControl('Keep branch confidential').selected = True71 >>> browser.getControl('Keep branch confidential').selected = True
@@ -71,7 +97,8 @@
71 The branch is now publicly accessible.97 The branch is now publicly accessible.
7298
7399
74== Private Only branches ==100Private Only branches
101---------------------
75102
76If the policy specifies private only, then the user is able to make103If the policy specifies private only, then the user is able to make
77a public branch private, but not vice versa.104a public branch private, but not vice versa.