launchpad-reviewers team mailing list archive

[Данило Шеган <danilo@xxxxxxxxxxxxx>]

Данило Шеган has proposed merging lp:~danilo/launchpad/bug-690254 into lp:launchpad/db-devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #690254 Update or remove TranslationMessage.shareIfPossible
  https://bugs.launchpad.net/bugs/690254

For more details, see:
https://code.launchpad.net/~danilo/launchpad/bug-690254/+merge/44960

= Bug 690254 =

TranslationMessage.shareIfPossible still uses the old model code (getCurrentTranslationMessage, getI$

== Implementation details ==

While it seems as if shareIfPossible is still in the old model, it is not.  It is just using old mod$
So, nothing is really changing except the way we get the data.

== Tests ==

Unfortunately, shareIfPossible is unit-tested only indirectly through test_message_sharing_migration$

  bin/test -cvvt test_message_sharing_migration

== Demo and Q/A ==

Part of general "translations sharing" feature QA.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/model/translationmessage.py
-- 
https://code.launchpad.net/~danilo/launchpad/bug-690254/+merge/44960
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/launchpad/bug-690254 into lp:launchpad/db-devel.

=== modified file 'lib/lp/translations/model/translationmessage.py'
--- lib/lp/translations/model/translationmessage.py	2010-12-17 10:30:55 +0000
+++ lib/lp/translations/model/translationmessage.py	2010-12-31 14:43:34 +0000
@@ -38,6 +38,7 @@
 from canonical.launchpad.interfaces.lpstorm import IStore
 from lp.registry.interfaces.person import validate_public_person
 from lp.services.propertycache import cachedproperty
+from lp.translations.interfaces.side import TranslationSide
 from lp.translations.interfaces.translationmessage import (
     ITranslationMessage,
     ITranslationMessageSet,
@@ -398,9 +399,6 @@
 
     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
@@ -408,22 +406,24 @@
         # Existing shared direct equivalent to this message, if any.
         shared = self.getSharedEquivalent()
 
-        # Existing shared current translation for this POTMsgSet, if
+        # Existing shared ubuntu translation for this POTMsgSet, if
         # any.
-        current = self.potmsgset.getCurrentTranslationMessage(
-            potemplate=None, language=self.language)
+        ubuntu = self.potmsgset.getCurrentTranslation(
+            potemplate=None, language=self.language,
+            side=TranslationSide.UBUNTU)
 
         # Existing shared upstream translation for this POTMsgSet, if
         # any.
-        upstream = self.potmsgset.getImportedTranslationMessage(
-            potemplate=None, language=self.language)
+        upstream = self.potmsgset.getCurrentTranslation(
+            potemplate=None, language=self.language,
+            side=TranslationSide.UPSTREAM)
 
         if shared is None:
-            clash_with_shared_current = (
-                current is not None and self.is_current_ubuntu)
+            clash_with_shared_ubuntu = (
+                ubuntu is not None and self.is_current_ubuntu)
             clash_with_shared_upstream = (
                 upstream is not None and self.is_current_upstream)
-            if clash_with_shared_current or clash_with_shared_upstream:
+            if clash_with_shared_ubuntu or clash_with_shared_upstream:
                 # Keep this message diverged, so it won't usurp the
                 # ubuntu or upstream message that the templates share.
                 pass
@@ -432,16 +432,16 @@
                 self.potemplate = None
         elif self.is_current_ubuntu or self.is_current_upstream:
             # Bequeathe ubuntu/upstream flags to shared equivalent.
-            if self.is_current_ubuntu and current is None:
+            if self.is_current_ubuntu and ubuntu is None:
                 shared.is_current_ubuntu = True
             if self.is_current_upstream and upstream is None:
                 shared.is_current_upstream = True
 
-            current_diverged = (
+            ubuntu_diverged = (
                 self.is_current_ubuntu and not shared.is_current_ubuntu)
             upstream_diverged = (
                 self.is_current_upstream and not shared.is_current_upstream)
-            if not (current_diverged or upstream_diverged):
+            if not (ubuntu_diverged or upstream_diverged):
                 # This message is now totally redundant.
                 self.destroySelf()
         else: