Launchpad itself

Merge lp:~jtv/launchpad/recife-review into lp:~launchpad/launchpad/recife

  1. recife-review
  2. Merge into recife
Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 9159
Proposed branch: lp:~jtv/launchpad/recife-review
Merge into: lp:~launchpad/launchpad/recife
Diff against target: 201 lines (+67/-66)
3 files modified
lib/lp/translations/model/potmsgset.py (+4/-42)
lib/lp/translations/model/translationmessage.py (+1/-1)
lib/lp/translations/tests/test_clearcurrenttranslation.py (+62/-23)
To merge this branch: bzr merge lp:~jtv/launchpad/recife-review
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Review via email: mp+33183@code.launchpad.net

Commit message

Review messages in clearCurrentTranslation.

Description of the change

= 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

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

All comments were on IRC :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/translations/model/potmsgset.py'
2--- lib/lp/translations/model/potmsgset.py 2010-08-19 09:37:13 +0000
3+++ lib/lp/translations/model/potmsgset.py 2010-08-20 05:59:41 +0000
4@@ -1275,48 +1275,10 @@
5 def clearCurrentTranslation(self, pofile, submitter, origin,
6 share_with_other_side=False):
7 """See `IPOTMsgSet`."""
8- template = pofile.potemplate
9- traits = getUtility(ITranslationSideTraitsSet).getTraits(
10- template.translation_side)
11-
12- current = traits.getCurrentMessage(self, template, pofile.language)
13- if current is None:
14- # Trivial case: there's nothing to disable.
15- return
16-
17- if current.is_diverged:
18- # Disable the current message.
19- if current.getSharedEquivalent() is not None:
20- current.destroySelf()
21- else:
22- traits.setFlag(current, False)
23-
24- shared = traits.getCurrentMessage(self, template, pofile.language)
25- assert shared is None or not shared.is_diverged, (
26- "I killed a divergence but the current message is still "
27- "diverged.")
28- if shared is not None and not shared.is_empty:
29- # Mask the shared message with a diverged empty message.
30- existing_empty_message = self._findTranslationMessage(
31- pofile, self._findPOTranslations([]), prefer_shared=True)
32- can_reuse = not (
33- existing_empty_message is None or
34- existing_empty_message.is_current_upstream or
35- existing_empty_message.is_current_ubuntu)
36- if can_reuse:
37- existing_empty_message.potemplate = template
38- mask = existing_empty_message
39- else:
40- mask = self._makeTranslationMessage(
41- pofile, submitter, {}, origin, diverged=True)
42-
43- Store.of(mask).add_flush_order(current, mask)
44- mask.markReviewed(submitter, UTC_NOW)
45- traits.setFlag(mask, True)
46- else:
47- traits.setFlag(current, False)
48- if share_with_other_side:
49- traits.other_side_traits.setFlag(current, False)
50+ message = self.setCurrentTranslation(
51+ pofile, submitter, [], origin,
52+ share_with_other_side=share_with_other_side)
53+ message.markReviewed(submitter)
54
55 def applySanityFixes(self, text):
56 """See `IPOTMsgSet`."""
57
58=== modified file 'lib/lp/translations/model/translationmessage.py'
59--- lib/lp/translations/model/translationmessage.py 2010-08-17 06:12:20 +0000
60+++ lib/lp/translations/model/translationmessage.py 2010-08-20 05:59:41 +0000
61@@ -101,7 +101,7 @@
62 def markReviewed(self, reviewer, timestamp=None):
63 """See `ITranslationMessage`."""
64 if timestamp is None:
65- timestamp = datetime.now(UTC)
66+ timestamp = UTC_NOW
67
68 self.reviewer = reviewer
69 self.date_reviewed = timestamp
70
71=== renamed file 'lib/lp/translations/tests/test_clearCurrentTranslation.py' => 'lib/lp/translations/tests/test_clearcurrenttranslation.py'
72--- lib/lp/translations/tests/test_clearCurrentTranslation.py 2010-08-10 07:39:01 +0000
73+++ lib/lp/translations/tests/test_clearcurrenttranslation.py 2010-08-20 05:59:41 +0000
74@@ -5,10 +5,19 @@
75
76 __metaclass__ = type
77
78+from datetime import (
79+ datetime,
80+ timedelta,
81+ )
82+
83+from pytz import UTC
84+
85 from zope.component import getUtility
86 from zope.security.proxy import removeSecurityProxy
87
88+from canonical.database.constants import UTC_NOW
89 from canonical.testing import DatabaseFunctionalLayer
90+
91 from lp.testing import TestCaseWithFactory
92 from lp.translations.interfaces.side import ITranslationSideTraitsSet
93 from lp.translations.interfaces.translationmessage import (
94@@ -59,16 +68,15 @@
95 def _makeTranslationMessage(self, potmsgset, pofile, translations=None,
96 diverged=False):
97 """Create a (non-current) TranslationMessage for potmsgset."""
98- if translations is None:
99- translations = {0: self.factory.getUniqueString()}
100- message = potmsgset.submitSuggestion(
101- pofile, pofile.potemplate.owner, translations)
102-
103+ message = self.factory.makeSuggestion(
104+ pofile=pofile, potmsgset=potmsgset, translations=translations)
105 if diverged:
106 removeSecurityProxy(message).potemplate = pofile.potemplate
107 return message
108
109- def test_does_nothing_if_not_translated(self):
110+ def test_creates_empty_message(self):
111+ # Even if there is no current message, clearCurrentTranslation
112+ # will create an empty message so as to mark the review time.
113 pofile = self._makePOFile()
114 template = pofile.potemplate
115 potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
116@@ -77,7 +85,9 @@
117
118 current = get_traits(template).getCurrentMessage(
119 potmsgset, template, pofile.language)
120- self.assertIs(None, current)
121+ self.assertEqual(
122+ [],
123+ [msgstr for msgstr in current.translations if msgstr is not None])
124
125 def test_deactivates_shared_message(self):
126 pofile = self._makePOFile()
127@@ -176,22 +186,6 @@
128 self.assertFalse(traits.getFlag(tm))
129 self.assertFalse(traits.other_side_traits.getFlag(tm))
130
131- def test_discards_redundant_suggestion(self):
132- translations = [self.factory.getUniqueString()]
133- pofile = self._makePOFile()
134- template = pofile.potemplate
135- potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
136- tm = self._makeTranslationMessage(potmsgset, pofile, translations)
137- get_traits(template).setFlag(tm, True)
138- suggestion = self._makeTranslationMessage(
139- potmsgset, pofile, translations)
140-
141- potmsgset.clearCurrentTranslation(pofile, template.owner, ORIGIN)
142-
143- remaining_tms = list(potmsgset.getAllTranslationMessages())
144- self.assertEqual(1, len(remaining_tms))
145- self.assertIn(remaining_tms[0], [tm, suggestion])
146-
147 def test_converges_with_empty_shared_message(self):
148 pofile = self._makePOFile()
149 template = pofile.potemplate
150@@ -210,6 +204,51 @@
151 potmsgset, template, pofile.language)
152 self.assertEqual(blank_shared_tm, current)
153
154+ def test_reviews_new_blank(self):
155+ # When clearCurrentTranslation creates a blank message in order
156+ # to mark the review, the blank message does indeed have its
157+ # review fields set.
158+ pofile = self._makePOFile()
159+ template = pofile.potemplate
160+ potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
161+ reviewer = self.factory.makePerson()
162+
163+ potmsgset.clearCurrentTranslation(pofile, reviewer, ORIGIN)
164+
165+ blank = get_traits(template).getCurrentMessage(
166+ potmsgset, template, pofile.language)
167+
168+ self.assertNotEqual(None, blank.date_reviewed)
169+ self.assertEqual(reviewer, blank.reviewer)
170+
171+ def test_reviews_existing_blank(self):
172+ # When clearCurrentTranslation reuses an existing blank message
173+ # in order to mark the review, the blank message's review
174+ # information is updated.
175+ pofile = self._makePOFile()
176+ template = pofile.potemplate
177+ traits = get_traits(template)
178+ potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
179+ blank = self.factory.makeSuggestion(
180+ potmsgset=potmsgset, pofile=pofile, translations=[])
181+
182+ old_review_date = datetime.now(UTC) - timedelta(days=7)
183+ old_reviewer = self.factory.makePerson()
184+ blank.markReviewed(old_reviewer, timestamp=old_review_date)
185+
186+ current = self.factory.makeCurrentTranslationMessage(
187+ pofile=pofile, potmsgset=potmsgset)
188+
189+ new_reviewer = self.factory.makePerson()
190+
191+ potmsgset.clearCurrentTranslation(pofile, new_reviewer, ORIGIN)
192+
193+ current = traits.getCurrentMessage(
194+ potmsgset, template, pofile.language)
195+
196+ self.assertEqual(new_reviewer, current.reviewer)
197+ self.assertSqlAttributeEqualsDate(current, 'date_reviewed', UTC_NOW)
198+
199
200 class TestClearCurrentTranslationUpstream(TestCaseWithFactory,
201 ScenarioMixin):

Subscribers

People subscribed via source and target branches