launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00840
[Merge] lp:~jtv/launchpad/recife-diverge into lp:~launchpad/launchpad/recife
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/recife-diverge into lp:~launchpad/launchpad/recife.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers): code
= approveAsDiverged =
For the Recife feature branch. This implements the option to approve a translation suggestion, not as the new shared translation as you'd normally want, but as a diverged translation specific to one particular POTemplate.
Interface-wise, the new method is on TranslationMessage: "approve this one in this POFile." The actual implementation is in POTMsgSet, where all the supporting infrastructure is, and the published one forwards to it.
To test:
{{{
./bin/test -vvc lp.translations.tests.test_translationmessage -t ApproveAsDiverged
}}}
(Although of course I ran the rest of 'em as well).
I also seized this opportunity to replace a factory method, previously a hack to work around the lack of this new method, to do things more properly.
No lint,
Jeroen
--
https://code.launchpad.net/~jtv/launchpad/recife-diverge/+merge/34407
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/recife-diverge into lp:~launchpad/launchpad/recife.
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-09-01 13:47:24 +0000
+++ lib/lp/testing/factory.py 2010-09-02 11:57:45 +0000
@@ -237,7 +237,6 @@
time_counter,
)
from lp.translations.interfaces.potemplate import IPOTemplateSet
-from lp.translations.interfaces.side import ITranslationSideTraitsSet
from lp.translations.interfaces.translationfileformat import (
TranslationFileFormat,
)
@@ -443,7 +442,6 @@
registry_team.addMember(user, registry_team.teamowner)
return user
-
def makeCopyArchiveLocation(self, distribution=None, owner=None,
name=None, enabled=True):
"""Create and return a new arbitrary location for copy packages."""
@@ -2300,18 +2298,16 @@
"""Create a diverged, current `TranslationMessage`."""
if pofile is None:
pofile = self.makePOFile('lt')
+ if reviewer is None:
+ reviewer = self.makePerson()
# This creates a suggestion, then diverges it, then activates it.
# Once we have a method for diverging messages, do this in a more
# proper way.
- message = self.makeSharedTranslationMessage(
+ message = self.makeSuggestion(
pofile=pofile, potmsgset=potmsgset, translator=translator,
- reviewer=reviewer, translations=translations, suggestion=True)
- traits = getUtility(ITranslationSideTraitsSet).getTraits(
- pofile.potemplate.translation_side)
- removeSecurityProxy(message).potemplate = pofile.potemplate
- traits.setFlag(message, True)
- return message
+ translations=translations)
+ return message.approveAsDiverged(pofile, reviewer)
def makeTranslation(self, pofile, sequence,
english=None, translated=None,
=== modified file 'lib/lp/testing/tests/test_factory.py'
--- lib/lp/testing/tests/test_factory.py 2010-08-24 07:12:53 +0000
+++ lib/lp/testing/tests/test_factory.py 2010-09-02 11:57:45 +0000
@@ -6,7 +6,6 @@
__metaclass__ = type
from datetime import datetime
-import unittest
import pytz
from zope.component import getUtility
=== modified file 'lib/lp/translations/interfaces/translationmessage.py'
--- lib/lp/translations/interfaces/translationmessage.py 2010-08-24 11:39:06 +0000
+++ lib/lp/translations/interfaces/translationmessage.py 2010-09-02 11:57:45 +0000
@@ -259,6 +259,9 @@
lock_timestamp=None):
"""Approve this suggestion, making it a current translation."""
+ def approveAsDiverged(pofile, reviewer, lock_timestamp=None):
+ """Approve this suggestion, as a diverged 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 2010-09-01 15:11:18 +0000
+++ lib/lp/translations/model/potmsgset.py 2010-09-02 11:57:45 +0000
@@ -1145,6 +1145,61 @@
template.awardKarma(translator, 'translationsuggestionapproved')
template.awardKarma(reviewer, 'translationreview')
+ def approveAsDiverged(self, pofile, suggestion, reviewer,
+ lock_timestamp=None):
+ """Approve a suggestion to become a diverged translation."""
+ template = pofile.potemplate
+ traits = getUtility(ITranslationSideTraitsSet).getTraits(
+ template.translation_side)
+ if suggestion.potemplate == template and traits.getFlag(suggestion):
+ # The suggestion is already current and diverged for the
+ # right template.
+ return suggestion
+
+ incumbent = traits.getCurrentMessage(self, template, pofile.language)
+ if incumbent is not None:
+ self._checkForConflict(incumbent, lock_timestamp)
+ if incumbent.is_diverged:
+ # The incumbent is in the way. Disable it.
+ traits.setFlag(incumbent, False)
+ incumbent.shareIfPossible()
+ else:
+ # This incumbent isn't really in the way.
+ incumbent = None
+
+ if traits.other_side_traits.getFlag(suggestion):
+ # The suggestion is already current on the other side. Can't
+ # reuse it as a diverged message here, so clone it.
+ message = None
+ elif not traits.getFlag(suggestion):
+ # No obstacles. Diverge & activate.
+ suggestion.potemplate = template
+ traits.setFlag(suggestion, True)
+ message = reviewed_message = suggestion
+ elif suggestion.is_diverged:
+ # The suggestion is already current in another template.
+ # Can't reuse it as a diverged message here, so clone it.
+ message = None
+ else:
+ # This message is already the shared current message. If it
+ # was previously masked by a diverged message, it no longer
+ # is. This is probably the behaviour the user would expect.
+ message = suggestion
+ reviewed_message = incumbent
+
+ if message is None:
+ potranslations = self._findPOTranslations(
+ dict(enumerate(suggestion.translations)))
+ message = reviewed_message = self._makeTranslationMessage(
+ pofile, suggestion.submitter, potranslations,
+ suggestion.origin, diverged=True)
+ traits.setFlag(message, True)
+
+ pofile.markChanged()
+ if reviewed_message is not None:
+ reviewed_message.markReviewed(reviewer)
+ return message
+
def setCurrentTranslation(self, pofile, submitter, translations, origin,
share_with_other_side=False,
lock_timestamp=None):
=== modified file 'lib/lp/translations/model/translationmessage.py'
--- lib/lp/translations/model/translationmessage.py 2010-08-24 11:39:06 +0000
+++ lib/lp/translations/model/translationmessage.py 2010-09-02 11:57:45 +0000
@@ -175,6 +175,10 @@
"""See `ITranslationMessage`."""
raise NotImplementedError()
+ def approveAsDiverged(self, *args, **kwargs):
+ """See `ITranslationMessage`."""
+ raise NotImplementedError()
+
def getOnePOFile(self):
"""See `ITranslationMessage`."""
return None
@@ -348,6 +352,11 @@
share_with_other_side=share_with_other_side,
lock_timestamp=lock_timestamp)
+ def approveAsDiverged(self, pofile, reviewer, lock_timestamp=None):
+ """See `ITranslationMessage`."""
+ return self.potmsgset.approveAsDiverged(
+ pofile, self, reviewer, 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 2010-08-24 11:39:06 +0000
+++ lib/lp/translations/tests/test_translationmessage.py 2010-09-02 11:57:45 +0000
@@ -269,6 +269,210 @@
pofile, self.factory.makePerson(), lock_timestamp=old)
+class TestApproveAsDiverged(TestCaseWithFactory):
+ """Tests for `TranslationMessage.approveAsDiverged`."""
+
+ layer = ZopelessDatabaseLayer
+
+ def _makeCounterpartPOFile(self, pofile):
+ """Make a `POFile` on the opposite translation side.
+
+ :param pofile: A `POFile` to match. Assumed to be on the
+ "upstream" translation side.
+ """
+ assert pofile.potemplate.productseries is not None, (
+ "This test needs a product template; got a package template.""")
+ other_template = self.factory.makePOTemplate(
+ distroseries=self.factory.makeDistroSeries(),
+ sourcepackagename=self.factory.makeSourcePackageName())
+
+ return self.factory.makePOFile(
+ pofile.language.code, potemplate=other_template)
+
+ def test_makeCounterpartPOFile(self):
+ # self.factory.makePOFile makes POFiles on the upstream side by
+ # default. self._makeCounterpartPOFile makes POFiles on the
+ # Ubuntu side.
+ pofile = self.factory.makePOFile('es')
+ other_pofile = self._makeCounterpartPOFile(pofile)
+
+ self.assertEqual(pofile.language, other_pofile.language)
+ self.assertNotEqual(
+ pofile.potemplate.translation_side,
+ other_pofile.potemplate.translation_side)
+
+ def test_no_change(self):
+ # Approving and diverging a message that's already active and
+ # diverged for this POFile does nothing. Even the reviewer
+ # stays unchanged.
+ translator = self.factory.makePerson()
+ original_reviewer = self.factory.makePerson()
+ later_reviewer = self.factory.makePerson()
+ pofile = self.factory.makePOFile('es_AR')
+ message = self.factory.makeDivergedTranslationMessage(
+ pofile=pofile, translator=translator, reviewer=original_reviewer)
+
+ resulting_message = message.approveAsDiverged(pofile, later_reviewer)
+
+ self.assertEqual(message, resulting_message)
+ self.assertEqual(translator, message.submitter)
+ self.assertEqual(original_reviewer, message.reviewer)
+ self.assertEqual(pofile.potemplate, message.potemplate)
+
+ def test_activates_and_diverges(self):
+ # Approving a simple suggestion activates and diverges it.
+ pofile = self.factory.makePOFile('es_BO')
+ suggestion = self.factory.makeSuggestion(pofile=pofile)
+
+ resulting_message = suggestion.approveAsDiverged(
+ pofile, self.factory.makePerson())
+
+ self.assertEqual(suggestion, resulting_message)
+ self.assertTrue(suggestion.is_current_upstream)
+ self.assertEqual(pofile.potemplate, suggestion.potemplate)
+
+ def test_activating_reviews(self):
+ # Activating a message marks it as reviewed.
+ pofile = self.factory.makePOFile('es_BO')
+ reviewer = self.factory.makePerson()
+ suggestion = self.factory.makeSuggestion(pofile=pofile)
+
+ resulting_message = suggestion.approveAsDiverged(pofile, reviewer)
+
+ self.assertEqual(reviewer, resulting_message.reviewer)
+
+ def test_diverge_current_shared_leaves_message_intact(self):
+ # Approving and diverging a message that's already the current
+ # shared message leaves it untouched.
+ original_reviewer = self.factory.makePerson()
+ later_reviewer = self.factory.makePerson()
+ pofile = self.factory.makePOFile('es_CL')
+ message = self.factory.makeCurrentTranslationMessage(
+ pofile=pofile, reviewer=original_reviewer)
+
+ resulting_message = message.approveAsDiverged(pofile, later_reviewer)
+
+ self.assertEqual(message, resulting_message)
+ self.assertEqual(original_reviewer, message.reviewer)
+ self.assertIs(None, message.potemplate)
+
+ def test_diverge_current_shared_message_unmasks_it(self):
+ pofile = self.factory.makePOFile('es_CO')
+ reviewer = self.factory.makePerson()
+ shared = self.factory.makeCurrentTranslationMessage(pofile=pofile)
+ diverged = self.factory.makeDivergedTranslationMessage(
+ pofile=pofile, potmsgset=shared.potmsgset)
+
+ self.assertTrue(shared.is_current_upstream)
+ self.assertTrue(diverged.is_current_upstream)
+ self.assertFalse(shared.is_diverged)
+ self.assertTrue(diverged.is_diverged)
+
+ shared.approveAsDiverged(pofile, reviewer)
+
+ self.assertTrue(shared.is_current_upstream)
+ self.assertFalse(diverged.is_current_upstream)
+ self.assertFalse(shared.is_diverged)
+
+ def test_does_not_affect_other_side(self):
+ # Approving a message that's current on the other side clones
+ # it, so that the other side remains unaffected by this local
+ # change.
+ reviewer = self.factory.makePerson()
+ pofile = self.factory.makePOFile('es_CR')
+ other_pofile = self._makeCounterpartPOFile(pofile)
+ suggestion = self.factory.makeCurrentTranslationMessage(
+ pofile=other_pofile)
+ self.assertEqual(
+ (False, True),
+ (suggestion.is_current_upstream, suggestion.is_current_ubuntu))
+
+ resulting_message = suggestion.approveAsDiverged(pofile, reviewer)
+
+ self.assertNotEqual(suggestion, resulting_message)
+ self.assertEqual(pofile.potemplate, resulting_message.potemplate)
+ self.assertIs(None, suggestion.potemplate)
+ self.assertTrue(resulting_message.is_current_upstream)
+ self.assertEqual(
+ (False, True),
+ (suggestion.is_current_upstream, suggestion.is_current_ubuntu))
+
+ def test_does_not_affect_diverged_elsewhere(self):
+ # Approving a message that's current and diverged to another
+ # template clones it, so that the other template remains
+ # unaffected by this local change.
+ reviewer = self.factory.makePerson()
+ pofile = self.factory.makePOFile('es_DO')
+ elsewhere = self.factory.makePOFile('es_DO')
+
+ suggestion = self.factory.makeDivergedTranslationMessage(
+ pofile=elsewhere)
+
+ resulting_message = suggestion.approveAsDiverged(pofile, reviewer)
+
+ self.assertNotEqual(suggestion, resulting_message)
+ self.assertEqual(pofile.potemplate, resulting_message.potemplate)
+ self.assertEqual(elsewhere.potemplate, suggestion.potemplate)
+ self.assertTrue(resulting_message.is_current_upstream)
+ self.assertTrue(suggestion.is_current_upstream)
+
+ def test_detects_conflict(self):
+ # Trying to approve and diverge a message based on outdated
+ # information raises TranslationConflict.
+ reviewer = self.factory.makePerson()
+ pofile = self.factory.makePOFile('es_EC')
+ suggestion = self.factory.makeSuggestion(pofile=pofile)
+ current = self.factory.makeDivergedTranslationMessage(
+ pofile=pofile, potmsgset=suggestion.potmsgset)
+ earlier = current.date_reviewed - timedelta(1)
+
+ self.assertRaises(
+ TranslationConflict,
+ suggestion.approveAsDiverged,
+ pofile, reviewer, lock_timestamp=earlier)
+
+ def test_passes_conflict_check_if_no_conflict(self):
+ # Trying to approve and diverge a message based on up-to-date
+ # works without raising a conflict.
+ reviewer = self.factory.makePerson()
+ pofile = self.factory.makePOFile('es_GT')
+ suggestion = self.factory.makeSuggestion(pofile=pofile)
+ current = self.factory.makeDivergedTranslationMessage(
+ pofile=pofile, potmsgset=suggestion.potmsgset)
+ later = current.date_reviewed + timedelta(1)
+
+ suggestion.approveAsDiverged(pofile, reviewer, lock_timestamp=later)
+
+ self.assertTrue(suggestion.is_current_upstream)
+
+ def test_passes_conflict_check_if_same_translations(self):
+ # Trying to approve and diverge a message based on outdated
+ # information works just fine if in the meantime the suggestion
+ # has become the diverged current translation.
+ reviewer = self.factory.makePerson()
+ pofile = self.factory.makePOFile('es_HN')
+ current = self.factory.makeDivergedTranslationMessage(pofile=pofile)
+ earlier = current.date_reviewed - timedelta(1)
+
+ current.approveAsDiverged(pofile, reviewer, lock_timestamp=earlier)
+
+ self.assertTrue(current.is_current_upstream)
+
+ def test_updates_pofile(self):
+ # Approving a message as a diverged translation marks the POFile
+ # as updated.
+ reviewer = self.factory.makePerson()
+ pofile = self.factory.makePOFile('es_MX')
+ suggestion = self.factory.makeSuggestion(pofile=pofile)
+ earlier = pofile.date_changed - timedelta(1)
+ pofile.markChanged(timestamp=earlier)
+
+ self.assertEqual(earlier, pofile.date_changed)
+ suggestion.approveAsDiverged(pofile, reviewer)
+ self.assertNotEqual(earlier, pofile.date_changed)
+ self.assertEqual(suggestion.date_reviewed, pofile.date_changed)
+
+
class TestTranslationMessageFindIdenticalMessage(TestCaseWithFactory):
"""Tests for `TranslationMessage.findIdenticalMessage`."""
