Merge lp:~jtv/launchpad/recife-traits into lp:~launchpad/launchpad/recife

[Jeroen T. Vermeulen]

Translation side traits.

= Translation side traits =

For merging into our "recife" feature branch, which changes how we model how Ubuntu and non-Ubuntu translations interact and which translation messages are the "current" ones.

This is a pattern that came in handy while implementing POTMsgSet.setCurrentTranslation: some external "traits" classes that summarize the parameters of the side of translation that you're currently working on—upstream or Ubuntu. In the "Recife" model that Translations is migrating to, the two use separate flags to indicate which TranslationMessages they consider current.

A lot of the changes appear unrelated: since the traits are provided by a utility, I had to involve Zope in some existing tests that then stumbled over security proxies. That meant more logins and one or two interface extensions.

There's a few tiny bits of lint left where I'm not sure the linter is right: a comment that counts as a blank line and two cases where the formatting for list comprehensions that we were once told to employ is considered whitespace-before-]. The other stuff I cleaned up.

To test:
{{{
./bin/test -vvc -m lp.translations.tests -t potmsgset -t setcurrent
}}}

Jeroen

[Henning Eggers (henninge)]

Thanks for the traits! ;-)
I think they are a good idea but we will have to watch to keep them simple. We talked about the helper class that is left in here for historical reason and I would be glad to see it removed in another branch soon.
Other than that I didn't find any problems. ;-)

review: Approve (code)

=== modified file 'lib/lp/translations/configure.zcml'
--- lib/lp/translations/configure.zcml	2010-07-23 09:41:07 +0000
+++ lib/lp/translations/configure.zcml	2010-08-06 07:43:48 +0000
@@ -570,6 +570,19 @@
             <allow
                 interface="lp.translations.interfaces.translationmessage.ITranslationMessageSet"/>
         </securedutility>
+
+        <!-- TranslationSideTraits -->
+        <class class="lp.translations.model.side.TranslationSideTraits">
+          <allow interface="lp.translations.interfaces.side.ITranslationSideTraits"/>
+        </class>
+
+        <!-- TranslationSideTraitsSet -->
+        <securedutility
+            class="lp.translations.model.side.TranslationSideTraitsSet"
+            provides="lp.translations.interfaces.side.ITranslationSideTraitsSet">
+            <allow
+              interface="lp.translations.interfaces.side.ITranslationSideTraitsSet"/>
+        </securedutility>
     </facet>
     <adapter
         provides="canonical.launchpad.webapp.interfaces.IBreadcrumb"

=== added file 'lib/lp/translations/interfaces/side.py'
--- lib/lp/translations/interfaces/side.py	1970-01-01 00:00:00 +0000
+++ lib/lp/translations/interfaces/side.py	2010-08-06 07:43:48 +0000
@@ -0,0 +1,71 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Traits of the two "sides" of translation: Ubuntu and upstream."""
+
+__metaclass__ = type
+__all__ = [
+    'ITranslationSideTraits',
+    'ITranslationSideTraitsSet',
+    'TranslationSide',
+    ]
+
+from zope.interface import Attribute, Interface
+from zope.schema import TextLine
+
+from lazr.restful.fields import Reference
+
+
+class TranslationSide:
+    """The two "sides" of software that can be translated in Launchpad.
+
+    These are "upstream" and "Ubuntu."
+    """
+    UPSTREAM = 1
+    UBUNTU = 2
+
+
+class ITranslationSideTraits(Interface):
+    """Traits describing a "side": upstream or Ubuntu.
+
+    Encapsulates primitives that depend on translation side: finding the
+    message that is current on the given side, checking the flag that
+    says whether a message is current on this side, setting or clearing
+    the flag, and providing the same capabilities for the other side.
+
+    For an introduction to the Traits pattern, see
+    http://www.cantrip.org/traits.html
+    """
+    side = Attribute("This TranslationSide")
+    other_side_traits = Reference(
+        Interface, title=u"Traits for other side.", required=True,
+        readonly=True)
+    flag_name = TextLine(
+        title=u"The TranslationMessage flag for this side",
+        required=True, readonly=True)
+
+    def getCurrentMessage(potemplate, potmsgset, language):
+        """Find the current message on this side, if any."""
+
+    def getFlag(translationmessage):
+        """Retrieve a message's "current" flag for this side."""
+
+    def setFlag(translationmessage, value):
+        """Set a message's "current" flag for this side.
+
+        This is a dumb operation.  It does not worry about conflicting
+        other messages.
+        """
+
+
+class ITranslationSideTraitsSet(Interface):
+    """Utility for `TranslationSideTraits`."""
+
+    def getTraits(side):
+        """Retrieve the `TranslationSideTraits` for `side`."""
+
+    def getForTemplate(potemplate):
+        """Get the `TranslationSideTraits` for `potemplate`s side."""
+
+    def getAllTraits():
+        """Return dict mapping `TranslationSide` to traits objects."""

=== modified file 'lib/lp/translations/interfaces/translationmessage.py'
--- lib/lp/translations/interfaces/translationmessage.py	2010-06-18 10:50:07 +0000
+++ lib/lp/translations/interfaces/translationmessage.py	2010-08-06 07:43:48 +0000
@@ -125,9 +125,8 @@
         readonly=False, required=False)
 
     reviewer = Object(
-        title=_(
-            "The person who did the review and accepted current translations"
-            ), readonly=False, required=False, schema=IPerson)
+        title=_("The person who reviewed and accepted this translation"),
+        readonly=False, required=False, schema=IPerson)
 
     # Message references for up to TranslationConstants.MAX_PLURAL_FORMS
     # plural forms.
@@ -218,6 +217,14 @@
     def getOnePOFile():
         """Get any POFile containing this translation."""
 
+    def shareIfPossible():
+        """Make this message shared, if possible.
+
+        If there is already a similar message that is shared, this
+        message's information is merged into that of the existing one,
+        and self is deleted.
+        """
+
     def isHidden(pofile):
         """Whether this is an unused, hidden suggestion in `pofile`.
 
@@ -258,7 +265,6 @@
         """
 
 
-
 class ITranslationMessageSuggestions(Interface):
     """Suggested `ITranslationMessage`s for a `POTMsgSet`.
 

=== modified file 'lib/lp/translations/interfaces/translations.py'
--- lib/lp/translations/interfaces/translations.py	2010-08-04 11:00:51 +0000
+++ lib/lp/translations/interfaces/translations.py	2010-08-06 07:43:48 +0000
@@ -10,7 +10,6 @@
 __all__ = [
     'TranslationConstants',
     'TranslationsBranchImportMode',
-    'TranslationSide',
     ]
 
 
@@ -52,12 +51,3 @@
         Import all translation files (templates and translations)
         found in the branch.
         """)
-
-
-class TranslationSide:
-    """The two "sides" of software that can be translated in Launchpad.
-
-    These are "upstream" and "Ubuntu."
-    """
-    UPSTREAM = 1
-    UBUNTU = 2

=== modified file 'lib/lp/translations/model/potmsgset.py'
--- lib/lp/translations/model/potmsgset.py	2010-08-04 11:00:51 +0000
+++ lib/lp/translations/model/potmsgset.py	2010-08-06 07:43:48 +0000
@@ -5,7 +5,7 @@
 
 __metaclass__ = type
 __all__ = [
-    'make_translation_side_message_traits',
+    'make_message_side_helpers',
     'POTMsgSet',
     ]
 
@@ -38,6 +38,8 @@
     IPOTMsgSet,
     POTMsgSetInIncompatibleTemplatesError,
     TranslationCreditsType)
+from lp.translations.interfaces.side import (
+    ITranslationSideTraitsSet, TranslationSide)
 from lp.translations.interfaces.translationfileformat import (
     TranslationFileFormat)
 from lp.translations.interfaces.translationimporter import (
@@ -46,8 +48,7 @@
     RosettaTranslationOrigin,
     TranslationConflict,
     TranslationValidationStatus)
-from lp.translations.interfaces.translations import (
-    TranslationConstants, TranslationSide)
+from lp.translations.interfaces.translations import TranslationConstants
 from lp.translations.model.pomsgid import POMsgID
 from lp.translations.model.potranslation import POTranslation
 from lp.translations.model.translationmessage import (
@@ -85,99 +86,68 @@
                        u'credits are counted as translated.')
 
 
-class TranslationSideMessageTraits:
-    """Dealing with a `POTMsgSet` on either `TranslationSide`.
-
-    Encapsulates primitives that depend on translation side: finding the
-    message that is current on the given side, checking the flag that
-    says whether a message is current on this side, setting or clearing
-    the flag, and providing the same capabilities for the other side.
-
-    For an introduction to the Traits pattern, see
-    http://www.cantrip.org/traits.html
+# Marker for "no incumbent message found yet."
+incumbent_unknown = object()
+
+
+class MessageSideHelper:
+    """Helper for manipulating messages on one `TranslationSide`.
+
+    Does some caching so that the caller doesn't need to worry about
+    unnecessary queries e.g. when disabling a previously current
+    message.
     """
-    # The TranslationSide that these Traits are for.
-    side = None
-
-    # TranslationSideMessageTraits for this message on the "other side."
+
+    # The TranslationSideTraits that this helper is for.
+    traits = None
+
+    # MessageSideHelper for this message on the "other side."
     other_side = None
 
-    # Name of this side's flag.
-    flag_name = None
+    _incumbent = incumbent_unknown
 
-    def __init__(self, potmsgset, potemplate=None, language=None,
-                 variant=None):
+    def __init__(self, side, potmsgset, potemplate=None, language=None):
+        self.traits = getUtility(ITranslationSideTraitsSet).getTraits(side)
         self.potmsgset = potmsgset
         self.potemplate = potemplate
         self.language = language
-        self.variant = variant
-
-        self._found_incumbent = False
 
     @property
     def incumbent_message(self):
         """Message that currently has the flag."""
-        if not self._found_incumbent:
-            self._incumbent = self._getIncumbentMessage()
-            self._found_incumbent = True
+        if self._incumbent == incumbent_unknown:
+            self._incumbent = self.traits.getCurrentMessage(
+                self.potmsgset, self.potemplate, self.language)
         return self._incumbent
 
-    def getFlag(self, translationmessage):
-        """Is this message the current one on this side?"""
-        return getattr(translationmessage, self.flag_name)
-
     def setFlag(self, translationmessage, value):
         """Set or clear a message's "current" flag for this side."""
-        if value == self.getFlag(translationmessage):
+        if value == self.traits.getFlag(translationmessage):
             return
 
-        if value and self.incumbent_message is not None:
-            Store.of(self.incumbent_message).add_flush_order(
-                self.incumbent_message, translationmessage)
-            self.setFlag(self.incumbent_message, False)
-
-        setattr(translationmessage, self.flag_name, value)
-        self._found_incumbent = False
-
-    def _getIncumbentMessage(self):
-        """Get the message that is current on this side, if any."""
-        raise NotImplementedError('_getIncumbentMessage')
-
-
-class UpstreamSideTraits(TranslationSideMessageTraits):
-    """Traits for upstream translations."""
-
-    side = TranslationSide.UPSTREAM
-
-    flag_name = 'is_current_upstream'
-
-    def _getIncumbentMessage(self):
-        """See `TranslationSideMessageTraits`."""
-        return self.potmsgset.getImportedTranslationMessage(
-            self.potemplate, self.language, variant=self.variant)
-
-
-class UbuntuSideTraits(TranslationSideMessageTraits):
-    """Traits for Ubuntu translations."""
-
-    side = TranslationSide.UBUNTU
-
-    flag_name = 'is_current_ubuntu'
-
-    def _getIncumbentMessage(self):
-        """See `TranslationSideMessageTraits`."""
-        return self.potmsgset.getCurrentTranslationMessage(
-            self.potemplate, self.language, variant=self.variant)
-
-
-def make_translation_side_message_traits(side, potmsgset, potemplate,
-                                         language, variant=None):
-    """Create `TranslationSideTraits` object of the appropriate subtype."""
-    ubuntu = UbuntuSideTraits(potmsgset, potemplate, language, variant)
-    upstream = UpstreamSideTraits(potmsgset, potemplate, language, variant)
+        if value:
+            if self.incumbent_message is not None:
+                Store.of(self.incumbent_message).add_flush_order(
+                    self.incumbent_message, translationmessage)
+                self.setFlag(self.incumbent_message, False)
+            self._incumbent = translationmessage
+        else:
+            self._incumbent = incumbent_unknown
+
+        self.traits.setFlag(translationmessage, value)
+
+
+def make_message_side_helpers(side, potmsgset, potemplate, language):
+    """Create `MessageSideHelper` object of the appropriate subtype."""
+    upstream = MessageSideHelper(
+        TranslationSide.UPSTREAM, potmsgset, potemplate, language)
+    ubuntu = MessageSideHelper(
+        TranslationSide.UBUNTU, potmsgset, potemplate, language)
     upstream.other_side = ubuntu
     ubuntu.other_side = upstream
-    mapping = dict((traits.side, traits) for traits in (ubuntu, upstream))
+    mapping = dict(
+        (helper.traits.side, helper)
+        for helper in (ubuntu, upstream))
     return mapping[side]
 
 
@@ -469,8 +439,9 @@
                 WHERE
                     POTMsgSet.id <> %s AND
                     msgid_singular = %s AND
-                    POTemplate.iscurrent AND
-                    (Product.official_rosetta OR Distribution.official_rosetta)
+                    POTemplate.iscurrent AND (
+                        Product.official_rosetta OR
+                        Distribution.official_rosetta)
             )''' % sqlvalues(self, self.msgid_singular))
 
         # Subquery to find the ids of TranslationMessages that are
@@ -485,7 +456,7 @@
             for form in xrange(TranslationConstants.MAX_PLURAL_FORMS)])
         ids_query_params = {
             'msgstrs': msgstrs,
-            'where': ' AND '.join(query)
+            'where': ' AND '.join(query),
         }
         ids_query = '''
             SELECT DISTINCT ON (%(msgstrs)s)
@@ -826,7 +797,6 @@
         if is_current_upstream or new_message == upstream_message:
             new_message.makeCurrentUpstream()
 
-
     def _isTranslationMessageASuggestion(self, force_suggestion,
                                          pofile, submitter,
                                          force_edition_rights,
@@ -1079,7 +1049,7 @@
             return 'none'
         elif message.is_diverged:
             return 'diverged'
-        elif translation_side_traits.other_side.getFlag(message):
+        elif translation_side_traits.other_side_traits.getFlag(message):
             return 'other_shared'
         else:
             return 'shared'
@@ -1100,8 +1070,7 @@
 
         translation_args = dict(
             ('msgstr%d' % form, translation)
-            for form, translation in translations.iteritems()
-            )
+            for form, translation in translations.iteritems())
 
         return TranslationMessage(
             potmsgset=self,
@@ -1117,14 +1086,13 @@
     def setCurrentTranslation(self, pofile, submitter, translations, origin,
                               translation_side, share_with_other_side=False):
         """See `IPOTMsgSet`."""
-        traits = make_translation_side_message_traits(
-            translation_side, self, pofile.potemplate, pofile.language,
-            variant=pofile.variant)
+        helper = make_message_side_helpers(
+            translation_side, self, pofile.potemplate, pofile.language)
 
         translations = self._findPOTranslations(translations)
 
         # The current message on this translation side, if any.
-        incumbent_message = traits.incumbent_message
+        incumbent_message = helper.incumbent_message
 
         # An already existing message, if any, that's either shared, or
         # diverged for the template/pofile we're working on, whose
@@ -1174,8 +1142,8 @@
         }
 
         incumbent_state = "incumbent_%s" % self._nameMessageStatus(
-            incumbent_message, traits)
-        twin_state = "twin_%s" % self._nameMessageStatus(twin, traits)
+            incumbent_message, helper.traits)
+        twin_state = "twin_%s" % self._nameMessageStatus(twin, helper.traits)
 
         decisions = decision_matrix[incumbent_state][twin_state]
         assert re.match('[ABZ]?[124567]?[+*]?$', decisions), (
@@ -1185,11 +1153,11 @@
             if character == 'A':
                 # Deactivate & converge.
                 # There may be an identical shared message.
-                traits.setFlag(incumbent_message, False)
+                helper.traits.setFlag(incumbent_message, False)
                 incumbent_message.shareIfPossible()
             elif character == 'B':
                 # Deactivate.
-                traits.setFlag(incumbent_message, False)
+                helper.setFlag(incumbent_message, False)
             elif character == 'Z':
                 # There is no incumbent message, so do nothing to it.
                 assert incumbent_message is None, (
@@ -1210,14 +1178,14 @@
                 # (If not, it's already active and has been unmasked by
                 # our deactivating the incumbent).
                 message = twin
-                if not traits.getFlag(twin):
-                    assert not traits.other_side.getFlag(twin), (
+                if not helper.traits.getFlag(twin):
+                    assert not helper.other_side.traits.getFlag(twin), (
                         "Trying to diverge a message that is current on the "
                         "other side.")
                     message.potemplate = pofile.potemplate
             elif character == '6':
                 # If other is not active, fork a diverged message.
-                if traits.getFlag(twin):
+                if helper.traits.getFlag(twin):
                     message = twin
                 else:
                     # The twin is used on the other side, so we can't
@@ -1232,11 +1200,11 @@
                 message.shareIfPossible()
             elif character == '*':
                 if share_with_other_side:
-                    if traits.other_side.incumbent_message is None:
-                        traits.other_side.setFlag(message, True)
+                    if helper.other_side.incumbent_message is None:
+                        helper.other_side.setFlag(message, True)
             elif character == '+':
                 if share_with_other_side:
-                    traits.other_side.setFlag(message, True)
+                    helper.other_side.setFlag(message, True)
             else:
                 raise AssertionError(
                     "Bad character in decision string: %s" % character)
@@ -1244,7 +1212,7 @@
         if decisions == '':
             message = twin
 
-        traits.setFlag(message, True)
+        helper.setFlag(message, True)
 
         return message
 
@@ -1263,7 +1231,7 @@
             current.is_current_ubuntu = False
             # Converge the current translation only if it is diverged and not
             # current upstream.
-            is_diverged =  current.potemplate is not None
+            is_diverged = current.potemplate is not None
             if is_diverged and not current.is_current_upstream:
                 current.potemplate = None
             pofile.date_changed = UTC_NOW
@@ -1494,4 +1462,3 @@
         """See `IPOTMsgSet`."""
         return TranslationTemplateItem.selectBy(
             potmsgset=self, orderBy=['id'])
-

=== added file 'lib/lp/translations/model/side.py'
--- lib/lp/translations/model/side.py	1970-01-01 00:00:00 +0000
+++ lib/lp/translations/model/side.py	2010-08-06 07:43:48 +0000
@@ -0,0 +1,79 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""`TranslationSideTraits` implementations."""
+
+__metaclass__ = type
+__all__ = [
+    'TranslationSideTraits',
+    'TranslationSideTraitsSet',
+    ]
+
+from zope.interface import implements
+
+from lp.translations.interfaces.side import (
+    ITranslationSideTraits,
+    ITranslationSideTraitsSet,
+    TranslationSide)
+
+
+class TranslationSideTraits:
+    """See `ITranslationSideTraits`."""
+    implements(ITranslationSideTraits)
+
+    def __init__(self, side, flag_name):
+        self.side = side
+        self.other_side_traits = None
+        self.flag_name = flag_name
+
+    def getFlag(self, translationmessage):
+        """See `ITranslationSideTraits`."""
+        return getattr(translationmessage, self.flag_name)
+
+    def getCurrentMessage(self, potmsgset, potemplate, language):
+        """See `ITranslationSideTraits`."""
+        if self.side == TranslationSide.UPSTREAM:
+            return potmsgset.getImportedTranslationMessage(
+                potemplate, language)
+        else:
+            return potmsgset.getCurrentTranslationMessage(
+                potemplate, language)
+
+    def setFlag(self, translationmessage, value):
+        """See `ITranslationSideTraits`."""
+        if self.side == TranslationSide.UPSTREAM:
+            translationmessage.makeCurrentUpstream(value)
+        else:
+            translationmessage.makeCurrentUbuntu(value)
+
+
+class TranslationSideTraitsSet:
+    """See `ITranslationSideTraitsSet`."""
+    implements(ITranslationSideTraitsSet)
+
+    def __init__(self):
+        upstream = TranslationSideTraits(
+            TranslationSide.UPSTREAM, 'is_current_upstream')
+        ubuntu = TranslationSideTraits(
+            TranslationSide.UBUNTU, 'is_current_ubuntu')
+        ubuntu.other_side_traits = upstream
+        upstream.other_side_traits = ubuntu
+        self.traits = dict(
+            (traits.side, traits)
+            for traits in [ubuntu, upstream])
+
+    def getTraits(self, side):
+        """See `ITranslationSideTraitsSet`."""
+        return self.traits[side]
+
+    def getForTemplate(self, potemplate):
+        """See `ITranslationSideTraitsSet`."""
+        if potemplate.productseries is not None:
+            side = TranslationSide.UPSTREAM
+        else:
+            side = TranslationSide.UBUNTU
+        return self.getTraits(side)
+
+    def getAllTraits(self):
+        """See `ITranslationSideTraitsSet`."""
+        return self.traits

=== modified file 'lib/lp/translations/model/translationmessage.py'
--- lib/lp/translations/model/translationmessage.py	2010-08-04 11:00:51 +0000
+++ lib/lp/translations/model/translationmessage.py	2010-08-06 07:43:48 +0000
@@ -161,11 +161,16 @@
         return
 
     def makeCurrentUbuntu(self, new_value=True):
+        """See `ITranslationMessage`."""
         self.is_current_ubuntu = new_value
 
     def makeCurrentUpstream(self, new_value=True):
+        """See `ITranslationMessage`."""
         self.is_current_upstream = new_value
 
+    def shareIfPossible(self):
+        """See `ITranslationMessage`."""
+
 
 class TranslationMessage(SQLBase, TranslationMessageMixIn):
     implements(ITranslationMessage)
@@ -350,12 +355,7 @@
         return Store.of(self).find(TranslationMessage, where_clause).one()
 
     def shareIfPossible(self):
-        """Make this message shared, if possible.
-
-        If there is already a similar message that is shared, this
-        message's information is merged into that of the existing one,
-        and self is deleted.
-        """
+        """See `ITranslationMessage`."""
         if self.potemplate is None:
             # Already converged.
             return
@@ -458,7 +458,6 @@
         self.is_current_upstream = new_value
 
 
-
 class TranslationMessageSet:
     """See `ITranslationMessageSet`."""
     implements(ITranslationMessageSet)

=== modified file 'lib/lp/translations/tests/test_potmsgset.py'
--- lib/lp/translations/tests/test_potmsgset.py	2010-07-27 11:31:46 +0000
+++ lib/lp/translations/tests/test_potmsgset.py	2010-08-06 07:43:48 +0000
@@ -24,32 +24,32 @@
     TranslationFileFormat)
 from lp.translations.interfaces.translationmessage import (
     RosettaTranslationOrigin, TranslationConflict)
-from lp.translations.interfaces.translations import TranslationSide
-from lp.translations.model.potmsgset import (
-    make_translation_side_message_traits)
+from lp.translations.interfaces.side import TranslationSide
+from lp.translations.model.potmsgset import make_message_side_helpers
 from lp.translations.model.translationmessage import (
     DummyTranslationMessage)
 
 from lp.testing import TestCaseWithFactory
-from canonical.testing import ZopelessDatabaseLayer
+from canonical.testing import DatabaseFunctionalLayer, ZopelessDatabaseLayer
 
 
 class TestTranslationSharedPOTMsgSets(TestCaseWithFactory):
     """Test discovery of translation suggestions."""
 
-    layer = ZopelessDatabaseLayer
+    layer = DatabaseFunctionalLayer
 
     def setUp(self):
         """Set up context to test in."""
         # Create a product with two series and a shared POTemplate
         # in different series ('devel' and 'stable').
-        super(TestTranslationSharedPOTMsgSets, self).setUp()
+        super(TestTranslationSharedPOTMsgSets, self).setUp(
+            '[e-mail]')
         self.foo = self.factory.makeProduct()
         self.foo_devel = self.factory.makeProductSeries(
             name='devel', product=self.foo)
         self.foo_stable = self.factory.makeProductSeries(
             name='stable', product=self.foo)
-        self.foo.official_rosetta = True
+        removeSecurityProxy(self.foo).official_rosetta = True
 
         # POTemplate is 'shared' if it has the same name ('messages').
         self.devel_potemplate = self.factory.makePOTemplate(
@@ -154,8 +154,7 @@
         # is not used as a singular_text.
         translation = self.factory.makeTranslationMessage(
             pofile=en_pofile, potmsgset=potmsgset,
-            translations=[DIVERGED_ENGLISH_STRING])
-        translation.potemplate = self.devel_potemplate
+            translations=[DIVERGED_ENGLISH_STRING], force_diverged=True)
         self.assertEquals(potmsgset.singular_text, ENGLISH_STRING)
 
     def test_getCurrentDummyTranslationMessage(self):
@@ -324,7 +323,8 @@
         # Create an external POTemplate with a POTMsgSet using
         # the same English string as the one in self.potmsgset.
         external_template = self.factory.makePOTemplate()
-        external_template.productseries.product.official_rosetta = True
+        product = external_template.productseries.product
+        removeSecurityProxy(product).official_rosetta = True
         external_potmsgset = self.factory.makePOTMsgSet(
             external_template,
             singular=self.potmsgset.singular_text)
@@ -357,7 +357,7 @@
         imported_translation = self.factory.makeSharedTranslationMessage(
             pofile=external_pofile, potmsgset=external_potmsgset,
             suggestion=False, is_current_upstream=True)
-        imported_translation.is_current_ubuntu = False
+        imported_translation.makeCurrentUbuntu(False)
 
         transaction.commit()
 
@@ -383,7 +383,8 @@
         # Create an external POTemplate with a POTMsgSet using
         # the same English string as the one in self.potmsgset.
         external_template = self.factory.makePOTemplate()
-        external_template.productseries.product.official_rosetta = True
+        product = external_template.productseries.product
+        removeSecurityProxy(product).official_rosetta = True
         external_potmsgset = self.factory.makePOTMsgSet(
             external_template,
             singular=self.potmsgset.singular_text)
@@ -416,7 +417,7 @@
         imported_translation = self.factory.makeSharedTranslationMessage(
             pofile=external_pofile, potmsgset=external_potmsgset,
             suggestion=False, is_current_upstream=True)
-        imported_translation.is_current_ubuntu = False
+        imported_translation.makeCurrentUbuntu(False)
 
         transaction.commit()
 
@@ -460,7 +461,7 @@
 
         # If the current upstream translation is also current in Ubuntu,
         # it's not changed in Ubuntu.
-        current_shared.is_current_ubuntu = False
+        current_shared.makeCurrentUbuntu(False)
         imported_shared = self.factory.makeSharedTranslationMessage(
             pofile=sr_pofile, potmsgset=self.potmsgset,
             is_current_upstream=True)
@@ -471,7 +472,7 @@
 
         # If there's a current, diverged translation, and an imported
         # non-current one, it's changed in Ubuntu.
-        imported_shared.is_current_ubuntu = False
+        imported_shared.makeCurrentUbuntu(False)
         current_diverged = self.factory.makeTranslationMessage(
             pofile=sr_pofile, potmsgset=self.potmsgset,
             is_current_upstream=False)
@@ -482,7 +483,7 @@
 
         # If the upstream one is shared and used in Ubuntu, yet there is
         # a diverged Ubuntu translation as well, it is changed in Ubuntu.
-        imported_shared.is_current_ubuntu = False
+        imported_shared.makeCurrentUbuntu(False)
         self.assertEquals(
             self.potmsgset.hasTranslationChangedInLaunchpad(
                 self.devel_potemplate, serbian),
@@ -688,8 +689,8 @@
             potemplate, es_pofile.language)
         # Let's make sure this message is also marked as imported
         # and diverged.
-        es_current.is_current_upstream = True
-        es_current.potemplate = potemplate
+        es_current.makeCurrentUpstream(True)
+        removeSecurityProxy(es_current).potemplate = potemplate
 
         self.assertTrue(es_current.is_current_ubuntu)
         self.assertNotEqual(None, es_current.potemplate)
@@ -720,13 +721,19 @@
 class TestPOTMsgSetSuggestions(TestCaseWithFactory):
     """Test retrieval and dismissal of translation suggestions."""
 
-    layer = ZopelessDatabaseLayer
+    layer = DatabaseFunctionalLayer
 
     def _setDateCreated(self, tm):
         removeSecurityProxy(tm).date_created = self.now()
 
     def _setDateReviewed(self, tm):
-        removeSecurityProxy(tm).date_reviewed = self.now()
+        naked_tm = removeSecurityProxy(tm)
+        if naked_tm.reviewer is None:
+            naked_tm.reviewer = self.factory.makePerson()
+        naked_tm.date_reviewed = self.now()
+
+    def _setDateUpdated(self, tm):
+        removeSecurityProxy(tm).date_updated = self.now()
 
     def gen_now(self):
         now = datetime.now(pytz.UTC)
@@ -737,12 +744,12 @@
     def setUp(self):
         # Create a product with all the boilerplate objects to be able to
         # create TranslationMessage objects.
-        super(TestPOTMsgSetSuggestions, self).setUp()
+        super(TestPOTMsgSetSuggestions, self).setUp('[e-mail]')
         self.now = self.gen_now().next
         self.foo = self.factory.makeProduct()
         self.foo_main = self.factory.makeProductSeries(
             name='main', product=self.foo)
-        self.foo.official_rosetta = True
+        removeSecurityProxy(self.foo).official_rosetta = True
 
         self.potemplate = self.factory.makePOTemplate(
             productseries=self.foo_main, name="messages")
@@ -752,14 +759,17 @@
         # Set up some translation messages with dummy timestamps that will be
         # changed in the tests.
         self.translation = self.factory.makeTranslationMessage(
-            self.pofile, self.potmsgset, translations=[u'trans1'],
-            reviewer=self.factory.makePerson(), date_updated=self.now())
+            removeSecurityProxy(self.pofile), self.potmsgset,
+            translations=[u'trans1'], reviewer=self.factory.makePerson(),
+            is_current_upstream=True, date_updated=self.now())
         self.suggestion1 = self.factory.makeTranslationMessage(
             self.pofile, self.potmsgset, suggestion=True,
-            translations=[u'sugg1'], date_updated=self.now())
+            translations=[u'sugg1'], reviewer=self.factory.makePerson(),
+            date_updated=self.now())
         self.suggestion2 = self.factory.makeTranslationMessage(
             self.pofile, self.potmsgset, suggestion=True,
             translations=[u'sugg2'], date_updated=self.now())
+        self._setDateCreated(self.suggestion2)
 
     def test_dismiss_all(self):
         # Set order of creation and review.
@@ -841,7 +851,8 @@
         transaction.commit()
         # Make the translation a suggestion, too.
         suggestion3 = self.translation
-        suggestion3.is_current_ubuntu = False
+        suggestion3.makeCurrentUbuntu(False)
+        suggestion3.makeCurrentUpstream(False)
         self._setDateCreated(suggestion3)
         transaction.commit()
         # All suggestions are visible.
@@ -914,7 +925,7 @@
 class TestPOTMsgSetResetTranslation(TestCaseWithFactory):
     """Test resetting the current translation."""
 
-    layer = ZopelessDatabaseLayer
+    layer = DatabaseFunctionalLayer
 
     def gen_now(self):
         now = datetime.now(pytz.UTC)
@@ -925,12 +936,13 @@
     def setUp(self):
         # Create a product with all the boilerplate objects to be able to
         # create TranslationMessage objects.
-        super(TestPOTMsgSetResetTranslation, self).setUp()
+        super(TestPOTMsgSetResetTranslation, self).setUp(
+            '[e-mail]')
         self.now = self.gen_now().next
         self.foo = self.factory.makeProduct()
         self.foo_main = self.factory.makeProductSeries(
             name='main', product=self.foo)
-        self.foo.official_rosetta = True
+        removeSecurityProxy(self.foo).official_rosetta = True
 
         self.potemplate = self.factory.makePOTemplate(
             productseries=self.foo_main, name="messages")
@@ -997,7 +1009,7 @@
 class TestPOTMsgSetCornerCases(TestCaseWithFactory):
     """Test corner cases and constraints."""
 
-    layer = ZopelessDatabaseLayer
+    layer = DatabaseFunctionalLayer
 
     def gen_now(self):
         now = datetime.now(pytz.UTC)
@@ -1009,7 +1021,7 @@
         """Set up context to test in."""
         # Create a product with two series and a shared POTemplate
         # in different series ('devel' and 'stable').
-        super(TestPOTMsgSetCornerCases, self).setUp()
+        super(TestPOTMsgSetCornerCases, self).setUp('[e-mail]')
 
         self.pofile = self.factory.makePOFile('sr')
         self.potemplate = self.pofile.potemplate
@@ -1126,12 +1138,11 @@
         tm1 = self.potmsgset.updateTranslation(
             self.pofile, self.uploader, [u"tm1"], lock_timestamp=self.now(),
             is_current_upstream=True, force_shared=True)
+        self.assertTrue(tm1.is_current_ubuntu)
         tm2 = self.potmsgset.updateTranslation(
             self.pofile, self.uploader, [u"tm2"], lock_timestamp=self.now(),
             is_current_upstream=True, force_diverged=True)
-        tm2.is_current_ubuntu = False
-        self.assertTrue(tm1.is_current_ubuntu)
-        self.assertFalse(tm2.is_current_ubuntu)
+        tm2.makeCurrentUbuntu(False)
 
         self.potmsgset.updateTranslation(
             self.pofile, self.uploader, [u"tm1"], lock_timestamp=self.now(),
@@ -1184,7 +1195,7 @@
         tm2 = self.potmsgset.updateTranslation(
             self.pofile, self.uploader, [u"tm2"], lock_timestamp=self.now(),
             is_current_upstream=True, force_diverged=True)
-        tm2.is_current_ubuntu = False
+        tm2.makeCurrentUbuntu(False)
 
         self.assertEquals(None, tm1.potemplate)
         self.assertEquals(self.pofile.potemplate, tm2.potemplate)
@@ -1249,10 +1260,11 @@
 class TestPOTMsgSetTranslationCredits(TestCaseWithFactory):
     """Test methods related to TranslationCredits."""
 
-    layer = ZopelessDatabaseLayer
+    layer = DatabaseFunctionalLayer
 
     def setUp(self):
-        super(TestPOTMsgSetTranslationCredits, self).setUp()
+        super(TestPOTMsgSetTranslationCredits, self).setUp(
+            '[e-mail]')
         self.potemplate = self.factory.makePOTemplate()
 
     def test_creation_credits(self):
@@ -1368,10 +1380,8 @@
         """Set up `KarmaRecorder` on `pofile`."""
         template = pofile.potemplate
         return self.installKarmaRecorder(
-            person=template.owner,
-            action_name='translationsuggestionadded',
-            product=template.product,
-            distribution=template.distribution,
+            person=template.owner, action_name='translationsuggestionadded',
+            product=template.product, distribution=template.distribution,
             sourcepackagename=template.sourcepackagename)
 
     def test_new_suggestion(self):
@@ -1571,7 +1581,10 @@
 
 
 class TestSetCurrentTranslation(TestCaseWithFactory):
-    layer = ZopelessDatabaseLayer
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestSetCurrentTranslation, self).setUp('[e-mail]')
 
     def _makePOFileAndPOTMsgSet(self):
         pofile = self.factory.makePOFile('nl')
@@ -1599,62 +1612,62 @@
             pofile.potemplate, pofile.language))
         self.assertEqual(origin, message.origin)
 
-    def test_make_translation_side_message_traits(self):
-        # make_translation_side_message_traits is a factory for traits
-        # objects that help setCurrentTranslations deal with the
-        # dichotomy between upstream and Ubuntu translations.
+    def test_make_message_side_helpers(self):
+        # make_message_side_helpers is a factory for helpers that help
+        # setCurrentTranslations deal with the dichotomy between
+        # upstream and Ubuntu translations.
         pofile, potmsgset = self._makePOFileAndPOTMsgSet()
         sides = (TranslationSide.UPSTREAM, TranslationSide.UBUNTU)
         for side in sides:
-            traits = make_translation_side_message_traits(
+            helper = make_message_side_helpers(
                 side, potmsgset, pofile.potemplate, pofile.language)
-            self.assertEqual(side, traits.side)
-            self.assertNotEqual(side, traits.other_side.side)
-            self.assertIn(traits.other_side.side, sides)
-            self.assertIs(traits, traits.other_side.other_side)
+            self.assertEqual(side, helper.traits.side)
+            self.assertNotEqual(side, helper.other_side.traits.side)
+            self.assertIn(helper.other_side.traits.side, sides)
+            self.assertIs(helper, helper.other_side.other_side)
 
     def test_UpstreamSideTraits_upstream(self):
         pofile, potmsgset = self._makePOFileAndPOTMsgSet()
         message = self.factory.makeTranslationMessage(
             pofile=pofile, potmsgset=potmsgset)
 
-        traits = make_translation_side_message_traits(
+        helper = make_message_side_helpers(
             TranslationSide.UPSTREAM, potmsgset, pofile.potemplate,
             pofile.language)
 
-        self.assertEqual('is_current_upstream', traits.flag_name)
+        self.assertEqual('is_current_upstream', helper.traits.flag_name)
 
-        self.assertFalse(traits.getFlag(message))
+        self.assertFalse(helper.traits.getFlag(message))
         self.assertFalse(message.is_current_upstream)
-        self.assertEquals(None, traits.incumbent_message)
-
-        traits.setFlag(message, True)
-
-        self.assertTrue(traits.getFlag(message))
+        self.assertEquals(None, helper.incumbent_message)
+
+        helper.setFlag(message, True)
+
+        self.assertTrue(helper.traits.getFlag(message))
         self.assertTrue(message.is_current_upstream)
-        self.assertEquals(message, traits.incumbent_message)
+        self.assertEquals(message, helper.incumbent_message)
 
     def test_UpstreamSideTraits_ubuntu(self):
         pofile, potmsgset = self._makePOFileAndPOTMsgSet()
         message = self.factory.makeTranslationMessage(
             pofile=pofile, potmsgset=potmsgset)
-        message.is_current_ubuntu = False
+        message.makeCurrentUbuntu(False)
 
-        traits = make_translation_side_message_traits(
+        helper = make_message_side_helpers(
             TranslationSide.UBUNTU, potmsgset, pofile.potemplate,
             pofile.language)
 
-        self.assertEqual('is_current_ubuntu', traits.flag_name)
+        self.assertEqual('is_current_ubuntu', helper.traits.flag_name)
 
-        self.assertFalse(traits.getFlag(message))
+        self.assertFalse(helper.traits.getFlag(message))
         self.assertFalse(message.is_current_ubuntu)
-        self.assertEquals(None, traits.incumbent_message)
-
-        traits.setFlag(message, True)
-
-        self.assertTrue(traits.getFlag(message))
+        self.assertEquals(None, helper.incumbent_message)
+
+        helper.setFlag(message, True)
+
+        self.assertTrue(helper.traits.getFlag(message))
         self.assertTrue(message.is_current_ubuntu)
-        self.assertEquals(message, traits.incumbent_message)
+        self.assertEquals(message, helper.incumbent_message)
 
     def test_identical(self):
         # Setting the same message twice leaves the original as-is.

=== modified file 'lib/lp/translations/tests/test_setcurrenttranslation.py'
--- lib/lp/translations/tests/test_setcurrenttranslation.py	2010-07-06 09:27:37 +0000
+++ lib/lp/translations/tests/test_setcurrenttranslation.py	2010-08-06 07:43:48 +0000
@@ -13,8 +13,7 @@
 from lp.testing import TestCaseWithFactory
 from lp.translations.interfaces.translationmessage import (
     RosettaTranslationOrigin)
-from lp.translations.interfaces.translations import (
-    TranslationSide)
+from lp.translations.interfaces.side import TranslationSide
 from lp.translations.model.translationmessage import (
     TranslationMessage)
 
@@ -87,7 +86,8 @@
          * current: represents a current shared translation for this context.
          * diverged: represents a diverged translation for this context.
          * other_shared: represents a shared translation for "other" context.
-         * divergences_elsewhere: a list of other divergences in both contexts.
+         * divergences_elsewhere: a list of other divergences in both
+            contexts.
         """
         new_current, new_diverged, new_other, new_divergences = (
             summarize_current_translations(self.pofile, self.potmsgset))

=== added file 'lib/lp/translations/tests/test_side.py'
--- lib/lp/translations/tests/test_side.py	1970-01-01 00:00:00 +0000
+++ lib/lp/translations/tests/test_side.py	2010-08-06 07:43:48 +0000
@@ -0,0 +1,186 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test `TranslationSide` and friends."""
+
+__metaclass__ = type
+
+from canonical.testing import DatabaseFunctionalLayer
+from lp.testing import TestCaseWithFactory
+
+from zope.component import getUtility
+from zope.interface.verify import verifyObject
+
+from lp.translations.interfaces.side import (
+    ITranslationSideTraits,
+    ITranslationSideTraitsSet,
+    TranslationSide)
+
+
+class TestTranslationSideTraitsSet(TestCaseWithFactory):
+    layer = DatabaseFunctionalLayer
+
+    def test_baseline(self):
+        utility = getUtility(ITranslationSideTraitsSet)
+        self.assertTrue(verifyObject(ITranslationSideTraitsSet, utility))
+        for traits in utility.getAllTraits().itervalues():
+            self.assertTrue(verifyObject(ITranslationSideTraits, traits))
+
+    def test_other_sides(self):
+        utility = getUtility(ITranslationSideTraitsSet)
+        upstream = utility.getTraits(TranslationSide.UPSTREAM)
+        ubuntu = utility.getTraits(TranslationSide.UBUNTU)
+
+        self.assertEqual(ubuntu, upstream.other_side_traits)
+        self.assertEqual(upstream, ubuntu.other_side_traits)
+
+    def test_getTraits(self):
+        utility = getUtility(ITranslationSideTraitsSet)
+        for side in [TranslationSide.UPSTREAM, TranslationSide.UBUNTU]:
+            traits = utility.getTraits(side)
+            self.assertTrue(verifyObject(ITranslationSideTraits, traits))
+
+    def test_getForTemplate_upstream(self):
+        utility = getUtility(ITranslationSideTraitsSet)
+        productseries = self.factory.makeProductSeries()
+        template = self.factory.makePOTemplate(productseries=productseries)
+        traits = utility.getForTemplate(template)
+        self.assertEqual(TranslationSide.UPSTREAM, traits.side)
+
+    def test_getForTemplate_ubuntu(self):
+        utility = getUtility(ITranslationSideTraitsSet)
+        package = self.factory.makeSourcePackage()
+        template = self.factory.makePOTemplate(
+            distroseries=package.distroseries,
+            sourcepackagename=package.sourcepackagename)
+        traits = utility.getForTemplate(template)
+        self.assertEqual(TranslationSide.UBUNTU, traits.side)
+
+    def test_getAllTraits(self):
+        utility = getUtility(ITranslationSideTraitsSet)
+        traits_dict = utility.getAllTraits()
+
+        self.assertContentEqual(
+            [TranslationSide.UPSTREAM, TranslationSide.UBUNTU],
+            traits_dict.keys())
+
+        for side, traits in traits_dict.iteritems():
+            self.assertEqual(side, traits.side)
+            self.assertEqual(traits, utility.getTraits(side))
+
+
+class TraitsScenario:
+    """Tests that can be run on either the upstream or the Ubuntu side."""
+
+    def _makeTemplate(self):
+        """Create a template for the side being tested."""
+        raise NotImplementedError()
+
+    def _makeTemplateAndTranslationMessage(self):
+        """Create a POTemplate with a TranslationMessage.
+
+        Creates a POFile and POTMsgSet along the way.
+
+        The TranslationMessage will not be current.
+        """
+        template = self._makeTemplate()
+        pofile = self.factory.makePOFile('nl', template)
+        potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
+        translationmessage = potmsgset.submitSuggestion(
+            pofile, self.factory.makePerson(),
+            [self.factory.getUniqueString()])
+        return template, translationmessage
+
+    def _getTraits(self, template):
+        """Shortcut: get TranslationSideTraits for template."""
+        return getUtility(ITranslationSideTraitsSet).getForTemplate(template)
+
+    def test_getFlag_and_setFlag(self):
+        template, message = self._makeTemplateAndTranslationMessage()
+        traits = self._getTraits(template)
+        other_side_traits = traits.other_side_traits
+
+        traits.setFlag(message, True)
+
+        self.assertEqual(
+            (True, False),
+            (traits.getFlag(message), other_side_traits.getFlag(message)))
+
+        traits.setFlag(message, False)
+
+        self.assertEqual(
+            (False, False),
+            (traits.getFlag(message), other_side_traits.getFlag(message)))
+
+    def test_getCurrentMessage(self):
+        template, message = self._makeTemplateAndTranslationMessage()
+        traits = self._getTraits(template)
+
+        traits.setFlag(message, True)
+
+        current_message = traits.getCurrentMessage(
+            message.potmsgset, template, message.language)
+        self.assertEqual(message, current_message)
+
+        traits.setFlag(message, False)
+
+        current_message = traits.getCurrentMessage(
+            message.potmsgset, template, message.language)
+        self.assertIs(None, current_message)
+
+    def test_getCurrentMessage_ignores_other_flag(self):
+        template, message = self._makeTemplateAndTranslationMessage()
+        traits = self._getTraits(template)
+        other_side_traits = traits.other_side_traits
+
+        other_side_traits.setFlag(message, True)
+
+        current_message = traits.getCurrentMessage(
+            message.potmsgset, template, message.language)
+        self.assertIs(None, current_message)
+
+        other_side_traits.setFlag(message, False)
+
+        current_message = traits.getCurrentMessage(
+            message.potmsgset, template, message.language)
+        self.assertIs(None, current_message)
+
+
+class UpstreamTranslationSideTraitsTest(TraitsScenario, TestCaseWithFactory):
+    """Run the TraitsScenario tests on the upstream side."""
+    layer = DatabaseFunctionalLayer
+
+    def _makeTemplate(self):
+        """See `TraitsScenario`."""
+        return self.factory.makePOTemplate(
+            productseries=self.factory.makeProductSeries())
+
+    def test_getFlag_reads_upstream_flag(self):
+        # This test case looks on the upstream side.  We're really
+        # working with the is_current_upstream flag underneath the
+        # traits interface.
+        template, message = self._makeTemplateAndTranslationMessage()
+        traits = self._getTraits(template)
+        traits.setFlag(message, True)
+        self.assertTrue(message.is_current_upstream)
+
+
+class UbuntuTranslationSideTraitsTest(TraitsScenario, TestCaseWithFactory):
+    """Run the TraitsScenario tests on the Ubuntu side."""
+    layer = DatabaseFunctionalLayer
+
+    def _makeTemplate(self):
+        """See `TraitsScenario`."""
+        package = self.factory.makeSourcePackage()
+        return self.factory.makePOTemplate(
+            distroseries=package.distroseries,
+            sourcepackagename=package.sourcepackagename)
+
+    def test_getFlag_reads_ubuntu_flag(self):
+        # This test case looks on the Ubuntu side.  We're really
+        # working with the is_current_ubuntu flag underneath the traits
+        # interface.
+        template, message = self._makeTemplateAndTranslationMessage()
+        traits = self._getTraits(template)
+        traits.setFlag(message, True)
+        self.assertTrue(message.is_current_ubuntu)