Merge lp:~sinzui/launchpad/delete-packaging-link into lp:launchpad

Proposed by Curtis Hovey
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 15987
Proposed branch: lp:~sinzui/launchpad/delete-packaging-link
Merge into: lp:launchpad
Diff against target: 642 lines (+86/-199)
11 files modified
lib/lp/registry/browser/tests/test_sourcepackage_views.py (+28/-31)
lib/lp/registry/doc/sourcepackage.txt (+0/-64)
lib/lp/registry/interfaces/packaging.py (+2/-3)
lib/lp/registry/model/packaging.py (+7/-6)
lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging-concurrent-deletion.txt (+2/-2)
lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging.txt (+2/-1)
lib/lp/registry/tests/test_packaging.py (+9/-20)
lib/lp/registry/tests/test_sourcepackage.py (+16/-59)
lib/lp/testing/factory.py (+6/-1)
lib/lp/translations/browser/tests/test_sharing_details.py (+12/-11)
lib/lp/translations/tests/test_translationpackagingjob.py (+2/-1)
To merge this branch: bzr merge lp:~sinzui/launchpad/delete-packaging-link
Reviewer Review Type Date Requested Status
Richard Harding (community) Approve
Review via email: mp+124809@code.launchpad.net

Commit message

Allow the Ubuntu community to manage their upstream packaging links.

Description of the change

Launchpad's zcml states that anyone can unlink a package from a series.
The rule is used on the distribution and source package pages, and on
the project's package pages. When translation message sharing was added,
a condition was added to the code that will raise a forbidden error if
the user attempts to unlink a package from a series that is sharing
translations.

There is no role or action that a user can do that definitely qualify a
user to manage packaging. We know that project users are more likely to
create bogus packaging links and that users create more packaging links
then are deleted.

The Packaging.userCanDelete() restrictions are naive. The rules assume
that shared messages are more important then the true links to the
upstream project. The ubuntu community cannot fix bogus data created by
project maintainers who do not understand packaging. Ubuntu encourages
projects to link to them by making it easy to create the link, and the
community gardens the packaging links. The restrictive code is solving a
problem that never existed, and introduces a new problem that distro
communities cannot manage their data.

The broken rules and tests favoured the package or productseries owner,
but these properties recored registrants, not people with
responsibility. In fact, most of these Packaging permission checks are
asking if the user is ~sinzui or ~jelmer because we created most of the
packaging links :(

--------------------------------------------------------------------

RULES

    Pre-implementation: jcsackett
    * Either remove Packaging.userCanDelete() or simplify it to verify
      if the user has launchpad experience.

QA

    * Visit https://qastaging.launchpad.net/ubuntu/q-series/+source/tweak
    * Verify there is a delete and edit icon next to the upstream series
    * Open each action and verify the pages loads without error
    * Visit https://translations.qastaging.launchpad.net/ubuntu/precise/+source/tweak/+sharing-details
    * Verify the edit and delete icons are show next to the package
    * Open each and verify the pages loads without error

LINT

    lib/lp/registry/interfaces/packaging.py
    lib/lp/registry/model/packaging.py
    lib/lp/translations/browser/tests/test_sharing_details.py

TEST

    ./bin/test -vvc -t packaging lp.translations.browser.tests.test_sharing

IMPLEMENTATION

I updated Packaging.userCanDelete() to check if the user is not
probationary, which is the experience test we use elsewhere when we do
not want novices creating bogus data. I updated the tests that care
about sharing and packaging to make it clear probationary users cannot
change collection packaging data.
    lib/lp/registry/interfaces/packaging.py
    lib/lp/registry/model/packaging.py
    lib/lp/translations/browser/tests/test_sharing_details.py

To post a comment you must log in.
Revision history for this message
Richard Harding (rharding) wrote :

Thanks for clearing up in irc that the karma changes are needed to get the user to not be probationary. For devs not totally aware of the distinction a comment to that effect would be helpful. Thanks for the change.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/registry/browser/tests/test_sourcepackage_views.py'
--- lib/lp/registry/browser/tests/test_sourcepackage_views.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/browser/tests/test_sourcepackage_views.py 2012-09-19 18:22:24 +0000
@@ -213,17 +213,14 @@
213213
214 layer = DatabaseFunctionalLayer214 layer = DatabaseFunctionalLayer
215215
216 def makeSourcePackageOverviewMenu(self, with_packaging, for_other_user):216 def makeSourcePackageOverviewMenu(self, with_packaging, karma=None):
217 sourcepackage = self.factory.makeSourcePackage()217 sourcepackage = self.factory.makeSourcePackage()
218 owner = self.factory.makePerson()218 registrant = self.factory.makePerson()
219 if with_packaging:219 if with_packaging:
220 self.factory.makePackagingLink(220 self.factory.makePackagingLink(
221 sourcepackagename=sourcepackage.sourcepackagename,221 sourcepackagename=sourcepackage.sourcepackagename,
222 distroseries=sourcepackage.distroseries, owner=owner)222 distroseries=sourcepackage.distroseries, owner=registrant)
223 if for_other_user:223 user = self.factory.makePerson(karma=karma)
224 user = self.factory.makePerson()
225 else:
226 user = owner
227 with person_logged_in(user):224 with person_logged_in(user):
228 menu = SourcePackageOverviewMenu(sourcepackage)225 menu = SourcePackageOverviewMenu(sourcepackage)
229 return menu, user226 return menu, user
@@ -231,62 +228,62 @@
231 def test_edit_packaging_link__enabled_without_packaging(self):228 def test_edit_packaging_link__enabled_without_packaging(self):
232 # If no packging exists, the edit_packaging link is always229 # If no packging exists, the edit_packaging link is always
233 # enabled.230 # enabled.
234 menu, user = self.makeSourcePackageOverviewMenu(False, False)231 menu, user = self.makeSourcePackageOverviewMenu(False, None)
235 with person_logged_in(user):232 with person_logged_in(user):
236 self.assertTrue(menu.edit_packaging().enabled)233 self.assertTrue(menu.edit_packaging().enabled)
237234
238 def test_set_upstrem_link__enabled_without_packaging(self):235 def test_set_upstrem_link__enabled_without_packaging(self):
239 # If no packging exists, the set_upstream link is always236 # If no packging exists, the set_upstream link is always
240 # enabled.237 # enabled.
241 menu, user = self.makeSourcePackageOverviewMenu(False, False)238 menu, user = self.makeSourcePackageOverviewMenu(False, None)
242 with person_logged_in(user):239 with person_logged_in(user):
243 self.assertTrue(menu.set_upstream().enabled)240 self.assertTrue(menu.set_upstream().enabled)
244241
245 def test_remove_packaging_link__enabled_without_packaging(self):242 def test_remove_packaging_link__enabled_without_packaging(self):
246 # If no packging exists, the remove_packaging link is always243 # If no packging exists, the remove_packaging link is always
247 # enabled.244 # enabled.
248 menu, user = self.makeSourcePackageOverviewMenu(False, False)245 menu, user = self.makeSourcePackageOverviewMenu(False, None)
249 with person_logged_in(user):246 with person_logged_in(user):
250 self.assertTrue(menu.remove_packaging().enabled)247 self.assertTrue(menu.remove_packaging().enabled)
251248
252 def test_edit_packaging_link__enabled_with_packaging_for_owner(self):249 def test_edit_packaging_link__enabled_with_packaging_non_probation(self):
253 # If a packging exists, the edit_packaging link is enabled250 # If a packging exists, the edit_packaging link is enabled
254 # for the packaging owner.251 # for the non-probationary users.
255 menu, user = self.makeSourcePackageOverviewMenu(True, False)252 menu, user = self.makeSourcePackageOverviewMenu(True, 100)
256 with person_logged_in(user):253 with person_logged_in(user):
257 self.assertTrue(menu.edit_packaging().enabled)254 self.assertTrue(menu.edit_packaging().enabled)
258255
259 def test_set_upstrem_link__enabled_with_packaging_for_owner(self):256 def test_set_upstrem_link__enabled_with_packaging_non_probation(self):
260 # If a packging exists, the set_upstream link is enabled257 # If a packging exists, the set_upstream link is enabled
261 # for the packaging owner.258 # for the non-probationary users.
262 menu, user = self.makeSourcePackageOverviewMenu(True, False)259 menu, user = self.makeSourcePackageOverviewMenu(True, 100)
263 with person_logged_in(user):260 with person_logged_in(user):
264 self.assertTrue(menu.set_upstream().enabled)261 self.assertTrue(menu.set_upstream().enabled)
265262
266 def test_remove_packaging_link__enabled_with_packaging_for_owner(self):263 def test_remove_packaging_link__enabled_with_packaging_non_probation(self):
267 # If a packging exists, the remove_packaging link is enabled264 # If a packging exists, the remove_packaging link is enabled
268 # for the packaging owner.265 # for the non-probationary users.
269 menu, user = self.makeSourcePackageOverviewMenu(True, False)266 menu, user = self.makeSourcePackageOverviewMenu(True, 100)
270 with person_logged_in(user):267 with person_logged_in(user):
271 self.assertTrue(menu.remove_packaging().enabled)268 self.assertTrue(menu.remove_packaging().enabled)
272269
273 def test_edit_packaging_link__enabled_with_packaging_for_others(self):270 def test_edit_packaging_link__enabled_with_packaging_probation(self):
274 # If a packging exists, the edit_packaging link is enabled271 # If a packging exists, the edit_packaging link is not enabled
275 # for the packaging owner.272 # for probationary users.
276 menu, user = self.makeSourcePackageOverviewMenu(True, True)273 menu, user = self.makeSourcePackageOverviewMenu(True, None)
277 with person_logged_in(user):274 with person_logged_in(user):
278 self.assertFalse(menu.edit_packaging().enabled)275 self.assertFalse(menu.edit_packaging().enabled)
279276
280 def test_set_upstrem_link__enabled_with_packaging_for_others(self):277 def test_set_upstrem_link__enabled_with_packaging_probation(self):
281 # If a packging exists, the set_upstream link is enabled278 # If a packging exists, the set_upstream link is not enabled
282 # for the packaging owner.279 # for probationary users.
283 menu, user = self.makeSourcePackageOverviewMenu(True, True)280 menu, user = self.makeSourcePackageOverviewMenu(True, None)
284 with person_logged_in(user):281 with person_logged_in(user):
285 self.assertFalse(menu.set_upstream().enabled)282 self.assertFalse(menu.set_upstream().enabled)
286283
287 def test_remove_packaging_link__enabled_with_packaging_for_others(self):284 def test_remove_packaging_link__enabled_with_packaging_probation(self):
288 # If a packging exists, the remove_packaging link is enabled285 # If a packging exists, the remove_packaging link is not enabled
289 # for the packaging owner.286 # for probationary users.
290 menu, user = self.makeSourcePackageOverviewMenu(True, True)287 menu, user = self.makeSourcePackageOverviewMenu(True, None)
291 with person_logged_in(user):288 with person_logged_in(user):
292 self.assertFalse(menu.remove_packaging().enabled)289 self.assertFalse(menu.remove_packaging().enabled)
293290
=== modified file 'lib/lp/registry/doc/sourcepackage.txt'
--- lib/lp/registry/doc/sourcepackage.txt 2012-08-07 02:31:56 +0000
+++ lib/lp/registry/doc/sourcepackage.txt 2012-09-19 18:22:24 +0000
@@ -273,18 +273,12 @@
273First, let's get some useful objects from the db.273First, let's get some useful objects from the db.
274274
275 >>> from lp.registry.model.sourcepackagename import SourcePackageName275 >>> from lp.registry.model.sourcepackagename import SourcePackageName
276 >>> evolution = SourcePackageName.byName('evolution')
277 >>> a52dec = SourcePackageName.byName('a52dec')
278 >>> netapplet = SourcePackageName.byName('netapplet')
279 >>> firefox = SourcePackageName.byName('mozilla-firefox')276 >>> firefox = SourcePackageName.byName('mozilla-firefox')
280 >>> pmount = SourcePackageName.byName('pmount')277 >>> pmount = SourcePackageName.byName('pmount')
281278
282 >>> from lp.registry.model.distroseries import DistroSeries279 >>> from lp.registry.model.distroseries import DistroSeries
283 >>> warty = DistroSeries.get(1)280 >>> warty = DistroSeries.get(1)
284 >>> hoary = DistroSeries.get(3)281 >>> hoary = DistroSeries.get(3)
285 >>> sarge = DistroSeries.get(7)
286 >>> sid = DistroSeries.get(8)
287 >>> g2k5 = DistroSeries.get(9)
288282
289Now let's make sure that we can see a productseries for a source283Now let's make sure that we can see a productseries for a source
290package.284package.
@@ -294,62 +288,6 @@
294 >>> print sp.productseries.name288 >>> print sp.productseries.name
295 1.0289 1.0
296290
297A source package's product series is None when it does not have a
298Packaging entry. Historical packaging does not affect the state of the
299productseries attribute.
300
301 >>> from lp.registry.model.packaging import PackagingUtil
302 >>> hoary_a52dec = SourcePackage(sourcepackagename=a52dec,
303 ... distroseries=hoary)
304 >>> PackagingUtil().packagingEntryExists(
305 ... productseries=hoary_a52dec.productseries,
306 ... sourcepackagename=a52dec,
307 ... distroseries=hoary)
308 False
309
310 >>> print hoary_a52dec.productseries
311 None
312
313Once a Packaging entry is created to link a distro series source package name
314to a product series, the source package does have a product series.
315
316 >>> from lp.registry.interfaces.packaging import PackagingType
317
318 >>> user = factory.makePerson()
319 >>> a52dec_series = factory.makeProductSeries(name='ratty')
320 >>> packaging = PackagingUtil().createPackaging(
321 ... productseries=a52dec_series,
322 ... sourcepackagename=a52dec,
323 ... distroseries=hoary,
324 ... packaging=PackagingType.PRIME,
325 ... owner=user)
326
327 >>> PackagingUtil().packagingEntryExists(
328 ... productseries=a52dec_series,
329 ... sourcepackagename=a52dec,
330 ... distroseries=hoary)
331 True
332
333 >>> print hoary_a52dec.productseries.name
334 ratty
335
336Packaging entries can be deleted using PackagingUtil.deletePackaging. That
337also removes the source package product series.
338
339 >>> from lp.testing import person_logged_in
340 >>> with person_logged_in(user):
341 ... PackagingUtil().deletePackaging(
342 ... productseries=a52dec_series,
343 ... sourcepackagename=a52dec,
344 ... distroseries=hoary)
345 >>> PackagingUtil().packagingEntryExists(
346 ... productseries=a52dec_series,
347 ... sourcepackagename=a52dec,
348 ... distroseries=hoary)
349 False
350
351 >>> print hoary_a52dec.productseries
352 None
353291
354Linkified changelogs are available through SourcePackageReleaseView: XXX292Linkified changelogs are available through SourcePackageReleaseView: XXX
355julian 2007-09-17 This is duplicating the page test. Instead it should293julian 2007-09-17 This is duplicating the page test. Instead it should
@@ -537,5 +475,3 @@
537475
538 >>> verifyObject(IHasTranslationImports, warty_firefox)476 >>> verifyObject(IHasTranslationImports, warty_firefox)
539 True477 True
540
541
542478
=== modified file 'lib/lp/registry/interfaces/packaging.py'
--- lib/lp/registry/interfaces/packaging.py 2011-12-24 16:54:44 +0000
+++ lib/lp/registry/interfaces/packaging.py 2012-09-19 18:22:24 +0000
@@ -98,9 +98,8 @@
98 """True, if the current user is allowed to delete this packaging,98 """True, if the current user is allowed to delete this packaging,
99 else False.99 else False.
100100
101 People who created the packaging can delete it, as well as101 Non-probationary users can delete packaging links that they believe
102 people with upload rights for a source package, distribution102 connect Ubuntu to bogus data.
103 owners, members of the registry team and LP admin team.
104 """103 """
105104
106105
107106
=== modified file 'lib/lp/registry/model/packaging.py'
--- lib/lp/registry/model/packaging.py 2011-12-30 06:14:56 +0000
+++ lib/lp/registry/model/packaging.py 2012-09-19 18:22:24 +0000
@@ -74,12 +74,13 @@
74 if user is None:74 if user is None:
75 return False75 return False
76 admin = getUtility(ILaunchpadCelebrities).admin76 admin = getUtility(ILaunchpadCelebrities).admin
77 registry_experts = (77 registry_experts = (getUtility(ILaunchpadCelebrities).registry_experts)
78 getUtility(ILaunchpadCelebrities).registry_experts)78 if (not user.is_probationary
79 return (79 or user.inTeam(self.productseries.product.owner)
80 user.inTeam(self.owner) or80 or user.canAccess(self.sourcepackage, 'setBranch')
81 user.canAccess(self.sourcepackage, 'setBranch') or81 or user.inTeam(registry_experts) or user.inTeam(admin)):
82 user.inTeam(registry_experts) or user.inTeam(admin))82 return True
83 return False
8384
84 def destroySelf(self):85 def destroySelf(self):
85 if not self.userCanDelete():86 if not self.userCanDelete():
8687
=== modified file 'lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging-concurrent-deletion.txt'
--- lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging-concurrent-deletion.txt 2010-03-03 00:40:03 +0000
+++ lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging-concurrent-deletion.txt 2012-09-19 18:22:24 +0000
@@ -7,10 +7,10 @@
77
8The No Privilege User may open the same page in two browser tabs.8The No Privilege User may open the same page in two browser tabs.
99
10 >>> first_browser = setupBrowser(auth='Basic no-priv@canonical.com:test')10 >>> first_browser = setupBrowser(auth='Basic test@canonical.com:test')
11 >>> first_browser.open('http://launchpad.dev/ubuntu/+source/alsa-utils')11 >>> first_browser.open('http://launchpad.dev/ubuntu/+source/alsa-utils')
1212
13 >>> second_browser = setupBrowser(auth='Basic no-priv@canonical.com:test')13 >>> second_browser = setupBrowser(auth='Basic test@canonical.com:test')
14 >>> second_browser.open('http://launchpad.dev/ubuntu/+source/alsa-utils')14 >>> second_browser.open('http://launchpad.dev/ubuntu/+source/alsa-utils')
1515
16Then the user click the "Delete Link" button in the first tab. The16Then the user click the "Delete Link" button in the first tab. The
1717
=== modified file 'lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging.txt'
--- lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging.txt 2012-06-14 10:34:55 +0000
+++ lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging.txt 2012-09-19 18:22:24 +0000
@@ -20,7 +20,7 @@
20 >>> print extract_text(find_tag_by_id(content, 'packages_list'))20 >>> print extract_text(find_tag_by_id(content, 'packages_list'))
21 The Hoary Hedgehog Release (active development) Set upstream link21 The Hoary Hedgehog Release (active development) Set upstream link
22 1.0.9a-4ubuntu1 release (main) 2005-09-1522 1.0.9a-4ubuntu1 release (main) 2005-09-15
23 The Warty Warthog Release (current stable release) alsa-utils trunk series23 The Warty Warthog Release (current stable release) alsa-utils trunk series
24 1.0.8-1ubuntu1 release (main) 2005-09-1524 1.0.8-1ubuntu1 release (main) 2005-09-15
25 1.0.9a-4 release (main) 2005-09-1625 1.0.9a-4 release (main) 2005-09-16
2626
@@ -31,6 +31,7 @@
31A button is displayed to authenticated users to delete existing31A button is displayed to authenticated users to delete existing
32packaging links.32packaging links.
3333
34 >>> user_browser = setupBrowser(auth='Basic test@canonical.com:test')
34 >>> user_browser.open('http://launchpad.dev/ubuntu/+source/alsa-utils')35 >>> user_browser.open('http://launchpad.dev/ubuntu/+source/alsa-utils')
35 >>> link = user_browser.getLink(36 >>> link = user_browser.getLink(
36 ... url='/ubuntu/warty/+source/alsa-utils/+remove-packaging')37 ... url='/ubuntu/warty/+source/alsa-utils/+remove-packaging')
3738
=== modified file 'lib/lp/registry/tests/test_packaging.py'
--- lib/lp/registry/tests/test_packaging.py 2012-06-29 08:40:05 +0000
+++ lib/lp/registry/tests/test_packaging.py 2012-09-19 18:22:24 +0000
@@ -51,7 +51,8 @@
51 def test_destroySelf_notifies(self):51 def test_destroySelf_notifies(self):
52 """destroySelf creates a notification."""52 """destroySelf creates a notification."""
53 packaging = self.factory.makePackagingLink()53 packaging = self.factory.makePackagingLink()
54 with person_logged_in(packaging.owner):54 user = self.factory.makePerson(karma=200)
55 with person_logged_in(user):
55 with EventRecorder() as recorder:56 with EventRecorder() as recorder:
56 removeSecurityProxy(packaging).destroySelf()57 removeSecurityProxy(packaging).destroySelf()
57 (event,) = recorder.events58 (event,) = recorder.events
@@ -67,7 +68,7 @@
67 packaging.productseries, packaging.sourcepackagename,68 packaging.productseries, packaging.sourcepackagename,
68 packaging.distroseries)69 packaging.distroseries)
6970
70 def test_destroySelf__not_allowed_for_arbitrary_user(self):71 def test_destroySelf__not_allowed_for_probationary_user(self):
71 """Arbitrary users cannot delete a packaging."""72 """Arbitrary users cannot delete a packaging."""
72 packaging = self.factory.makePackagingLink()73 packaging = self.factory.makePackagingLink()
73 packaging_util = getUtility(IPackagingUtil)74 packaging_util = getUtility(IPackagingUtil)
@@ -77,14 +78,15 @@
77 packaging.productseries, packaging.sourcepackagename,78 packaging.productseries, packaging.sourcepackagename,
78 packaging.distroseries)79 packaging.distroseries)
7980
80 def test_destroySelf__allowed_for_packaging_owner(self):81 def test_destroySelf__allowed_for_non_probationary_user(self):
81 """A packaging owner can delete a packaging."""82 """An experienced user can delete a packaging."""
82 packaging = self.factory.makePackagingLink()83 packaging = self.factory.makePackagingLink()
83 sourcepackagename = packaging.sourcepackagename84 sourcepackagename = packaging.sourcepackagename
84 distroseries = packaging.distroseries85 distroseries = packaging.distroseries
85 productseries = packaging.productseries86 productseries = packaging.productseries
86 packaging_util = getUtility(IPackagingUtil)87 packaging_util = getUtility(IPackagingUtil)
87 with person_logged_in(packaging.owner):88 user = self.factory.makePerson(karma=200)
89 with person_logged_in(user):
88 packaging_util.deletePackaging(90 packaging_util.deletePackaging(
89 packaging.productseries, packaging.sourcepackagename,91 packaging.productseries, packaging.sourcepackagename,
90 packaging.distroseries)92 packaging.distroseries)
@@ -92,20 +94,6 @@
92 packaging_util.packagingEntryExists(94 packaging_util.packagingEntryExists(
93 sourcepackagename, distroseries, productseries))95 sourcepackagename, distroseries, productseries))
9496
95 def test_destroySelf__allowed_for_distro_owner(self):
96 """A distribution owner can delete a packaging link."""
97 packaging = self.factory.makePackagingLink()
98 sourcepackagename = packaging.sourcepackagename
99 distroseries = packaging.distroseries
100 productseries = packaging.productseries
101 packaging_util = getUtility(IPackagingUtil)
102 with person_logged_in(distroseries.distribution.owner):
103 packaging_util.deletePackaging(
104 productseries, sourcepackagename, distroseries)
105 self.assertFalse(
106 packaging_util.packagingEntryExists(
107 sourcepackagename, distroseries, productseries))
108
109 def test_destroySelf__allowed_for_uploader(self):97 def test_destroySelf__allowed_for_uploader(self):
110 """A person with upload rights for the sourcepackage can98 """A person with upload rights for the sourcepackage can
111 delete a packaging link.99 delete a packaging link.
@@ -287,7 +275,8 @@
287 """Deleting a Packaging creates a notification."""275 """Deleting a Packaging creates a notification."""
288 packaging_util = getUtility(IPackagingUtil)276 packaging_util = getUtility(IPackagingUtil)
289 packaging = self.factory.makePackagingLink()277 packaging = self.factory.makePackagingLink()
290 with person_logged_in(packaging.owner):278 user = self.factory.makePerson(karma=200)
279 with person_logged_in(user):
291 with EventRecorder() as recorder:280 with EventRecorder() as recorder:
292 packaging_util.deletePackaging(281 packaging_util.deletePackaging(
293 packaging.productseries, packaging.sourcepackagename,282 packaging.productseries, packaging.sourcepackagename,
294283
=== modified file 'lib/lp/registry/tests/test_sourcepackage.py'
--- lib/lp/registry/tests/test_sourcepackage.py 2012-06-28 01:14:33 +0000
+++ lib/lp/registry/tests/test_sourcepackage.py 2012-09-19 18:22:24 +0000
@@ -14,7 +14,6 @@
14from zope.component import getUtility14from zope.component import getUtility
15from zope.interface.verify import verifyObject15from zope.interface.verify import verifyObject
16from zope.security.checker import canAccess16from zope.security.checker import canAccess
17from zope.security.interfaces import Unauthorized
18from zope.security.management import checkPermission17from zope.security.management import checkPermission
19from zope.security.proxy import removeSecurityProxy18from zope.security.proxy import removeSecurityProxy
2019
@@ -283,10 +282,11 @@
283282
284 def test_deletePackaging(self):283 def test_deletePackaging(self):
285 """Ensure deletePackaging completely removes packaging."""284 """Ensure deletePackaging completely removes packaging."""
285 user = self.factory.makePerson(karma=200)
286 packaging = self.factory.makePackagingLink()286 packaging = self.factory.makePackagingLink()
287 packaging_id = packaging.id287 packaging_id = packaging.id
288 store = Store.of(packaging)288 store = Store.of(packaging)
289 with person_logged_in(packaging.owner):289 with person_logged_in(user):
290 packaging.sourcepackage.deletePackaging()290 packaging.sourcepackage.deletePackaging()
291 result = store.find(Packaging, Packaging.id == packaging_id)291 result = store.find(Packaging, Packaging.id == packaging_id)
292 self.assertIs(None, result.one())292 self.assertIs(None, result.one())
@@ -305,11 +305,12 @@
305 sourcepackage = self.factory.makeSourcePackage()305 sourcepackage = self.factory.makeSourcePackage()
306 productseries = self.factory.makeProductSeries()306 productseries = self.factory.makeProductSeries()
307 other_series = self.factory.makeProductSeries()307 other_series = self.factory.makeProductSeries()
308 owner = self.factory.makePerson()308 user = self.factory.makePerson(karma=200)
309 registrant = self.factory.makePerson()
309 with EventRecorder() as recorder:310 with EventRecorder() as recorder:
310 with person_logged_in(owner):311 with person_logged_in(user):
311 sourcepackage.setPackaging(productseries, owner=owner)312 sourcepackage.setPackaging(productseries, owner=registrant)
312 sourcepackage.setPackaging(other_series, owner=owner)313 sourcepackage.setPackaging(other_series, owner=registrant)
313 packaging = sourcepackage.direct_packaging314 packaging = sourcepackage.direct_packaging
314 self.assertEqual(packaging.productseries, other_series)315 self.assertEqual(packaging.productseries, other_series)
315 # The first call of setPackaging() created an ObjectCreatedEvent;316 # The first call of setPackaging() created an ObjectCreatedEvent;
@@ -321,21 +322,6 @@
321 self.assertIsInstance(event2, ObjectDeletedEvent)322 self.assertIsInstance(event2, ObjectDeletedEvent)
322 self.assertIsInstance(event3, ObjectCreatedEvent)323 self.assertIsInstance(event3, ObjectCreatedEvent)
323324
324 def test_setPackaging__change_existing_entry_different_users(self):
325 """An ordinary user cannot change a Packaging defined by
326 somebody else.
327 """
328 sourcepackage = self.factory.makeSourcePackage()
329 productseries = self.factory.makeProductSeries()
330 other_series = self.factory.makeProductSeries()
331 owner = self.factory.makePerson()
332 other_user = self.factory.makePerson()
333 sourcepackage.setPackaging(productseries, owner=owner)
334 with person_logged_in(other_user):
335 self.assertRaises(
336 Unauthorized, sourcepackage.setPackaging,
337 other_series, owner=other_user)
338
339 def test_setPackagingReturnSharingDetailPermissions__ordinary_user(self):325 def test_setPackagingReturnSharingDetailPermissions__ordinary_user(self):
340 """An ordinary user can create a packaging link but he cannot326 """An ordinary user can create a packaging link but he cannot
341 set the series' branch or translation syncronisation settings,327 set the series' branch or translation syncronisation settings,
@@ -343,7 +329,7 @@
343 """329 """
344 sourcepackage = self.factory.makeSourcePackage()330 sourcepackage = self.factory.makeSourcePackage()
345 productseries = self.factory.makeProductSeries()331 productseries = self.factory.makeProductSeries()
346 packaging_owner = self.factory.makePerson()332 packaging_owner = self.factory.makePerson(karma=100)
347 with person_logged_in(packaging_owner):333 with person_logged_in(packaging_owner):
348 permissions = (334 permissions = (
349 sourcepackage.setPackagingReturnSharingDetailPermissions(335 sourcepackage.setPackagingReturnSharingDetailPermissions(
@@ -370,19 +356,20 @@
370 synchronisation settings, or the translation usage settings of the356 synchronisation settings, or the translation usage settings of the
371 product.357 product.
372 """358 """
359 user = self.factory.makePerson(karma=100)
373 packaging = self.factory.makePackagingLink()360 packaging = self.factory.makePackagingLink()
374 sourcepackage = packaging.sourcepackage361 sourcepackage = packaging.sourcepackage
375 productseries = packaging.productseries362 productseries = packaging.productseries
376 with person_logged_in(packaging.owner):363 with person_logged_in(user):
377 permissions = sourcepackage.getSharingDetailPermissions()364 permissions = sourcepackage.getSharingDetailPermissions()
378 self.assertEqual(productseries, sourcepackage.productseries)365 self.assertEqual(productseries, sourcepackage.productseries)
379 self.assertFalse(366 self.assertFalse(
380 packaging.owner.canWrite(productseries, 'branch'))367 user.canWrite(productseries, 'branch'))
381 self.assertFalse(368 self.assertFalse(
382 packaging.owner.canWrite(369 user.canWrite(
383 productseries, 'translations_autoimport_mode'))370 productseries, 'translations_autoimport_mode'))
384 self.assertFalse(371 self.assertFalse(
385 packaging.owner.canWrite(372 user.canWrite(
386 productseries.product, 'translations_usage'))373 productseries.product, 'translations_usage'))
387 expected = {374 expected = {
388 'user_can_change_product_series': True,375 'user_can_change_product_series': True,
@@ -397,37 +384,6 @@
397 return self.factory.makeProductSeries(384 return self.factory.makeProductSeries(
398 owner=self.factory.makePerson())385 owner=self.factory.makePerson())
399386
400 def test_getSharingDetailPermissions__series_owner(self):
401 """A product series owner can create a packaging link, and he can
402 set the series' branch or translation syncronisation settings,
403 but he cannot set the translation usage settings of the product.
404 """
405 productseries = self.makeDistinctOwnerProductSeries()
406 series_owner = productseries.owner
407 # Ensure productseries owner is distinct from product owner.
408 productseries = self.factory.makeProductSeries(
409 owner=series_owner)
410 with person_logged_in(series_owner):
411 packaging = self.factory.makePackagingLink(
412 productseries=productseries, owner=series_owner)
413 sourcepackage = packaging.sourcepackage
414 permissions = sourcepackage.getSharingDetailPermissions()
415 self.assertEqual(productseries, sourcepackage.productseries)
416 self.assertTrue(series_owner.canWrite(productseries, 'branch'))
417 self.assertTrue(
418 series_owner.canWrite(
419 productseries, 'translations_autoimport_mode'))
420 self.assertFalse(
421 series_owner.canWrite(
422 productseries.product, 'translations_usage'))
423 expected = {
424 'user_can_change_product_series': True,
425 'user_can_change_branch': True,
426 'user_can_change_translation_usage': False,
427 'user_can_change_translations_autoimport_mode': True,
428 }
429 self.assertEqual(expected, permissions)
430
431 def test_getSharingDetailPermissions__product_owner(self):387 def test_getSharingDetailPermissions__product_owner(self):
432 """A product owner can create a packaging link, and he can set the388 """A product owner can create a packaging link, and he can set the
433 series' branch and the translation syncronisation settings, and the389 series' branch and the translation syncronisation settings, and the
@@ -463,7 +419,7 @@
463 Afterward, random people cannot change product series.419 Afterward, random people cannot change product series.
464 """420 """
465 sourcepackage = self.factory.makeSourcePackage()421 sourcepackage = self.factory.makeSourcePackage()
466 person1 = self.factory.makePerson()422 person1 = self.factory.makePerson(karma=100)
467 person2 = self.factory.makePerson()423 person2 = self.factory.makePerson()
468424
469 def can_change_product_series():425 def can_change_product_series():
@@ -553,10 +509,11 @@
553509
554 def test_deletePackaging(self):510 def test_deletePackaging(self):
555 """Deleting a packaging should work."""511 """Deleting a packaging should work."""
512 user = self.factory.makePerson(karma=200)
556 packaging = self.factory.makePackagingLink()513 packaging = self.factory.makePackagingLink()
557 sourcepackage = packaging.sourcepackage514 sourcepackage = packaging.sourcepackage
558 transaction.commit()515 transaction.commit()
559 self.wsObject(sourcepackage, user=packaging.owner).deletePackaging()516 self.wsObject(sourcepackage, user=user).deletePackaging()
560 transaction.commit()517 transaction.commit()
561 self.assertIs(None, sourcepackage.direct_packaging)518 self.assertIs(None, sourcepackage.direct_packaging)
562519
563520
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2012-09-14 18:23:50 +0000
+++ lib/lp/testing/factory.py 2012-09-19 18:22:24 +0000
@@ -214,6 +214,7 @@
214from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet214from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
215from lp.registry.interfaces.ssh import ISSHKeySet215from lp.registry.interfaces.ssh import ISSHKeySet
216from lp.registry.model.commercialsubscription import CommercialSubscription216from lp.registry.model.commercialsubscription import CommercialSubscription
217from lp.registry.model.karma import KarmaTotalCache
217from lp.registry.model.milestone import Milestone218from lp.registry.model.milestone import Milestone
218from lp.registry.model.suitesourcepackage import SuiteSourcePackage219from lp.registry.model.suitesourcepackage import SuiteSourcePackage
219from lp.services.config import config220from lp.services.config import config
@@ -607,7 +608,7 @@
607 self, email=None, name=None, displayname=None, account_status=None,608 self, email=None, name=None, displayname=None, account_status=None,
608 email_address_status=None, hide_email_addresses=False,609 email_address_status=None, hide_email_addresses=False,
609 time_zone=None, latitude=None, longitude=None, description=None,610 time_zone=None, latitude=None, longitude=None, description=None,
610 selfgenerated_bugnotifications=False, member_of=()):611 selfgenerated_bugnotifications=False, member_of=(), karma=None):
611 """Create and return a new, arbitrary Person.612 """Create and return a new, arbitrary Person.
612613
613 :param email: The email address for the new person.614 :param email: The email address for the new person.
@@ -668,6 +669,10 @@
668 with person_logged_in(team.teamowner):669 with person_logged_in(team.teamowner):
669 team.addMember(person, team.teamowner)670 team.addMember(person, team.teamowner)
670671
672 if karma is not None:
673 with dbuser('karma'):
674 # Give the user karma to make the user non-probationary.
675 KarmaTotalCache(person=person.id, karma_total=karma)
671 # Ensure updated ValidPersonCache676 # Ensure updated ValidPersonCache
672 flush_database_updates()677 flush_database_updates()
673 return person678 return person
674679
=== modified file 'lib/lp/translations/browser/tests/test_sharing_details.py'
--- lib/lp/translations/browser/tests/test_sharing_details.py 2012-08-06 06:09:19 +0000
+++ lib/lp/translations/browser/tests/test_sharing_details.py 2012-09-19 18:22:24 +0000
@@ -5,7 +5,6 @@
55
66
7import re7import re
8
9from lazr.restful.interfaces import IJSONRequestCache8from lazr.restful.interfaces import IJSONRequestCache
10from soupmatchers import (9from soupmatchers import (
11 HTMLContains,10 HTMLContains,
@@ -101,7 +100,9 @@
101 sourcepackage=self.sourcepackage, name='ubuntu-only')100 sourcepackage=self.sourcepackage, name='ubuntu-only')
102 self.shared_template_ubuntu_side = self.factory.makePOTemplate(101 self.shared_template_ubuntu_side = self.factory.makePOTemplate(
103 sourcepackage=self.sourcepackage, name='shared-template')102 sourcepackage=self.sourcepackage, name='shared-template')
104 self.productseries = self.factory.makeProductSeries()103 self.privileged_user = self.factory.makePerson(karma=200)
104 product = self.factory.makeProduct(owner=self.privileged_user)
105 self.productseries = self.factory.makeProductSeries(product=product)
105 self.shared_template_upstream_side = self.factory.makePOTemplate(106 self.shared_template_upstream_side = self.factory.makePOTemplate(
106 productseries=self.productseries, name='shared-template')107 productseries=self.productseries, name='shared-template')
107 self.upstream_only_template = self.factory.makePOTemplate(108 self.upstream_only_template = self.factory.makePOTemplate(
@@ -566,8 +567,8 @@
566 self.assertEqual(567 self.assertEqual(
567 expected, self.view.set_packaging_link.escapedtext)568 expected, self.view.set_packaging_link.escapedtext)
568569
569 def test_set_packaging_link__with_packaging_any_user(self):570 def test_set_packaging_link__with_packaging_probationary_user(self):
570 # If packaging is configured, arbitrary users do no see571 # If packaging is configured, probationary users do no see
571 # the "set packaging" link.572 # the "set packaging" link.
572 self.configureSharing()573 self.configureSharing()
573 expected = self._getExpectedPackagingLink(574 expected = self._getExpectedPackagingLink(
@@ -588,7 +589,7 @@
588 expected = self._getExpectedPackagingLink(589 expected = self._getExpectedPackagingLink(
589 id='set-packaging', url='+edit-packaging', icon='add',590 id='set-packaging', url='+edit-packaging', icon='add',
590 text='Set upstream link', visible=True)591 text='Set upstream link', visible=True)
591 with person_logged_in(self.sourcepackage.packaging.owner):592 with person_logged_in(self.privileged_user):
592 view = SourcePackageTranslationSharingDetailsView(593 view = SourcePackageTranslationSharingDetailsView(
593 self.sourcepackage, LaunchpadTestRequest())594 self.sourcepackage, LaunchpadTestRequest())
594 view.initialize()595 view.initialize()
@@ -617,8 +618,8 @@
617 self.assertEqual(618 self.assertEqual(
618 expected, self.view.change_packaging_link.escapedtext)619 expected, self.view.change_packaging_link.escapedtext)
619620
620 def test_change_packaging_link__with_packaging_any_user(self):621 def test_change_packaging_link__with_packaging_probationary_user(self):
621 # If packaging is configured, arbitrary users do no see622 # If packaging is configured, probationary users do no see
622 # the "change packaging" link.623 # the "change packaging" link.
623 self.configureSharing()624 self.configureSharing()
624 expected = self._getExpectedPackagingLink(625 expected = self._getExpectedPackagingLink(
@@ -639,7 +640,7 @@
639 expected = self._getExpectedPackagingLink(640 expected = self._getExpectedPackagingLink(
640 id='change-packaging', url='+edit-packaging', icon='edit',641 id='change-packaging', url='+edit-packaging', icon='edit',
641 text='Change upstream link', visible=True)642 text='Change upstream link', visible=True)
642 with person_logged_in(self.sourcepackage.packaging.owner):643 with person_logged_in(self.privileged_user):
643 view = SourcePackageTranslationSharingDetailsView(644 view = SourcePackageTranslationSharingDetailsView(
644 self.sourcepackage, LaunchpadTestRequest())645 self.sourcepackage, LaunchpadTestRequest())
645 view.initialize()646 view.initialize()
@@ -668,8 +669,8 @@
668 self.assertEqual(669 self.assertEqual(
669 expected, self.view.remove_packaging_link.escapedtext)670 expected, self.view.remove_packaging_link.escapedtext)
670671
671 def test_remove_packaging_link__with_packaging_any_user(self):672 def test_remove_packaging_link__with_packaging_probationary_user(self):
672 # If packaging is configured, arbitrary users do no see673 # If packaging is configured, probationary users do no see
673 # the "remove packaging" link.674 # the "remove packaging" link.
674 self.configureSharing()675 self.configureSharing()
675 expected = self._getExpectedPackagingLink(676 expected = self._getExpectedPackagingLink(
@@ -690,7 +691,7 @@
690 expected = self._getExpectedPackagingLink(691 expected = self._getExpectedPackagingLink(
691 id='remove-packaging', url='+remove-packaging', icon='remove',692 id='remove-packaging', url='+remove-packaging', icon='remove',
692 text='Remove upstream link', visible=True)693 text='Remove upstream link', visible=True)
693 with person_logged_in(self.sourcepackage.packaging.owner):694 with person_logged_in(self.privileged_user):
694 view = SourcePackageTranslationSharingDetailsView(695 view = SourcePackageTranslationSharingDetailsView(
695 self.sourcepackage, LaunchpadTestRequest())696 self.sourcepackage, LaunchpadTestRequest())
696 view.initialize()697 view.initialize()
697698
=== modified file 'lib/lp/translations/tests/test_translationpackagingjob.py'
--- lib/lp/translations/tests/test_translationpackagingjob.py 2012-04-24 18:41:35 +0000
+++ lib/lp/translations/tests/test_translationpackagingjob.py 2012-09-19 18:22:24 +0000
@@ -288,7 +288,8 @@
288 packaging.productseries, packaging.sourcepackage,288 packaging.productseries, packaging.sourcepackage,
289 TranslationSplitJob)289 TranslationSplitJob)
290 self.assertEqual([], finder.find())290 self.assertEqual([], finder.find())
291 with person_logged_in(packaging.owner):291 user = self.factory.makePerson(karma=200)
292 with person_logged_in(user):
292 getUtility(IPackagingUtil).deletePackaging(293 getUtility(IPackagingUtil).deletePackaging(
293 packaging.productseries, packaging.sourcepackagename,294 packaging.productseries, packaging.sourcepackagename,
294 packaging.distroseries)295 packaging.distroseries)