Launchpad itself

Merge lp:~jml/launchpad/p3a-commercial-subscription into lp:launchpad

  1. p3a-commercial-subscription
  2. Merge into devel
Proposed by Jonathan Lange
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 15403
Proposed branch: lp:~jml/launchpad/p3a-commercial-subscription
Merge into: lp:launchpad
Prerequisite: lp:~jml/launchpad/p3a-private-team
Diff against target: 195 lines (+59/-30)
5 files modified
lib/lp/registry/browser/tests/test_team.py (+2/-4)
lib/lp/soyuz/model/archive.py (+8/-11)
lib/lp/soyuz/tests/test_archive.py (+8/-1)
lib/lp/soyuz/tests/test_archive_privacy.py (+35/-14)
lib/lp/testing/factory.py (+6/-0)
To merge this branch: bzr merge lp:~jml/launchpad/p3a-commercial-subscription
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Review via email: mp+109602@code.launchpad.net

Commit message

Allow people with commercial subscriptions to create private PPAs via the API.

Description of the change

We want to be able to create private PPAs without having commercial admin privileges. After an implementation discussion with sinzui (summarized https://code.launchpad.net/~jml/launchpad/db-distro-level-ppa-privacy/+merge/109336/comments/234987), we decided that folk with commercial subscriptions should be allowed to create private PPAs on public teams. This branch enables that change, via the createPPA API call.

What it does *not* do is allow people with commercial subscription to change privacy of existing PPAs. This is because I can't figure out how to separate the 'private' attribute from the rest of the attributes requiring 'launchpad.Commercial' and because I don't want to grant 'launchpad.Commercial' privileges to anyone with a commercial subscription, as the permission seems to grant low-level admin privileges than rather than things one would expect a any paying user to be able to do.

Also, this branch does not update the web UI.

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

Thank you. I think this is fine to land.

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/registry/browser/tests/test_team.py'
2--- lib/lp/registry/browser/tests/test_team.py 2012-06-10 13:14:04 +0000
3+++ lib/lp/registry/browser/tests/test_team.py 2012-06-13 10:24:21 +0000
4@@ -501,8 +501,7 @@
5 personset = getUtility(IPersonSet)
6 team = self.factory.makeTeam(
7 subscription_policy=TeamSubscriptionPolicy.MODERATED)
8- product = self.factory.makeProduct(owner=team)
9- self.factory.makeCommercialSubscription(product)
10+ self.factory.grantCommercialSubscription(team)
11 with person_logged_in(team.teamowner):
12 view = create_initialized_view(
13 personset, name=self.view_name, principal=team.teamowner)
14@@ -514,8 +513,7 @@
15 personset = getUtility(IPersonSet)
16 team = self.factory.makeTeam(
17 subscription_policy=TeamSubscriptionPolicy.MODERATED)
18- product = self.factory.makeProduct(owner=team)
19- self.factory.makeCommercialSubscription(product)
20+ self.factory.grantCommercialSubscription(team)
21 team_name = self.factory.getUniqueString()
22 form = {
23 'field.name': team_name,
24
25=== modified file 'lib/lp/soyuz/model/archive.py'
26--- lib/lp/soyuz/model/archive.py 2012-06-12 15:14:40 +0000
27+++ lib/lp/soyuz/model/archive.py 2012-06-13 10:24:21 +0000
28@@ -66,10 +66,7 @@
29 validate_person,
30 )
31 from lp.registry.interfaces.pocket import PackagePublishingPocket
32-from lp.registry.interfaces.role import (
33- IHasOwner,
34- IPersonRoles,
35- )
36+from lp.registry.interfaces.role import IHasOwner
37 from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
38 from lp.registry.model.sourcepackagename import SourcePackageName
39 from lp.registry.model.teammembership import TeamParticipation
40@@ -2097,13 +2094,13 @@
41 creator = getUtility(ILaunchBag).user
42 ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
43 if private:
44- # NOTE: This duplicates the policy in lp/soyuz/configure.zcml
45- # which says that one needs 'launchpad.Commercial' permission to
46- # set 'private', and the logic in `AdminByCommercialTeamOrAdmins`
47- # which determines who is granted launchpad.Commercial
48- # permissions.
49- role = IPersonRoles(creator)
50- if not (owner.private or role.in_admin or role.in_commercial_admin):
51+ # NOTE: This duplicates the policy in lp/soyuz/configure.zcml which
52+ # says that one needs 'launchpad.Commercial' permission to set
53+ # 'private', and the logic in `AdminByCommercialTeamOrAdmins` which
54+ # determines who is granted launchpad.Commercial permissions. The
55+ # difference is that here we grant ability to set 'private' to people
56+ # with a commercial subscription.
57+ if not (owner.private or creator.checkAllowVisibility()):
58 return '%s is not allowed to make private PPAs' % creator.name
59 elif owner.private:
60 return 'Private teams may not have public archives.'
61
62=== modified file 'lib/lp/soyuz/tests/test_archive.py'
63--- lib/lp/soyuz/tests/test_archive.py 2012-06-12 15:14:40 +0000
64+++ lib/lp/soyuz/tests/test_archive.py 2012-06-13 10:24:21 +0000
65@@ -1760,7 +1760,7 @@
66 'A PPA cannot have the same name as its distribution.',
67 validate_ppa(ppa_owner, 'ubuntu'))
68
69- def test_private_ppa_non_commercial_admin(self):
70+ def test_private_ppa_standard_user(self):
71 ppa_owner = self.factory.makePerson()
72 with person_logged_in(ppa_owner):
73 errors = validate_ppa(
74@@ -1769,6 +1769,13 @@
75 '%s is not allowed to make private PPAs' % (ppa_owner.name,),
76 errors)
77
78+ def test_private_ppa_commercial_subscription(self):
79+ owner = self.factory.makePerson()
80+ self.factory.grantCommercialSubscription(owner)
81+ with person_logged_in(owner):
82+ errors = validate_ppa(owner, 'ppa', private=True)
83+ self.assertIsNone(errors)
84+
85 def test_private_ppa_commercial_admin(self):
86 ppa_owner = self.factory.makePerson()
87 with celebrity_logged_in('admin'):
88
89=== modified file 'lib/lp/soyuz/tests/test_archive_privacy.py'
90--- lib/lp/soyuz/tests/test_archive_privacy.py 2012-05-25 22:14:21 +0000
91+++ lib/lp/soyuz/tests/test_archive_privacy.py 2012-06-13 10:24:21 +0000
92@@ -8,6 +8,7 @@
93 from lp.soyuz.interfaces.archive import CannotSwitchPrivacy
94 from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
95 from lp.testing import (
96+ celebrity_logged_in,
97 person_logged_in,
98 TestCaseWithFactory,
99 )
100@@ -37,24 +38,46 @@
101 with person_logged_in(subscriber):
102 self.assertEqual(ppa.description, "Foo")
103
104-
105-class TestArchivePrivacySwitching(TestCaseWithFactory):
106+ def test_owner_changing_privacy(self):
107+ ppa = self.factory.makeArchive()
108+ with person_logged_in(ppa.owner):
109+ self.assertRaises(Unauthorized, setattr, ppa, 'private', True)
110+
111+ def test_owner_with_commercial_subscription_changing_privacy(self):
112+ ppa = self.factory.makeArchive()
113+ self.factory.grantCommercialSubscription(ppa.owner)
114+ with person_logged_in(ppa.owner):
115+ # XXX: jml 2012-06-11: We actually want this to be allowed, but I
116+ # can't think of any way to grant this without also granting other
117+ # attributes that have launchpad.Commercial.
118+ self.assertRaises(Unauthorized, setattr, ppa, 'private', True)
119+
120+ def test_admin_changing_privacy(self):
121+ ppa = self.factory.makeArchive()
122+ with celebrity_logged_in('admin'):
123+ ppa.private = True
124+ self.assertEqual(True, ppa.private)
125+
126+ def test_commercial_admin_changing_privacy(self):
127+ ppa = self.factory.makeArchive()
128+ with celebrity_logged_in('commercial_admin'):
129+ ppa.private = True
130+ self.assertEqual(True, ppa.private)
131+
132+
133+class TestPrivacySwitching(TestCaseWithFactory):
134
135 layer = LaunchpadZopelessLayer
136
137- def set_ppa_privacy(self, ppa, private):
138- """Helper method to privatise a ppa."""
139- ppa.private = private
140-
141 def test_switch_privacy_no_pubs_succeeds(self):
142 # Changing the privacy is fine if there are no publishing
143 # records.
144 public_ppa = self.factory.makeArchive()
145- self.set_ppa_privacy(public_ppa, private=True)
146+ public_ppa.private = True
147 self.assertTrue(public_ppa.private)
148
149 private_ppa = self.factory.makeArchive(private=True)
150- self.set_ppa_privacy(private_ppa, private=False)
151+ private_ppa.private = False
152 self.assertFalse(private_ppa.private)
153
154 def test_switch_privacy_with_pubs_fails(self):
155@@ -69,19 +92,17 @@
156 publisher.getPubSource(archive=private_ppa)
157
158 self.assertRaises(
159- CannotSwitchPrivacy,
160- self.set_ppa_privacy, public_ppa, private=True)
161+ CannotSwitchPrivacy, setattr, public_ppa, 'private', True)
162
163 self.assertRaises(
164- CannotSwitchPrivacy,
165- self.set_ppa_privacy, private_ppa, private=False)
166+ CannotSwitchPrivacy, setattr, private_ppa, 'private', False)
167
168 def test_buildd_secret_was_generated(self):
169 public_ppa = self.factory.makeArchive()
170- self.set_ppa_privacy(public_ppa, private=True)
171+ public_ppa.private = True
172 self.assertNotEqual(public_ppa.buildd_secret, None)
173
174 def test_discard_buildd_was_discarded(self):
175 private_ppa = self.factory.makeArchive(private=True)
176- self.set_ppa_privacy(private_ppa, private=False)
177+ private_ppa.private = False
178 self.assertEqual(private_ppa.buildd_secret, None)
179
180=== modified file 'lib/lp/testing/factory.py'
181--- lib/lp/testing/factory.py 2012-06-08 06:01:50 +0000
182+++ lib/lp/testing/factory.py 2012-06-13 10:24:21 +0000
183@@ -4338,6 +4338,12 @@
184 sales_system_id='new',
185 whiteboard='')
186
187+ def grantCommercialSubscription(self, person, months=12):
188+ """Give 'person' a commercial subscription."""
189+ product = self.makeProduct(owner=person)
190+ product.redeemSubscriptionVoucher(
191+ self.getUniqueString(), person, person, months)
192+
193
194 # Some factory methods return simple Python types. We don't add
195 # security wrappers for them, as well as for objects created by