launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00485
[Merge] lp:~jtv/launchpad/recife-562858 into lp:~launchpad/launchpad/recife
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/recife-562858 into lp:~launchpad/launchpad/recife.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers): code
Related bugs:
#562858 Clearing translations for POTMsgSet
https://bugs.launchpad.net/bugs/562858
= Bug 562858 =
This is for merging into the Recife feature branch.
A new method, clearCurrentTranslation, makes a translated message untranslated.
This is not as easy as it seems because there can be both a shared translation and a diverged translation masking it; and a translated message can actually be empty which also means it's untranslated (we need this sometimes). When we remove a diverged message we may need to put an empty diverged message in its place so that "clearing" the translation doesn't merely revert to the shared translation, but we shouldn't just create a new message if that just duplicates an existing one—though again we must if the existing one is already in use in some other way.
A shared message may also be the current translation on the "other side" (i.e. in Ubuntu when we're looking at an upstream project, or vice versa). If the message being deactivated is the current translation on both sides, we may want to clear it on both sides, or just on the side we're looking at. This is a policy choice, so passed in by parameter.
You'll also notice some cleanups. In particular, you can now find the translation side traits object for a given POTemplate through a property of the POTemplate itself. After all, potemplate.translation_side_traits is much more convenient than getUtility(ITranslationSideTraitsSet).getForTemplate(potemplate).
To test:
{{{
./bin/test -vvc lp.translations -t test_clearCurrentTranslation -t test_side
}}}
Actually I just ran a larger selection:
{{{
./bin/test -vvc lp.translations.tests
}}}
A few bits of lint left, mainly where comments upset the blank-lines count, but several bits cleaned up as well.
Jeroen
--
https://code.launchpad.net/~jtv/launchpad/recife-562858/+merge/32075
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/recife-562858 into lp:~launchpad/launchpad/recife.
=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py 2010-08-04 11:00:51 +0000
+++ lib/lp/testing/__init__.py 2010-08-09 09:18:46 +0000
@@ -576,7 +576,8 @@
bzr_branch = self.createBranchAtURL(db_branch.getInternalBzrUrl())
if parent:
bzr_branch.pull(parent)
- removeSecurityProxy(db_branch).last_scanned_id = bzr_branch.last_revision()
+ naked_branch = removeSecurityProxy(db_branch)
+ naked_branch.last_scanned_id = bzr_branch.last_revision()
return bzr_branch
@staticmethod
=== modified file 'lib/lp/translations/interfaces/potemplate.py'
--- lib/lp/translations/interfaces/potemplate.py 2010-08-04 11:00:51 +0000
+++ lib/lp/translations/interfaces/potemplate.py 2010-08-09 09:18:46 +0000
@@ -17,6 +17,7 @@
from lp.app.errors import NotFoundError
from lp.registry.interfaces.distribution import IDistribution
from lp.translations.interfaces.rosettastats import IRosettaStats
+from lp.translations.interfaces.side import ITranslationSideTraits
from lp.registry.interfaces.sourcepackagename import (
ISourcePackageName)
from lp.translations.interfaces.translationfileformat import (
@@ -289,6 +290,11 @@
gettext uses the original English strings to identify messages.
"""))
+ translation_side_traits = Object(
+ schema=ITranslationSideTraits,
+ title=_("TranslationSideTraits for this template."), required=True,
+ readonly=True)
+
def __iter__():
"""Return an iterator over current `IPOTMsgSet` in this template."""
@@ -816,6 +822,7 @@
exist for it.
"""
+
class ITranslationTemplatesCollection(Interface):
"""A `Collection` of `POTemplate`s."""
=== modified file 'lib/lp/translations/interfaces/potmsgset.py'
--- lib/lp/translations/interfaces/potmsgset.py 2010-06-28 11:52:52 +0000
+++ lib/lp/translations/interfaces/potmsgset.py 2010-08-09 09:18:46 +0000
@@ -51,6 +51,7 @@
class BrokenTextError(ValueError):
"""Exception raised when we detect values on a text that aren't valid."""
+
class POTMsgSetInIncompatibleTemplatesError(Exception):
"""Raised when a POTMsgSet appears in multiple incompatible templates.
@@ -294,6 +295,25 @@
If a translation conflict is detected, TranslationConflict is raised.
"""
+ def clearCurrentTranslation(pofile, submitter, origin,
+ share_with_other_side=False):
+ """Set the current message in `pofile` to be untranslated.
+
+ If the current message is shared, this will also clear it in
+ other translations that share the same message.
+
+ :param pofile: The translation file that should have its current
+ translation for this `POTMsgSet` cleared. If the message is
+ shared, this may not be the only translation file that will
+ be affected.
+ :param submitter: The person responsible for clearing the message.
+ :param origin: `RosettaTranslationOrigin`.
+ :param share_with_other_side: If the current message is also
+ current on the other side (i.e. the Ubuntu side if working
+ upstream, or vice versa) then should it be cleared there as
+ well?
+ """
+
def applySanityFixes(unicode_text):
"""Return 'unicode_text' or None after doing some sanitization.
=== modified file 'lib/lp/translations/interfaces/translationmessage.py'
--- lib/lp/translations/interfaces/translationmessage.py 2010-08-04 10:22:08 +0000
+++ lib/lp/translations/interfaces/translationmessage.py 2010-08-09 09:18:46 +0000
@@ -217,6 +217,9 @@
def getOnePOFile():
"""Get any POFile containing this translation."""
+ def getSharedEquivalent():
+ """Find shared message that otherwise exactly matches this one."""
+
def shareIfPossible():
"""Make this message shared, if possible.
=== modified file 'lib/lp/translations/model/potemplate.py'
--- lib/lp/translations/model/potemplate.py 2010-08-04 11:00:51 +0000
+++ lib/lp/translations/model/potemplate.py 2010-08-09 09:18:46 +0000
@@ -64,6 +64,9 @@
IPOTemplateSharingSubset,
IPOTemplateSubset,
LanguageNotFound)
+from lp.translations.interfaces.side import (
+ ITranslationSideTraitsSet,
+ TranslationSide)
from lp.translations.interfaces.translationcommonformat import (
ITranslationFileData)
from lp.translations.interfaces.translationexporter import (
@@ -576,8 +579,7 @@
if sharing_templates:
clauses.append(
'TranslationTemplateItem.potemplate in %s' % sqlvalues(
- self._sharing_ids)
- )
+ self._sharing_ids))
else:
clauses.append(
'TranslationTemplateItem.potemplate = %s' % sqlvalues(self))
@@ -978,6 +980,15 @@
for row in rows:
yield VPOTExport(self, *row)
+ @property
+ def translation_side_traits(self):
+ """See `IPOTemplate`."""
+ if self.productseries:
+ side = TranslationSide.UPSTREAM
+ else:
+ side = TranslationSide.UBUNTU
+ return getUtility(ITranslationSideTraitsSet).getTraits(side)
+
class POTemplateSubset:
implements(IPOTemplateSubset)
@@ -1410,7 +1421,7 @@
The translation focus is used to pick a distroseries, so a source
package instance can be created. If no translation focus is set,
the distribution's current series is used."""
-
+
from lp.registry.model.sourcepackage import SourcePackage
distroseries = distribution.translation_focus
if distroseries is None:
@@ -1431,12 +1442,11 @@
origin = Join(
Packaging, DistroSeries,
Packaging.distroseries == DistroSeries.id)
- result = Store.of(distribution).using(origin).find(
+ matches = Store.of(distribution).using(origin).find(
Packaging,
And(DistroSeries.distribution == distribution.id,
- Packaging.sourcepackagename == sourcepackagename.id)
- ).count()
- return result == 0
+ Packaging.sourcepackagename == sourcepackagename.id))
+ return not bool(matches.any())
def _get_potemplate_equivalence_class(self, template):
"""Return whatever we group `POTemplate`s by for sharing purposes."""
@@ -1463,23 +1473,21 @@
origin = LeftJoin(
LeftJoin(
POTemplate, ProductSeries,
- POTemplate.productseriesID == ProductSeries.id),
+ POTemplate.productseriesID == ProductSeries.id),
Join(
Packaging, ProductSeries1,
Packaging.productseriesID == ProductSeries1.id),
And(
Packaging.distroseriesID == POTemplate.distroseriesID,
Packaging.sourcepackagenameID == (
- POTemplate.sourcepackagenameID)
- )
+ POTemplate.sourcepackagenameID))
)
return Store.of(self.product).using(origin).find(
POTemplate,
And(
Or(
ProductSeries.productID == self.product.id,
- ProductSeries1.productID == self.product.id
- ),
+ ProductSeries1.productID == self.product.id),
templatename_clause
))
@@ -1500,8 +1508,7 @@
And(
DistroSeries.distributionID == self.distribution.id,
POTemplate.sourcepackagename == self.sourcepackagename,
- templatename_clause
- ))
+ templatename_clause))
def _queryByDistribution(self, templatename_clause):
"""Special case when templates are searched across a distribution."""
=== modified file 'lib/lp/translations/model/potmsgset.py'
--- lib/lp/translations/model/potmsgset.py 2010-08-09 03:26:48 +0000
+++ lib/lp/translations/model/potmsgset.py 2010-08-09 09:18:46 +0000
@@ -439,8 +439,9 @@
WHERE
POTMsgSet.id <> %s AND
msgid_singular = %s AND
- POTemplate.iscurrent AND (
- Product.official_rosetta OR
+ POTemplate.iscurrent AND
+ COALESCE(
+ Product.official_rosetta,
Distribution.official_rosetta)
)''' % sqlvalues(self, self.msgid_singular))
@@ -1236,6 +1237,52 @@
current.potemplate = None
pofile.date_changed = UTC_NOW
+ def clearCurrentTranslation(self, pofile, submitter, origin,
+ share_with_other_side=False):
+ """See `IPOTMsgSet`."""
+ template = pofile.potemplate
+ traits = template.translation_side_traits
+
+ current = traits.getCurrentMessage(self, template, pofile.language)
+ if current is None:
+ # Trivial case: there's nothing to disable.
+ return
+
+ if current.is_diverged:
+ # Disable the current message.
+ if current.getSharedEquivalent() is not None:
+ current.destroySelf()
+ else:
+ traits.setFlag(current, False)
+
+ shared = traits.getCurrentMessage(self, template, pofile.language)
+ assert shared is None or not shared.is_diverged, (
+ "I killed a divergence but the current message is still "
+ "diverged.")
+ if shared is not None and not shared.is_empty:
+ # Mask the shared message with a diverged empty message.
+ existing_empty_message = self._findTranslationMessage(
+ pofile, self._findPOTranslations([]), prefer_shared=True)
+ can_reuse = not (
+ existing_empty_message is None or
+ existing_empty_message.is_current_upstream or
+ existing_empty_message.is_current_ubuntu)
+ if can_reuse:
+ existing_empty_message.potemplate = template
+ mask = existing_empty_message
+ else:
+ mask = self._makeTranslationMessage(
+ pofile, submitter, {}, origin, diverged=True)
+
+ Store.of(mask).add_flush_order(current, mask)
+ mask.reviewer = submitter
+ mask.date_reviewed = UTC_NOW
+ traits.setFlag(mask, True)
+ else:
+ traits.setFlag(current, False)
+ if share_with_other_side:
+ traits.other_side_traits.setFlag(current, False)
+
def applySanityFixes(self, text):
"""See `IPOTMsgSet`."""
if text is None:
=== modified file 'lib/lp/translations/model/translationmessage.py'
--- lib/lp/translations/model/translationmessage.py 2010-08-09 03:26:48 +0000
+++ lib/lp/translations/model/translationmessage.py 2010-08-09 09:18:46 +0000
@@ -168,6 +168,10 @@
"""See `ITranslationMessage`."""
self.is_current_upstream = new_value
+ def getSharedEquivalent(self):
+ """See `ITranslationMessage`."""
+ raise NotImplementedError()
+
def shareIfPossible(self):
"""See `ITranslationMessage`."""
@@ -327,9 +331,8 @@
else:
return None
- def _getSharedEquivalent(self):
- """Get shared message that otherwise exactly matches this one.
- """
+ def getSharedEquivalent(self):
+ """See `ITranslationMessage`."""
clauses = [
'potemplate IS NULL',
'potmsgset = %s' % sqlvalues(self.potmsgset),
@@ -361,7 +364,7 @@
return
# Existing shared direct equivalent to this message, if any.
- shared = self._getSharedEquivalent()
+ shared = self.getSharedEquivalent()
# Existing shared current translation for this POTMsgSet, if
# any.
=== added file 'lib/lp/translations/tests/test_clearCurrentTranslation.py'
--- lib/lp/translations/tests/test_clearCurrentTranslation.py 1970-01-01 00:00:00 +0000
+++ lib/lp/translations/tests/test_clearCurrentTranslation.py 2010-08-09 09:18:46 +0000
@@ -0,0 +1,217 @@
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for `POTMsgSet.clearCurrentTranslation`."""
+
+__metaclass__ = type
+
+from zope.security.proxy import removeSecurityProxy
+
+from canonical.testing import DatabaseFunctionalLayer
+from lp.testing import TestCaseWithFactory
+from lp.translations.interfaces.translationmessage import (
+ RosettaTranslationOrigin)
+
+ORIGIN = RosettaTranslationOrigin.SCM
+
+
+class ScenarioMixin:
+ layer = DatabaseFunctionalLayer
+
+ def makeUpstreamTemplate(self):
+ """Create a POTemplate for a project."""
+ productseries = self.factory.makeProductSeries()
+ return self.factory.makePOTemplate(productseries=productseries)
+
+ def makeUbuntuTemplate(self):
+ """Create a POTemplate for an Ubuntu package."""
+ package = self.factory.makeSourcePackage()
+ return self.factory.makePOTemplate(
+ distroseries=package.distroseries,
+ sourcepackagename=package.sourcepackagename)
+
+ def _makePOFile(self, potemplate=None):
+ """Create a `POFile` for the given template.
+
+ Also creates a POTemplate if none is given, using
+ self.makePOTemplate.
+ """
+ if potemplate is None:
+ potemplate = self.makePOTemplate()
+ return self.factory.makePOFile('nl', potemplate=potemplate)
+
+ def _makeTranslationMessage(self, potmsgset, pofile, translations=None,
+ diverged=False):
+ """Create a (non-current) TranslationMessage for potmsgset."""
+ if translations is None:
+ translations = {0: self.factory.getUniqueString()}
+ message = potmsgset.submitSuggestion(
+ pofile, pofile.potemplate.owner, translations)
+
+ if diverged:
+ removeSecurityProxy(message).potemplate = pofile.potemplate
+ return message
+
+ def test_does_nothing_if_not_translated(self):
+ pofile = self._makePOFile()
+ template = pofile.potemplate
+ potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
+
+ potmsgset.clearCurrentTranslation(pofile, template.owner, ORIGIN)
+
+ current = template.translation_side_traits.getCurrentMessage(
+ potmsgset, template, pofile.language)
+ self.assertIs(None, current)
+
+ def test_deactivates_shared_message(self):
+ pofile = self._makePOFile()
+ template = pofile.potemplate
+ traits = template.translation_side_traits
+ potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
+ tm = self._makeTranslationMessage(potmsgset, pofile)
+ traits.setFlag(tm, True)
+ self.assertTrue(traits.getFlag(tm))
+
+ potmsgset.clearCurrentTranslation(pofile, template.owner, ORIGIN)
+
+ self.assertFalse(traits.getFlag(tm))
+
+ def test_deactivates_diverged_message(self):
+ pofile = self._makePOFile()
+ template = pofile.potemplate
+ traits = template.translation_side_traits
+ potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
+ tm = self._makeTranslationMessage(potmsgset, pofile, diverged=True)
+ traits.setFlag(tm, True)
+
+ potmsgset.clearCurrentTranslation(pofile, template.owner, ORIGIN)
+
+ self.assertFalse(traits.getFlag(tm))
+
+ def test_hides_unmasked_shared_message(self):
+ # When disabling a diverged message that masks a (nonempty)
+ # shared message, clearCurrentTranslation leaves an empty
+ # diverged message to mask the shared message.
+ pofile = self._makePOFile()
+ template = pofile.potemplate
+ traits = template.translation_side_traits
+ potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
+ shared_tm = self._makeTranslationMessage(potmsgset, pofile)
+ traits.setFlag(shared_tm, True)
+ diverged_tm = self._makeTranslationMessage(
+ potmsgset, pofile, diverged=True)
+ traits.setFlag(diverged_tm, True)
+
+ potmsgset.clearCurrentTranslation(pofile, template.owner, ORIGIN)
+
+ current = traits.getCurrentMessage(
+ potmsgset, template, pofile.language)
+ self.assertNotEqual(shared_tm, current)
+ self.assertNotEqual(diverged_tm, current)
+ self.assertTrue(current.is_empty)
+ self.assertTrue(current.is_diverged)
+ self.assertEqual(template.owner, current.reviewer)
+
+ self.assertTrue(traits.getFlag(shared_tm))
+
+ def test_ignores_other_message(self):
+ pofile = self._makePOFile()
+ template = pofile.potemplate
+ traits = template.translation_side_traits
+ potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
+ tm = self._makeTranslationMessage(potmsgset, pofile)
+ traits.setFlag(tm, True)
+
+ other_template = self.makeOtherPOTemplate()
+ other_pofile = self._makePOFile(potemplate=other_template)
+ other_tm = self._makeTranslationMessage(potmsgset, pofile)
+ traits.other_side_traits.setFlag(other_tm, True)
+
+ potmsgset.clearCurrentTranslation(pofile, template.owner, ORIGIN)
+
+ self.assertTrue(traits.other_side_traits.getFlag(other_tm))
+
+ def test_deactivates_one_side(self):
+ pofile = self._makePOFile()
+ template = pofile.potemplate
+ traits = template.translation_side_traits
+ potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
+ tm = self._makeTranslationMessage(potmsgset, pofile)
+ traits.setFlag(tm, True)
+ traits.other_side_traits.setFlag(tm, True)
+
+ potmsgset.clearCurrentTranslation(pofile, template.owner, ORIGIN)
+
+ self.assertFalse(traits.getFlag(tm))
+ self.assertTrue(traits.other_side_traits.getFlag(tm))
+
+ def test_deactivates_both_sides(self):
+ pofile = self._makePOFile()
+ template = pofile.potemplate
+ traits = template.translation_side_traits
+ potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
+ tm = self._makeTranslationMessage(potmsgset, pofile)
+ traits.setFlag(tm, True)
+ traits.other_side_traits.setFlag(tm, True)
+
+ potmsgset.clearCurrentTranslation(
+ pofile, template.owner, ORIGIN, share_with_other_side=True)
+
+ self.assertFalse(traits.getFlag(tm))
+ self.assertFalse(traits.other_side_traits.getFlag(tm))
+
+ def test_discards_redundant_suggestion(self):
+ translations = [self.factory.getUniqueString()]
+ pofile = self._makePOFile()
+ template = pofile.potemplate
+ potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
+ tm = self._makeTranslationMessage(potmsgset, pofile, translations)
+ template.translation_side_traits.setFlag(tm, True)
+ suggestion = self._makeTranslationMessage(
+ potmsgset, pofile, translations)
+
+ potmsgset.clearCurrentTranslation(pofile, template.owner, ORIGIN)
+
+ remaining_tms = list(potmsgset.getAllTranslationMessages())
+ self.assertEqual(1, len(remaining_tms))
+ self.assertIn(remaining_tms[0], [tm, suggestion])
+
+ def test_converges_with_empty_shared_message(self):
+ pofile = self._makePOFile()
+ template = pofile.potemplate
+ traits = template.translation_side_traits
+ potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
+ diverged_tm = self._makeTranslationMessage(
+ potmsgset, pofile, diverged=True)
+ traits.setFlag(diverged_tm, True)
+ blank_shared_tm = self._makeTranslationMessage(potmsgset, pofile, [])
+ traits.setFlag(blank_shared_tm, True)
+
+ potmsgset.clearCurrentTranslation(pofile, template.owner, ORIGIN)
+
+ self.assertTrue(traits.getFlag(blank_shared_tm))
+ current = traits.getCurrentMessage(
+ potmsgset, template, pofile.language)
+ self.assertEqual(blank_shared_tm, current)
+
+
+class TestClearCurrentTranslationsUpstream(TestCaseWithFactory,
+ ScenarioMixin):
+ """Test clearCurrentTranslationsUpstream on upstream side."""
+ makePOTemplate = ScenarioMixin.makeUpstreamTemplate
+ makeOtherPOTemplate = ScenarioMixin.makeUbuntuTemplate
+
+ def setUp(self):
+ super(TestClearCurrentTranslationsUpstream, self).setUp(
+ 'carlos@xxxxxxxxxxxxx')
+
+
+class TestClearCurrentTranslationsUbuntu(TestCaseWithFactory,
+ ScenarioMixin):
+ """Test clearCurrentTranslationsUpstream on Ubuntu side."""
+ makePOTemplate = ScenarioMixin.makeUbuntuTemplate
+ makeOtherPOTemplate = ScenarioMixin.makeUpstreamTemplate
+
+ def setUp(self):
+ super(TestClearCurrentTranslationsUbuntu, self).setUp(
+ 'carlos@xxxxxxxxxxxxx')
