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

[Curtis Hovey]

Allow the Ubuntu community to manage their upstream packaging links.

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

[Richard Harding (rharding)]

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

=== 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 @@
 
     layer = DatabaseFunctionalLayer
 
-    def makeSourcePackageOverviewMenu(self, with_packaging, for_other_user):
+    def makeSourcePackageOverviewMenu(self, with_packaging, karma=None):
         sourcepackage = self.factory.makeSourcePackage()
-        owner = self.factory.makePerson()
+        registrant = self.factory.makePerson()
         if with_packaging:
             self.factory.makePackagingLink(
                 sourcepackagename=sourcepackage.sourcepackagename,
-                distroseries=sourcepackage.distroseries, owner=owner)
-        if for_other_user:
-            user = self.factory.makePerson()
-        else:
-            user = owner
+                distroseries=sourcepackage.distroseries, owner=registrant)
+        user = self.factory.makePerson(karma=karma)
         with person_logged_in(user):
             menu = SourcePackageOverviewMenu(sourcepackage)
         return menu, user
@@ -231,62 +228,62 @@
     def test_edit_packaging_link__enabled_without_packaging(self):
         # If no packging exists, the edit_packaging link is always
         # enabled.
-        menu, user = self.makeSourcePackageOverviewMenu(False, False)
+        menu, user = self.makeSourcePackageOverviewMenu(False, None)
         with person_logged_in(user):
             self.assertTrue(menu.edit_packaging().enabled)
 
     def test_set_upstrem_link__enabled_without_packaging(self):
         # If no packging exists, the set_upstream link is always
         # enabled.
-        menu, user = self.makeSourcePackageOverviewMenu(False, False)
+        menu, user = self.makeSourcePackageOverviewMenu(False, None)
         with person_logged_in(user):
             self.assertTrue(menu.set_upstream().enabled)
 
     def test_remove_packaging_link__enabled_without_packaging(self):
         # If no packging exists, the remove_packaging link is always
         # enabled.
-        menu, user = self.makeSourcePackageOverviewMenu(False, False)
+        menu, user = self.makeSourcePackageOverviewMenu(False, None)
         with person_logged_in(user):
             self.assertTrue(menu.remove_packaging().enabled)
 
-    def test_edit_packaging_link__enabled_with_packaging_for_owner(self):
+    def test_edit_packaging_link__enabled_with_packaging_non_probation(self):
         # If a packging exists, the edit_packaging link is enabled
-        # for the packaging owner.
-        menu, user = self.makeSourcePackageOverviewMenu(True, False)
+        # for the non-probationary users.
+        menu, user = self.makeSourcePackageOverviewMenu(True, 100)
         with person_logged_in(user):
             self.assertTrue(menu.edit_packaging().enabled)
 
-    def test_set_upstrem_link__enabled_with_packaging_for_owner(self):
+    def test_set_upstrem_link__enabled_with_packaging_non_probation(self):
         # If a packging exists, the set_upstream link is enabled
-        # for the packaging owner.
-        menu, user = self.makeSourcePackageOverviewMenu(True, False)
+        # for the non-probationary users.
+        menu, user = self.makeSourcePackageOverviewMenu(True, 100)
         with person_logged_in(user):
             self.assertTrue(menu.set_upstream().enabled)
 
-    def test_remove_packaging_link__enabled_with_packaging_for_owner(self):
+    def test_remove_packaging_link__enabled_with_packaging_non_probation(self):
         # If a packging exists, the remove_packaging link is enabled
-        # for the packaging owner.
-        menu, user = self.makeSourcePackageOverviewMenu(True, False)
+        # for the non-probationary users.
+        menu, user = self.makeSourcePackageOverviewMenu(True, 100)
         with person_logged_in(user):
             self.assertTrue(menu.remove_packaging().enabled)
 
-    def test_edit_packaging_link__enabled_with_packaging_for_others(self):
-        # If a packging exists, the edit_packaging link is enabled
-        # for the packaging owner.
-        menu, user = self.makeSourcePackageOverviewMenu(True, True)
+    def test_edit_packaging_link__enabled_with_packaging_probation(self):
+        # If a packging exists, the edit_packaging link is not enabled
+        # for probationary users.
+        menu, user = self.makeSourcePackageOverviewMenu(True, None)
         with person_logged_in(user):
             self.assertFalse(menu.edit_packaging().enabled)
 
-    def test_set_upstrem_link__enabled_with_packaging_for_others(self):
-        # If a packging exists, the set_upstream link is enabled
-        # for the packaging owner.
-        menu, user = self.makeSourcePackageOverviewMenu(True, True)
+    def test_set_upstrem_link__enabled_with_packaging_probation(self):
+        # If a packging exists, the set_upstream link is not enabled
+        # for probationary users.
+        menu, user = self.makeSourcePackageOverviewMenu(True, None)
         with person_logged_in(user):
             self.assertFalse(menu.set_upstream().enabled)
 
-    def test_remove_packaging_link__enabled_with_packaging_for_others(self):
-        # If a packging exists, the remove_packaging link is enabled
-        # for the packaging owner.
-        menu, user = self.makeSourcePackageOverviewMenu(True, True)
+    def test_remove_packaging_link__enabled_with_packaging_probation(self):
+        # If a packging exists, the remove_packaging link is not enabled
+        # for probationary users.
+        menu, user = self.makeSourcePackageOverviewMenu(True, None)
         with person_logged_in(user):
             self.assertFalse(menu.remove_packaging().enabled)

=== 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 @@
 First, let's get some useful objects from the db.
 
     >>> from lp.registry.model.sourcepackagename import SourcePackageName
-    >>> evolution = SourcePackageName.byName('evolution')
-    >>> a52dec = SourcePackageName.byName('a52dec')
-    >>> netapplet = SourcePackageName.byName('netapplet')
     >>> firefox = SourcePackageName.byName('mozilla-firefox')
     >>> pmount = SourcePackageName.byName('pmount')
 
     >>> from lp.registry.model.distroseries import DistroSeries
     >>> warty = DistroSeries.get(1)
     >>> hoary = DistroSeries.get(3)
-    >>> sarge = DistroSeries.get(7)
-    >>> sid = DistroSeries.get(8)
-    >>> g2k5 = DistroSeries.get(9)
 
 Now let's make sure that we can see a productseries for a source
 package.
@@ -294,62 +288,6 @@
     >>> print sp.productseries.name
     1.0
 
-A source package's product series is None when it does not have a
-Packaging entry. Historical packaging does not affect the state of the
-productseries attribute.
-
-    >>> from lp.registry.model.packaging import PackagingUtil
-    >>> hoary_a52dec = SourcePackage(sourcepackagename=a52dec,
-    ...     distroseries=hoary)
-    >>> PackagingUtil().packagingEntryExists(
-    ...     productseries=hoary_a52dec.productseries,
-    ...     sourcepackagename=a52dec,
-    ...     distroseries=hoary)
-    False
-
-    >>> print hoary_a52dec.productseries
-    None
-
-Once a Packaging entry is created to link a distro series source package name
-to a product series, the source package does have a product series.
-
-    >>> from lp.registry.interfaces.packaging import PackagingType
-
-    >>> user = factory.makePerson()
-    >>> a52dec_series = factory.makeProductSeries(name='ratty')
-    >>> packaging = PackagingUtil().createPackaging(
-    ...     productseries=a52dec_series,
-    ...     sourcepackagename=a52dec,
-    ...     distroseries=hoary,
-    ...     packaging=PackagingType.PRIME,
-    ...     owner=user)
-
-    >>> PackagingUtil().packagingEntryExists(
-    ...     productseries=a52dec_series,
-    ...     sourcepackagename=a52dec,
-    ...     distroseries=hoary)
-    True
-
-    >>> print hoary_a52dec.productseries.name
-    ratty
-
-Packaging entries can be deleted using PackagingUtil.deletePackaging. That
-also removes the source package product series.
-
-    >>> from lp.testing import person_logged_in
-    >>> with person_logged_in(user):
-    ...     PackagingUtil().deletePackaging(
-    ...         productseries=a52dec_series,
-    ...         sourcepackagename=a52dec,
-    ...         distroseries=hoary)
-    >>> PackagingUtil().packagingEntryExists(
-    ...     productseries=a52dec_series,
-    ...     sourcepackagename=a52dec,
-    ...     distroseries=hoary)
-    False
-
-    >>> print hoary_a52dec.productseries
-    None
 
 Linkified changelogs are available through SourcePackageReleaseView: XXX
 julian 2007-09-17 This is duplicating the page test. Instead it should
@@ -537,5 +475,3 @@
 
     >>> verifyObject(IHasTranslationImports, warty_firefox)
     True
-
-

=== 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 @@
         """True, if the current user is allowed to delete this packaging,
         else False.
 
-        People who created the packaging can delete it, as well as
-        people with upload rights for a source package, distribution
-        owners, members of the registry team and LP admin team.
+        Non-probationary users can delete packaging links that they believe
+        connect Ubuntu to bogus data.
         """
 
 

=== 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 @@
         if user is None:
             return False
         admin = getUtility(ILaunchpadCelebrities).admin
-        registry_experts = (
-            getUtility(ILaunchpadCelebrities).registry_experts)
-        return (
-            user.inTeam(self.owner) or
-            user.canAccess(self.sourcepackage, 'setBranch') or
-            user.inTeam(registry_experts) or user.inTeam(admin))
+        registry_experts = (getUtility(ILaunchpadCelebrities).registry_experts)
+        if (not user.is_probationary
+            or user.inTeam(self.productseries.product.owner)
+            or user.canAccess(self.sourcepackage, 'setBranch')
+            or user.inTeam(registry_experts) or user.inTeam(admin)):
+            return True
+        return False
 
     def destroySelf(self):
         if not self.userCanDelete():

=== 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 @@
 
 The No Privilege User may open the same page in two browser tabs.
 
-    >>> first_browser = setupBrowser(auth='Basic [e-mail]:test')
+    >>> first_browser = setupBrowser(auth='Basic [e-mail]:test')
     >>> first_browser.open('http://launchpad.dev/ubuntu/+source/alsa-utils')
 
-    >>> second_browser = setupBrowser(auth='Basic [e-mail]:test')
+    >>> second_browser = setupBrowser(auth='Basic [e-mail]:test')
     >>> second_browser.open('http://launchpad.dev/ubuntu/+source/alsa-utils')
 
 Then the user click the "Delete Link" button in the first tab. The

=== 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 @@
     >>> print extract_text(find_tag_by_id(content, 'packages_list'))
     The Hoary Hedgehog Release (active development)     Set upstream link
       1.0.9a-4ubuntu1 release (main) 2005-09-15
-    The Warty Warthog Release (current stable release)  alsa-utils trunk series
+    The Warty Warthog Release (current stable release) alsa-utils trunk series
       1.0.8-1ubuntu1  release (main) 2005-09-15
       1.0.9a-4        release (main) 2005-09-16
 
@@ -31,6 +31,7 @@
 A button is displayed to authenticated users to delete existing
 packaging links.
 
+    >>> user_browser = setupBrowser(auth='Basic [e-mail]:test')
     >>> user_browser.open('http://launchpad.dev/ubuntu/+source/alsa-utils')
     >>> link = user_browser.getLink(
     ...     url='/ubuntu/warty/+source/alsa-utils/+remove-packaging')

=== 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 @@
     def test_destroySelf_notifies(self):
         """destroySelf creates a notification."""
         packaging = self.factory.makePackagingLink()
-        with person_logged_in(packaging.owner):
+        user = self.factory.makePerson(karma=200)
+        with person_logged_in(user):
             with EventRecorder() as recorder:
                 removeSecurityProxy(packaging).destroySelf()
         (event,) = recorder.events
@@ -67,7 +68,7 @@
             packaging.productseries, packaging.sourcepackagename,
             packaging.distroseries)
 
-    def test_destroySelf__not_allowed_for_arbitrary_user(self):
+    def test_destroySelf__not_allowed_for_probationary_user(self):
         """Arbitrary users cannot delete a packaging."""
         packaging = self.factory.makePackagingLink()
         packaging_util = getUtility(IPackagingUtil)
@@ -77,14 +78,15 @@
                 packaging.productseries, packaging.sourcepackagename,
                 packaging.distroseries)
 
-    def test_destroySelf__allowed_for_packaging_owner(self):
-        """A packaging owner can delete a packaging."""
+    def test_destroySelf__allowed_for_non_probationary_user(self):
+        """An experienced user can delete a packaging."""
         packaging = self.factory.makePackagingLink()
         sourcepackagename = packaging.sourcepackagename
         distroseries = packaging.distroseries
         productseries = packaging.productseries
         packaging_util = getUtility(IPackagingUtil)
-        with person_logged_in(packaging.owner):
+        user = self.factory.makePerson(karma=200)
+        with person_logged_in(user):
             packaging_util.deletePackaging(
                 packaging.productseries, packaging.sourcepackagename,
                 packaging.distroseries)
@@ -92,20 +94,6 @@
             packaging_util.packagingEntryExists(
                 sourcepackagename, distroseries, productseries))
 
-    def test_destroySelf__allowed_for_distro_owner(self):
-        """A distribution owner can delete a packaging link."""
-        packaging = self.factory.makePackagingLink()
-        sourcepackagename = packaging.sourcepackagename
-        distroseries = packaging.distroseries
-        productseries = packaging.productseries
-        packaging_util = getUtility(IPackagingUtil)
-        with person_logged_in(distroseries.distribution.owner):
-            packaging_util.deletePackaging(
-                productseries, sourcepackagename, distroseries)
-        self.assertFalse(
-            packaging_util.packagingEntryExists(
-                sourcepackagename, distroseries, productseries))
-
     def test_destroySelf__allowed_for_uploader(self):
         """A person with upload rights for the sourcepackage can
         delete a packaging link.
@@ -287,7 +275,8 @@
         """Deleting a Packaging creates a notification."""
         packaging_util = getUtility(IPackagingUtil)
         packaging = self.factory.makePackagingLink()
-        with person_logged_in(packaging.owner):
+        user = self.factory.makePerson(karma=200)
+        with person_logged_in(user):
             with EventRecorder() as recorder:
                 packaging_util.deletePackaging(
                     packaging.productseries, packaging.sourcepackagename,

=== 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 @@
 from zope.component import getUtility
 from zope.interface.verify import verifyObject
 from zope.security.checker import canAccess
-from zope.security.interfaces import Unauthorized
 from zope.security.management import checkPermission
 from zope.security.proxy import removeSecurityProxy
 
@@ -283,10 +282,11 @@
 
     def test_deletePackaging(self):
         """Ensure deletePackaging completely removes packaging."""
+        user = self.factory.makePerson(karma=200)
         packaging = self.factory.makePackagingLink()
         packaging_id = packaging.id
         store = Store.of(packaging)
-        with person_logged_in(packaging.owner):
+        with person_logged_in(user):
             packaging.sourcepackage.deletePackaging()
         result = store.find(Packaging, Packaging.id == packaging_id)
         self.assertIs(None, result.one())
@@ -305,11 +305,12 @@
         sourcepackage = self.factory.makeSourcePackage()
         productseries = self.factory.makeProductSeries()
         other_series = self.factory.makeProductSeries()
-        owner = self.factory.makePerson()
+        user = self.factory.makePerson(karma=200)
+        registrant = self.factory.makePerson()
         with EventRecorder() as recorder:
-            with person_logged_in(owner):
-                sourcepackage.setPackaging(productseries, owner=owner)
-                sourcepackage.setPackaging(other_series, owner=owner)
+            with person_logged_in(user):
+                sourcepackage.setPackaging(productseries, owner=registrant)
+                sourcepackage.setPackaging(other_series, owner=registrant)
                 packaging = sourcepackage.direct_packaging
                 self.assertEqual(packaging.productseries, other_series)
         # The first call of setPackaging() created an ObjectCreatedEvent;
@@ -321,21 +322,6 @@
         self.assertIsInstance(event2, ObjectDeletedEvent)
         self.assertIsInstance(event3, ObjectCreatedEvent)
 
-    def test_setPackaging__change_existing_entry_different_users(self):
-        """An ordinary user cannot change a Packaging defined by
-        somebody else.
-        """
-        sourcepackage = self.factory.makeSourcePackage()
-        productseries = self.factory.makeProductSeries()
-        other_series = self.factory.makeProductSeries()
-        owner = self.factory.makePerson()
-        other_user = self.factory.makePerson()
-        sourcepackage.setPackaging(productseries, owner=owner)
-        with person_logged_in(other_user):
-            self.assertRaises(
-                Unauthorized, sourcepackage.setPackaging,
-                other_series, owner=other_user)
-
     def test_setPackagingReturnSharingDetailPermissions__ordinary_user(self):
         """An ordinary user can create a packaging link but he cannot
         set the series' branch or translation syncronisation settings,
@@ -343,7 +329,7 @@
         """
         sourcepackage = self.factory.makeSourcePackage()
         productseries = self.factory.makeProductSeries()
-        packaging_owner = self.factory.makePerson()
+        packaging_owner = self.factory.makePerson(karma=100)
         with person_logged_in(packaging_owner):
             permissions = (
                 sourcepackage.setPackagingReturnSharingDetailPermissions(
@@ -370,19 +356,20 @@
         synchronisation settings, or the translation usage settings of the
         product.
         """
+        user = self.factory.makePerson(karma=100)
         packaging = self.factory.makePackagingLink()
         sourcepackage = packaging.sourcepackage
         productseries = packaging.productseries
-        with person_logged_in(packaging.owner):
+        with person_logged_in(user):
             permissions = sourcepackage.getSharingDetailPermissions()
             self.assertEqual(productseries, sourcepackage.productseries)
             self.assertFalse(
-                packaging.owner.canWrite(productseries, 'branch'))
+                user.canWrite(productseries, 'branch'))
             self.assertFalse(
-                packaging.owner.canWrite(
+                user.canWrite(
                     productseries, 'translations_autoimport_mode'))
             self.assertFalse(
-                packaging.owner.canWrite(
+                user.canWrite(
                     productseries.product, 'translations_usage'))
             expected = {
                 'user_can_change_product_series': True,
@@ -397,37 +384,6 @@
         return self.factory.makeProductSeries(
             owner=self.factory.makePerson())
 
-    def test_getSharingDetailPermissions__series_owner(self):
-        """A product series owner can create a packaging link, and he can
-        set the series' branch or translation syncronisation settings,
-        but he cannot set the translation usage settings of the product.
-        """
-        productseries = self.makeDistinctOwnerProductSeries()
-        series_owner = productseries.owner
-        # Ensure productseries owner is distinct from product owner.
-        productseries = self.factory.makeProductSeries(
-            owner=series_owner)
-        with person_logged_in(series_owner):
-            packaging = self.factory.makePackagingLink(
-                productseries=productseries, owner=series_owner)
-            sourcepackage = packaging.sourcepackage
-            permissions = sourcepackage.getSharingDetailPermissions()
-            self.assertEqual(productseries, sourcepackage.productseries)
-            self.assertTrue(series_owner.canWrite(productseries, 'branch'))
-            self.assertTrue(
-                series_owner.canWrite(
-                    productseries, 'translations_autoimport_mode'))
-            self.assertFalse(
-                series_owner.canWrite(
-                    productseries.product, 'translations_usage'))
-            expected = {
-                'user_can_change_product_series': True,
-                'user_can_change_branch': True,
-                'user_can_change_translation_usage': False,
-                'user_can_change_translations_autoimport_mode': True,
-                }
-            self.assertEqual(expected, permissions)
-
     def test_getSharingDetailPermissions__product_owner(self):
         """A product owner can create a packaging link, and he can set the
         series' branch and the translation syncronisation settings, and the
@@ -463,7 +419,7 @@
         Afterward, random people cannot change product series.
         """
         sourcepackage = self.factory.makeSourcePackage()
-        person1 = self.factory.makePerson()
+        person1 = self.factory.makePerson(karma=100)
         person2 = self.factory.makePerson()
 
         def can_change_product_series():
@@ -553,10 +509,11 @@
 
     def test_deletePackaging(self):
         """Deleting a packaging should work."""
+        user = self.factory.makePerson(karma=200)
         packaging = self.factory.makePackagingLink()
         sourcepackage = packaging.sourcepackage
         transaction.commit()
-        self.wsObject(sourcepackage, user=packaging.owner).deletePackaging()
+        self.wsObject(sourcepackage, user=user).deletePackaging()
         transaction.commit()
         self.assertIs(None, sourcepackage.direct_packaging)
 

=== 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 @@
 from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
 from lp.registry.interfaces.ssh import ISSHKeySet
 from lp.registry.model.commercialsubscription import CommercialSubscription
+from lp.registry.model.karma import KarmaTotalCache
 from lp.registry.model.milestone import Milestone
 from lp.registry.model.suitesourcepackage import SuiteSourcePackage
 from lp.services.config import config
@@ -607,7 +608,7 @@
         self, email=None, name=None, displayname=None, account_status=None,
         email_address_status=None, hide_email_addresses=False,
         time_zone=None, latitude=None, longitude=None, description=None,
-        selfgenerated_bugnotifications=False, member_of=()):
+        selfgenerated_bugnotifications=False, member_of=(), karma=None):
         """Create and return a new, arbitrary Person.
 
         :param email: The email address for the new person.
@@ -668,6 +669,10 @@
             with person_logged_in(team.teamowner):
                 team.addMember(person, team.teamowner)
 
+        if karma is not None:
+            with dbuser('karma'):
+                # Give the user karma to make the user non-probationary.
+                KarmaTotalCache(person=person.id, karma_total=karma)
         # Ensure updated ValidPersonCache
         flush_database_updates()
         return person

=== 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 @@
 
 
 import re
-
 from lazr.restful.interfaces import IJSONRequestCache
 from soupmatchers import (
     HTMLContains,
@@ -101,7 +100,9 @@
             sourcepackage=self.sourcepackage, name='ubuntu-only')
         self.shared_template_ubuntu_side = self.factory.makePOTemplate(
             sourcepackage=self.sourcepackage, name='shared-template')
-        self.productseries = self.factory.makeProductSeries()
+        self.privileged_user = self.factory.makePerson(karma=200)
+        product = self.factory.makeProduct(owner=self.privileged_user)
+        self.productseries = self.factory.makeProductSeries(product=product)
         self.shared_template_upstream_side = self.factory.makePOTemplate(
             productseries=self.productseries, name='shared-template')
         self.upstream_only_template = self.factory.makePOTemplate(
@@ -566,8 +567,8 @@
             self.assertEqual(
                 expected, self.view.set_packaging_link.escapedtext)
 
-    def test_set_packaging_link__with_packaging_any_user(self):
-        # If packaging is configured, arbitrary users do no see
+    def test_set_packaging_link__with_packaging_probationary_user(self):
+        # If packaging is configured, probationary users do no see
         # the "set packaging" link.
         self.configureSharing()
         expected = self._getExpectedPackagingLink(
@@ -588,7 +589,7 @@
         expected = self._getExpectedPackagingLink(
             id='set-packaging', url='+edit-packaging', icon='add',
             text='Set upstream link', visible=True)
-        with person_logged_in(self.sourcepackage.packaging.owner):
+        with person_logged_in(self.privileged_user):
             view = SourcePackageTranslationSharingDetailsView(
                 self.sourcepackage, LaunchpadTestRequest())
             view.initialize()
@@ -617,8 +618,8 @@
             self.assertEqual(
                 expected, self.view.change_packaging_link.escapedtext)
 
-    def test_change_packaging_link__with_packaging_any_user(self):
-        # If packaging is configured, arbitrary users do no see
+    def test_change_packaging_link__with_packaging_probationary_user(self):
+        # If packaging is configured, probationary users do no see
         # the "change packaging" link.
         self.configureSharing()
         expected = self._getExpectedPackagingLink(
@@ -639,7 +640,7 @@
         expected = self._getExpectedPackagingLink(
             id='change-packaging', url='+edit-packaging', icon='edit',
             text='Change upstream link', visible=True)
-        with person_logged_in(self.sourcepackage.packaging.owner):
+        with person_logged_in(self.privileged_user):
             view = SourcePackageTranslationSharingDetailsView(
                 self.sourcepackage, LaunchpadTestRequest())
             view.initialize()
@@ -668,8 +669,8 @@
             self.assertEqual(
                 expected, self.view.remove_packaging_link.escapedtext)
 
-    def test_remove_packaging_link__with_packaging_any_user(self):
-        # If packaging is configured, arbitrary users do no see
+    def test_remove_packaging_link__with_packaging_probationary_user(self):
+        # If packaging is configured, probationary users do no see
         # the "remove packaging" link.
         self.configureSharing()
         expected = self._getExpectedPackagingLink(
@@ -690,7 +691,7 @@
         expected = self._getExpectedPackagingLink(
             id='remove-packaging', url='+remove-packaging', icon='remove',
             text='Remove upstream link', visible=True)
-        with person_logged_in(self.sourcepackage.packaging.owner):
+        with person_logged_in(self.privileged_user):
             view = SourcePackageTranslationSharingDetailsView(
                 self.sourcepackage, LaunchpadTestRequest())
             view.initialize()

=== 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 @@
             packaging.productseries, packaging.sourcepackage,
             TranslationSplitJob)
         self.assertEqual([], finder.find())
-        with person_logged_in(packaging.owner):
+        user = self.factory.makePerson(karma=200)
+        with person_logged_in(user):
             getUtility(IPackagingUtil).deletePackaging(
                 packaging.productseries, packaging.sourcepackagename,
                 packaging.distroseries)