Merge lp:~adeuring/launchpad/bug-702468 into lp:launchpad

[Abel Deuring]

[r=henninge,jcsackett,sinzui][bug=702468] If a yet untranslated upstream message is translated for the first time, and if upstream -> Ubuntu translation sharing is enabled, the new translation becomes current for Ubuntu too, even if there was already a current Ubuntu translation.

This branch fixes bug 702468: First upstream translation does not replace Ubuntu-only translation.

The flags TranslationMessage.is_current_(upstream|ubuntu) are set in
POTMsgSet._setTranslation().

The bug describes the situation that we have no incumbent message
and the "twin" message (i.e. the new message) is either shared
or diverged but not shared with the other side. The decision_matrix
entries for these cases are
decision_matrix['incumbent_none']['twin_shared'] and
decision_matrix['incumbent_none']['twin_diverged'], respectively.

The treatment of the translation on the "other side" for these cases
is described by the "decision character" '*', so this branch changes
the "elif character == '*':" part of the the method _setTranslation().

The "decision character" '*' is also used in
decision_matrix['incumbent_shared'], but the "elif incumbent_message
is None:" in my patch ensures that the method behaves as before in
this case.

The existing tests in test_setcurrenttranslation.py covered the
case "existing translation in Ubuntu, no translation in upstream"
already, so I only changed the expected data for the two "relevant"
tests.

test: ./bin/test -vvt lp.translations.tests.test_setcurrenttranslation

no lint

[j.c.sackett (jcsackett)]

This looks good to land.

review: Approve (code*)

[Curtis Hovey (sinzui)]

Thanks.

review: Approve (code)

[Abel Deuring (adeuring)]

Henning,

As discussed last week, I've disabled the flow "new current translation without a previous current translation becomes the current translation on the other side, if the other side already has a current translation" for the direction "new current Ubuntu translation overrides current upstream translation".

Looking through the existing tests, I think that they cover all situations, so I did not add any.

review: Needs Resubmitting

[Henning Eggers (henninge)]

Abel,
thank you for this update! It looks very well done to me. I have just one remaining request.

Please add a stub for selectUpstreamTranslation to SetCurrentTranslationTestMixin with a docstring and a "raise NotImplementedError". That makes it clearer where that is coming from. Also, you should either fix the parameter documentation in selectUpstreamTranslation ("tm" twice) or remove it completely (because it is pretty obvious ;).

Cheers,
Henning

review: Approve (code)

=== modified file 'lib/lp/translations/model/potmsgset.py'
--- lib/lp/translations/model/potmsgset.py	2011-02-04 17:14:20 +0000
+++ lib/lp/translations/model/potmsgset.py	2011-02-15 11:46:10 +0000
@@ -1028,6 +1028,16 @@
                             self, pofile.potemplate, pofile.language))
                     if other_incumbent is None:
                         traits.other_side_traits.setFlag(message, True)
+                    elif (incumbent_message is None and
+                          traits.side == TranslationSide.UPSTREAM):
+                        # If this is the first upstream translation, we
+                        # we use it as the current Ubuntu translation
+                        # too, overriding a possibly existing current
+                        # Ubuntu translation.
+                        if other_incumbent is not None:
+                            traits.other_side_traits.setFlag(
+                                other_incumbent, False)
+                        traits.other_side_traits.setFlag(message, True)
             elif character == '+':
                 if share_with_other_side:
                     traits.other_side_traits.setFlag(incumbent_message, False)

=== modified file 'lib/lp/translations/tests/test_setcurrenttranslation.py'
--- lib/lp/translations/tests/test_setcurrenttranslation.py	2010-11-18 04:08:16 +0000
+++ lib/lp/translations/tests/test_setcurrenttranslation.py	2011-02-15 11:46:10 +0000
@@ -161,6 +161,12 @@
         self.assert_Current_Diverged_Other_DivergencesElsewhere_are(
             tm, None, tm, [])
 
+    def selectUpstreamTranslation(self, tm, tm_other):
+        # Return the upstream translation.
+        # :param tm: A translation for this side.
+        # :param tm_other: A translation for the other side.
+        raise NotImplementedError
+
     def test_c_None__n_None__o_shared(self, follows=False):
         # Current translation is None, and we have found no
         # existing TM matching new translations.
@@ -175,10 +181,17 @@
             new_translations, share_with_other_side=follows)
 
         # We end up with a shared current translation.
-        # Current for other context one stays the same.
+        # Current for other context one stays the same, if the
+        # other side does not follow this side.
         self.assertTrue(tm is not None)
+        if follows:
+            # Even if the other side is supposed to follow this side,
+            # we ovverride the other only if the current side is Ubuntu.
+            expected_other = self.selectUpstreamTranslation(tm, tm_other)
+        else:
+            expected_other = tm_other
         self.assert_Current_Diverged_Other_DivergencesElsewhere_are(
-            tm, None, tm_other, [])
+            tm, None, expected_other, [])
 
     def test_c_None__n_None__o_shared__follows(self):
         # There is no current translation, though there is a shared one
@@ -318,14 +331,17 @@
             new_translations, share_with_other_side=True)
 
         # tm_suggestion becomes current.
-        # Current for other context one stays the same.
         self.assertTrue(tm is not None)
         self.assertEquals(tm_suggestion, tm)
+        # If a translation is set for the first time in upstream,
+        # this translation becomes current in Ubuntu too, but if the
+        # translation is set for the first time in Ubuntu, this does
+        # not affect the upstream translation.
+        expected_other = self.selectUpstreamTranslation(tm, tm_other)
         self.assert_Current_Diverged_Other_DivergencesElsewhere_are(
-            tm, None, tm_other, [])
+            tm, None, expected_other, [])
 
-    def test_c_None__n_shared__o_shared__identical(self,
-                                                               follows=False):
+    def test_c_None__n_shared__o_shared__identical(self, follows=False):
         # Current translation is None, and we have found a
         # shared existing TM matching new translations and it's
         # also a current translation in "other" context.
@@ -1147,6 +1163,10 @@
         self.potmsgset = self.factory.makePOTMsgSet(
             potemplate=potemplate, sequence=1)
 
+    def selectUpstreamTranslation(self, tm, tm_other):
+        # See `SetCurrentTranslationTestMixin`
+        return tm_other
+
 
 class TestSetCurrentTranslation_Upstream(SetCurrentTranslationTestMixin,
                                          TestCaseWithFactory):
@@ -1182,3 +1202,7 @@
 
         self.potmsgset = self.factory.makePOTMsgSet(
             potemplate=potemplate, sequence=1)
+
+    def selectUpstreamTranslation(self, tm, tm_other):
+        # See `SetCurrentTranslationTestMixin`
+        return tm