launchpad-reviewers team mailing list archive

[Henning Eggers <henning.eggers@xxxxxxxxxxxxx>]

Henning Eggers has proposed merging lp:~henninge/launchpad/bug-720673-import-origin into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #720673 TranslationMessage.origin field set incorrectly during import.
  https://bugs.launchpad.net/bugs/720673

For more details, see:
https://code.launchpad.net/~henninge/launchpad/bug-720673-import-origin/+merge/50157

= Summary =

We had this great idea to simplify things in the import script by adding
translations first as a suggestion and then approving them if it verifies ok.
We missed the fact, though, that the suggest-approve mechanism was designed
for translations entered through the UI. So there behavior does not match
what the old import code did. I fixed a big part of that in the last branch
by introducing the "accept" methods as opposed to "approve". This branch now
deals with a little leftover in what submitSuggestion does differently. In
this case the difference is so little, though that I did not introduce a new
method but just added a parameter.

== Proposed fix ==

Add a parameter from_import to submitSuggestion to change it behavior as is
appropriate for imports, namely:
- set the origin to SCM (source code management)
- do not assign karma to the uploader.

We may have to rethink the karma story for uploads, maybe create its own
category.

== Pre-implementation notes ==

This is pretty obvious. ;-)

== Implementation details ==

Do you really need more detail?

== Tests ==

bin/test -vvcm lp.translations.tests.test_potmsgset -t submitSuggestion

== Demo and Q/A ==

Do an import and verify through a database query that the origin is set
correctly. It is not exposed in the UI.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/interfaces/potmsgset.py
  lib/lp/translations/tests/test_potmsgset.py
  lib/lp/translations/model/potmsgset.py
  lib/lp/translations/utilities/translation_import.py
-- 
https://code.launchpad.net/~henninge/launchpad/bug-720673-import-origin/+merge/50157
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~henninge/launchpad/bug-720673-import-origin into lp:launchpad.

=== modified file 'lib/lp/translations/interfaces/potmsgset.py'
--- lib/lp/translations/interfaces/potmsgset.py	2011-01-27 23:13:20 +0000
+++ lib/lp/translations/interfaces/potmsgset.py	2011-02-17 14:32:22 +0000
@@ -222,13 +222,17 @@
             translations.
         """
 
-    def submitSuggestion(pofile, submitter, new_translations):
+    def submitSuggestion(pofile, submitter, new_translations,
+                         from_import=False):
         """Submit a suggested translation for this message.
 
         If an identical message is already present, it will be returned
         (and it is not changed).  Otherwise, a new one is created and
         returned.  Suggestions for translation credits messages are
         ignored, and None is returned in that case.
+        Setting from_import to true will prevent karma assignment and
+        set the origin of the created message to SCM instead of
+        ROSETTAWEB.
         """
 
     def dismissAllSuggestions(pofile, reviewer, lock_timestamp):

=== modified file 'lib/lp/translations/model/potmsgset.py'
--- lib/lp/translations/model/potmsgset.py	2011-02-15 23:54:54 +0000
+++ lib/lp/translations/model/potmsgset.py	2011-02-17 14:32:22 +0000
@@ -549,7 +549,8 @@
         else:
             return None
 
-    def submitSuggestion(self, pofile, submitter, new_translations):
+    def submitSuggestion(self, pofile, submitter, new_translations,
+                         from_import=False):
         """See `IPOTMsgSet`."""
         if self.is_translation_credit:
             # We don't support suggestions on credits messages.
@@ -565,11 +566,16 @@
             ('msgstr%d' % form, potranslation)
             for form, potranslation in potranslations.iteritems())
 
-        pofile.potemplate.awardKarma(submitter, 'translationsuggestionadded')
+        if from_import:
+            origin = RosettaTranslationOrigin.SCM
+        else:
+            origin = RosettaTranslationOrigin.ROSETTAWEB
+            pofile.potemplate.awardKarma(
+                submitter, 'translationsuggestionadded')
 
         return TranslationMessage(
             potmsgset=self, language=pofile.language,
-            origin=RosettaTranslationOrigin.ROSETTAWEB, submitter=submitter,
+            origin=origin, submitter=submitter,
             **forms)
 
     def _checkForConflict(self, current_message, lock_timestamp,

=== modified file 'lib/lp/translations/tests/test_potmsgset.py'
--- lib/lp/translations/tests/test_potmsgset.py	2011-02-14 21:50:21 +0000
+++ lib/lp/translations/tests/test_potmsgset.py	2011-02-17 14:32:22 +0000
@@ -1396,6 +1396,29 @@
 
         self.assertEqual([], karma_listener.karma_events)
 
+    def test_from_import_origin(self):
+        # With from_import set, the origin is set to SCM.
+        pofile, potmsgset = self._makePOFileAndPOTMsgSet()
+        owner = pofile.potemplate.owner
+        translation = {0: self.factory.getUniqueString()}
+
+        suggestion = potmsgset.submitSuggestion(
+            pofile, owner, translation, from_import=True)
+
+        self.assertEqual(RosettaTranslationOrigin.SCM, suggestion.origin)
+
+    def test_from_import_karma(self):
+        # No karma is assigned if from_import is set.
+        pofile, potmsgset = self._makePOFileAndPOTMsgSet()
+        owner = pofile.potemplate.owner
+        translation = {0: self.factory.getUniqueString()}
+        karma_listener = self._listenForKarma(pofile)
+
+        potmsgset.submitSuggestion(
+            pofile, owner, translation, from_import=True)
+
+        self.assertEqual([], karma_listener.karma_events)
+
 
 class TestSetCurrentTranslation(TestCaseWithFactory):
     layer = DatabaseFunctionalLayer

=== modified file 'lib/lp/translations/utilities/translation_import.py'
--- lib/lp/translations/utilities/translation_import.py	2011-02-15 12:13:20 +0000
+++ lib/lp/translations/utilities/translation_import.py	2011-02-17 14:32:22 +0000
@@ -545,7 +545,8 @@
         # The message is first stored as a suggestion and only made
         # current if it validates.
         new_message = potmsgset.submitSuggestion(
-            self.pofile, self.last_translator, sanitized_translations)
+            self.pofile, self.last_translator, sanitized_translations,
+            from_import=True)
 
         validation_ok = self._validateMessage(
             potmsgset, new_message, sanitized_translations, message_data)