Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/recife-review into
lp:~launchpad/launchpad/recife.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers): code
= Set messages as reviewed =
For the Recife feature branch.
When a TranslationMessage is selected to be a message's current translation, we
update its review date (so we can later see which suggestions came in since
last review) and keep track of its reviewer. Here I implement this for the
Recife data model which is to replace our current one.
We have agreed not to do this in setCurrentTranslation, but one level higher in
the call tree. We already had it implemented in approveSuggestion, and we
haven't implemented the method for divergence yet. So that left the change to
be made in clearCurrentTranslation. This method didn't even always leave a
message current, which means there might not be any place to mark the review
date at all.
We do have a standard way of dealing with this: create an empty
TranslationMessage. This will show up as "untranslated," just like the absence
of a current TM does, but it also acts as a review marker to compare
suggestions' submission dates against.
Adding a message gets complicated: what of divergence, what of tracking between
upstream and Ubuntu, what of existing messages, what of clashes between demoted
diverged messages and existing shared ones? Rather than deal with it all
again, I just call setCurrentTranslation to enable a blank message. This
simplifies the code a lot. One white-box test failed, but it actually asserted
that there would be no current message left. That is no longer the case, so I
scratched the test.
We also gained some helpful factory methods for TranslationMessages since the
original clearCurrentTranslation was written, so I converted the tests to use
them. This exposed another white-box test that we no longer need: what if we
deactivate a shared translation message when there's already a suggestion (a
non-current shared translation message) with the same contents? That's not
really supposed to be possible and moreover, it is now for
setCurrentTranslation to worry about.
To test,
{{{
./bin/test -vvc -m lp.translations.tests.test_clearcurrenttranslation
}}}
(I decapitalized the test name).
No lint.
Jeroen
--
https://code.launchpad.net/~jtv/launchpad/recife-review/+merge/33183
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~jtv/launchpad/recife-review into lp:~launchpad/launchpad/recife.
=== modified file 'lib/lp/translations/model/potmsgset.py'
--- lib/lp/translations/model/potmsgset.py 2010-08-19 09:37:13 +0000
+++ lib/lp/translations/model/potmsgset.py 2010-08-20 04:52:42 +0000
@@ -1275,48 +1275,10 @@
def clearCurrentTranslation(self, pofile, submitter, origin,
share_with_other_side=False):
"""See `IPOTMsgSet`."""
- template = pofile.potemplate
- traits = getUtility(ITranslationSideTraitsSet).getTraits(
- template.translation_side)
-
- 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.markReviewed(submitter, UTC_NOW)
- traits.setFlag(mask, True)
- else:
- traits.setFlag(current, False)
- if share_with_other_side:
- traits.other_side_traits.setFlag(current, False)
+ message = self.setCurrentTranslation(
+ pofile, submitter, [], origin,
+ share_with_other_side=share_with_other_side)
+ message.markReviewed(submitter)
def applySanityFixes(self, text):
"""See `IPOTMsgSet`."""
=== modified file 'lib/lp/translations/tests/test_clearCurrentTranslation.py'
--- lib/lp/translations/tests/test_clearCurrentTranslation.py 2010-08-10 07:39:01 +0000
+++ lib/lp/translations/tests/test_clearCurrentTranslation.py 2010-08-20 04:52:42 +0000
@@ -5,6 +5,13 @@
__metaclass__ = type
+from datetime import (
+ datetime,
+ timedelta,
+ )
+
+from pytz import UTC
+
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
@@ -59,16 +66,15 @@
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)
-
+ message = self.factory.makeSuggestion(
+ pofile=pofile, potmsgset=potmsgset, translations=translations)
if diverged:
removeSecurityProxy(message).potemplate = pofile.potemplate
return message
- def test_does_nothing_if_not_translated(self):
+ def test_creates_empty_message(self):
+ # Even if there is no current message, clearCurrentTranslation
+ # will create an empty message so as to mark the review time.
pofile = self._makePOFile()
template = pofile.potemplate
potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
@@ -77,7 +83,9 @@
current = get_traits(template).getCurrentMessage(
potmsgset, template, pofile.language)
- self.assertIs(None, current)
+ self.assertEqual(
+ [],
+ [msgstr for msgstr in current.translations if msgstr is not None])
def test_deactivates_shared_message(self):
pofile = self._makePOFile()
@@ -176,22 +184,6 @@
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)
- get_traits(template).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
@@ -210,6 +202,54 @@
potmsgset, template, pofile.language)
self.assertEqual(blank_shared_tm, current)
+ def test_reviews_new_blank(self):
+ # When clearCurrentTranslation creates a blank message in order
+ # to mark the review, the blank message does indeed have its
+ # review fields set.
+ pofile = self._makePOFile()
+ template = pofile.potemplate
+ potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
+ reviewer = self.factory.makePerson()
+
+ potmsgset.clearCurrentTranslation(pofile, reviewer, ORIGIN)
+
+ blank = get_traits(template).getCurrentMessage(
+ potmsgset, template, pofile.language)
+
+ self.assertNotEqual(None, blank.date_reviewed)
+ self.assertEqual(reviewer, blank.reviewer)
+
+ def test_reviews_existing_blank(self):
+ # When clearCurrentTranslation reuses an existing blank message
+ # in order to mark the review, the blank message's review
+ # information is updated.
+ pofile = self._makePOFile()
+ template = pofile.potemplate
+ traits = get_traits(template)
+ potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
+ blank = self.factory.makeSuggestion(
+ potmsgset=potmsgset, pofile=pofile, translations=[])
+
+ old_review_date = datetime.now(UTC) - timedelta(days=7, seconds=1)
+ old_reviewer = self.factory.makePerson()
+ blank.markReviewed(old_reviewer, timestamp=old_review_date)
+
+ current = self.factory.makeCurrentTranslationMessage(
+ pofile=pofile, potmsgset=potmsgset)
+
+ new_reviewer = self.factory.makePerson()
+
+ potmsgset.clearCurrentTranslation(pofile, new_reviewer, ORIGIN)
+
+ current = traits.getCurrentMessage(
+ potmsgset, template, pofile.language)
+
+ self.assertEqual(new_reviewer, current.reviewer)
+ self.assertBetween(
+ old_review_date + timedelta(days=7),
+ current.date_reviewed,
+ datetime.now(UTC) + timedelta(seconds=1))
+
class TestClearCurrentTranslationUpstream(TestCaseWithFactory,
ScenarioMixin):
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
