launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-1340010 into lp:launchpad.

Commit message:
TranslationMessageSet.preloadDetails now copes with TranslationMessages that have no corresponding POFile.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1340010 in Launchpad itself: "POFile:+translate OOPSes when loading a suggestion with no POFile"
  https://bugs.launchpad.net/launchpad/+bug/1340010

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-1340010/+merge/226246

POFile:+translate OOPSes like OOPS-179ec78410b8af059254cb47ab76a328 when it attempts to preloadDetails on a suggestion that doesn't have a corresponding POFile.

In the only known case so far, POTMsgSet 3203927 ("Delete") exists in POTemplate 14011 (https://launchpad.net/listen/0.4/+pots/listen) and has a TranslationMessage for Language 540 (en_GB), but there's no POFile for Language 540 in POTemplate 14011. TranslationMessageSet.preloadDetails crashes when it tries to preload the POTemplate for a POFile that is None.

_buildAllSuggestions has a browser_pofile is not None check to handle this case, so it probably is a legal situation. The fix is trivial, the test slightly less so.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-1340010/+merge/226246
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-1340010 into lp:launchpad.

=== modified file 'lib/lp/translations/model/translationmessage.py'
--- lib/lp/translations/model/translationmessage.py	2014-07-02 01:31:08 +0000
+++ lib/lp/translations/model/translationmessage.py	2014-07-10 05:54:31 +0000
@@ -558,7 +558,9 @@
         if need_pofile:
             self.preloadPOFilesAndSequences(tms, pofile)
         if need_potemplate:
-            pofiles = [tm.browser_pofile for tm in tms]
+            pofiles = [
+                tm.browser_pofile for tm in tms
+                if tm.browser_pofile is not None]
             pots = load_related(
                 POTemplate,
                 (removeSecurityProxy(pofile) for pofile in pofiles),

=== modified file 'lib/lp/translations/tests/test_translationmessage.py'
--- lib/lp/translations/tests/test_translationmessage.py	2014-06-10 11:25:51 +0000
+++ lib/lp/translations/tests/test_translationmessage.py	2014-07-10 05:54:31 +0000
@@ -12,21 +12,27 @@
 
 from pytz import UTC
 from storm.locals import Store
+import transaction
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
+from lp.services.propertycache import clear_property_cache
 from lp.services.worlddata.interfaces.language import ILanguageSet
 from lp.testing import (
     TestCaseWithFactory,
     verifyObject,
     )
-from lp.testing.layers import ZopelessDatabaseLayer
+from lp.testing.layers import (
+    DatabaseFunctionalLayer,
+    ZopelessDatabaseLayer,
+    )
 from lp.translations.interfaces.side import (
     ITranslationSideTraitsSet,
     TranslationSide,
     )
 from lp.translations.interfaces.translationmessage import (
     ITranslationMessage,
+    ITranslationMessageSet,
     TranslationConflict,
     )
 from lp.translations.interfaces.translations import TranslationConstants
@@ -1096,3 +1102,30 @@
         self.assertTrue(translation.is_diverged)
         translation.shareIfPossible()
         self.assertTrue(translation.is_diverged)
+
+
+class TestPreloading(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_preloadDetails_handles_orphaned_message(self):
+        # preloadDetails works when given a TranslationMessage that has
+        # no POFile. This happens occasionally on production, and the
+        # suggestion rendering code copes with it, so we should too.
+        lang = self.factory.makeLanguage()
+        tm1 = self.factory.makeCurrentTranslationMessage(language=lang)
+        tm2 = self.factory.makeCurrentTranslationMessage(language=lang)
+
+        tm1_pofile = tm1.getOnePOFile()
+        tm2_pofile = tm2.getOnePOFile()
+
+        # Get rid of tm1's POFile.
+        removeSecurityProxy(tm1_pofile).language = self.factory.makeLanguage()
+
+        getUtility(ITranslationMessageSet).preloadDetails(
+            [tm1, tm2], need_pofile=True, need_potemplate=True,
+            need_potemplate_context=True, need_potranslation=True,
+            need_potmsgset=True, need_people=True)
+
+        self.assertIs(None, tm1.browser_pofile)
+        self.assertEqual(tm2_pofile, tm2.browser_pofile)