Merge lp:~henninge/launchpad/db-devel-688519-getusedtranslationmessage into lp:launchpad/db-devel

[Henning Eggers]

[r=jcsackett,sinzui][ui=none][bug=688519] Replacement for getImportedTranslationMessage and getSharedTranslationMessage. Stop using makeCurrent*.

= Summary =

The goal of this branch was to find an alternative for methods that use
POTMsgSet._getUsedTranslationMessage, which are
getCurrentTranslationMessage, getImportedTranslationMessage, and
getSharedTranslationMessage. These alternatives were found but cannot be
used in all places so that the old methods have to be kept around.

The branch is quite long because a lot of call sites could still be updated.

Another fix that came up while doing this was to remove the use of
TranslationMessage.makeCurrentUbunut and makeCurrentUpstream from code that
uses the new model. In the new model the work done by them has been moved to
higher levels.

A few more other things were fixed, see the details below.

== Proposed fix ==

The getImportedTranslationMessage and getSharedTranslationMessage were
replaced by getOtherTranslation and getSharedTranslation which use
getCurrentTranslation which already existed.

The makeCurrent* methods were called in the central
TranslationSideTraits.setFlag method. Its implemetation now sets the
respective flag attributes directly without checking if other flags need to
be cleared first. Only one call site needed to be fixed because it did not
unset a flag properly.

== Pre-implementation notes ==

More and earlier pre-implementation chatting with Danilo would have
prevented some turns that this branch took but it's OK now. There are a lot
more things that could be fixed but they should not become part of this
branch.

== Implementation details ==

Any change not mentioned here is either a mechanical replacement or was
already mentioned.

lib/lp/testing/factory.py:
The factory method makeCurrentTranslationMessage did not create diverged
message properly which was now causing tests to fail. That now routes
through makeDivergedTranslationMessage which already existed.

lib/lp/translations/model/potmsgset.py:
The singular_text property was patched to work with the new model by looking
for English translations on both sides. That approach is still subject to
discussion but works for now.
An added setFlag call fixes setCurrentTranslation.

lib/lp/translations/tests/helpers.py:
I found that get_all_translations_current_anywhere was named wrongly because
it explicitely looks for diverged messages. Also its implementation and also
its call site were overly complicated. I fixed it to be simpler.

lib/lp/translations/tests/test_potmsgset.py:
The bigger test changes in here were the incentive for this branch. Some of
the concepts tested here don't apply any more, so I updated the test to be
more useful. This could be done for a lot more tests but not in this branch.

I just saw that I left two tests disabled in there. They should pass now,
though. I will re-enable them an push the change.

lib/lp/translations/tests/test_translatablemessage.py:
The factory method makeCurrentTranslationMessage now takes date_created and
date_reviewed parameters.

== Tests ==

Best to run all translations tests.
bin/test -vvm lp.translations

== Demo and Q/A ==

QA is part of the QA for the recife branch.

= Launchpad lint =

I removed a lot of lint as you can see in the branch.

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/scripts/tests/test_message_sharing_migration.py
  lib/lp/translations/utilities/tests/import-flags.txt
  lib/lp/translations/model/side.py
  lib/lp/translations/interfaces/potmsgset.py
  lib/lp/translations/scripts/message_sharing_migration.py
  lib/lp/testing/factory.py
  lib/lp/translations/tests/helpers.py
  lib/lp/translations/tests/test_potmsgset.py
  lib/lp/translations/utilities/tests/test_xpi_import.py
  lib/lp/translations/doc/poimport.txt
  lib/lp/translations/tests/potmsgset-update-translation.txt
  lib/lp/translations/tests/test_translatablemessage.py
  lib/lp/translations/model/potmsgset.py
  lib/lp/translations/model/translatablemessage.py
  lib/lp/translations/doc/potmsgset.txt
  lib/lp/translations/tests/test_suggestions.py
  lib/lp/translations/model/translationmessage.py

./lib/lp/translations/utilities/tests/import-flags.txt
       1: narrative uses a moin header.
      14: narrative uses a moin header.
      23: source exceeds 78 characters.
      91: narrative uses a moin header.
     242: narrative exceeds 78 characters.
./lib/lp/translations/model/potmsgset.py
     307: W291 trailing whitespace
     307: Line has trailing whitespace.

[j.c.sackett (jcsackett)]

Henninge--

This is quite a branch.

Is it safe to assume this is preceding further work? I like the changes, but it seems like this introduces yet another way to do things without getting rid of the old way that I think this branch is supposed to cleanup.

Overall, there's a lot of cleanup in this branch I appreciate. I have some other questions below and a few commenting nitpicks/questions. Feel free to answer here or track me down in IRC.

> === modified file 'lib/lp/testing/factory.py'
> --- lib/lp/testing/factory.py 2010-12-15 04:45:54 +0000
> +++ lib/lp/testing/factory.py 2010-12-16 16:04:00 +0000
> @@ -2671,24 +2671,25 @@
> potmsgset.singular_text, translations,
> pofile.language.pluralforms)
>
> - message = potmsgset.setCurrentTranslation(
> - pofile, translator, translations,
> - RosettaTranslationOrigin.ROSETTAWEB,
> - share_with_other_side=current_other)
> - naked_message = removeSecurityProxy(message)
> -
> if diverged:
> - naked_message.potemplate = pofile.potemplate
> + message = self.makeDivergedTranslationMessage(
> + pofile, potmsgset, translator, reviewer,
> + translations, date_created)
> + else:
> + message = potmsgset.setCurrentTranslation(
> + pofile, translator, translations,
> + RosettaTranslationOrigin.ROSETTAWEB,
> + share_with_other_side=current_other)
> + if date_created is not None:
> + removeSecurityProxy(message).date_created = date_created
>
> - if date_created is not None:
> - naked_message.date_created = date_created
> message.markReviewed(reviewer, date_reviewed)
>
> return message

Thanks for improving the use of naked objects through that stretch.

> === modified file 'lib/lp/translations/model/potmsgset.py'
> --- lib/lp/translations/model/potmsgset.py 2010-12-02 16:13:51 +0000
> +++ lib/lp/translations/model/potmsgset.py 2010-12-16 16:04:00 +0000
> @@ -245,10 +245,17 @@
> self._cached_singular_text = self.msgid_singular.msgid
> return self._cached_singular_text
>
> - # Singular text is stored as an "English translation."
> - translation_message = self.getCurrentTranslationMessage(
> - potemplate=None,
> - language=getUtility(ILaunchpadCelebrities).english)
> + # Singular text is stored as an "English translation." Search on
> + # both sides.
> + # XXX henninge 2010-12-14, bug=690196 This may still need some
> + # re-thinking. Maybe even add "getAnyCurrentTranslation".
> + translation_message = self.getCurrentTranslation(
> + None, getUtility(ILaunchpadCelebrities).english,
> + TranslationSide.UBUNTU)
> + if translation_message is None:
> + translation_message = self.getCurrentTranslation(
> + None, getUtility(ILaunchpadCelebrities).english,
> + TranslationSide.UPSTREAM)
> if translation_message is not None:
> msgstr0 = translation_message.msgstr0
> ...

review: Needs Information (code*)

[Henning Eggers (henninge)]

Am 16.12.2010 19:33, schrieb j.c.sackett:
> Review: Needs Information code*
> Henninge--
>
> This is quite a branch.

I know. Sorry about that.

>
> Is it safe to assume this is preceding further work?

Yes, there is more to come. I am sorry I forgot to mention the big picture. We
recently merged our "recife" feature branch which introduces a new model to
share translations between Ubuntu and upstream projects. This is not primarily
a change in the database model (some column and index renaming) but in the way
we use it. The work is mostly done but there is still a lot of cruft flying
around. The work to remove that is ongoing and this branch is part of it.

Thank you very much for ploughing through this branch. Please find my comments
below.

Cheers,
Henning

>> === modified file 'lib/lp/translations/model/potmsgset.py'
>> --- lib/lp/translations/model/potmsgset.py 2010-12-02 16:13:51 +0000
>> +++ lib/lp/translations/model/potmsgset.py 2010-12-16 16:04:00 +0000
>> @@ -245,10 +245,17 @@
>> self._cached_singular_text = self.msgid_singular.msgid
>> return self._cached_singular_text
>>
>> - # Singular text is stored as an "English translation."
>> - translation_message = self.getCurrentTranslationMessage(
>> - potemplate=None,
>> - language=getUtility(ILaunchpadCelebrities).english)
>> + # Singular text is stored as an "English translation." Search on
>> + # both sides.
>> + # XXX henninge 2010-12-14, bug=690196 This may still need some
>> + # re-thinking. Maybe even add "getAnyCurrentTranslation".
>> + translation_message = self.getCurrentTranslation(
>> + None, getUtility(ILaunchpadCelebrities).english,
>> + TranslationSide.UBUNTU)
>> + if translation_message is None:
>> + translation_message = self.getCurrentTranslation(
>> + None, getUtility(ILaunchpadCelebrities).english,
>> + TranslationSide.UPSTREAM)
>> if translation_message is not None:
>> msgstr0 = translation_message.msgstr0
>> if msgstr0 is not None:
>
> I'm not sure I understand what's going on here; could you explain more what
> the possible re-thinking entails?
>
> If I understand correctly, a new model requires looking at both UPSTREAM
> and UBUNTU translations to see if anything exists;

Well, that is what needs to be discussed. ;) Looking on both sides is the safe
approach for now to make sure we find anything. The bug report suggests other
solution, including a model change.

> I'm guessing the new model is the side.py thing? Or is this a new "conceptual model" ?

The TranslationSideTraits in side.py are a central part of the new conceptual
model. Another one is the setCurrentTranslation method which is the code that
is now doing what updateTranslation used to do.

Along with introducing the new model, we did some major restructuring in the
code because the old code (mainly updateTranslation) is too complex (in a bad
way) to try and change it to the new model. We now have more smaller methods
that share the work that updateTranslation used to do.

>
>> @@ -335,6 +343,18...

[j.c.sackett (jcsackett)]

Henning--

Thanks for addressing my comments and questions.

>> I know this method already existed, but as the use of assert statements has
>> become deprecated, it wouldn't hurt to clean this up into a normal,
>> more informative exception.
>
>Hm, I have not heard of that deprecation yet. The style guide points to
>https://dev.launchpad.net/AssertionsInLaunchpad
>which suggests that this is a correct use of an assert statement.

I hadn't seen that page; my understanding was that we were removing them from lp code, but that page makes a strong argument in your favor. I'll drop the issue, thanks for pointing me to those docs.

I look forwards to the future branches that continue cleaning this up, thanks for the start!

review: Approve (code*)

[Curtis Hovey (sinzui)]

I have no remarks.

review: Approve (code)

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-12-17 04:26:11 +0000
+++ lib/lp/testing/factory.py	2010-12-17 10:33:38 +0000
@@ -2656,24 +2656,25 @@
                 potmsgset.singular_text, translations,
                 pofile.language.pluralforms)
 
-        message = potmsgset.setCurrentTranslation(
-            pofile, translator, translations,
-            RosettaTranslationOrigin.ROSETTAWEB,
-            share_with_other_side=current_other)
-        naked_message = removeSecurityProxy(message)
-
         if diverged:
-            naked_message.potemplate = pofile.potemplate
+            message = self.makeDivergedTranslationMessage(
+                pofile, potmsgset, translator, reviewer,
+                translations, date_created)
+        else:
+            message = potmsgset.setCurrentTranslation(
+                pofile, translator, translations,
+                RosettaTranslationOrigin.ROSETTAWEB,
+                share_with_other_side=current_other)
+            if date_created is not None:
+                removeSecurityProxy(message).date_created = date_created
 
-        if date_created is not None:
-            naked_message.date_created = date_created
         message.markReviewed(reviewer, date_reviewed)
 
         return message
 
     def makeDivergedTranslationMessage(self, pofile=None, potmsgset=None,
                                        translator=None, reviewer=None,
-                                       translations=None):
+                                       translations=None, date_created=None):
         """Create a diverged, current `TranslationMessage`."""
         if pofile is None:
             pofile = self.makePOFile('lt')
@@ -2682,7 +2683,7 @@
 
         message = self.makeSuggestion(
             pofile=pofile, potmsgset=potmsgset, translator=translator,
-            translations=translations)
+            translations=translations, date_created=date_created)
         return message.approveAsDiverged(pofile, reviewer)
 
     def makeTranslation(self, pofile, sequence,

=== modified file 'lib/lp/translations/doc/poimport.txt'
--- lib/lp/translations/doc/poimport.txt	2010-12-02 16:13:51 +0000
+++ lib/lp/translations/doc/poimport.txt	2010-12-17 10:33:38 +0000
@@ -318,8 +318,8 @@
 
 And should be accepted by our validator.
 
-    >>> upstream_message = message.potmsgset.getImportedTranslationMessage(
-    ...     pofile.potemplate, pofile.language)
+    >>> upstream_message = message.potmsgset.getOtherTranslation(
+    ...     pofile.language, pofile.potemplate.translation_side)
     >>> upstream_message.validation_status == TranslationValidationStatus.OK
     True
 
@@ -337,8 +337,8 @@
 
     >>> potmsgset = pofile.potemplate.getPOTMsgSetByMsgIDText(
     ...     u'Singular %d', u'Plural %d')
-    >>> imported_translationmessage = potmsgset.getImportedTranslationMessage(
-    ...     pofile.potemplate, pofile.language)
+    >>> imported_translationmessage = potmsgset.getOtherTranslation(
+    ...     pofile.language, pofile.potemplate.translation_side)
     >>> print imported_translationmessage.validation_status.name
     OK
 

=== modified file 'lib/lp/translations/doc/potmsgset.txt'
--- lib/lp/translations/doc/potmsgset.txt	2010-12-02 16:13:51 +0000
+++ lib/lp/translations/doc/potmsgset.txt	2010-12-17 10:33:38 +0000
@@ -245,8 +245,8 @@
     >>> print upstream_potmsgset.getCurrentTranslationMessage(
     ...     evolution_potemplate, catalan)
     None
-    >>> print upstream_potmsgset.getImportedTranslationMessage(
-    ...     evolution_potemplate, catalan)
+    >>> print upstream_potmsgset.getOtherTranslation(
+    ...     catalan, evolution_potemplate.translation_side)
     None
     >>> list(upstream_potmsgset.getLocalTranslationMessages(
     ...          evolution_potemplate, catalan))
@@ -290,8 +290,8 @@
     >>> print upstream_potmsgset.getCurrentTranslationMessage(
     ...     evolution_potemplate, catalan)
     None
-    >>> print upstream_potmsgset.getImportedTranslationMessage(
-    ...     evolution_potemplate, catalan)
+    >>> print upstream_potmsgset.getOtherTranslation(
+    ...     catalan, evolution_potemplate.translation_side)
     None
     >>> list(upstream_potmsgset.getLocalTranslationMessages(
     ...     evolution_potemplate, catalan))
@@ -339,8 +339,8 @@
     ...     evolution_potemplate, catalan)
     >>> print current
     None
-    >>> imported = upstream_potmsgset.getImportedTranslationMessage(
-    ...     evolution_potemplate, catalan)
+    >>> imported = upstream_potmsgset.getOtherTranslation(
+    ...     catalan, evolution_potemplate.translation_side)
     >>> print imported
     None
 
@@ -871,8 +871,8 @@
     ...     pmount_man_template, pofile.language)
     >>> print current
     None
-    >>> imported = potmsgset_untranslated.getImportedTranslationMessage(
-    ...     pmount_man_template, pofile.language)
+    >>> imported = potmsgset_untranslated.getOtherTranslation(
+    ...     pofile.language, pmount_man_template.translation_side)
     >>> print imported
     None
 

=== modified file 'lib/lp/translations/interfaces/potmsgset.py'
--- lib/lp/translations/interfaces/potmsgset.py	2010-12-02 16:13:51 +0000
+++ lib/lp/translations/interfaces/potmsgset.py	2010-12-17 10:33:38 +0000
@@ -157,6 +157,20 @@
     def getSharedTranslationMessage(language):
         """Returns a shared TranslationMessage."""
 
+    def getOtherTranslation(language, side):
+        """Returns the TranslationMessage that is current on the other side.
+
+        :param language: The language in which to find the message.
+        :param side: The side from which this message is seen.
+        """
+
+    def getSharedTranslation(language, side):
+        """Returns a shared TranslationMessage.
+
+        :param language: The language in which to find the message.
+        :param side: The side from which this message is seen.
+        """
+
     def getLocalTranslationMessages(potemplate, language,
                                     include_dismissed=False,
                                     include_unreviewed=True):

=== modified file 'lib/lp/translations/model/potmsgset.py'
--- lib/lp/translations/model/potmsgset.py	2010-12-02 16:13:51 +0000
+++ lib/lp/translations/model/potmsgset.py	2010-12-17 10:33:38 +0000
@@ -245,10 +245,17 @@
             self._cached_singular_text = self.msgid_singular.msgid
             return self._cached_singular_text
 
-        # Singular text is stored as an "English translation."
-        translation_message = self.getCurrentTranslationMessage(
-            potemplate=None,
-            language=getUtility(ILaunchpadCelebrities).english)
+        # Singular text is stored as an "English translation." Search on
+        # both sides.
+        # XXX henninge 2010-12-14, bug=690196 This may still need some
+        # re-thinking. Maybe even add "getAnyCurrentTranslation".
+        translation_message = self.getCurrentTranslation(
+            None, getUtility(ILaunchpadCelebrities).english,
+            TranslationSide.UBUNTU)
+        if translation_message is None:
+            translation_message = self.getCurrentTranslation(
+                None, getUtility(ILaunchpadCelebrities).english,
+                TranslationSide.UPSTREAM)
         if translation_message is not None:
             msgstr0 = translation_message.msgstr0
             if msgstr0 is not None:
@@ -335,6 +342,18 @@
         return self._getUsedTranslationMessage(
             None, language, current=True)
 
+    def getOtherTranslation(self, language, side):
+        """See `IPOTMsgSet`."""
+        traits = getUtility(
+            ITranslationSideTraitsSet).getTraits(side)
+        return self.getCurrentTranslation(
+            None, language, traits.other_side_traits.side)
+
+    def getSharedTranslation(self, language, side):
+        """See `IPOTMsgSet`."""
+        return self.getCurrentTranslation(
+            None, language, side)
+
     def getCurrentTranslation(self, potemplate, language, side):
         """See `IPOTMsgSet`."""
         assert side is not None, "Translation side must be specified."
@@ -487,12 +506,12 @@
 
     def hasTranslationChangedInLaunchpad(self, potemplate, language):
         """See `IPOTMsgSet`."""
-        imported_translation = self.getImportedTranslationMessage(
-            potemplate, language)
-        current_translation = self.getCurrentTranslationMessage(
-            potemplate, language)
-        return (imported_translation is not None and
-                imported_translation != current_translation)
+        other_translation = self.getOtherTranslation(
+            language, potemplate.translation_side)
+        current_translation = self.getCurrentTranslation(
+            potemplate, language, potemplate.translation_side)
+        return (other_translation is not None and
+                other_translation != current_translation)
 
     def isTranslationNewerThan(self, pofile, timestamp):
         """See `IPOTMsgSet`."""
@@ -1239,6 +1258,8 @@
                         lock_timestamp=None):
         """Set the current translation.
 
+        https://dev.launchpad.net/Translations/Specs/setCurrentTranslation
+
         :param pofile: The `POFile` to set the translation in.
         :param submitter: The `Person` who produced this translation.
         :param origin: The translation's `RosettaTranslationOrigin`.
@@ -1373,6 +1394,7 @@
                         traits.other_side_traits.setFlag(message, True)
             elif character == '+':
                 if share_with_other_side:
+                    traits.other_side_traits.setFlag(incumbent_message, False)
                     traits.other_side_traits.setFlag(message, True)
             else:
                 raise AssertionError(
@@ -1603,8 +1625,8 @@
         if not self.is_translation_credit:
             return
 
-        shared_upstream_translation = self.getCurrentTranslation(
-            None, pofile.language, TranslationSide.UPSTREAM)
+        shared_upstream_translation = self.getSharedTranslation(
+            pofile.language, TranslationSide.UPSTREAM)
 
         if shared_upstream_translation is not None:
             return

=== modified file 'lib/lp/translations/model/side.py'
--- lib/lp/translations/model/side.py	2010-12-02 11:01:43 +0000
+++ lib/lp/translations/model/side.py	2010-12-17 10:33:38 +0000
@@ -10,6 +10,7 @@
     ]
 
 from zope.interface import implements
+from zope.security.proxy import removeSecurityProxy
 
 from lp.translations.interfaces.side import (
     ITranslationSideTraits,
@@ -39,10 +40,8 @@
 
     def setFlag(self, translationmessage, value):
         """See `ITranslationSideTraits`."""
-        if self.side == TranslationSide.UPSTREAM:
-            translationmessage.makeCurrentUpstream(value)
-        else:
-            translationmessage.makeCurrentUbuntu(value)
+        naked_tm = removeSecurityProxy(translationmessage)
+        setattr(naked_tm, self.flag_name, value)
 
 
 class TranslationSideTraitsSet:

=== modified file 'lib/lp/translations/model/translatablemessage.py'
--- lib/lp/translations/model/translatablemessage.py	2010-12-02 16:13:51 +0000
+++ lib/lp/translations/model/translatablemessage.py	2010-12-17 10:33:38 +0000
@@ -92,12 +92,13 @@
 
     def getImportedTranslation(self):
         """See `ITranslatableMessage`"""
-        return self.potmsgset.getImportedTranslationMessage(self.potemplate,
-                                                            self.language)
+        return self.potmsgset.getOtherTranslation(
+            self.language, self.potemplate.translation_side)
 
     def getSharedTranslation(self):
         """See `ITranslatableMessage`"""
-        return self.potmsgset.getSharedTranslationMessage(self.language)
+        return self.potmsgset.getSharedTranslation(
+            self.language, self.potemplate.translation_side)
 
     def getAllSuggestions(self):
         """See `ITranslatableMessage`"""

=== modified file 'lib/lp/translations/model/translationmessage.py'
--- lib/lp/translations/model/translationmessage.py	2010-12-02 16:13:51 +0000
+++ lib/lp/translations/model/translationmessage.py	2010-12-17 10:33:38 +0000
@@ -398,6 +398,9 @@
 
     def shareIfPossible(self):
         """See `ITranslationMessage`."""
+        # XXX henninge 2010-12-14 bug=690254: This method still needs some
+        # thought on how it relates to the new model.
+
         if self.potemplate is None:
             # Already converged.
             return

=== modified file 'lib/lp/translations/scripts/message_sharing_migration.py'
--- lib/lp/translations/scripts/message_sharing_migration.py	2010-12-02 16:13:51 +0000
+++ lib/lp/translations/scripts/message_sharing_migration.py	2010-12-17 10:33:38 +0000
@@ -3,7 +3,7 @@
 
 __metaclass__ = type
 __all__ = [
-    'MessageSharingMerge'
+    'MessageSharingMerge',
     ]
 
 
@@ -78,7 +78,7 @@
 
     This adds all of the subordinate's TranslationTemplateItems to the
     representative's set of TranslationTemplateItems.
-    
+
     Any duplicates are deleted, so after this, the subordinate will no
     longer have any TranslationTemplateItems.
     """
@@ -127,8 +127,8 @@
 
     :param message: a `TranslationMessage` to drop flags on.
     :param incumbents: a sequence of reference messages.  If any of
-        these has either is_current_ubuntu or is_current_upstream set, that same
-        flag will be dropped on message (if set).
+        these has either is_current_ubuntu or is_current_upstream set, that
+        same flag will be dropped on message (if set).
     """
     if incumbents:
         for incumbent in incumbents:
@@ -458,8 +458,7 @@
         """Get list of ids for `template`'s `POTMsgSet`s."""
         return [
             potmsgset.id
-            for potmsgset in template.getPOTMsgSets(False, prefetch=False)
-            ]
+            for potmsgset in template.getPOTMsgSets(False, prefetch=False)]
 
     def _mergeTranslationMessages(self, potemplates):
         """Share `TranslationMessage`s between templates where possible."""
@@ -505,8 +504,7 @@
         tm = removeSecurityProxy(tm)
         msgstr_ids = tuple([
             getattr(tm, 'msgstr%dID' % form)
-            for form in xrange(TranslationConstants.MAX_PLURAL_FORMS)
-            ])
+            for form in xrange(TranslationConstants.MAX_PLURAL_FORMS)])
 
         return (tm.potemplateID, tm.languageID) + msgstr_ids
 

=== modified file 'lib/lp/translations/scripts/tests/test_message_sharing_migration.py'
--- lib/lp/translations/scripts/tests/test_message_sharing_migration.py	2010-12-02 16:13:51 +0000
+++ lib/lp/translations/scripts/tests/test_message_sharing_migration.py	2010-12-17 10:33:38 +0000
@@ -212,8 +212,8 @@
             potemplate=template, language=self.dutch)
         if not message:
             # No diverged message here, so check for a shared one.
-            message = potmsgset.getSharedTranslationMessage(
-                language=self.dutch)
+            message = potmsgset.getSharedTranslation(
+                language=self.dutch, side=template.translation_side)
         return message
 
     def _getMessages(self):
@@ -594,7 +594,7 @@
 
         # The duplicates have been cleaned up.
         self.assertEqual(potmsgset.getAllTranslationMessages().count(), 1)
-        
+
         # The is_current_ubuntu and is_current_upstream flags from the
         # duplicate messages have been merged into a single message,
         # current in both ubuntu and upstream.

=== modified file 'lib/lp/translations/tests/helpers.py'
--- lib/lp/translations/tests/helpers.py	2010-11-17 12:30:06 +0000
+++ lib/lp/translations/tests/helpers.py	2010-12-17 10:33:38 +0000
@@ -7,10 +7,7 @@
     'make_translationmessage_for_context',
     ]
 
-from storm.expr import (
-    Coalesce,
-    Or,
-    )
+from storm.expr import Or
 from storm.store import Store
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
@@ -88,11 +85,10 @@
     return new_message
 
 
-def get_all_translations_current_anywhere(pofile, potmsgset):
-    """Get current `TranslationMessage`s for this `POTMsgSet` and language.
+def get_all_translations_diverged_anywhere(pofile, potmsgset):
+    """Get diverged `TranslationMessage`s for this `POTMsgSet` and language.
 
-    The `TranslationMessage`s can be in use anywhere; the `pofile` only
-    selects the language.
+    Leave out translations diverged to pofile.potemplate.
     """
     result = Store.of(potmsgset).find(
         TranslationMessage,
@@ -101,8 +97,9 @@
             TranslationMessage.is_current_ubuntu == True,
             TranslationMessage.is_current_upstream == True),
         TranslationMessage.potemplate != None,
+        TranslationMessage.potemplate != pofile.potemplate,
         TranslationMessage.language == pofile.language)
-    return result.order_by(-Coalesce(TranslationMessage.potemplateID, -1))
+    return result.order_by(-TranslationMessage.potemplateID)
 
 
 def summarize_current_translations(pofile, potmsgset):
@@ -135,16 +132,7 @@
         "There is a diverged 'other' translation for "
         "this same template, which should be impossible.")
 
-    all_used = get_all_translations_current_anywhere(
-        pofile, potmsgset)
-    diverged = []
-    for suggestion in all_used:
-        if ((suggestion.potemplate is not None and
-             suggestion.potemplate != template) and
-            (suggestion.is_current_ubuntu or
-             suggestion.is_current_upstream)):
-            # It's diverged for another template and current somewhere.
-            diverged.append(suggestion)
+    diverged = list(get_all_translations_diverged_anywhere(pofile, potmsgset))
     return (
         current_shared, current_diverged,
         other_shared, diverged)

=== modified file 'lib/lp/translations/tests/potmsgset-update-translation.txt'
--- lib/lp/translations/tests/potmsgset-update-translation.txt	2010-12-02 16:13:51 +0000
+++ lib/lp/translations/tests/potmsgset-update-translation.txt	2010-12-17 10:33:38 +0000
@@ -53,16 +53,16 @@
 We change the upstream one, and thus, the one in Launchpad will be changed
 too at the same time.
 
-  >>> message = potmsgset.updateTranslation(
-  ...     pofile_es, carlos, {0: u'foo '}, is_current_upstream=True,
-  ...     lock_timestamp=datetime.datetime.now(UTC))
+    >>> message = potmsgset.updateTranslation(
+    ...     pofile_es, carlos, {0: u'foo '}, is_current_upstream=True,
+    ...     lock_timestamp=datetime.datetime.now(UTC))
 
-  >>> message.msgstr0.translation
-  u'foo '
-  >>> message.is_complete
-  True
-  >>> message.is_current_upstream
-  True
+    >>> message.msgstr0.translation
+    u'foo '
+    >>> message.is_complete
+    True
+    >>> message.is_current_upstream
+    True
 
 Even more, if there is no translation at all in Launchpad, we still get
 the update from upstream.
@@ -131,7 +131,8 @@
     >>> message.is_complete
     True
 
-    >>> message = potmsgset.getImportedTranslationMessage(upstream_evo, spanish)
+    >>> message = potmsgset.getImportedTranslationMessage(
+    ...     upstream_evo, spanish)
     >>> message.translations
     [u'bars ']
 

=== modified file 'lib/lp/translations/tests/test_potmsgset.py'
--- lib/lp/translations/tests/test_potmsgset.py	2010-12-10 13:44:57 +0000
+++ lib/lp/translations/tests/test_potmsgset.py	2010-12-17 10:33:38 +0000
@@ -246,55 +246,57 @@
         self.assertEquals(self.potmsgset.getCurrentTranslationMessage(
             self.stable_potemplate, serbian), shared_translation)
 
-
-    # XXX henninge 2010-12-10 bug=688519: The meaning of  imported is gone
-    # and so this test and the method it is testing need to be updated.
-    def test_getImportedTranslationMessage(self):
-        """Test how shared and diverged current translation messages
-        interact."""
-        # Share a POTMsgSet in two templates, and get a Serbian POFile.
-        self.potmsgset.setSequence(self.stable_potemplate, 1)
-        sr_pofile = self.factory.makePOFile('sr', self.devel_potemplate)
-        serbian = sr_pofile.language
-
-        # A shared translation is imported in both templates.
-        shared_translation = self.factory.makeSharedTranslationMessage(
-            pofile=sr_pofile, potmsgset=self.potmsgset,
-            is_current_upstream=True)
-        self.assertEquals(self.potmsgset.getImportedTranslationMessage(
-            self.devel_potemplate, serbian), shared_translation)
-        self.assertEquals(self.potmsgset.getImportedTranslationMessage(
-            self.stable_potemplate, serbian), shared_translation)
-
-        # Adding a diverged translation in one template makes that one
-        # an imported translation there.
-        diverged_translation = self.factory.makeTranslationMessage(
-            pofile=sr_pofile, potmsgset=self.potmsgset,
-            is_current_upstream=True, force_diverged=True)
-        self.assertEquals(self.potmsgset.getImportedTranslationMessage(
-            self.devel_potemplate, serbian), diverged_translation)
-        self.assertEquals(self.potmsgset.getImportedTranslationMessage(
-            self.stable_potemplate, serbian), shared_translation)
-
-    def test_getSharedTranslationMessage(self):
-        """Test how shared and diverged current translation messages
-        interact."""
+    def test_getOtherTranslation(self):
+        # Get the translation on the other side.
+        # Share POTMsgSet in a template on the other side.
+        distroseries = self.factory.makeUbuntuDistroSeries()
+        sourcepackagename = self.factory.makeSourcePackageName()
+        ubuntu_potemplate = self.factory.makePOTemplate(
+            distroseries=distroseries, sourcepackagename=sourcepackagename)
+        self.potmsgset.setSequence(ubuntu_potemplate, 1)
+        # Get POFiles on both sides.
+        pofile = self.factory.makePOFile(potemplate=self.devel_potemplate)
+        ubuntu_pofile = self.factory.makePOFile(
+            potemplate=ubuntu_potemplate, language=pofile.language)
+
+        # A shared translation is current on both sides.
+        shared_translation = self.factory.makeCurrentTranslationMessage(
+            pofile=pofile, potmsgset=self.potmsgset, current_other=True)
+        self.assertEquals(self.potmsgset.getCurrentTranslationMessage(
+            ubuntu_potemplate, ubuntu_pofile.language), shared_translation)
+        self.assertEquals(self.potmsgset.getOtherTranslation(
+            pofile.language, self.devel_potemplate.translation_side),
+            shared_translation)
+
+        # A diverted translation on the other side is not returned.
+        diverged_translation = self.factory.makeCurrentTranslationMessage(
+            pofile=ubuntu_pofile, potmsgset=self.potmsgset, diverged=True)
+        self.assertEquals(self.potmsgset.getCurrentTranslationMessage(
+            ubuntu_potemplate, ubuntu_pofile.language), diverged_translation)
+        self.assertEquals(self.potmsgset.getOtherTranslation(
+            pofile.language, self.devel_potemplate.translation_side),
+            shared_translation)
+
+    def test_getSharedTranslation(self):
+        # Get the current shared translation, not  the diverged one.
         # Share a POTMsgSet in two templates, and get a Serbian POFile.
         sr_pofile = self.factory.makePOFile('sr', self.devel_potemplate)
         serbian = sr_pofile.language
 
         # A shared translation matches the current one.
-        shared_translation = self.factory.makeSharedTranslationMessage(
+        shared_translation = self.factory.makeCurrentTranslationMessage(
             pofile=sr_pofile, potmsgset=self.potmsgset)
         self.assertEquals(
-            self.potmsgset.getSharedTranslationMessage(serbian),
+            self.potmsgset.getSharedTranslation(
+                serbian, self.stable_potemplate.translation_side),
             shared_translation)
 
         # Adding a diverged translation doesn't break getSharedTM.
         diverged_translation = self.factory.makeCurrentTranslationMessage(
             pofile=sr_pofile, potmsgset=self.potmsgset, diverged=True)
         self.assertEquals(
-            self.potmsgset.getSharedTranslationMessage(serbian),
+            self.potmsgset.getSharedTranslation(
+                serbian, self.stable_potemplate.translation_side),
             shared_translation)
 
     def test_getLocalTranslationMessages(self):
@@ -489,9 +491,8 @@
 
         # If only a current, non-imported translation exists, it's not
         # changed in Ubuntu.
-        current_shared = self.factory.makeSharedTranslationMessage(
-            pofile=sr_pofile, potmsgset=self.potmsgset,
-            is_current_upstream=False)
+        current_shared = self.factory.makeCurrentTranslationMessage(
+            pofile=sr_pofile, potmsgset=self.potmsgset)
         self.assertEquals(
             self.potmsgset.hasTranslationChangedInLaunchpad(
                 self.devel_potemplate, serbian),
@@ -499,10 +500,9 @@
 
         # If the current upstream translation is also current in Ubuntu,
         # it's not changed in Ubuntu.
-        current_shared.makeCurrentUbuntu(False)
-        imported_shared = self.factory.makeSharedTranslationMessage(
+        imported_shared = self.factory.makeCurrentTranslationMessage(
             pofile=sr_pofile, potmsgset=self.potmsgset,
-            is_current_upstream=True)
+            current_other=True)
         self.assertEquals(
             self.potmsgset.hasTranslationChangedInLaunchpad(
                 self.devel_potemplate, serbian),
@@ -510,18 +510,19 @@
 
         # If there's a current, diverged translation, and an imported
         # non-current one, it's changed in Ubuntu.
-        imported_shared.makeCurrentUbuntu(False)
         current_diverged = self.factory.makeCurrentTranslationMessage(
-            pofile=sr_pofile, potmsgset=self.potmsgset,
-            current_other=False)
+            pofile=sr_pofile, potmsgset=self.potmsgset, diverged=True)
         self.assertEquals(
             self.potmsgset.hasTranslationChangedInLaunchpad(
                 self.devel_potemplate, serbian),
             True)
 
-        # 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.makeCurrentUbuntu(False)
+        # If there is a different non-diverged translation,
+        # it's changed in Ubuntu.
+        current = self.factory.makeCurrentTranslationMessage(
+            pofile=sr_pofile, potmsgset=self.potmsgset)
+        self.assertTrue(current.is_current_upstream)
+        self.assertTrue(imported_shared.is_current_ubuntu)
         self.assertEquals(
             self.potmsgset.hasTranslationChangedInLaunchpad(
                 self.devel_potemplate, serbian),
@@ -747,23 +748,26 @@
         sr_pofile = self.factory.makePOFile('sr', self.devel_potemplate)
         credits_potmsgset = self.factory.makePOTMsgSet(
             self.devel_potemplate, singular=u'translator-credits')
-        diverged_credits = credits_potmsgset.setCurrentTranslation(
-            sr_pofile, sr_pofile.potemplate.owner, {0: 'credits'},
-            RosettaTranslationOrigin.SCM, share_with_other_side=True)
+        diverged_credits = self.factory.makeCurrentTranslationMessage(
+            sr_pofile, credits_potmsgset)
         # Since translation credits are special, we can't easily create
         # a diverged translation on it, though it may already exist in
         # the DB.
         removeSecurityProxy(diverged_credits).potemplate = (
             sr_pofile.potemplate)
         # Make sure that worked (not a real test).
-        self.assertTrue(diverged_credits.is_current_upstream)
-        self.assertEquals(sr_pofile.potemplate, diverged_credits.potemplate)
+        test_diverged_credits = credits_potmsgset.getCurrentTranslation(
+            sr_pofile.potemplate, sr_pofile.language,
+            sr_pofile.potemplate.translation_side)
+        self.assertTrue(test_diverged_credits.is_current_upstream)
+        self.assertEquals(
+            sr_pofile.potemplate, test_diverged_credits.potemplate)
 
         credits_potmsgset.setTranslationCreditsToTranslated(sr_pofile)
 
         # Shared translation is generated.
-        shared = credits_potmsgset.getCurrentTranslation(
-            None, sr_pofile.language, TranslationSide.UPSTREAM)
+        shared = credits_potmsgset.getSharedTranslation(
+            sr_pofile.language, sr_pofile.potemplate.translation_side)
         self.assertNotEquals(diverged_credits, shared)
         self.assertIsNot(None, shared)
 
@@ -1437,8 +1441,9 @@
             eo_pofile, credits, translations=[imported_credits],
             current_other=True)
 
-        eo_translation = credits.getCurrentTranslationMessage(
-            self.potemplate, eo_pofile.language)
+        eo_translation = credits.getCurrentTranslation(
+            self.potemplate, eo_pofile.language,
+            self.potemplate.translation_side)
         self.assertEqual(imported_credits, eo_translation.msgstr0.translation,
             "Imported translation credits do not replace dummy credits.")
 
@@ -1712,7 +1717,7 @@
             for form in xrange(forms))
 
     def test_baseline(self):
-        # setCurrentTranslation sets the current upstream translation
+        # setCurrentTranslation sets the current translation
         # for a message.
         pofile, potmsgset = self._makePOFileAndPOTMsgSet()
         translations = self._makeTranslations(potmsgset)
@@ -1721,8 +1726,9 @@
         message = potmsgset.setCurrentTranslation(
             pofile, pofile.potemplate.owner, translations, origin)
 
-        self.assertEqual(message, potmsgset.getImportedTranslationMessage(
-            pofile.potemplate, pofile.language))
+        self.assertEqual(message, potmsgset.getCurrentTranslation(
+            pofile.potemplate, pofile.language,
+            pofile.potemplate.translation_side))
         self.assertEqual(origin, message.origin)
 
     def test_identical(self):
@@ -1742,18 +1748,18 @@
         self.assertEqual(pofile.potemplate.owner, message.submitter)
         self.assertEqual(RosettaTranslationOrigin.ROSETTAWEB, message.origin)
 
-    def test_upstream_also_sets_initial_ubuntu(self):
-        # Setting an upstream translation also initializes the Ubuntu
-        # translation.
+    def test_share_with_other_side(self):
+        # If requested, the translation can also be set on the other side.
         pofile, potmsgset = self._makePOFileAndPOTMsgSet()
         translations = self._makeTranslations(potmsgset)
         origin = RosettaTranslationOrigin.ROSETTAWEB
 
         message = potmsgset.setCurrentTranslation(
-            pofile, pofile.potemplate.owner, translations, origin)
+            pofile, pofile.potemplate.owner, translations, origin,
+            share_with_other_side=True)
 
-        self.assertEqual(message, potmsgset.getImportedTranslationMessage(
-            pofile.potemplate, pofile.language))
+        self.assertEqual(message, potmsgset.getOtherTranslation(
+            pofile.language, pofile.potemplate.translation_side))
 
     def test_detects_conflict(self):
         pofile, potmsgset = self._makePOFileAndPOTMsgSet()

=== modified file 'lib/lp/translations/tests/test_suggestions.py'
--- lib/lp/translations/tests/test_suggestions.py	2010-12-02 16:13:51 +0000
+++ lib/lp/translations/tests/test_suggestions.py	2010-12-17 10:33:38 +0000
@@ -9,7 +9,6 @@
     )
 import unittest
 
-import gettextpo
 from pytz import timezone
 import transaction
 from zope.component import getUtility
@@ -197,8 +196,8 @@
             is_current_upstream=True, lock_timestamp=None)
         current_translation = potmsgset.getCurrentTranslationMessage(
             self.foo_template, self.foo_nl.language)
-        imported_translation = potmsgset.getImportedTranslationMessage(
-            self.foo_template, self.foo_nl.language)
+        imported_translation = potmsgset.getOtherTranslation(
+            self.foo_nl.language, self.foo_template.translation_side)
 
         self.assertEquals(
             current_translation, imported_translation,

=== modified file 'lib/lp/translations/tests/test_translatablemessage.py'
--- lib/lp/translations/tests/test_translatablemessage.py	2010-12-10 11:44:07 +0000
+++ lib/lp/translations/tests/test_translatablemessage.py	2010-12-17 10:33:38 +0000
@@ -13,7 +13,6 @@
 import pytz
 import transaction
 from zope.component import getUtility
-from zope.security.proxy import removeSecurityProxy
 
 from canonical.testing.layers import ZopelessDatabaseLayer
 from lp.app.enums import ServiceUsage
@@ -55,11 +54,9 @@
                 pofile=self.pofile, potmsgset=self.potmsgset,
                 translations=translation,
                 current_other=is_current_ubuntu,
-                diverged=is_diverged)
-            if date_updated is not None:
-                naked_message = removeSecurityProxy(message)
-                naked_message.date_created = date_updated
-                naked_message.date_reviewed = date_updated
+                diverged=is_diverged,
+                date_created=date_updated,
+                date_reviewed=date_updated)
         else:
             message = self.factory.makeSuggestion(
                 pofile=self.pofile, potmsgset=self.potmsgset,
@@ -133,14 +130,14 @@
         self.assertEqual(translation, current)
 
     def test_getImportedTranslation(self):
-        translation = self._createTranslation(is_current_upstream=True)
+        translation = self._createTranslation(is_current_ubuntu=True)
 
         message = TranslatableMessage(self.potmsgset, self.pofile)
         imported = message.getImportedTranslation()
         self.assertEqual(translation, imported)
 
     def test_getSharedTranslation(self):
-        translation = self._createTranslation(is_current_ubuntu=True)
+        translation = self._createTranslation(is_current_upstream=True)
         message = TranslatableMessage(self.potmsgset, self.pofile)
         shared = message.getSharedTranslation()
         self.assertEqual(translation, shared)

=== modified file 'lib/lp/translations/utilities/tests/import-flags.txt'
--- lib/lp/translations/utilities/tests/import-flags.txt	2010-12-02 16:13:51 +0000
+++ lib/lp/translations/utilities/tests/import-flags.txt	2010-12-17 10:33:38 +0000
@@ -49,8 +49,8 @@
     ...         msgid = potmsgset.singular_text
     ...         current = potmsgset.getCurrentTranslationMessage(
     ...             pofile.potemplate, pofile.language)
-    ...         imported = potmsgset.getImportedTranslationMessage(
-    ...             pofile.potemplate, pofile.language)
+    ...         imported = potmsgset.getOtherTranslation(
+    ...             pofile.language, pofile.potemplate.translation_side)
     ...         imported_translation = translation = None
     ...         if imported is not None:
     ...             imported_translation = imported.msgstr0.translation

=== modified file 'lib/lp/translations/utilities/tests/test_xpi_import.py'
--- lib/lp/translations/utilities/tests/test_xpi_import.py	2010-12-02 16:13:51 +0000
+++ lib/lp/translations/utilities/tests/test_xpi_import.py	2010-12-17 10:33:38 +0000
@@ -257,9 +257,9 @@
         # With this first import, upstream and Ubuntu translations must match.
         self.assertEquals(
             translation.translations,
-            potmsgset.getImportedTranslationMessage(
-                self.firefox_template,
-                self.spanish_firefox.language).translations)
+            potmsgset.getOtherTranslation(
+                self.spanish_firefox.language,
+                self.firefox_template.translation_side).translations)
 
         potmsgset = self.firefox_template.getPOTMsgSetByMsgIDText(
             u'foozilla.menu.accesskey', context='main/subdir/test2.dtd')
@@ -274,9 +274,9 @@
             unwrap(access_key_source_comment))
         # But for the translation import, we get the key directly.
         self.assertEquals(
-            potmsgset.getImportedTranslationMessage(
-                self.firefox_template,
-                self.spanish_firefox.language).translations,
+            potmsgset.getOtherTranslation(
+                self.spanish_firefox.language,
+                self.firefox_template.translation_side).translations,
             [u'M'])
 
         potmsgset = self.firefox_template.getPOTMsgSetByMsgIDText(
@@ -292,9 +292,9 @@
             unwrap(command_key_source_comment))
         # But for the translation import, we get the key directly.
         self.assertEquals(
-            potmsgset.getImportedTranslationMessage(
-                self.firefox_template,
-                self.spanish_firefox.language).translations,
+            potmsgset.getOtherTranslation(
+                self.spanish_firefox.language,
+                self.firefox_template.translation_side).translations,
             [u'm'])
 
     def test_GetLastTranslator(self):