launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02508
lp:~henninge/launchpad/devel-710591-setcurrenttranslation-extension into lp:launchpad
Henning Eggers has proposed merging lp:~henninge/launchpad/devel-710591-setcurrenttranslation-extension into lp:launchpad with lp:~henninge/launchpad/devel-710591-sharing-info-groundwork-0 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#710591 Ubuntu upstream translation imports overwrite Ubuntu translations
https://bugs.launchpad.net/bugs/710591
For more details, see:
https://code.launchpad.net/~henninge/launchpad/devel-710591-setcurrenttranslation-extension/+merge/48594
= Summary =
Provide a method to import translations in old style. Old style means that
translations coming from an import are marked as "imported" by setting the
"current" flag on the other side. It also follow the precedence rules as
outlined here:
https://wiki.ubuntu.com/Translations/KnowledgeBase/TranslationsPrecedence
== Proposed fix ==
Provide a method "TranslationMessage.acceptAsImported" to be used instead of
"TranslationMessage.approve" during import. Using the "approve" method here
is misleading as translations are not really approved. Hence the new method
does not take a "reviewer" parameter and does not set the "reviewer" and
"date_reviewed" attributes of a TranslationMessage.
== Pre-implementation notes ==
Talked to danilo about the need to fix this as a special case for
setCurrentTranslation.
== Implementation details ==
== Tests ==
bin/test -vvcm lp.translations.tests.test_translationmessage \
-t AcceptAsImported
== Demo and Q/A ==
None.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/translations/interfaces/translationmessage.py
lib/lp/translations/tests/test_translationmessage.py
lib/lp/testing/factory.py
lib/lp/translations/model/potmsgset.py
lib/lp/translations/model/translationmessage.py
--
https://code.launchpad.net/~henninge/launchpad/devel-710591-setcurrenttranslation-extension/+merge/48594
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~henninge/launchpad/devel-710591-setcurrenttranslation-extension into lp:launchpad.
=== removed file 'lib/lp/app/widgets/__init__.py'
--- lib/lp/app/widgets/__init__.py 2011-02-02 15:37:35 +0000
+++ lib/lp/app/widgets/__init__.py 1970-01-01 00:00:00 +0000
@@ -1,2 +0,0 @@
-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2011-02-03 05:23:55 +0000
+++ lib/lp/testing/factory.py 2011-02-04 11:07:33 +0000
@@ -2705,7 +2705,7 @@
removeSecurityProxy(potmsgset).sync()
return potmsgset
- def makePOFileAndPOTMsgSet(self, language_code, msgid=None,
+ def makePOFileAndPOTMsgSet(self, language_code=None, msgid=None,
with_plural=False):
"""Make a `POFile` with a `POTMsgSet`."""
pofile = self.makePOFile(language_code)
=== modified file 'lib/lp/translations/interfaces/translationmessage.py'
--- lib/lp/translations/interfaces/translationmessage.py 2011-01-26 22:19:28 +0000
+++ lib/lp/translations/interfaces/translationmessage.py 2011-02-04 11:07:33 +0000
@@ -267,6 +267,10 @@
def approveAsDiverged(pofile, reviewer, lock_timestamp=None):
"""Approve this suggestion, as a diverged translation."""
+ def acceptAsImported(pofile, share_with_other_side=False,
+ sold_style_import=False, lock_timestamp=None):
+ """Accept a suggestion as an imported translation."""
+
# XXX CarlosPerelloMarin 20071022: We should move this into browser code.
def makeHTMLID(description):
"""Unique identifier for self, suitable for use in HTML element ids.
=== modified file 'lib/lp/translations/model/potmsgset.py'
--- lib/lp/translations/model/potmsgset.py 2011-01-28 01:08:34 +0000
+++ lib/lp/translations/model/potmsgset.py 2011-02-04 11:07:33 +0000
@@ -739,6 +739,69 @@
template.awardKarma(translator, 'translationsuggestionapproved')
template.awardKarma(reviewer, 'translationreview')
+ def acceptAsImported(self, pofile, suggestion,
+ share_with_other_side=False, old_style_import=False,
+ lock_timestamp=None):
+ """Accept a suggestion as an imported translation.
+
+ When importing translations, these are first added as a suggestion
+ and only after successful validation they are made current. This is
+ slightly different to approving a suggestion because no reviewer is
+ credited.
+
+ Also, this method allows to do old style imports that set the flag on
+ the other side to indicate that this message has been imported. In
+ this mode it will replace existing translations only when this is the
+ first import for this message. This is used for package uploads when
+ the upstream project has not yet been created.
+
+ :param pofile: The `POFile` that the suggestion is being approved for.
+ :param suggestion: The `TranslationMessage` being approved.
+ :param share_with_other_side: Policy selector: share this change with
+ the other translation side if possible?
+ :param old_style_import: Do an old style import.
+ :param lock_timestamp: Timestamp of the original translation state
+ that this change is based on.
+ """
+ template = pofile.potemplate
+ traits = getUtility(ITranslationSideTraitsSet).getTraits(
+ template.translation_side)
+ if not old_style_import:
+ if traits.getFlag(suggestion):
+ # Message is already current.
+ return
+
+ translator = suggestion.submitter
+ potranslations = dictify_translations(suggestion.all_msgstrs)
+ self._setTranslation(
+ pofile, translator, suggestion.origin, potranslations,
+ share_with_other_side=share_with_other_side,
+ identical_message=suggestion, lock_timestamp=lock_timestamp)
+ else:
+ if traits.getFlag(suggestion) and (
+ traits.other_side_traits.getFlag(suggestion)):
+ # Message is already current.
+ return
+ current = self.getCurrentTranslation(
+ template, pofile.language, template.translation_side)
+ other = self.getOtherTranslation(
+ pofile.language, template.translation_side)
+ if other is not None:
+ traits.other_side_traits.setFlag(other, False)
+ if current is None or other is None:
+ translator = suggestion.submitter
+ potranslations = dictify_translations(suggestion.all_msgstrs)
+ self._setTranslation(
+ pofile, translator, suggestion.origin, potranslations,
+ share_with_other_side=True,
+ identical_message=suggestion,
+ lock_timestamp=lock_timestamp)
+ else:
+ # Make it only current on the other side.
+ traits.other_side_traits.setFlag(suggestion, True)
+ if suggestion != other:
+ pofile.markChanged(translator=suggestion.submitter)
+
def _cloneAndDiverge(self, original_message, pofile):
"""Create a diverged clone of a `TranslationMessage`.
=== modified file 'lib/lp/translations/model/translationmessage.py'
--- lib/lp/translations/model/translationmessage.py 2011-01-26 15:11:43 +0000
+++ lib/lp/translations/model/translationmessage.py 2011-02-04 11:07:33 +0000
@@ -343,6 +343,13 @@
return self.potmsgset.approveAsDiverged(
pofile, self, reviewer, lock_timestamp=lock_timestamp)
+ def acceptAsImported(self, pofile, share_with_other_side=False,
+ old_style_import=False, lock_timestamp=None):
+ """Accept a suggestion as an imported translation."""
+ self.potmsgset.acceptAsImported(
+ pofile, self, share_with_other_side=share_with_other_side,
+ old_style_import=old_style_import, lock_timestamp=lock_timestamp)
+
def getOnePOFile(self):
"""See `ITranslationMessage`."""
from lp.translations.model.pofile import POFile
=== modified file 'lib/lp/translations/tests/test_translationmessage.py'
--- lib/lp/translations/tests/test_translationmessage.py 2011-01-25 16:51:45 +0000
+++ lib/lp/translations/tests/test_translationmessage.py 2011-02-04 11:07:33 +0000
@@ -73,6 +73,8 @@
class TestApprove(TestCaseWithFactory):
+ """Tests for `TranslationMessage.approve`."""
+
layer = ZopelessDatabaseLayer
def test_approve_activates_message(self):
@@ -290,6 +292,172 @@
self.assertNotEqual(ubuntu_tm, upstream_tm)
+class TestAcceptAsImported(TestCaseWithFactory):
+ """Tests for `TranslationMessage.acceptAsImported`.
+
+ This method is a lot like `TranslationMessage.approve`, so this test
+ mainly exercises what it does differently.
+ """
+
+ layer = ZopelessDatabaseLayer
+
+ def test_accept_activates_message(self):
+ # An untranslated message receives an imported translation.
+ pofile = self.factory.makePOFile()
+ suggestion = self.factory.makeSuggestion(pofile=pofile)
+ reviewer = self.factory.makePerson()
+ self.assertFalse(suggestion.is_current_upstream)
+ self.assertFalse(suggestion.is_current_ubuntu)
+
+ suggestion.acceptAsImported(pofile)
+
+ # By default the suggestion becomes current only on the side
+ # (upstream or Ubuntu) that it's being approved on.
+ self.assertTrue(suggestion.is_current_upstream)
+ self.assertFalse(suggestion.is_current_ubuntu)
+
+ def test_accept_can_make_other_side_track(self):
+ # In some situations (see POTMsgSet.setCurrentTranslation for
+ # details) the acceptance can be made to propagate to the other
+ # side, subject to the share_with_other_side parameter.
+ pofile = self.factory.makePOFile()
+ suggestion = self.factory.makeSuggestion(pofile=pofile)
+ reviewer = self.factory.makePerson()
+
+ self.assertFalse(suggestion.is_current_upstream)
+ self.assertFalse(suggestion.is_current_ubuntu)
+
+ suggestion.acceptAsImported(pofile, share_with_other_side=True)
+
+ self.assertTrue(suggestion.is_current_upstream)
+ self.assertTrue(suggestion.is_current_ubuntu)
+
+ def test_accept_marks_not_reviewed(self):
+ # Accepting a suggestion does not update its review fields.
+ pofile = self.factory.makePOFile()
+ suggestion = self.factory.makeSuggestion(pofile=pofile)
+ reviewer = self.factory.makePerson()
+
+ self.assertIs(None, suggestion.reviewer)
+ self.assertIs(None, suggestion.date_reviewed)
+
+ suggestion.acceptAsImported(pofile)
+
+ self.assertIs(None, suggestion.reviewer)
+ self.assertIs(None, suggestion.date_reviewed)
+
+ def test_accept_awards_no_karma(self):
+ # The translator receives no karma.
+ translator = self.factory.makePerson()
+ pofile = self.factory.makePOFile()
+ suggestion = self.factory.makeSuggestion(
+ pofile=pofile, translator=translator)
+
+ karmarecorder = self.installKarmaRecorder(person=translator)
+ suggestion.acceptAsImported(pofile)
+
+ self.assertEqual([], karmarecorder.karma_events)
+
+ def test_accept_old_style_activates_message_if_untranslated(self):
+ # An untranslated message accepts an imported translation.
+ pofile = self.factory.makePOFile()
+ suggestion = self.factory.makeSuggestion(pofile=pofile)
+ reviewer = self.factory.makePerson()
+ self.assertFalse(suggestion.is_current_upstream)
+ self.assertFalse(suggestion.is_current_ubuntu)
+
+ suggestion.acceptAsImported(pofile, old_style_import=True)
+
+ # Messages are always accepted on the other side, too.
+ self.assertTrue(suggestion.is_current_upstream)
+ self.assertTrue(suggestion.is_current_ubuntu)
+
+ def test_accept_old_style_no_previously_imported(self):
+ # If there was already a current translation, but no previously
+ # imported one, it is disabled when a suggestion is accepted.
+ pofile, potmsgset = self.factory.makePOFileAndPOTMsgSet()
+ suggestion = self.factory.makeSuggestion(
+ pofile=pofile, potmsgset=potmsgset)
+ incumbent_message = self.factory.makeCurrentTranslationMessage(
+ pofile=pofile, potmsgset=potmsgset)
+
+ self.assertTrue(incumbent_message.is_current_upstream)
+ self.assertFalse(suggestion.is_current_upstream)
+
+ suggestion.acceptAsImported(pofile, old_style_import=True)
+
+ self.assertFalse(incumbent_message.is_current_upstream)
+ self.assertTrue(suggestion.is_current_upstream)
+ # Messages are always accepted on the other side, too.
+ self.assertTrue(suggestion.is_current_ubuntu)
+
+ def test_accept_old_style_previously_imported(self):
+ # If there was already a current translation, and a previously
+ # imported one, the current translation is left untouched.
+ pofile, potmsgset = self.factory.makePOFileAndPOTMsgSet()
+ imported_message = self.factory.makeCurrentTranslationMessage(
+ pofile=pofile, potmsgset=potmsgset, current_other=True)
+ imported_message.is_current_upstream = False
+
+ suggestion = self.factory.makeSuggestion(
+ pofile=pofile, potmsgset=potmsgset)
+ incumbent_message = self.factory.makeCurrentTranslationMessage(
+ pofile=pofile, potmsgset=potmsgset)
+
+ self.assertTrue(incumbent_message.is_current_upstream)
+ self.assertFalse(suggestion.is_current_upstream)
+ self.assertTrue(imported_message.is_current_ubuntu)
+
+ suggestion.acceptAsImported(pofile, old_style_import=True)
+
+ self.assertTrue(incumbent_message.is_current_upstream)
+ self.assertFalse(suggestion.is_current_upstream)
+ # Messages are always accepted on the other side, too.
+ self.assertFalse(imported_message.is_current_ubuntu)
+ self.assertTrue(suggestion.is_current_ubuntu)
+
+ def test_accept_old_style_current_message(self):
+ # Accepting a message that's already current does nothing on this
+ # side but makes sure the other side's flag is set.
+ pofile = self.factory.makePOFile()
+ translation = self.factory.makeCurrentTranslationMessage(
+ pofile=pofile)
+ self.assertTrue(translation.is_current_upstream)
+ self.assertFalse(translation.is_current_ubuntu)
+
+ translation.acceptAsImported(pofile, old_style_import=True)
+
+ self.assertTrue(translation.is_current_upstream)
+ self.assertTrue(translation.is_current_ubuntu)
+
+ def test_accept_old_style_current_and_imported_message(self):
+ # Accepting a message that's already current and was also imported
+ # does nothing.
+ pofile = self.factory.makePOFile()
+ translation = self.factory.makeCurrentTranslationMessage(
+ pofile=pofile, current_other=True)
+ self.assertTrue(translation.is_current_upstream)
+ self.assertTrue(translation.is_current_ubuntu)
+
+ translation.acceptAsImported(pofile, old_style_import=True)
+
+ self.assertTrue(translation.is_current_upstream)
+ self.assertTrue(translation.is_current_ubuntu)
+
+ def test_accept_old_style_detects_conflict(self):
+ pofile = self.factory.makePOFile()
+ current = self.factory.makeCurrentTranslationMessage(pofile=pofile)
+ potmsgset = current.potmsgset
+ suggestion = self.factory.makeSuggestion(
+ pofile=pofile, potmsgset=potmsgset)
+ old = datetime.now(UTC) - timedelta(days=1)
+
+ self.assertRaises(
+ TranslationConflict,
+ suggestion.acceptAsImported,
+ pofile, old_style_import=True, lock_timestamp=old)
+
+
class TestApproveAsDiverged(TestCaseWithFactory):
"""Tests for `TranslationMessage.approveAsDiverged`."""
