Merge lp:~jtv/launchpad/recife-policy-invites-allows into lp:~launchpad/launchpad/recife

[Jeroen T. Vermeulen]

Recife model's translations access and sharing policies.

This is a complete overhaul of the translation permissions code. Be warned. It's probably oversized, too, though I've been splitting off lots of work into separate branches to keep sizes down.

The merge target is Recife, a feature branch we've been working on all year.

I hope the best explanation I can give for the new model is in the code itself. But to set the right frame of reference:

 * Whether a user can edit a translation, or enter suggestions there was governed by code in the pofile module.

 * That code was at least 5 years old, and not unit-tested directly until I replaced the doctest with more systematic unit tests recently.

 * Throughout the ages, as empires rose and fell and continents collided and drifted apart, the human race kept piling more checks into the access model but also began using it in more and different ways. Scientists today are hard-pressed to describe exactly what these checks are and aren't meant to be responsible for.

 * My apologies for the little David Attenborough speech there. No more National Geographic Channel for me.

 * Unfortunately there are still two checks for read-only mode in the "access" checks for POFile. I'd like to clean those up later, but note that during a slow submission they can catch cases where the server goes read-only after the check at the beginning of the request, and so turn oopses into slightly less annoying exceptions.

 * We now need even more from the access checks: we need to know whether a translation made in Ubuntu (by a particular person and in a particular language) should be shared with a linked upstream product (if any) or vice versa. We call this issue "sharing policy."

 * Sharing policy turns out not to depend on access permissions exactly. It touches on access rights, but is fundamentally a workflow choice.

 * Therefore in this branch I tease those two aspects of the access rights apart, and move them into TranslationPolicy. You'll also find sharing policy defined there.

I actually duplicate a lot of testing here. The POFile permissions test I added recently covers much of the same ground as the tests you see here. That was absolutely necessary to guard continuity during these changes. I could possibly slim them down now that we have direct tests on the policy system. On the other hand, the POFile permissions test is structured in a very different way and both seem worthwhile. They may complement each other somewhat. Also of course, I'm not throwing any more changes into this diff!

There are bits of lint left, most of which I intend to clean up after review (together with automated copyright updates and import re-sorting):
{{{

./lib/lp/registry/model/distribution.py
    1236: Line exceeds 78 characters.
./lib/lp/translations/model/pofile.py
      72: 'TranslationPermission' imported but unused
     102: 'ITranslationsPerson' imported but unused
./lib/lp/translations/utilities/translation_import.py
      36: 'ITranslationSideTraitsSet' imported but unused
      35: 'IPOTemplateSet' imported but unused
      36: 'TranslationSide' imported but unused
}}}

The distribution one however is not one I'd like to touch. Better for Registry people to deal with it.

Jeroen

[Māris Fogels (mars)]

Hi Jeroen,

This change looks good. The code and tests are clear, even for someone like myself who is not entirely familiar with the model. In all cases the new code is easier to read than what it replaced.

I mostly looked at changes to test coverage, since you said you overhauled the doctests in preference to unit tests. Given that, are the changes to the translationtarget property tested? What I see represents a big change to the method's return type and type checking.

Similarly, share_with_other_side() changed - is it tested elsewhere as well?

I think the change looks good enough to land as-is, if you want. r=mars

review: Approve

[Jeroen T. Vermeulen (jtv)]

Thanks for going the extra mile (on such a big review) and looking that up! It turns out that one of the two, POTemplate.translationtarget, was not covered by unit tests. I fixed that.

Jeroen

=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py	2010-11-09 06:55:07 +0000
+++ lib/lp/registry/model/distribution.py	2010-11-10 04:40:00 +0000
@@ -1753,6 +1753,32 @@
         # see: https://bugs.launchpad.net/soyuz/+bug/246200
         return results.any() != None
 
+    def sharesTranslationsWithOtherSide(self, person, language,
+                                        sourcepackage=None,
+                                        purportedly_upstream=False):
+        """See `ITranslationPolicy`."""
+        assert sourcepackage is not None, (
+            "Translations sharing policy requires a SourcePackage.")
+
+        sharing_productseries = sourcepackage.productseries
+        if sharing_productseries is None:
+            # There is no known upstream series.  Take the uploader's
+            # word for whether these are upstream translations (in which
+            # case they're shared) or not.
+            # What are the consequences if that value is incorrect?  In
+            # the case where translations from upstream are purportedly
+            # from Ubuntu, we miss a chance at sharing when the package
+            # is eventually matched up with a productseries.  An import
+            # or sharing-script run will fix that.  In the case where
+            # Ubuntu translations are purportedly from upstream, an
+            # import can fix it once a productseries is selected; or a
+            # merge done by a script will give precedence to the Product
+            # translations for upstream.
+            return purportedly_upstream
+
+        upstream_product = sharing_productseries.product
+        return upstream_product.invitesTranslationEdits(person, language)
+
 
 class DistributionSet:
     """This class is to deal with Distribution related stuff"""

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2010-11-09 06:55:07 +0000
+++ lib/lp/registry/model/product.py	2010-11-10 04:40:00 +0000
@@ -1059,6 +1059,15 @@
         # policy from its ProjectGroup, if any.
         return self.project
 
+    def sharesTranslationsWithOtherSide(self, person, language,
+                                        sourcepackage=None,
+                                        purportedly_upstream=False):
+        """See `ITranslationPolicy`."""
+        assert sourcepackage is None, "Got a SourcePackage for a Product!"
+        # Product translations are considered upstream.  They are
+        # automatically shared.
+        return True
+
     @property
     def has_any_specifications(self):
         """See `IHasSpecifications`."""

=== modified file 'lib/lp/registry/model/projectgroup.py'
--- lib/lp/registry/model/projectgroup.py	2010-11-09 06:55:07 +0000
+++ lib/lp/registry/model/projectgroup.py	2010-11-10 04:40:00 +0000
@@ -208,6 +208,16 @@
         # return not self.translatables().is_empty()
         return self.translatables().count() != 0
 
+    def sharesTranslationsWithOtherSide(self, person, language,
+                                        sourcepackage=None,
+                                        purportedly_upstream=False):
+        """See `ITranslationPolicy`."""
+        assert sourcepackage is None, (
+            "Got a SourcePackage for a ProjectGroup!")
+        # ProjectGroup translations are considered upstream.  They are
+        # automatically shared.
+        return True
+
     def has_branches(self):
         """ See `IProjectGroup`."""
         return not self.getBranches().is_empty()

=== modified file 'lib/lp/translations/interfaces/potemplate.py'
--- lib/lp/translations/interfaces/potemplate.py	2010-11-09 06:44:07 +0000
+++ lib/lp/translations/interfaces/potemplate.py	2010-11-10 04:40:00 +0000
@@ -34,6 +34,7 @@
 from canonical.launchpad.interfaces.librarian import ILibraryFileAlias
 from lp.app.errors import NotFoundError
 from lp.registry.interfaces.distribution import IDistribution
+from lp.registry.interfaces.sourcepackage import ISourcePackage
 from lp.registry.interfaces.sourcepackagename import ISourcePackageName
 from lp.services.fields import PersonChoice
 from lp.translations.interfaces.rosettastats import IRosettaStats
@@ -168,6 +169,10 @@
         required=False,
         vocabulary="SourcePackageName")
 
+    sourcepackage = Reference(
+        ISourcePackage, title=u"Source package this template is for, if any.",
+        required=False, readonly=True)
+
     from_sourcepackagename = Choice(
         title=_("From Source Package Name"),
         description=_(

=== modified file 'lib/lp/translations/interfaces/translationpolicy.py'
--- lib/lp/translations/interfaces/translationpolicy.py	2010-11-09 15:12:40 +0000
+++ lib/lp/translations/interfaces/translationpolicy.py	2010-11-10 04:40:00 +0000
@@ -22,6 +22,14 @@
     add suggestions.  (The ability to edit also implies the ability to
     enter suggestions).  Everyone else is allowed only to view the
     translations.
+
+    The policy can "invite" the user to edit or suggest; or it can
+    merely "allow" them to.  Whoever is invited is also allowed, but
+    administrators and certain other special users may be allowed
+    without actually being invited.
+
+    The invitation is based purely on the access model configured by the
+    user: translation team and translation policy.
     """
 
     translationgroup = Choice(
@@ -79,3 +87,67 @@
         `self.translationpermission` and any inherited
         `TranslationPermission`.
         """
+
+    def invitesTranslationEdits(person, language):
+        """Does this policy invite `person` to edit translations?
+
+        The decision is based on the chosen `TranslationPermission`,
+        `TranslationGroup`(s), the presence of a translation team, and
+        `person`s membership of the translation team.
+
+        As one extreme, the OPEN model invites editing by anyone.  The
+        opposite extreme is CLOSED, which invites editing only by
+        members of the applicable translation team.
+
+        :param person: The user.
+        :type person: IPerson
+        :param language: The language to translate to.  This will be
+            used to look up the applicable translation team(s).
+        :type language: ILanguage
+        """
+
+    def invitesTranslationSuggestions(person, language):
+        """Does this policy invite `person` to enter suggestions?
+
+        Similar to `invitesTranslationEdits`, but for the activity of
+        entering suggestions.  This carries less risk, so generally a
+        wider public is invited to do this than to edit.
+        """
+
+    def allowsTranslationEdits(person, language):
+        """Is `person` allowed to edit translations to `language`?
+
+        Similar to `invitesTranslationEdits`, except administrators and
+        in the case of Product translations, owners of the product are
+        always allowed even if they are not invited.
+        """
+
+    def allowsTranslationSuggestions(person, language):
+        """Is `person` allowed to enter suggestions for `language`?
+
+        Similar to `invitesTranslationSuggestions, except administrators
+        and in the case of Product translations, owners of the product
+        are always allowed even if they are not invited.
+        """
+
+    def sharesTranslationsWithOtherSide(person, language,
+                                        sourcepackage=None,
+                                        purportedly_upstream=False):
+        """Should translations be shared across `TranslationSide`s?
+
+        Should translations to this object, as reviewed by `person`,
+        into `language` be shared with the other `TranslationSide`?
+
+        The answer depends on whether the user is invited to edit the
+        translations on the other side.  Administrators and other
+        specially privileged users are allowed to do that, but that
+        does not automatically mean that their translations should be
+        shared there.
+
+        :param person: The `Person` providing translations.
+        :param language: The `Language` being translated to.
+        :param sourcepackage: When translating a `Distribution`, the
+            `SourcePackage` that is being translated.
+        :param purportedly_upstream: Whether `person` provides the
+            translations in question as coming from upstream.
+        """

=== modified file 'lib/lp/translations/model/pofile.py'
--- lib/lp/translations/model/pofile.py	2010-11-09 09:46:20 +0000
+++ lib/lp/translations/model/pofile.py	2010-11-10 04:40:00 +0000
@@ -69,10 +69,7 @@
 from canonical.launchpad.webapp.publisher import canonical_url
 from lp.registry.interfaces.person import validate_public_person
 from lp.services.propertycache import cachedproperty
-from lp.translations.enums import (
-    RosettaImportStatus,
-    TranslationPermission,
-    )
+from lp.translations.enums import RosettaImportStatus
 from lp.translations.interfaces.pofile import (
     IPOFile,
     IPOFileSet,
@@ -99,7 +96,6 @@
     TranslationValidationStatus,
     )
 from lp.translations.interfaces.translations import TranslationConstants
-from lp.translations.interfaces.translationsperson import ITranslationsPerson
 from lp.translations.model.pomsgid import POMsgID
 from lp.translations.model.potmsgset import POTMsgSet
 from lp.translations.model.translatablemessage import TranslatableMessage
@@ -117,170 +113,6 @@
     )
 
 
-def _check_translation_perms(permission, translators, person):
-    """Return True or False dependening on whether the person is part of the
-    right group of translators, and the permission on the relevant project,
-    product or distribution.
-
-    :param permission: The kind of TranslationPermission.
-    :param translators: The list of official translators for the
-        product/project/distribution.
-    :param person: The person that we want to check if has translation
-        permissions.
-    """
-    # Let's determine if the person is part of a designated translation team
-    is_designated_translator = False
-    # XXX sabdfl 2005-05-25:
-    # This code could be improved when we have implemented CrowdControl.
-    for translator in translators:
-        if person.inTeam(translator):
-            is_designated_translator = True
-            break
-
-    # have a look at the applicable permission policy
-    if permission == TranslationPermission.OPEN:
-        # if the translation policy is "open", then yes, anybody is an
-        # editor of any translation
-        return True
-    elif permission == TranslationPermission.STRUCTURED:
-        # in the case of a STRUCTURED permission, designated translators
-        # can edit, unless there are no translators, in which case
-        # anybody can translate
-        if len(translators) > 0:
-            # when there are designated translators, only they can edit
-            if is_designated_translator is True:
-                return True
-        else:
-            # since there are no translators, anyone can edit
-            return True
-    elif permission in (TranslationPermission.RESTRICTED,
-                        TranslationPermission.CLOSED):
-        # if the translation policy is "restricted" or "closed", then check if
-        # the person is in the set of translators
-        if is_designated_translator:
-            return True
-    else:
-        raise NotImplementedError('Unknown permission %s' % permission.name)
-
-    # ok, thats all we can check, and so we must assume the answer is no
-    return False
-
-
-def _person_has_not_licensed_translations(person):
-    """Whether a person has declined to BSD-license their translations."""
-    t_p = ITranslationsPerson(person)
-    if (t_p.translations_relicensing_agreement is not None and
-        t_p.translations_relicensing_agreement is False):
-        return True
-    else:
-        return False
-
-
-def is_admin(user):
-    """Is `user` an admin or Translations admin?"""
-    celebs = getUtility(ILaunchpadCelebrities)
-    return user.inTeam(celebs.admin) or user.inTeam(celebs.rosetta_experts)
-
-
-def is_product_owner(user, potemplate):
-    """Is `user` the owner of a `Product` that `potemplate` belongs to?
-
-    A product's owners have edit rights on the product's translations.
-    """
-    productseries = potemplate.productseries
-    if productseries is None:
-        return False
-
-    return user.inTeam(productseries.product.owner)
-
-
-def _can_edit_translations(pofile, person):
-    """Say if a person is able to edit existing translations.
-
-    Return True or False indicating whether the person is allowed
-    to edit these translations.
-
-    Admins and Rosetta experts are always able to edit any translation.
-    If the `IPOFile` is for an `IProductSeries`, the owner of the `IProduct`
-    has also permissions.
-    Any other mortal will have rights depending on if he/she is on the right
-    translation team for the given `IPOFile`.translationpermission and the
-    language associated with this `IPOFile`.
-    """
-    if person is None:
-        # Anonymous users can't edit anything.
-        return False
-
-    if is_read_only():
-        # Nothing can be edited in read-only mode.
-        return False
-
-    # XXX Carlos Perello Marin 2006-02-07 bug=4814:
-    # We should not check the permissions here but use the standard
-    # security system.
-
-    # Rosetta experts and admins can always edit translations.
-    if is_admin(person):
-        return True
-
-    # The owner of the product is also able to edit translations.
-    if is_product_owner(person, pofile.potemplate):
-        return True
-
-    # If a person has decided not to license their translations under BSD
-    # license they can't edit translations.
-    if _person_has_not_licensed_translations(person):
-        return False
-
-    # Finally, check whether the user is a member of the translation team.
-    translators = [t.translator for t in pofile.translators]
-    return _check_translation_perms(
-        pofile.translationpermission, translators, person)
-
-
-def _can_add_suggestions(pofile, person):
-    """Whether a person is able to add suggestions.
-
-    Besides people who have permission to edit the translation, this
-    includes any logged-in user for translations in STRUCTURED mode, and
-    any logged-in user for translations in RESTRICTED mode that have a
-    translation team assigned.
-    """
-    if is_read_only():
-        # No suggestions can be added in read-only mode.
-        return False
-
-    if person is None:
-        return False
-
-    # If a person has decided not to license their translations under BSD
-    # license they can't edit translations.
-    if _person_has_not_licensed_translations(person):
-        return False
-
-    if _can_edit_translations(pofile, person):
-        return True
-
-    if pofile.translationpermission == TranslationPermission.OPEN:
-        # We would return True here, except OPEN mode already allows
-        # anyone to edit (see above).
-        raise AssertionError(
-            "Translation is OPEN, but user is not allowed to edit.")
-    elif pofile.translationpermission == TranslationPermission.STRUCTURED:
-        return True
-    elif pofile.translationpermission == TranslationPermission.RESTRICTED:
-        # Only allow suggestions if there is someone to review them.
-        groups = pofile.potemplate.translationgroups
-        for group in groups:
-            if group.query_translator(pofile.language):
-                return True
-        return False
-    elif pofile.translationpermission == TranslationPermission.CLOSED:
-        return False
-
-    raise AssertionError("Unknown translation mode.")
-
-
 class POFileMixIn(RosettaStats):
     """Base class for `POFile` and `DummyPOFile`.
 
@@ -306,11 +138,19 @@
 
     def canEditTranslations(self, person):
         """See `IPOFile`."""
-        return _can_edit_translations(self, person)
+        if is_read_only():
+            # Nothing can be edited in read-only mode.
+            return False
+        policy = self.potemplate.getTranslationPolicy()
+        return policy.allowsTranslationEdits(person, self.language)
 
     def canAddSuggestions(self, person):
         """See `IPOFile`."""
-        return _can_add_suggestions(self, person)
+        if is_read_only():
+            # No data can be entered in read-only mode.
+            return False
+        policy = self.potemplate.getTranslationPolicy()
+        return policy.allowsTranslationSuggestions(person, self.language)
 
     def getHeader(self):
         """See `IPOFile`."""

=== modified file 'lib/lp/translations/model/potemplate.py'
--- lib/lp/translations/model/potemplate.py	2010-11-09 14:25:41 +0000
+++ lib/lp/translations/model/potemplate.py	2010-11-10 04:40:00 +0000
@@ -366,16 +366,25 @@
             clauseTables=['POFile'],
             distinct=True).count()
 
+    @cachedproperty
+    def sourcepackage(self):
+        """See `IPOTemplate`."""
+        # Avoid circular imports
+        from lp.registry.model.sourcepackage import SourcePackage
+
+        if self.distroseries is None:
+            return None
+        return SourcePackage(
+            distroseries=self.distroseries,
+            sourcepackagename=self.sourcepackagename)
+
     @property
     def translationtarget(self):
         """See `IPOTemplate`."""
         if self.productseries is not None:
             return self.productseries
-        elif self.distroseries is not None:
-            from lp.registry.model.sourcepackage import SourcePackage
-            return SourcePackage(distroseries=self.distroseries,
-                sourcepackagename=self.sourcepackagename)
-        raise AssertionError('Unknown POTemplate translation target')
+        else:
+            return self.sourcepackage
 
     def getHeader(self):
         """See `IPOTemplate`."""

=== modified file 'lib/lp/translations/model/translationpolicy.py'
--- lib/lp/translations/model/translationpolicy.py	2010-11-08 08:58:38 +0000
+++ lib/lp/translations/model/translationpolicy.py	2010-11-10 04:40:00 +0000
@@ -17,11 +17,28 @@
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.interfaces.lpstorm import IStore
 from lp.registry.model.person import Person
+from lp.translations.enums import TranslationPermission
 from lp.translations.interfaces.translationsperson import ITranslationsPerson
 from lp.translations.model.translationgroup import TranslationGroup
 from lp.translations.model.translator import Translator
 
 
+def has_translators(translators_list):
+    """Did `getTranslators` find any translators?"""
+    for group, translator, team in translators_list:
+        if team is not None:
+            return True
+    return False
+
+
+def is_in_one_of_translators(translators_list, person):
+    """Is `person` a member of one of the entries in `getTranslators`?"""
+    for group, translator, team in translators_list:
+        if team is not None and person.inTeam(team):
+            return True
+    return False
+
+
 class TranslationPolicyMixin:
     """Implementation mixin for `ITranslationPolicy`."""
 
@@ -102,3 +119,68 @@
             return max([
                 self.translationpermission,
                 inherited.getEffectiveTranslationPermission()])
+
+    def invitesTranslationEdits(self, person, language):
+        """See `ITranslationPolicy`."""
+        if person is None:
+            return False
+
+        model = self.getEffectiveTranslationPermission()
+        if model == TranslationPermission.OPEN:
+            # Open permissions invite all contributions.
+            return True
+
+        translators = self.getTranslators(language)
+        if model == TranslationPermission.STRUCTURED:
+            # Structured permissions act like Open if no translators
+            # have been assigned for the language.
+            if not has_translators(translators):
+                return True
+
+        # Translation-team members are always invited to edit.
+        return is_in_one_of_translators(translators, person)
+
+    def invitesTranslationSuggestions(self, person, language):
+        """See `ITranslationPolicy`."""
+        if person is None:
+            return False
+
+        model = self.getEffectiveTranslationPermission()
+
+        # These models always invite suggestions from anyone.
+        welcoming_models = [
+            TranslationPermission.OPEN,
+            TranslationPermission.STRUCTURED,
+            ]
+        if model in welcoming_models:
+            return True
+
+        translators = self.getTranslators(language)
+        if model == TranslationPermission.RESTRICTED:
+            if has_translators(translators):
+                # Restricted invites any user's suggestions as long as
+                # there is a translation team to handle them.
+                return True
+
+        # Translation-team members are always invited to suggest.
+        return is_in_one_of_translators(translators, person)
+
+    def allowsTranslationEdits(self, person, language):
+        """See `ITranslationPolicy`."""
+        if person is None:
+            return False
+        if self._hasSpecialTranslationPrivileges(person):
+            return True
+        return (
+            self._canTranslate(person) and
+            self.invitesTranslationEdits(person, language))
+
+    def allowsTranslationSuggestions(self, person, language):
+        """See `ITranslationPolicy`."""
+        if person is None:
+            return False
+        if self._hasSpecialTranslationPrivileges(person):
+            return True
+        return (
+            self._canTranslate(person) and
+            self.invitesTranslationSuggestions(person, language))

=== modified file 'lib/lp/translations/tests/test_potemplate.py'
--- lib/lp/translations/tests/test_potemplate.py	2010-10-19 12:52:33 +0000
+++ lib/lp/translations/tests/test_potemplate.py	2010-11-10 04:40:00 +0000
@@ -139,6 +139,18 @@
         self.assertEqual(1, len(karma_events))
         self.assertEqual(action, karma_events[0].action.name)
 
+    def test_translationtarget_can_be_productseries(self):
+        productseries = self.factory.makeProductSeries()
+        template = self.factory.makePOTemplate(productseries=productseries)
+        self.assertEqual(productseries, template.translationtarget)
+
+    def test_translationtarget_can_be_sourcepackage(self):
+        package = self.factory.makeSourcePackage()
+        template = self.factory.makePOTemplate(
+            distroseries=package.distroseries,
+            sourcepackagename=package.sourcepackagename)
+        self.assertEqual(package, template.translationtarget)
+
 
 class EquivalenceClassTestMixin:
     """Helper for POTemplate equivalence class tests."""

=== modified file 'lib/lp/translations/tests/test_translationpolicy.py'
--- lib/lp/translations/tests/test_translationpolicy.py	2010-11-09 14:25:41 +0000
+++ lib/lp/translations/tests/test_translationpolicy.py	2010-11-10 04:40:00 +0000
@@ -7,6 +7,7 @@
 
 from zope.component import getUtility
 from zope.interface import implements
+from zope.security.proxy import removeSecurityProxy
 
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.testing.layers import ZopelessDatabaseLayer
@@ -187,3 +188,224 @@
         self.assertEqual(TranslationPermission.CLOSED, max(
             TranslationPermission.RESTRICTED,
             TranslationPermission.CLOSED))
+
+    def test_nobodies_stay_out(self):
+        # We neither allow nor invite suggestions or edits by anonymous
+        # users.
+        language = self.factory.makeLanguage()
+        self.assertFalse(self.policy.invitesTranslationEdits(None, language))
+        self.assertFalse(
+            self.policy.invitesTranslationSuggestions(None, language))
+        self.assertFalse(self.policy.allowsTranslationEdits(None, language))
+        self.assertFalse(
+            self.policy.allowsTranslationSuggestions(None, language))
+
+    def test_privileged_users_allowed_but_not_invited(self):
+        # Specially privileged users such as administrators and
+        # "translations owners" can enter suggestions and edit
+        # translations, but are not particularly invited to do so.
+        owner = self.factory.makePerson()
+        language = self.factory.makeLanguage()
+        self.policy.translationpermission = TranslationPermission.CLOSED
+        self.policy.isTranslationsOwner = FakeMethod(result=True)
+        self.assertFalse(self.policy.invitesTranslationEdits(owner, language))
+        self.assertFalse(
+            self.policy.invitesTranslationSuggestions(owner, language))
+        self.assertTrue(self.policy.allowsTranslationEdits(owner, language))
+        self.assertTrue(
+            self.policy.allowsTranslationSuggestions(owner, language))
+
+    def test_open_invites_anyone(self):
+        # The OPEN model invites anyone to enter suggestions or even
+        # edit translations.
+        joe = self.factory.makePerson()
+        language = self.factory.makeLanguage()
+        self.policy.translationpermission = TranslationPermission.OPEN
+        self.assertTrue(self.policy.invitesTranslationEdits(joe, language))
+        self.assertTrue(
+            self.policy.invitesTranslationSuggestions(joe, language))
+
+    def test_translation_team_members_are_invited(self):
+        # Members of a translation team are invited (and thus allowed)
+        # to enter suggestions for or edit translations covered by the
+        # translation team.
+        language = self.factory.makeLanguage()
+        translator = self._makeTranslator(language)
+        for permission in TranslationPermission.items:
+            self.policy.translationpermission = permission
+            self.assertTrue(
+                self.policy.invitesTranslationEdits(translator, language))
+            self.assertTrue(
+                self.policy.invitesTranslationSuggestions(
+                    translator, language))
+
+    def test_structured_is_open_for_untended_translations(self):
+        # Without a translation team, STRUCTURED is like OPEN.
+        joe = self.factory.makePerson()
+        language = self.factory.makeLanguage()
+        self.policy.translationpermission = TranslationPermission.STRUCTURED
+        self.assertTrue(self.policy.invitesTranslationEdits(joe, language))
+        self.assertTrue(
+            self.policy.invitesTranslationSuggestions(joe, language))
+
+    def test_restricted_is_closed_for_untended_translations(self):
+        # Without a translation team, RESTRICTED is like CLOSED.
+        joe = self.factory.makePerson()
+        language = self.factory.makeLanguage()
+        self.policy.translationpermission = TranslationPermission.RESTRICTED
+        self.assertFalse(self.policy.invitesTranslationEdits(joe, language))
+        self.assertFalse(
+            self.policy.invitesTranslationSuggestions(joe, language))
+
+    def test_structured_and_restricted_for_tended_translations(self):
+        # If there's a translation team, STRUCTURED and RESTRICTED both
+        # invite suggestions (but not editing) by non-members.
+        joe = self.factory.makePerson()
+        language = self.factory.makeLanguage()
+        self._makeTranslator(language)
+        intermediate_permissions = [
+            TranslationPermission.STRUCTURED,
+            TranslationPermission.RESTRICTED,
+            ]
+        for permission in intermediate_permissions:
+            self.policy.translationpermission = permission
+            self.assertFalse(
+                self.policy.invitesTranslationEdits(joe, language))
+            self.assertTrue(
+                self.policy.invitesTranslationSuggestions(joe, language))
+
+    def test_closed_invites_nobody_for_untended_translations(self):
+        # The CLOSED model does not invite anyone for untended
+        # translations.
+        joe = self.factory.makePerson()
+        language = self.factory.makeLanguage()
+        self.policy.translationpermission = TranslationPermission.CLOSED
+
+        self.assertFalse(self.policy.invitesTranslationEdits(joe, language))
+        self.assertFalse(
+            self.policy.invitesTranslationSuggestions(joe, language))
+
+    def test_closed_does_not_invite_nonmembers_for_tended_translations(self):
+        # The CLOSED model invites nobody outside the translation team.
+        joe = self.factory.makePerson()
+        language = self.factory.makeLanguage()
+        self._makeTranslator(language)
+        self.policy.translationpermission = TranslationPermission.CLOSED
+
+        self.assertFalse(self.policy.invitesTranslationEdits(joe, language))
+        self.assertFalse(
+            self.policy.invitesTranslationSuggestions(joe, language))
+
+    def test_untended_translation_means_no_team(self):
+        # A translation is "untended" if there is no translation team,
+        # even if there is a translation group.
+        joe = self.factory.makePerson()
+        language = self.factory.makeLanguage()
+        self.policy.translationpermission = TranslationPermission.RESTRICTED
+
+        self.assertFalse(
+            self.policy.invitesTranslationSuggestions(joe, language))
+        self.policy.translationgroup = self.factory.makeTranslationGroup()
+        self.assertFalse(
+            self.policy.invitesTranslationSuggestions(joe, language))
+
+    def test_translation_can_be_tended_by_empty_team(self):
+        # A translation that has an empty translation team is tended.
+        joe = self.factory.makePerson()
+        language = self.factory.makeLanguage()
+        self.policy.translationgroup = self.factory.makeTranslationGroup()
+        getUtility(ITranslatorSet).new(
+            self.policy.translationgroup, language, self.factory.makeTeam(),
+            None)
+        self.policy.translationpermission = TranslationPermission.RESTRICTED
+
+        self.assertTrue(
+            self.policy.invitesTranslationSuggestions(joe, language))
+
+
+class TestTranslationsOwners(TestCaseWithFactory):
+    """Who exactly are "translations owners"?
+
+    :ivar owner: A `Person` to be used as an owner of various things.
+    """
+    layer = ZopelessDatabaseLayer
+
+    def setUp(self):
+        super(TestTranslationsOwners, self).setUp()
+        self.owner = self.factory.makePerson()
+
+    def isTranslationsOwnerOf(self, pillar):
+        """Is `self.owner` a translations owner of `pillar`?"""
+        return removeSecurityProxy(pillar).isTranslationsOwner(self.owner)
+
+    def test_product_owners(self):
+        # Product owners are "translations owners."
+        product = self.factory.makeProduct(owner=self.owner)
+        self.assertTrue(self.isTranslationsOwnerOf(product))
+
+    def test_projectgroup_owners(self):
+        # ProjectGroup owners are not translations owners.
+        project = self.factory.makeProject(owner=self.owner)
+        self.assertFalse(self.isTranslationsOwnerOf(project))
+
+    def test_distribution_owners(self):
+        # Distribution owners are not translations owners.
+        distro = self.factory.makeDistribution(owner=self.owner)
+        self.assertFalse(self.isTranslationsOwnerOf(distro))
+
+    def test_product_translationgroup_owners(self):
+        # Translation group owners are not translations owners in the
+        # case of Products.
+        group = self.factory.makeTranslationGroup(owner=self.owner)
+        product = self.factory.makeProject()
+        product.translationgroup = group
+        self.assertFalse(self.isTranslationsOwnerOf(product))
+
+    def test_distro_translationgroup_owners(self):
+        # Translation group owners are not translations owners in the
+        # case of Distributions.
+        group = self.factory.makeTranslationGroup(owner=self.owner)
+        distro = self.factory.makeDistribution()
+        distro.translationgroup = group
+        self.assertFalse(self.isTranslationsOwnerOf(distro))
+
+
+class TestSharingPolicy(TestCaseWithFactory):
+    """Test `ITranslationPolicy`'s sharing between Ubuntu and upstream."""
+    layer = ZopelessDatabaseLayer
+
+    def setUp(self):
+        super(TestSharingPolicy, self).setUp()
+        self.user = self.factory.makePerson()
+        self.language = self.factory.makeLanguage()
+
+    def _doesPackageShare(self, sourcepackage, from_upstream=False):
+        """Does this `SourcePackage` share with upstream?"""
+        distro = sourcepackage.distroseries.distribution
+        return distro.sharesTranslationsWithOtherSide(
+            self.user, self.language, sourcepackage=sourcepackage,
+            purportedly_upstream=from_upstream)
+
+    def test_product_always_shares(self):
+        product = self.factory.makeProduct()
+        self.assertTrue(
+            product.sharesTranslationsWithOtherSide(self.user, self.language))
+
+    def test_distribution_shares_only_if_invited(self):
+        package = self.factory.makeSourcePackage()
+        self.factory.makePackagingLink(
+            sourcepackagename=package.sourcepackagename,
+            distroseries=package.distroseries)
+        product = package.productseries.product
+
+        product.translationpermission = TranslationPermission.OPEN
+        self.assertTrue(self._doesPackageShare(package))
+        product.translationpermission = TranslationPermission.CLOSED
+        self.assertFalse(self._doesPackageShare(package))
+
+    def test_unlinked_package_shares_only_upstream_translations(self):
+        package = self.factory.makeSourcePackage()
+        distro = package.distroseries.distribution
+        for from_upstream in [False, True]:
+            self.assertEqual(
+                from_upstream, self._doesPackageShare(package, from_upstream))

=== modified file 'lib/lp/translations/utilities/translation_import.py'
--- lib/lp/translations/utilities/translation_import.py	2010-11-09 09:46:20 +0000
+++ lib/lp/translations/utilities/translation_import.py	2010-11-10 04:40:00 +0000
@@ -32,11 +32,6 @@
     )
 from lp.services.propertycache import cachedproperty
 from lp.translations.enums import RosettaImportStatus
-from lp.translations.interfaces.potemplate import IPOTemplateSet
-from lp.translations.interfaces.side import (
-    ITranslationSideTraitsSet,
-    TranslationSide,
-    )
 from lp.translations.interfaces.translationexporter import (
     ITranslationExporter,
     )
@@ -462,29 +457,13 @@
     def share_with_other_side(self):
         """Returns True if translations should be shared with the other side.
         """
-        traits = getUtility(
-            ITranslationSideTraitsSet).getForTemplate(self.potemplate)
-        if traits.side == TranslationSide.UPSTREAM:
-            return True
-        # Maintainer uploads are always shared with Ubuntu.
-        if self.translation_import_queue_entry.by_maintainer:
-            return True
-        # Find the sharing POFile and check permissions.
-        productseries = self.potemplate.distroseries.getSourcePackage(
-            self.potemplate.sourcepackagename).productseries
-        if productseries is None:
-            return False
-        upstream_template = getUtility(IPOTemplateSet).getSubset(
-            productseries=productseries).getPOTemplateByName(
-                self.potemplate.name)
-        upstream_pofile = upstream_template.getPOFileByLang(
-            self.pofile.language.code)
-        if upstream_pofile is not None:
-            uploader_person = self.translation_import_queue_entry.importer
-            if upstream_pofile.canEditTranslations(uploader_person):
-                return True
-        # Deny the rest.
-        return False
+        from_upstream = self.translation_import_queue_entry.by_maintainer
+        potemplate = self.potemplate
+        policy = potemplate.getTranslationPolicy()
+        return policy.sharesTranslationsWithOtherSide(
+            self.importer, self.pofile.language,
+            sourcepackage=potemplate.sourcepackage,
+            purportedly_upstream=from_upstream)
 
     @cachedproperty
     def translations_are_msgids(self):