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
=== modified file 'lib/lp/translations/utilities/tests/test_file_importer.py'
--- lib/lp/translations/utilities/tests/test_file_importer.py 2010-08-29 19:55:16 +0000
+++ lib/lp/translations/utilities/tests/test_file_importer.py 2010-09-03 14:17:42 +0000
@@ -20,6 +20,8 @@
20 ITranslationImportQueue,20 ITranslationImportQueue,
21 )21 )
22from lp.translations.utilities.gettext_po_importer import GettextPOImporter22from lp.translations.utilities.gettext_po_importer import GettextPOImporter
23from lp.translations.utilities.translation_common_format import (
24 TranslationMessageData)
23from lp.translations.utilities.translation_import import (25from lp.translations.utilities.translation_import import (
24 FileImporter,26 FileImporter,
25 POFileImporter,27 POFileImporter,
@@ -217,6 +219,34 @@
217 "FileImporter.getOrCreatePOTMessageSet did not get an existing "219 "FileImporter.getOrCreatePOTMessageSet did not get an existing "
218 "IPOTMsgSet object from the database.")220 "IPOTMsgSet object from the database.")
219221
222 def _test_storeTranslationsInDatabase_empty(self, is_published=True):
223 """Check whether we store empty messages appropriately."""
224 # Construct a POFile importer.
225 pot_importer = self._createPOTFileImporter(
226 TEST_TEMPLATE_EXPORTED, is_published=True)
227 importer = self._createPOFileImporter(
228 pot_importer, TEST_TRANSLATION_EXPORTED,
229 is_published=is_published, person=self.importer_person)
230
231 # Empty message to import.
232 message = TranslationMessageData()
233 message.addTranslation(0, u'')
234
235 potmsgset = self.factory.makePOTMsgSet(
236 potemplate = importer.potemplate, sequence=50)
237 translation = importer.storeTranslationsInDatabase(
238 message, potmsgset)
239 # No TranslationMessage is created.
240 self.assertIs(None, translation)
241
242 def test_storeTranslationsInDatabase_empty_imported(self):
243 """Storing empty messages for published imports appropriately."""
244 self._test_storeTranslationsInDatabase_empty(is_published=True)
245
246 def test_storeTranslationsInDatabase_empty_user(self):
247 """Store empty messages for user uploads appropriately."""
248 self._test_storeTranslationsInDatabase_empty(is_published=False)
249
220 def test_FileImporter_storeTranslationsInDatabase_privileges(self):250 def test_FileImporter_storeTranslationsInDatabase_privileges(self):
221 """Test `storeTranslationsInDatabase` privileges."""251 """Test `storeTranslationsInDatabase` privileges."""
222252
223253
=== modified file 'lib/lp/translations/utilities/tests/test_translation_importer.py'
--- lib/lp/translations/utilities/tests/test_translation_importer.py 2010-08-20 20:31:18 +0000
+++ lib/lp/translations/utilities/tests/test_translation_importer.py 2010-09-03 14:17:42 +0000
@@ -5,13 +5,12 @@
55
6__metaclass__ = type6__metaclass__ = type
77
8import unittest
9
10from zope.component import getUtility8from zope.component import getUtility
11from zope.interface.verify import verifyObject9from zope.interface.verify import verifyObject
1210
13from canonical.testing import LaunchpadZopelessLayer11from canonical.testing import LaunchpadZopelessLayer
14from lp.registry.interfaces.product import IProductSet12from lp.registry.interfaces.product import IProductSet
13from lp.testing import TestCaseWithFactory
15from lp.translations.interfaces.potemplate import IPOTemplateSet14from lp.translations.interfaces.potemplate import IPOTemplateSet
16from lp.translations.interfaces.translationfileformat import (15from lp.translations.interfaces.translationfileformat import (
17 TranslationFileFormat,16 TranslationFileFormat,
@@ -29,11 +28,12 @@
29 )28 )
3029
3130
32class TranslationImporterTestCase(unittest.TestCase):31class TranslationImporterTestCase(TestCaseWithFactory):
33 """Class test for translation importer component"""32 """Class test for translation importer component"""
34 layer = LaunchpadZopelessLayer33 layer = LaunchpadZopelessLayer
3534
36 def setUp(self):35 def setUp(self):
36 super(TranslationImporterTestCase, self).setUp()
37 # Add a new entry for testing purposes. It's a template one.37 # Add a new entry for testing purposes. It's a template one.
38 productset = getUtility(IProductSet)38 productset = getUtility(IProductSet)
39 evolution = productset.getByName('evolution')39 evolution = productset.getByName('evolution')
@@ -246,10 +246,3 @@
246 msg2._translations = ["le foo", "les foos", "beaucoup des foos", None]246 msg2._translations = ["le foo", "les foos", "beaucoup des foos", None]
247 self.assertTrue(is_identical_translation(msg1, msg2),247 self.assertTrue(is_identical_translation(msg1, msg2),
248 "Identical multi-form messages not accepted as identical.")248 "Identical multi-form messages not accepted as identical.")
249
250
251def test_suite():
252 suite = unittest.TestSuite()
253 suite.addTest(unittest.makeSuite(TranslationImporterTestCase))
254 return suite
255
256249
=== modified file 'lib/lp/translations/utilities/translation_import.py'
--- lib/lp/translations/utilities/translation_import.py 2010-08-24 10:45:57 +0000
+++ lib/lp/translations/utilities/translation_import.py 2010-09-03 14:17:42 +0000
@@ -11,7 +11,6 @@
1111
12import datetime12import datetime
13from operator import attrgetter13from operator import attrgetter
14import os
1514
16import gettextpo15import gettextpo
17import posixpath16import posixpath
@@ -282,6 +281,7 @@
282 else:281 else:
283 return False282 return False
284283
284
285class TranslationImporter:285class TranslationImporter:
286 """Handle translation resources imports."""286 """Handle translation resources imports."""
287287
@@ -345,13 +345,13 @@
345 def importFile(self, translation_import_queue_entry, logger=None):345 def importFile(self, translation_import_queue_entry, logger=None):
346 """See ITranslationImporter."""346 """See ITranslationImporter."""
347 assert translation_import_queue_entry is not None, (347 assert translation_import_queue_entry is not None, (
348 "The translation import queue entry cannot be None.")348 "Import queue entry cannot be None.")
349 assert (translation_import_queue_entry.status ==349 assert (translation_import_queue_entry.status ==
350 RosettaImportStatus.APPROVED), (350 RosettaImportStatus.APPROVED), (
351 "The entry is not approved!.")351 "Import queue entry is not approved.")
352 assert (translation_import_queue_entry.potemplate is not None or352 assert (translation_import_queue_entry.potemplate is not None or
353 translation_import_queue_entry.pofile is not None), (353 translation_import_queue_entry.pofile is not None), (
354 "The entry has not any import target.")354 "Import queue entry has no import target.")
355355
356 importer = self.getTranslationFormatImporter(356 importer = self.getTranslationFormatImporter(
357 translation_import_queue_entry.format)357 translation_import_queue_entry.format)
@@ -438,7 +438,7 @@
438 is added to the list in self.errors but the translations are stored438 is added to the list in self.errors but the translations are stored
439 anyway, marked as having an error.439 anyway, marked as having an error.
440440
441 :param message: The message who's translations will be stored.441 :param message: The message for which translations will be stored.
442 :param potmsgset: The POTMsgSet that this message belongs to.442 :param potmsgset: The POTMsgSet that this message belongs to.
443443
444 :return: The updated translation_message entry or None, if no storing444 :return: The updated translation_message entry or None, if no storing
@@ -449,7 +449,9 @@
449 # store English strings in an IPOFile.449 # store English strings in an IPOFile.
450 return None450 return None
451451
452 if not message.translations:452 if (not message.translations or
453 set(message.translations) == set([u'']) or
454 set(message.translations) == set([None])):
453 # We don't have anything to import.455 # We don't have anything to import.
454 return None456 return None
455457
@@ -488,7 +490,6 @@
488 potmsgset.id)490 potmsgset.id)
489 return None491 return None
490492
491
492 just_replaced_msgid = (493 just_replaced_msgid = (
493 self.importer.uses_source_string_msgids and494 self.importer.uses_source_string_msgids and
494 self.pofile.language.code == 'en')495 self.pofile.language.code == 'en')
@@ -548,7 +549,6 @@
548 self.translation_import_queue_entry.format)549 self.translation_import_queue_entry.format)
549 return self._cached_format_exporter550 return self._cached_format_exporter
550551
551
552 def _addUpdateError(self, message, potmsgset, errormsg):552 def _addUpdateError(self, message, potmsgset, errormsg):
553 """Add an error returned by updateTranslation.553 """Add an error returned by updateTranslation.
554554
@@ -567,7 +567,7 @@
567 'pofile': self.pofile,567 'pofile': self.pofile,
568 'pomessage': self.format_exporter.exportTranslationMessageData(568 'pomessage': self.format_exporter.exportTranslationMessageData(
569 message),569 message),
570 'error-message': unicode(errormsg)570 'error-message': unicode(errormsg),
571 })571 })
572572
573 def _addConflictError(self, message, potmsgset):573 def _addConflictError(self, message, potmsgset):