← Back to team overview

launchpad-reviewers team mailing list archive

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

 

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/recife-policy-invites-allows into lp:~launchpad/launchpad/recife with lp:~jtv/launchpad/recife-pre-cleanups as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


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
-- 
https://code.launchpad.net/~jtv/launchpad/recife-policy-invites-allows/+merge/40438
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/recife-policy-invites-allows into lp:~launchpad/launchpad/recife.
=== 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-09 14:56:26 +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-09 14:56:26 +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-09 14:56:26 +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-09 14:56:26 +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 11:11:23 +0000
+++ lib/lp/translations/interfaces/translationpolicy.py	2010-11-09 14:56:26 +0000
@@ -9,12 +9,10 @@
     ]
 
 from zope.interface import Interface
-from zope.schema import (
-    Choice,
-    )
+from zope.schema import Choice
 
 from canonical.launchpad import _
-from lp.translations.interfaces.translationgroup import TranslationPermission
+from lp.translations.enums import TranslationPermission
 
 
 class ITranslationPolicy(Interface):
@@ -24,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(
@@ -81,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-09 14:56:26 +0000
@@ -117,170 +117,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 +142,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 06:44:07 +0000
+++ lib/lp/translations/model/potemplate.py	2010-11-09 14:56:26 +0000
@@ -366,15 +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-09 14:56:26 +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_translationpermission.py'
--- lib/lp/translations/tests/test_translationpermission.py	2010-11-08 10:02:51 +0000
+++ lib/lp/translations/tests/test_translationpermission.py	2010-11-09 14:56:26 +0000
@@ -13,23 +13,6 @@
 from lp.translations.interfaces.translator import ITranslatorSet
 
 
-# Description of the translations permissions model:
-# * OPEN lets anyone edit or suggest.
-# * STRUCTURED lets translation team members edit and anyone
-# suggest, but acts like OPEN when no translation team
-# applies.
-# * RESTRICTED lets translation team members edit and anyone
-# suggest, but acts like CLOSED when no translation team
-# applies.
-# * CLOSED lets only translation team members edit translations
-# or enter suggestions.
-translation_permissions = [
-    TranslationPermission.OPEN,
-    TranslationPermission.STRUCTURED,
-    TranslationPermission.RESTRICTED,
-    TranslationPermission.CLOSED,
-    ]
-
 # A user can be translating either a translation that's not covered by a
 # translation team ("untended"), or one that is ("tended"), or one whose
 # translation team the user is a member of ("member").
@@ -161,7 +144,7 @@
             permissions_model[permission, coverage],
             privilege_level,
             "Wrong privileges for %s with translation team coverage '%s'." % (
-                permission, coverage))
+                permission.name, coverage))
 
     def test_translationgroup_models(self):
         # Test that a translation group bestows the expected privilege
@@ -171,7 +154,7 @@
         user = self.factory.makePerson()
         product = self.factory.makeProduct()
         pofiles = self.makePOFilesForCoverageLevels(product, user)
-        for permission in translation_permissions:
+        for permission in TranslationPermission.items:
             product.translationpermission = permission
             for coverage in team_coverage:
                 pofile = pofiles[coverage]
@@ -185,7 +168,7 @@
         user = self.factory.makePerson()
         pofile = self.factory.makePOFile()
         product = pofile.potemplate.productseries.product
-        for permission in translation_permissions:
+        for permission in TranslationPermission.items:
             product.translationpermission = permission
             privilege_level = PrivilegeLevel.check(pofile, user)
             self.assertPrivilege(permission, 'untended', privilege_level)
@@ -234,9 +217,9 @@
         product = self.makeProductInProjectGroup()
         user = self.factory.makePerson()
         pofiles = self.makePOFilesForCoverageLevels(product, user)
-        for project_permission in translation_permissions:
+        for project_permission in TranslationPermission.items:
             product.project.translationpermission = project_permission
-            for product_permission in translation_permissions:
+            for product_permission in TranslationPermission.items:
                 product.translationpermission = product_permission
                 effective_permission = combine_permissions(product)
 
@@ -290,9 +273,9 @@
 
         # The strictest of Open and something else is always the
         # something else.
-        for project_permission in translation_permissions:
+        for project_permission in TranslationPermission.items:
             product.project.translationpermission = project_permission
-            for product_permission in translation_permissions:
+            for product_permission in TranslationPermission.items:
                 product.translationpermission = product_permission
                 expected_permission = (
                     combinations[project_permission][product_permission])

=== modified file 'lib/lp/translations/tests/test_translationpolicy.py'
--- lib/lp/translations/tests/test_translationpolicy.py	2010-11-08 02:29:37 +0000
+++ lib/lp/translations/tests/test_translationpolicy.py	2010-11-09 14:56:26 +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
@@ -20,6 +21,7 @@
 
 
 class TranslationPolicyImplementation(TranslationPolicyMixin):
+    """An `ITranslationPolicy` implementation for testing."""
     implements(ITranslationPolicy)
 
     translationgroup = None
@@ -28,6 +30,10 @@
 
 
 class TestTranslationPolicy(TestCaseWithFactory):
+    """Test `TranslationPolicyMixin`.
+
+    :ivar policy: A `TranslationPolicyImplementation` for testing.
+    """
     layer = ZopelessDatabaseLayer
 
     def setUp(self):
@@ -182,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-09 14:56:26 +0000
@@ -462,6 +462,7 @@
     def share_with_other_side(self):
         """Returns True if translations should be shared with the other side.
         """
+<<<<<<< TREE
         traits = getUtility(
             ITranslationSideTraitsSet).getForTemplate(self.potemplate)
         if traits.side == TranslationSide.UPSTREAM:
@@ -485,6 +486,15 @@
                 return True
         # Deny the rest.
         return False
+=======
+        from_upstream = self.translation_import_queue_entry.from_upstream
+        potemplate = self.potemplate
+        policy = potemplate.getTranslationPolicy()
+        return policy.sharesTranslationsWithOtherSide(
+            self.importer, self.pofile.language,
+            sourcepackage=potemplate.sourcepackage,
+            purportedly_upstream=from_upstream)
+>>>>>>> MERGE-SOURCE
 
     @cachedproperty
     def translations_are_msgids(self):