Launchpad itself

Merge lp:~danilo/launchpad/bug-548375 into lp:launchpad

  1. bug-548375
  2. Merge into devel
Proposed by Данило Шеган
Status: Merged
Approved by: Данило Шеган
Approved revision: no longer in the source branch.
Merged at revision: 11501
Proposed branch: lp:~danilo/launchpad/bug-548375
Merge into: lp:launchpad
Diff against target: 173 lines (+42/-19)
3 files modified
lib/lp/translations/utilities/tests/test_file_importer.py (+30/-0)
lib/lp/translations/utilities/tests/test_translation_importer.py (+3/-10)
lib/lp/translations/utilities/translation_import.py (+9/-9)
To merge this branch: bzr merge lp:~danilo/launchpad/bug-548375
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+34544@code.launchpad.net

Commit message

Ignore empty translations for uploads (including user-uploaded PO files).

Description of the change

= Bug #548375 =

Ignore empty translations on import (always). We've also considered
keeping the ability to revert translations with non-published uploads,
but it would be more work for dubious benefit. For now, what users
want is for imports to simply not introduce empty messages. We get that
here.

Most of the changes are cleanups and lint fixes. Basically, only the change

@@ -449,7 +449,9 @@ class FileImporter(object):
             # store English strings in an IPOFile.
             return None

- if not message.translations:
+ if (not message.translations or
+ set(message.translations) == set([u'']) or
+ set(message.translations) == set([None])):
             # We don't have anything to import.
             return None

is important here. Since message.translations is never going to be
more than 6 items, I didn't bother optimizing this (i.e. do a
set(message.translations) once and use that in further comparisons).

The tests are split into two methods so it's clearer what we do for
both cases (I've first written tests which assert previous behavior,
then unified the code using a helper private method).

== Tests ==

bin/test -cvvt storeTranslationsInDatabase_empty

== Demo and Q/A ==

Export a PO file and re-import it as "updated translation" and watch for empty messages not being created.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/utilities/tests/test_translation_importer.py
  lib/lp/translations/utilities/tests/test_file_importer.py
  lib/lp/translations/utilities/translation_import.py

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

Looks good Danilos. Thanks for the branch and the clean up.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/translations/utilities/tests/test_file_importer.py'
2--- lib/lp/translations/utilities/tests/test_file_importer.py 2010-08-29 19:55:16 +0000
3+++ lib/lp/translations/utilities/tests/test_file_importer.py 2010-09-03 14:17:42 +0000
4@@ -20,6 +20,8 @@
5 ITranslationImportQueue,
6 )
7 from lp.translations.utilities.gettext_po_importer import GettextPOImporter
8+from lp.translations.utilities.translation_common_format import (
9+ TranslationMessageData)
10 from lp.translations.utilities.translation_import import (
11 FileImporter,
12 POFileImporter,
13@@ -217,6 +219,34 @@
14 "FileImporter.getOrCreatePOTMessageSet did not get an existing "
15 "IPOTMsgSet object from the database.")
16
17+ def _test_storeTranslationsInDatabase_empty(self, is_published=True):
18+ """Check whether we store empty messages appropriately."""
19+ # Construct a POFile importer.
20+ pot_importer = self._createPOTFileImporter(
21+ TEST_TEMPLATE_EXPORTED, is_published=True)
22+ importer = self._createPOFileImporter(
23+ pot_importer, TEST_TRANSLATION_EXPORTED,
24+ is_published=is_published, person=self.importer_person)
25+
26+ # Empty message to import.
27+ message = TranslationMessageData()
28+ message.addTranslation(0, u'')
29+
30+ potmsgset = self.factory.makePOTMsgSet(
31+ potemplate = importer.potemplate, sequence=50)
32+ translation = importer.storeTranslationsInDatabase(
33+ message, potmsgset)
34+ # No TranslationMessage is created.
35+ self.assertIs(None, translation)
36+
37+ def test_storeTranslationsInDatabase_empty_imported(self):
38+ """Storing empty messages for published imports appropriately."""
39+ self._test_storeTranslationsInDatabase_empty(is_published=True)
40+
41+ def test_storeTranslationsInDatabase_empty_user(self):
42+ """Store empty messages for user uploads appropriately."""
43+ self._test_storeTranslationsInDatabase_empty(is_published=False)
44+
45 def test_FileImporter_storeTranslationsInDatabase_privileges(self):
46 """Test `storeTranslationsInDatabase` privileges."""
47
48
49=== modified file 'lib/lp/translations/utilities/tests/test_translation_importer.py'
50--- lib/lp/translations/utilities/tests/test_translation_importer.py 2010-08-20 20:31:18 +0000
51+++ lib/lp/translations/utilities/tests/test_translation_importer.py 2010-09-03 14:17:42 +0000
52@@ -5,13 +5,12 @@
53
54 __metaclass__ = type
55
56-import unittest
57-
58 from zope.component import getUtility
59 from zope.interface.verify import verifyObject
60
61 from canonical.testing import LaunchpadZopelessLayer
62 from lp.registry.interfaces.product import IProductSet
63+from lp.testing import TestCaseWithFactory
64 from lp.translations.interfaces.potemplate import IPOTemplateSet
65 from lp.translations.interfaces.translationfileformat import (
66 TranslationFileFormat,
67@@ -29,11 +28,12 @@
68 )
69
70
71-class TranslationImporterTestCase(unittest.TestCase):
72+class TranslationImporterTestCase(TestCaseWithFactory):
73 """Class test for translation importer component"""
74 layer = LaunchpadZopelessLayer
75
76 def setUp(self):
77+ super(TranslationImporterTestCase, self).setUp()
78 # Add a new entry for testing purposes. It's a template one.
79 productset = getUtility(IProductSet)
80 evolution = productset.getByName('evolution')
81@@ -246,10 +246,3 @@
82 msg2._translations = ["le foo", "les foos", "beaucoup des foos", None]
83 self.assertTrue(is_identical_translation(msg1, msg2),
84 "Identical multi-form messages not accepted as identical.")
85-
86-
87-def test_suite():
88- suite = unittest.TestSuite()
89- suite.addTest(unittest.makeSuite(TranslationImporterTestCase))
90- return suite
91-
92
93=== modified file 'lib/lp/translations/utilities/translation_import.py'
94--- lib/lp/translations/utilities/translation_import.py 2010-08-24 10:45:57 +0000
95+++ lib/lp/translations/utilities/translation_import.py 2010-09-03 14:17:42 +0000
96@@ -11,7 +11,6 @@
97
98 import datetime
99 from operator import attrgetter
100-import os
101
102 import gettextpo
103 import posixpath
104@@ -282,6 +281,7 @@
105 else:
106 return False
107
108+
109 class TranslationImporter:
110 """Handle translation resources imports."""
111
112@@ -345,13 +345,13 @@
113 def importFile(self, translation_import_queue_entry, logger=None):
114 """See ITranslationImporter."""
115 assert translation_import_queue_entry is not None, (
116- "The translation import queue entry cannot be None.")
117+ "Import queue entry cannot be None.")
118 assert (translation_import_queue_entry.status ==
119 RosettaImportStatus.APPROVED), (
120- "The entry is not approved!.")
121+ "Import queue entry is not approved.")
122 assert (translation_import_queue_entry.potemplate is not None or
123 translation_import_queue_entry.pofile is not None), (
124- "The entry has not any import target.")
125+ "Import queue entry has no import target.")
126
127 importer = self.getTranslationFormatImporter(
128 translation_import_queue_entry.format)
129@@ -438,7 +438,7 @@
130 is added to the list in self.errors but the translations are stored
131 anyway, marked as having an error.
132
133- :param message: The message who's translations will be stored.
134+ :param message: The message for which translations will be stored.
135 :param potmsgset: The POTMsgSet that this message belongs to.
136
137 :return: The updated translation_message entry or None, if no storing
138@@ -449,7 +449,9 @@
139 # store English strings in an IPOFile.
140 return None
141
142- if not message.translations:
143+ if (not message.translations or
144+ set(message.translations) == set([u'']) or
145+ set(message.translations) == set([None])):
146 # We don't have anything to import.
147 return None
148
149@@ -488,7 +490,6 @@
150 potmsgset.id)
151 return None
152
153-
154 just_replaced_msgid = (
155 self.importer.uses_source_string_msgids and
156 self.pofile.language.code == 'en')
157@@ -548,7 +549,6 @@
158 self.translation_import_queue_entry.format)
159 return self._cached_format_exporter
160
161-
162 def _addUpdateError(self, message, potmsgset, errormsg):
163 """Add an error returned by updateTranslation.
164
165@@ -567,7 +567,7 @@
166 'pofile': self.pofile,
167 'pomessage': self.format_exporter.exportTranslationMessageData(
168 message),
169- 'error-message': unicode(errormsg)
170+ 'error-message': unicode(errormsg),
171 })
172
173 def _addConflictError(self, message, potmsgset):