Merge lp:~jml/launchpad/commercial-means-shut-up into lp:launchpad
- commercial-means-shut-up
- Merge into devel
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Brad Crittenden | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 15209 | ||||
Proposed branch: | lp:~jml/launchpad/commercial-means-shut-up | ||||
Merge into: | lp:launchpad | ||||
Prerequisite: | lp:~jml/launchpad/drop-special-commercial-permissions | ||||
Diff against target: |
532 lines (+109/-73) 15 files modified
lib/lp/registry/interfaces/person.py (+5/-2) lib/lp/registry/model/person.py (+4/-3) lib/lp/soyuz/browser/archive.py (+16/-6) lib/lp/soyuz/configure.zcml (+4/-3) lib/lp/soyuz/interfaces/archive.py (+15/-6) lib/lp/soyuz/mail/notifications.py (+4/-3) lib/lp/soyuz/model/archive.py (+17/-7) lib/lp/soyuz/stories/ppa/xx-ppa-workflow.txt (+9/-7) lib/lp/soyuz/stories/webservice/xx-archive-commercial.txt (+2/-1) lib/lp/soyuz/stories/webservice/xx-archive.txt (+5/-5) lib/lp/soyuz/tests/test_archive.py (+13/-18) lib/lp/soyuz/tests/test_archive_privacy.py (+2/-1) lib/lp/soyuz/tests/test_archive_subscriptions.py (+1/-1) lib/lp/soyuz/tests/test_person_createppa.py (+6/-5) lib/lp/testing/factory.py (+6/-5) |
||||
To merge this branch: | bzr merge lp:~jml/launchpad/commercial-means-shut-up | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Brad Crittenden (community) | code | Approve | |
Review via email: mp+104739@code.launchpad.net |
Commit message
Rename IArchive.commercial to IArchive.
Description of the change
This branch renames the 'commercial' attribute on 'IArchive' to 'suppress_
We don't do any queries across the attribute in Launchpad, so we're good there. Ubuntu Software Centre is the only user, and they (currently) don't even use it at an API level, so we're good for backwards compatibility.
Thus, we can un-expose 'commercial' and forbid any access to the attribute in code.
Future patches will:
- rename the column in the db
- change the permissions so that PPA owners (& any PPA creator) can change the attribute
- (possibly) migrate from boolean to an enum
Adds 36 lines, reducing us to 102 lines of credit.
Thanks,
jml
James Westby (james-w) wrote : | # |
Brad Crittenden (bac) wrote : | # |
Hi Jono,
* In lib/lp/
* On IRC we discussed backwards compatibility and you explained nothing in 1.0 or previous has changed. Thanks.
Thanks for the attentiveness and general cleanup you made.
Jonathan Lange (jml) wrote : | # |
I'm going to be offline for a while. If there are any test failures with this branch, please forward them to James Westby, who will be carrying on this work.
Preview Diff
1 | === modified file 'lib/lp/registry/interfaces/person.py' |
2 | --- lib/lp/registry/interfaces/person.py 2012-04-18 13:51:38 +0000 |
3 | +++ lib/lp/registry/interfaces/person.py 2012-05-04 15:09:32 +0000 |
4 | @@ -1487,12 +1487,12 @@ |
5 | displayname=TextLine(required=False), |
6 | description=TextLine(required=False), |
7 | private=Bool(required=False), |
8 | - commercial=Bool(required=False), |
9 | + suppress_subscription_notifications=Bool(required=False), |
10 | ) |
11 | @export_factory_operation(Interface, []) # Really IArchive. |
12 | @operation_for_version("beta") |
13 | def createPPA(name=None, displayname=None, description=None, |
14 | - private=False, commercial=False): |
15 | + private=False, suppress_subscription_notifications=False): |
16 | """Create a PPA. |
17 | |
18 | :param name: A string with the name of the new PPA to create. If |
19 | @@ -1501,6 +1501,9 @@ |
20 | :param description: The description for the new PPA. |
21 | :param private: Whether or not to create a private PPA. Defaults to |
22 | False, which means the PPA will be public. |
23 | + :param suppress_subscription_notifications: Whether or not to suppress |
24 | + emails to new subscribers about their subscriptions. Only |
25 | + meaningful for private PPAs. |
26 | :raises: `PPACreationError` if an error is encountered |
27 | |
28 | :return: a PPA `IArchive` record. |
29 | |
30 | === modified file 'lib/lp/registry/model/person.py' |
31 | --- lib/lp/registry/model/person.py 2012-04-20 07:01:18 +0000 |
32 | +++ lib/lp/registry/model/person.py 2012-05-04 15:09:32 +0000 |
33 | @@ -2972,14 +2972,15 @@ |
34 | return getUtility(IArchiveSet).getPPAOwnedByPerson(self, name) |
35 | |
36 | def createPPA(self, name=None, displayname=None, description=None, |
37 | - private=False, commercial=False): |
38 | + private=False, suppress_subscription_notifications=False): |
39 | """See `IPerson`.""" |
40 | # XXX: We pass through the Person on whom the PPA is being created, |
41 | # but validatePPA assumes that that Person is also the one creating |
42 | # the PPA. This is not true in general, and particularly not for |
43 | # teams. Instead, both the acting user and the target of the PPA |
44 | # creation ought to be passed through. |
45 | - errors = Archive.validatePPA(self, name, private, commercial) |
46 | + errors = Archive.validatePPA( |
47 | + self, name, private, suppress_subscription_notifications) |
48 | if errors: |
49 | raise PPACreationError(errors) |
50 | ubuntu = getUtility(ILaunchpadCelebrities).ubuntu |
51 | @@ -2987,7 +2988,7 @@ |
52 | owner=self, purpose=ArchivePurpose.PPA, |
53 | distribution=ubuntu, name=name, displayname=displayname, |
54 | description=description, private=private, |
55 | - commercial=commercial) |
56 | + suppress_subscription_notifications=suppress_subscription_notifications) |
57 | |
58 | def isBugContributor(self, user=None): |
59 | """See `IPerson`.""" |
60 | |
61 | === modified file 'lib/lp/soyuz/browser/archive.py' |
62 | --- lib/lp/soyuz/browser/archive.py 2012-02-10 10:00:53 +0000 |
63 | +++ lib/lp/soyuz/browser/archive.py 2012-05-04 15:09:32 +0000 |
64 | @@ -2107,9 +2107,17 @@ |
65 | |
66 | class ArchiveAdminView(BaseArchiveEditView, EnableRestrictedFamiliesMixin): |
67 | |
68 | - field_names = ['enabled', 'private', 'commercial', 'require_virtualized', |
69 | - 'build_debug_symbols', 'buildd_secret', 'authorized_size', |
70 | - 'relative_build_score', 'external_dependencies'] |
71 | + field_names = [ |
72 | + 'enabled', |
73 | + 'private', |
74 | + 'suppress_subscription_notifications', |
75 | + 'require_virtualized', |
76 | + 'build_debug_symbols', |
77 | + 'buildd_secret', |
78 | + 'authorized_size', |
79 | + 'relative_build_score', |
80 | + 'external_dependencies', |
81 | + ] |
82 | custom_widget('external_dependencies', TextAreaWidget, height=3) |
83 | custom_widget('enabled_restricted_families', LabeledMultiCheckBoxWidget) |
84 | page_title = 'Administer' |
85 | @@ -2164,10 +2172,12 @@ |
86 | error_text = "\n".join(errors) |
87 | self.setFieldError('external_dependencies', error_text) |
88 | |
89 | - if data.get('commercial') is True and not data['private']: |
90 | + if (data.get('suppress_subscription_notifications') is True |
91 | + and not data['private']): |
92 | self.setFieldError( |
93 | - 'commercial', |
94 | - 'Can only set commericial for private archives.') |
95 | + 'suppress_subscription_notifications', |
96 | + 'Can only suppress subscription notifications for private ' |
97 | + 'archives.') |
98 | |
99 | enabled_restricted_families = data.get('enabled_restricted_families') |
100 | require_virtualized = data.get('require_virtualized') |
101 | |
102 | === modified file 'lib/lp/soyuz/configure.zcml' |
103 | --- lib/lp/soyuz/configure.zcml 2012-04-17 22:10:05 +0000 |
104 | +++ lib/lp/soyuz/configure.zcml 2012-05-04 15:09:32 +0000 |
105 | @@ -409,11 +409,12 @@ |
106 | --> |
107 | <require |
108 | permission="launchpad.Commercial" |
109 | - interface="lp.soyuz.interfaces.archive.IArchiveCommercial" |
110 | + interface="lp.soyuz.interfaces.archive.IArchiveCommercial" |
111 | set_attributes="authorized_size build_debug_symbols buildd_secret |
112 | - commercial enabled_restricted_families |
113 | + enabled_restricted_families |
114 | external_dependencies private |
115 | - require_virtualized relative_build_score "/> |
116 | + require_virtualized relative_build_score |
117 | + suppress_subscription_notifications"/> |
118 | <require |
119 | permission="launchpad.Admin" |
120 | set_attributes="distribution name signing_key"/> |
121 | |
122 | === modified file 'lib/lp/soyuz/interfaces/archive.py' |
123 | --- lib/lp/soyuz/interfaces/archive.py 2012-05-01 11:15:58 +0000 |
124 | +++ lib/lp/soyuz/interfaces/archive.py 2012-05-04 15:09:32 +0000 |
125 | @@ -319,14 +319,21 @@ |
126 | is_main = Bool( |
127 | title=_("True if archive is a main archive type"), required=False) |
128 | |
129 | - commercial = exported( |
130 | + commercial = Bool( |
131 | + title=_("Commercial"), |
132 | + required=True, |
133 | + description=_( |
134 | + "True if the archive is for commercial applications in the " |
135 | + "Ubuntu Software Centre. Governs whether subscribers or " |
136 | + "uploaders get mail from Launchpad about archive events.")) |
137 | + |
138 | + suppress_subscription_notifications = exported( |
139 | Bool( |
140 | - title=_("Commercial"), |
141 | + title=_("Suppress subscription notifications"), |
142 | required=True, |
143 | description=_( |
144 | - "True if the archive is for commercial applications in the " |
145 | - "Ubuntu Software Centre. Governs whether subscribers or " |
146 | - "uploaders get mail from Launchpad about archive events."))) |
147 | + "Whether subscribers to private PPAs get emails about their " |
148 | + "subscriptions."))) |
149 | |
150 | def checkArchivePermission(person, component_or_package=None): |
151 | """Check to see if person is allowed to upload to component. |
152 | @@ -1743,7 +1750,7 @@ |
153 | |
154 | def new(purpose, owner, name=None, displayname=None, distribution=None, |
155 | description=None, enabled=True, require_virtualized=True, |
156 | - private=False, commercial=False): |
157 | + private=False, suppress_subscription_notifications=False): |
158 | """Create a new archive. |
159 | |
160 | On named-ppa creation, the signing key for the default PPA for the |
161 | @@ -1766,6 +1773,8 @@ |
162 | :param require_virtualized: whether builds for the new archive shall |
163 | be carried out on virtual builders |
164 | :param private: whether or not to make the PPA private |
165 | + :param suppress_subscription_notifications: whether to suppress |
166 | + emails to subscribers about new subscriptions. |
167 | |
168 | :return: an `IArchive` object. |
169 | :raises AssertionError if name is already taken within distribution. |
170 | |
171 | === modified file 'lib/lp/soyuz/mail/notifications.py' |
172 | --- lib/lp/soyuz/mail/notifications.py 2011-12-30 06:14:56 +0000 |
173 | +++ lib/lp/soyuz/mail/notifications.py 2012-05-04 15:09:32 +0000 |
174 | @@ -29,9 +29,10 @@ |
175 | |
176 | archive = subscription.archive |
177 | |
178 | - # We don't send notification emails for commercial PPAs as these |
179 | - # are purchased via software center (and do not mention Launchpad). |
180 | - if archive.commercial: |
181 | + # We don't send notification emails for some PPAs, particularly those that |
182 | + # are purchased via the Software Centre, so that its users do not have to |
183 | + # learn about Launchpad. |
184 | + if archive.suppress_subscription_notifications: |
185 | return |
186 | |
187 | registrant_name = subscription.registrant.displayname |
188 | |
189 | === modified file 'lib/lp/soyuz/model/archive.py' |
190 | --- lib/lp/soyuz/model/archive.py 2012-05-01 10:20:07 +0000 |
191 | +++ lib/lp/soyuz/model/archive.py 2012-05-04 15:09:32 +0000 |
192 | @@ -340,6 +340,14 @@ |
193 | else: |
194 | alsoProvides(self, IDistributionArchive) |
195 | |
196 | + @property |
197 | + def suppress_subscription_notifications(self): |
198 | + return self.commercial |
199 | + |
200 | + @suppress_subscription_notifications.setter |
201 | + def suppress_subscription_notifications(self, suppress): |
202 | + self.commercial = suppress |
203 | + |
204 | # Note: You may safely ignore lint when it complains about this |
205 | # declaration. As of Python 2.6, this is a perfectly valid way |
206 | # of adding a setter |
207 | @@ -1978,9 +1986,9 @@ |
208 | |
209 | @classmethod |
210 | def validatePPA(self, person, proposed_name, private=False, |
211 | - commercial=False): |
212 | + suppress_subscription_notifications=False): |
213 | ubuntu = getUtility(ILaunchpadCelebrities).ubuntu |
214 | - if private or commercial: |
215 | + if private or suppress_subscription_notifications: |
216 | # NOTE: This duplicates the policy in lp/soyuz/configure.zcml |
217 | # which says that one needs 'launchpad.Commercial' permission to |
218 | # set 'private', and the logic in `AdminByCommercialTeamOrAdmins` |
219 | @@ -1991,10 +1999,10 @@ |
220 | if private: |
221 | return ( |
222 | '%s is not allowed to make private PPAs' % person.name) |
223 | - if commercial: |
224 | + if suppress_subscription_notifications: |
225 | return ( |
226 | - '%s is not allowed to make commercial PPAs' |
227 | - % person.name) |
228 | + '%s is not allowed to make PPAs that suppress ' |
229 | + 'subscription notifications' % person.name) |
230 | if person.is_team and ( |
231 | person.subscriptionpolicy in OPEN_TEAM_POLICY): |
232 | return "Open teams cannot have PPAs." |
233 | @@ -2129,7 +2137,8 @@ |
234 | |
235 | def new(self, purpose, owner, name=None, displayname=None, |
236 | distribution=None, description=None, enabled=True, |
237 | - require_virtualized=True, private=False, commercial=False): |
238 | + require_virtualized=True, private=False, |
239 | + suppress_subscription_notifications=False): |
240 | """See `IArchiveSet`.""" |
241 | if distribution is None: |
242 | distribution = getUtility(ILaunchpadCelebrities).ubuntu |
243 | @@ -2206,7 +2215,8 @@ |
244 | else: |
245 | new_archive.private = private |
246 | |
247 | - new_archive.commercial = commercial |
248 | + new_archive.suppress_subscription_notifications = ( |
249 | + suppress_subscription_notifications) |
250 | |
251 | return new_archive |
252 | |
253 | |
254 | === modified file 'lib/lp/soyuz/stories/ppa/xx-ppa-workflow.txt' |
255 | --- lib/lp/soyuz/stories/ppa/xx-ppa-workflow.txt 2012-01-15 11:06:57 +0000 |
256 | +++ lib/lp/soyuz/stories/ppa/xx-ppa-workflow.txt 2012-05-04 15:09:32 +0000 |
257 | @@ -362,7 +362,8 @@ |
258 | True |
259 | >>> admin_browser.getControl(name="field.private").value |
260 | False |
261 | - >>> admin_browser.getControl(name="field.commercial").value |
262 | + >>> admin_browser.getControl( |
263 | + ... name="field.suppress_subscription_notifications").value |
264 | False |
265 | >>> admin_browser.getControl(name="field.require_virtualized").value |
266 | True |
267 | @@ -373,7 +374,8 @@ |
268 | |
269 | >>> admin_browser.getControl(name="field.enabled").value = False |
270 | >>> admin_browser.getControl(name="field.private").value = True |
271 | - >>> admin_browser.getControl(name="field.commercial").value = True |
272 | + >>> admin_browser.getControl( |
273 | + ... name="field.suppress_subscription_notifications").value = True |
274 | >>> admin_browser.getControl(name="field.buildd_secret").value = "secret" |
275 | >>> admin_browser.getControl( |
276 | ... name="field.require_virtualized").value = True |
277 | @@ -417,10 +419,10 @@ |
278 | 'deb not_a_url' is not a complete and valid sources.list entry |
279 | |
280 | |
281 | -Setting the buildd secret for non-private archives also generates |
282 | -an error. Because the "commercial" flag is also currently set, removing |
283 | -privacy will also trigger a validation error because the commercial flag can |
284 | -only be set on private archives: |
285 | +Setting the buildd secret for non-private archives also generates an error. |
286 | +Because the "suppress_subscription_notifications" flag is also currently set, |
287 | +removing privacy will also trigger a validation error because the |
288 | +suppress_subscription_notifications flag can only be set on private archives: |
289 | |
290 | >>> admin_browser.getControl( |
291 | ... name="field.external_dependencies").value = "" |
292 | @@ -431,7 +433,7 @@ |
293 | >>> for error in get_feedback_messages(admin_browser.contents): |
294 | ... print error |
295 | There are 2 errors. |
296 | - Can only set commericial for private archives. |
297 | + Can only suppress subscription notifications for private archives. |
298 | Do not specify for non-private archives |
299 | |
300 | |
301 | |
302 | === modified file 'lib/lp/soyuz/stories/webservice/xx-archive-commercial.txt' |
303 | --- lib/lp/soyuz/stories/webservice/xx-archive-commercial.txt 2012-05-04 10:43:42 +0000 |
304 | +++ lib/lp/soyuz/stories/webservice/xx-archive-commercial.txt 2012-05-04 15:09:32 +0000 |
305 | @@ -8,7 +8,8 @@ |
306 | Start by making a commercial PPA that we can fetch via the webservice: |
307 | |
308 | >>> login("admin@canonical.com") |
309 | - >>> ppa = factory.makeArchive(private=True, name="cpppa", commercial=True) |
310 | + >>> ppa = factory.makeArchive(private=True, name="cpppa", |
311 | + ... suppress_subscription_notifications=True) |
312 | >>> import transaction |
313 | >>> transaction.commit() |
314 | >>> logout() |
315 | |
316 | === modified file 'lib/lp/soyuz/stories/webservice/xx-archive.txt' |
317 | --- lib/lp/soyuz/stories/webservice/xx-archive.txt 2012-02-28 05:09:39 +0000 |
318 | +++ lib/lp/soyuz/stories/webservice/xx-archive.txt 2012-05-04 15:09:32 +0000 |
319 | @@ -17,7 +17,6 @@ |
320 | >>> from lazr.restful.testing.webservice import pprint_entry |
321 | >>> pprint_entry(cprov_archive) |
322 | authorized_size: 1024 |
323 | - commercial: False |
324 | dependencies_collection_link: |
325 | u'http://.../~cprov/+archive/ppa/dependencies' |
326 | description: u'packages to help my friends.' |
327 | @@ -31,6 +30,7 @@ |
328 | resource_type_link: u'http://.../#archive' |
329 | self_link: u'http://.../~cprov/+archive/ppa' |
330 | signing_key_fingerprint: None |
331 | + suppress_subscription_notifications: False |
332 | web_link: u'http://launchpad.../~cprov/+archive/ppa' |
333 | |
334 | For "devel" additional attributes are available. |
335 | @@ -39,7 +39,6 @@ |
336 | ... "/~cprov/+archive/ppa", api_version='devel').jsonBody() |
337 | >>> pprint_entry(cprov_archive_devel) |
338 | authorized_size: 1024 |
339 | - commercial: False |
340 | dependencies_collection_link: u'http://.../~cprov/+archive/ppa/dependencies' |
341 | description: u'packages to help my friends.' |
342 | displayname: u'PPA for Celso Providelo' |
343 | @@ -53,6 +52,7 @@ |
344 | resource_type_link: u'http://.../#archive' |
345 | self_link: u'http://.../~cprov/+archive/ppa' |
346 | signing_key_fingerprint: None |
347 | + suppress_subscription_notifications: False |
348 | web_link: u'http://launchpad.../~cprov/+archive/ppa' |
349 | |
350 | While the Archive signing key is being generated its |
351 | @@ -108,7 +108,6 @@ |
352 | ... ubuntutest['main_archive_link']).jsonBody() |
353 | >>> pprint_entry(ubuntu_main_archive) |
354 | authorized_size: None |
355 | - commercial: False |
356 | dependencies_collection_link: |
357 | u'http://.../ubuntutest/+archive/primary/dependencies' |
358 | description: None |
359 | @@ -122,6 +121,7 @@ |
360 | resource_type_link: u'http://.../#archive' |
361 | self_link: u'http://.../ubuntutest/+archive/primary' |
362 | signing_key_fingerprint: None |
363 | + suppress_subscription_notifications: False |
364 | web_link: u'http://launchpad.../ubuntutest/+archive/primary' |
365 | |
366 | A distribution can also provide a list of all its archives: |
367 | @@ -977,7 +977,6 @@ |
368 | |
369 | >>> pprint_entry(user_webservice.get("/~cprov/+archive/p3a").jsonBody()) |
370 | authorized_size: u'tag:launchpad.net:2008:redacted' |
371 | - commercial: False |
372 | dependencies_collection_link: |
373 | u'http://.../~cprov/+archive/p3a/dependencies' |
374 | description: u'tag:launchpad.net:2008:redacted' |
375 | @@ -991,11 +990,11 @@ |
376 | resource_type_link: u'http://.../#archive' |
377 | self_link: u'http://.../~cprov/+archive/p3a' |
378 | signing_key_fingerprint: u'tag:launchpad.net:2008:redacted' |
379 | + suppress_subscription_notifications: False |
380 | web_link: u'http://launchpad.../~cprov/+archive/p3a' |
381 | |
382 | >>> pprint_entry(cprov_webservice.get("/~cprov/+archive/p3a").jsonBody()) |
383 | authorized_size: 2048 |
384 | - commercial: False |
385 | dependencies_collection_link: |
386 | u'http://.../~cprov/+archive/p3a/dependencies' |
387 | description: u'packages to help my friends.' |
388 | @@ -1009,6 +1008,7 @@ |
389 | resource_type_link: u'http://.../#archive' |
390 | self_link: u'http://.../~cprov/+archive/p3a' |
391 | signing_key_fingerprint: u'ABCDEF0123456789ABCDDCBA0000111112345678' |
392 | + suppress_subscription_notifications: False |
393 | web_link: u'http://launchpad.../~cprov/+archive/p3a' |
394 | |
395 | Creating subscriptions to a (private) archive |
396 | |
397 | === modified file 'lib/lp/soyuz/tests/test_archive.py' |
398 | --- lib/lp/soyuz/tests/test_archive.py 2012-03-28 17:26:49 +0000 |
399 | +++ lib/lp/soyuz/tests/test_archive.py 2012-05-04 15:09:32 +0000 |
400 | @@ -1282,32 +1282,27 @@ |
401 | self.assertEqual(ArchiveStatus.DELETING, self.archive.status) |
402 | |
403 | |
404 | -class TestCommercialArchive(TestCaseWithFactory): |
405 | - """Tests relating to commercial archives.""" |
406 | +class TestSuppressSubscription(TestCaseWithFactory): |
407 | + """Tests relating to suppressing subscription.""" |
408 | |
409 | layer = DatabaseFunctionalLayer |
410 | |
411 | - def setUp(self): |
412 | - super(TestCommercialArchive, self).setUp() |
413 | - self.archive = self.factory.makeArchive() |
414 | - |
415 | - def setCommercial(self, archive, commercial): |
416 | - """Helper function.""" |
417 | - archive.commercial = commercial |
418 | - |
419 | - def test_set_and_get_commercial(self): |
420 | + def test_set_and_get_suppress(self): |
421 | # Basic set and get of the commercial property. Anyone can read |
422 | # it and it defaults to False. |
423 | - login_person(self.archive.owner) |
424 | - self.assertFalse(self.archive.commercial) |
425 | + archive = self.factory.makeArchive() |
426 | + with person_logged_in(archive.owner): |
427 | + self.assertFalse(archive.suppress_subscription_notifications) |
428 | |
429 | - # The archive owner can't change the value. |
430 | - self.assertRaises(Unauthorized, self.setCommercial, self.archive, True) |
431 | + # The archive owner can't change the value. |
432 | + self.assertRaises( |
433 | + Unauthorized, |
434 | + setattr, archive, 'suppress_subscription_notifications', True) |
435 | |
436 | # Commercial admins can change it. |
437 | - login(COMMERCIAL_ADMIN_EMAIL) |
438 | - self.setCommercial(self.archive, True) |
439 | - self.assertTrue(self.archive.commercial) |
440 | + with celebrity_logged_in('commercial_admin'): |
441 | + archive.suppress_subscription_notifications = True |
442 | + self.assertTrue(archive.suppress_subscription_notifications) |
443 | |
444 | |
445 | class TestBuildDebugSymbols(TestCaseWithFactory): |
446 | |
447 | === modified file 'lib/lp/soyuz/tests/test_archive_privacy.py' |
448 | --- lib/lp/soyuz/tests/test_archive_privacy.py 2012-05-01 11:13:33 +0000 |
449 | +++ lib/lp/soyuz/tests/test_archive_privacy.py 2012-05-04 15:09:32 +0000 |
450 | @@ -41,7 +41,8 @@ |
451 | # Commercial private PPAs cannot be accessed by non-subscribers. |
452 | ppa_name = self.factory.getUniqueString() |
453 | ppa = self.factory.makeArchive( |
454 | - private=True, commercial=True, name=ppa_name) |
455 | + private=True, suppress_subscription_notifications=True, |
456 | + name=ppa_name) |
457 | non_subscriber = self.factory.makePerson() |
458 | with person_logged_in(non_subscriber): |
459 | self.assertEqual(ppa_name, ppa.name) |
460 | |
461 | === modified file 'lib/lp/soyuz/tests/test_archive_subscriptions.py' |
462 | --- lib/lp/soyuz/tests/test_archive_subscriptions.py 2012-02-28 11:14:44 +0000 |
463 | +++ lib/lp/soyuz/tests/test_archive_subscriptions.py 2012-05-04 15:09:32 +0000 |
464 | @@ -109,7 +109,7 @@ |
465 | def test_new_commercial_subscription_no_email(self): |
466 | # As per bug 611568, an email is not sent for commercial PPAs. |
467 | with celebrity_logged_in('commercial_admin'): |
468 | - self.archive.commercial = True |
469 | + self.archive.suppress_subscription_notifications = True |
470 | |
471 | # Logging in as a celebrity team causes an email to be sent |
472 | # because a person is added as a member of the team, so this |
473 | |
474 | === modified file 'lib/lp/soyuz/tests/test_person_createppa.py' |
475 | --- lib/lp/soyuz/tests/test_person_createppa.py 2012-04-04 13:37:01 +0000 |
476 | +++ lib/lp/soyuz/tests/test_person_createppa.py 2012-05-04 15:09:32 +0000 |
477 | @@ -35,13 +35,14 @@ |
478 | self.assertRaises( |
479 | PPACreationError, person.createPPA, private=True) |
480 | |
481 | - def test_commercial(self): |
482 | + def test_suppress_subscription_notifications(self): |
483 | with celebrity_logged_in('commercial_admin') as person: |
484 | - ppa = person.createPPA(commercial=True) |
485 | - self.assertEqual(True, ppa.commercial) |
486 | + ppa = person.createPPA(suppress_subscription_notifications=True) |
487 | + self.assertEqual(True, ppa.suppress_subscription_notifications) |
488 | |
489 | - def test_commercial_without_permission(self): |
490 | + def test_suppress_without_permission(self): |
491 | person = self.factory.makePerson() |
492 | with person_logged_in(person): |
493 | self.assertRaises( |
494 | - PPACreationError, person.createPPA, commercial=True) |
495 | + PPACreationError, person.createPPA, |
496 | + suppress_subscription_notifications=True) |
497 | |
498 | === modified file 'lib/lp/testing/factory.py' |
499 | --- lib/lp/testing/factory.py 2012-05-02 05:25:11 +0000 |
500 | +++ lib/lp/testing/factory.py 2012-05-04 15:09:32 +0000 |
501 | @@ -2658,7 +2658,7 @@ |
502 | def makeArchive(self, distribution=None, owner=None, name=None, |
503 | purpose=None, enabled=True, private=False, |
504 | virtualized=True, description=None, displayname=None, |
505 | - commercial=False): |
506 | + suppress_subscription_notifications=False): |
507 | """Create and return a new arbitrary archive. |
508 | |
509 | :param distribution: Supply IDistribution, defaults to a new one |
510 | @@ -2671,8 +2671,9 @@ |
511 | :param private: Whether the archive is created private. |
512 | :param virtualized: Whether the archive is virtualized. |
513 | :param description: A description of the archive. |
514 | - :param commercial: Whether the archive is commercial. Defaults to |
515 | - False. |
516 | + :param suppress_subscription_notifications: Whether to suppress |
517 | + subscription notifications, defaults to False. Only useful |
518 | + for private archives. |
519 | """ |
520 | if purpose is None: |
521 | purpose = ArchivePurpose.PPA |
522 | @@ -2711,9 +2712,9 @@ |
523 | naked_archive.private = True |
524 | naked_archive.buildd_secret = "sekrit" |
525 | |
526 | - if commercial: |
527 | + if suppress_subscription_notifications: |
528 | naked_archive = removeSecurityProxy(archive) |
529 | - naked_archive.commercial = True |
530 | + naked_archive.suppress_subscription_notifications = True |
531 | |
532 | return archive |
533 |
Hi,
This change looks ok to me.
Mentioning the fact that it is only valid for private PPAs in the docstrings
might be useful in helping people understand the point, or avoiding them
learning the hard way by hitting the validatePPA check.
Thanks,
James