Merge lp:~jtv/launchpad/bug-488218 into lp:launchpad

Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~jtv/launchpad/bug-488218
Merge into: lp:launchpad
Diff against target: 325 lines (+105/-61)
7 files modified
lib/lp/translations/browser/person.py (+2/-0)
lib/lp/translations/doc/pofile.txt (+39/-12)
lib/lp/translations/model/pofile.py (+9/-2)
lib/lp/translations/model/potmsgset.py (+14/-11)
lib/lp/translations/stories/standalone/xx-translations-to-complete.txt (+25/-0)
lib/lp/translations/tests/test_pofile.py (+1/-0)
lib/lp/translations/tests/test_potmsgset.py (+15/-36)
To merge this branch: bzr merge lp:~jtv/launchpad/bug-488218
Reviewer Review Type Date Requested Status
Michael Nelson (community) code Approve
Review via email: mp+15247@code.launchpad.net

Commit message

Attribute "credits" translationmessages to rosetta_experts; never list rosetta_experts as contributor.

To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

= Bug 488218 =

Some translatable messages (POTMsgSets) are special. They are not
really translatable; they represent translation credits and their
"translations" are generated on-demand.

In order to stop these messages from showing up as untranslated, we
create special, "fake" translation messages for them.

But every time we create a TranslationMessage, the fact is recorded as a
contribution made by the message's translator. In this case, the last
translator for the POFile was recorded as the translator for the credits
message; or failing that, its owner.

So we had phantom "contributions" based on these automatically generated
messages. This branch fixes that by:

1. Making the Rosetta Experts team the translator for these messages.

2. Never listing Rosetta Experts as a contributor. Translations are
made by persons, not teams, and this team isn't for translation as such
anyway.

The tests for ownership are simplified; a new test shows explicitly that
the Rosetta Experts team is ignored in the contributors list. You'll
note a small drive-by there as well.

To test:
{{{
./bin/test -vv -t pofile.txt -t test_pofile -t test_potmsgset -t translations-to-complete
}}}

To Q/A: import a template with a translation credits message. For
example:

msgid ""
msgstr ""
"MIME-Version: 1.0\n"
"Content-Type: text/plain; charset=UTF-8\n"
"Content-Transfer-Encoding: 8bit\n"

msgid "Text"
msgstr ""

msgid "translation-credits"
msgstr ""

Then visit a (still blank) translation of the template, and log out from
there. This will create (because of a known, low-priority bug) a POFile
for that translation.

Log back in and look at your personal Translations page, under Activity.
There should be no reference to the POFile you just created; without
this branch, there is.

Also look at the translation credits message. Both before and after, it
will show your username. But previously it would show you as the
translator; now it will show Rosetta Administrators as the translator.

Finally, there turned out to be an oops waiting to happen when visiting
the Translations page for rosetta_experts, because the browser code
recognized that the team is now active as a translator, and was trying
to list translations the team could help complete. Teams should not be
considered translators in that context. I added a bit of pagetest.

Jeroen

Revision history for this message
Michael Nelson (michael.nelson) wrote :
Download full text (14.4 KiB)

Hi Jeroen,

Thanks for all the multiple drive-by's!

r=me, but two thoughts. The first is general - is there not a better way to handle
these translation credits so that they're not translatable messages
at all? I guess not, otherwise you would have done it.

Second thought: you said in your cover letter that "translations are made
by persons, not teams", but translations are still displayed for other
teams - just not the rosetta experts, is that right?
(eg. http://pastebin.ubuntu.com/328399/), if so, why? (just for my
understanding)

> === modified file 'lib/lp/translations/browser/person.py'
> --- lib/lp/translations/browser/person.py 2009-09-17 20:11:48 +0000
> +++ lib/lp/translations/browser/person.py 2009-11-26 10:47:06 +0000
> @@ -215,6 +215,8 @@
> @property
> def person_is_translator(self):
> """Is this person active in translations?"""
> + if self.context.isTeam():
> + return False
> person = ITranslationsPerson(self.context)
> history = person.getTranslationHistory(self.history_horizon).any()
> return history is not None
>
> === modified file 'lib/lp/translations/doc/pofile.txt'
> --- lib/lp/translations/doc/pofile.txt 2009-10-29 17:46:00 +0000
> +++ lib/lp/translations/doc/pofile.txt 2009-11-26 10:47:06 +0000
> @@ -1031,8 +1031,17 @@
> The 'contributors' property of a POFile returns all the people who contributed
> translations to it.
>
> - >>> [person.name for person in evolution_es.contributors]
> - [u'carlos', u'mark', u'no-priv']
> + >>> def print_names(persons):
> + ... """Print name for each of `persons`."""
> + ... for person in persons:
> + ... print person.name
> + ... print "--"
> +
> + >>> print_names(evolution_es.contributors)
> + carlos
> + mark
> + no-priv
> + --
>
> If you have a distroseries and want to know all the people who contributed
> translations on a given language for that distroseries, you can use
> @@ -1042,9 +1051,16 @@
> >>> from lp.registry.model.distroseries import DistroSeries
> >>> hoary = DistroSeries.selectOneBy(name="hoary")
> >>> spanish = Language.selectOneBy(code="es")
> - >>> [p.name for p in hoary.getPOFileContributorsByLanguage(spanish)]
> - [u'jorge-gonzalez-gonzalez', u'carlos', u'valyag', u'danner', u'name16',
> - u'name12', u'ubuntu-translators', u'tsukimi']
> + >>> print_names(hoary.getPOFileContributorsByLanguage(spanish))
> + jorge-gonzalez-gonzalez
> + carlos
> + valyag
> + danner
> + name16
> + name12
> + ubuntu-translators
> + tsukimi
> + --
>
> # We can see that there is another translator that doesn't appear in
> # previous list because the template he translated is not current.
> @@ -1052,8 +1068,9 @@
> >>> non_current_pofile.potemplate.iscurrent
> False
>
> - >>> [p.name for p in non_current_pofile.contributors]
> - [u'jordi']
> + >>> print_names(non_current_pofile.contributors)
> + jordi
> + --
>
> >>> non_current_pofile.potemplate.distroseries == hoary
> True
> @@ -1061,6 +1078,17 @@
> >>> non_current_pofile.language == spanish
> True...

review: Approve (code)
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Thanks for the review!

> r=me, but two thoughts. The first is general - is there not a better way to
> handle
> these translation credits so that they're not translatable messages
> at all? I guess not, otherwise you would have done it.

Right. It's actually an elegant solution in the problem domain that these are in all respects translatable messages. Another way of looking at it is that we, as an implementation, just happen to automate their translation.

> Second thought: you said in your cover letter that "translations are made
> by persons, not teams", but translations are still displayed for other
> teams - just not the rosetta experts, is that right?
> (eg. http://pastebin.ubuntu.com/328399/), if so, why? (just for my
> understanding)

Yes. One of the tests also showed translations made by another team, though I don't know how it was supposed to arrive at a situation where those can exist. One way I could think of, though, is by changing a person into a team. I believe that's possible, though I haven't checked.

Functionally, there's nothing wrong with crediting team contributions if they should exist. It's really just in the case of rosetta_experts that listing the team as a contributor becomes totally meaningless.

Jeroen

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/translations/browser/person.py'
2--- lib/lp/translations/browser/person.py 2009-09-17 20:11:48 +0000
3+++ lib/lp/translations/browser/person.py 2009-11-25 17:00:32 +0000
4@@ -215,6 +215,8 @@
5 @property
6 def person_is_translator(self):
7 """Is this person active in translations?"""
8+ if self.context.isTeam():
9+ return False
10 person = ITranslationsPerson(self.context)
11 history = person.getTranslationHistory(self.history_horizon).any()
12 return history is not None
13
14=== modified file 'lib/lp/translations/doc/pofile.txt'
15--- lib/lp/translations/doc/pofile.txt 2009-10-29 17:46:00 +0000
16+++ lib/lp/translations/doc/pofile.txt 2009-11-25 17:00:32 +0000
17@@ -1031,8 +1031,17 @@
18 The 'contributors' property of a POFile returns all the people who contributed
19 translations to it.
20
21- >>> [person.name for person in evolution_es.contributors]
22- [u'carlos', u'mark', u'no-priv']
23+ >>> def print_names(persons):
24+ ... """Print name for each of `persons`."""
25+ ... for person in persons:
26+ ... print person.name
27+ ... print "--"
28+
29+ >>> print_names(evolution_es.contributors)
30+ carlos
31+ mark
32+ no-priv
33+ --
34
35 If you have a distroseries and want to know all the people who contributed
36 translations on a given language for that distroseries, you can use
37@@ -1042,9 +1051,16 @@
38 >>> from lp.registry.model.distroseries import DistroSeries
39 >>> hoary = DistroSeries.selectOneBy(name="hoary")
40 >>> spanish = Language.selectOneBy(code="es")
41- >>> [p.name for p in hoary.getPOFileContributorsByLanguage(spanish)]
42- [u'jorge-gonzalez-gonzalez', u'carlos', u'valyag', u'danner', u'name16',
43- u'name12', u'ubuntu-translators', u'tsukimi']
44+ >>> print_names(hoary.getPOFileContributorsByLanguage(spanish))
45+ jorge-gonzalez-gonzalez
46+ carlos
47+ valyag
48+ danner
49+ name16
50+ name12
51+ ubuntu-translators
52+ tsukimi
53+ --
54
55 # We can see that there is another translator that doesn't appear in
56 # previous list because the template he translated is not current.
57@@ -1052,8 +1068,9 @@
58 >>> non_current_pofile.potemplate.iscurrent
59 False
60
61- >>> [p.name for p in non_current_pofile.contributors]
62- [u'jordi']
63+ >>> print_names(non_current_pofile.contributors)
64+ jordi
65+ --
66
67 >>> non_current_pofile.potemplate.distroseries == hoary
68 True
69@@ -1061,6 +1078,17 @@
70 >>> non_current_pofile.language == spanish
71 True
72
73+The rosetta_experts team is special: it never shows up in contributors
74+lists.
75+
76+ >>> experts_pofile = factory.makePOFile('nl')
77+ >>> experts_message = factory.makeTranslationMessage(
78+ ... pofile=experts_pofile, translator=rosetta_experts,
79+ ... reviewer=rosetta_experts, translations=['hi'])
80+
81+ >>> print_names(experts_pofile.contributors)
82+ --
83+
84
85 == getPOTMsgSetTranslated ==
86
87@@ -1191,7 +1219,6 @@
88 Happy translator
89 Launchpad Contributions:
90 Carlos ...
91- Rosetta Administrators...
92
93 Emails of translators just has the dummy translation.
94
95@@ -1210,7 +1237,7 @@
96
97 >>> print alsa_translation.prepareTranslationCredits(
98 ... emails_of_translators)
99- ,,carlos@canonical.com,rosetta@launchpad.net
100+ ,,carlos@canonical.com
101
102 Names of translators just has the dummy translation.
103
104@@ -1228,7 +1255,7 @@
105
106 >>> print alsa_translation.prepareTranslationCredits(
107 ... names_of_translators)
108- ,Launchpad Contributions:,Carlos P... M...,Rosetta Administrators
109+ ,Launchpad Contributions:,Carlos P... M...
110
111 Lets add a newly supported KDE context message with credit strings
112 split into two: context and message, instead of a single message with
113@@ -1241,7 +1268,7 @@
114 prepareTranslationCredits works on such messages as well:
115
116 >>> print alsa_translation.prepareTranslationCredits(new_kde_name_credits)
117- ,Launchpad Contributions:,Carlos P... M...,Rosetta Administrators
118+ ,Launchpad Contributions:,Carlos P... M...
119
120 Similar happens for email credits as with name credits.
121
122@@ -1249,7 +1276,7 @@
123 ... singular_text=u"Your emails", plural_text=None,
124 ... context=u"EMAIL OF TRANSLATORS")
125 >>> print alsa_translation.prepareTranslationCredits(new_kde_email_credits)
126- ,,carlos@canonical.com,rosetta@launchpad.net
127+ ,,carlos@canonical.com
128
129
130 == POFileToTranslationFileDataAdapter ==
131
132=== modified file 'lib/lp/translations/model/pofile.py'
133--- lib/lp/translations/model/pofile.py 2009-11-17 09:50:33 +0000
134+++ lib/lp/translations/model/pofile.py 2009-11-25 17:00:32 +0000
135@@ -36,6 +36,7 @@
136 from canonical.launchpad.webapp.publisher import canonical_url
137 from canonical.librarian.interfaces import ILibrarianClient
138 from lp.registry.interfaces.person import validate_public_person
139+from lp.registry.model.person import Person
140 from lp.translations.utilities.rosettastats import RosettaStats
141 from lp.translations.interfaces.pofile import IPOFile, IPOFileSet
142 from lp.translations.interfaces.potmsgset import (
143@@ -504,10 +505,15 @@
144 @property
145 def contributors(self):
146 """See `IPOFile`."""
147- from lp.registry.model.person import Person
148+ # Translation credit messages are "translated" by
149+ # rosetta_experts. Shouldn't show up in contributors lists
150+ # though.
151+ admin_team = getUtility(ILaunchpadCelebrities).rosetta_experts
152+
153 contributors = Person.select("""
154 POFileTranslator.person = Person.id AND
155- POFileTranslator.pofile = %s""" % quote(self),
156+ POFileTranslator.person <> %s AND
157+ POFileTranslator.pofile = %s""" % sqlvalues(admin_team, self),
158 clauseTables=["POFileTranslator"],
159 distinct=True,
160 # XXX: kiko 2006-10-19:
161@@ -516,6 +522,7 @@
162 # function to the column results and then ignore it -- just
163 # like selectAlso does, ironically.
164 orderBy=["Person.displayname", "Person.name"])
165+
166 return contributors
167
168 def prepareTranslationCredits(self, potmsgset):
169
170=== modified file 'lib/lp/translations/model/potmsgset.py'
171--- lib/lp/translations/model/potmsgset.py 2009-11-11 11:29:09 +0000
172+++ lib/lp/translations/model/potmsgset.py 2009-11-25 17:00:32 +0000
173@@ -1053,17 +1053,20 @@
174
175 def setTranslationCreditsToTranslated(self, pofile):
176 """See `IPOTMsgSet`."""
177- if self.is_translation_credit:
178- translation = self.getSharedTranslationMessage(pofile.language)
179- if translation is None:
180- translator = pofile.lasttranslator
181- if translator is None:
182- translator = pofile.owner
183- message = self.updateTranslation(
184- pofile, translator, [credits_message_str],
185- is_imported=False, allow_credits=True,
186- force_shared=True, force_edition_rights=True,
187- lock_timestamp=datetime.datetime.now(pytz.UTC))
188+ if not self.is_translation_credit:
189+ return
190+
191+ if self.getSharedTranslationMessage(pofile.language) is not None:
192+ return
193+
194+ # The credits message has a fixed "translator."
195+ translator = getUtility(ILaunchpadCelebrities).rosetta_experts
196+
197+ message = self.updateTranslation(
198+ pofile, translator, [credits_message_str],
199+ is_imported=False, allow_credits=True,
200+ force_shared=True, force_edition_rights=True,
201+ lock_timestamp=datetime.datetime.now(pytz.UTC))
202
203 def setSequence(self, potemplate, sequence):
204 """See `IPOTMsgSet`."""
205
206=== modified file 'lib/lp/translations/stories/standalone/xx-translations-to-complete.txt'
207--- lib/lp/translations/stories/standalone/xx-translations-to-complete.txt 2009-08-31 13:55:35 +0000
208+++ lib/lp/translations/stories/standalone/xx-translations-to-complete.txt 2009-11-25 17:00:32 +0000
209@@ -62,3 +62,28 @@
210 ... pierre_browser.contents, 'translations-to-complete-table')
211 >>> print tag
212 None
213+
214+
215+Teams
216+-----
217+
218+The Rosetta administrators, as a special case, can act as a translator
219+for automatically generated messages. Nevertheless, its Translations
220+page does not show any translations to complete--even to a member of the team.
221+
222+ >>> login('carlos@canonical.com')
223+ >>> from zope.component import getUtility
224+ >>> from canonical.launchpad.interfaces.launchpad import (
225+ ... ILaunchpadCelebrities)
226+ >>> pofile = factory.makePOFile('ru')
227+ >>> rosetta_experts = getUtility(ILaunchpadCelebrities).rosetta_experts
228+ >>> message = factory.makeTranslationMessage(
229+ ... pofile=pofile, translator=rosetta_experts, translations=['x'])
230+ >>> logout()
231+
232+ >>> carlos_browser = setupBrowser('Basic carlos@canonical.com:test')
233+ >>> experts_home = 'http://translations.launchpad.dev/~rosetta-admins'
234+ >>> carlos_browser.open(experts_home)
235+
236+ >>> print extract_text(find_tag_by_id(
237+ ... jean_browser.contents, 'translations-to-complete-table'))
238
239=== modified file 'lib/lp/translations/tests/test_pofile.py'
240--- lib/lp/translations/tests/test_pofile.py 2009-10-30 10:29:25 +0000
241+++ lib/lp/translations/tests/test_pofile.py 2009-11-25 17:00:32 +0000
242@@ -911,6 +911,7 @@
243 self.assertNotEqual(None, stable_potemplate.getPOFileByLang('eo'))
244 self.assertNotEqual(None, stable_potemplate.getPOFileByLang('de'))
245
246+
247 class TestTranslationCredits(TestCaseWithFactory):
248 """Test generation of translation credits."""
249
250
251=== modified file 'lib/lp/translations/tests/test_potmsgset.py'
252--- lib/lp/translations/tests/test_potmsgset.py 2009-11-11 11:38:22 +0000
253+++ lib/lp/translations/tests/test_potmsgset.py 2009-11-25 17:00:32 +0000
254@@ -15,6 +15,7 @@
255 from zope.security.proxy import isinstance as zope_isinstance
256 from zope.security.proxy import removeSecurityProxy
257
258+from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
259 from lp.registry.interfaces.person import IPersonSet
260 from lp.registry.interfaces.product import IProductSet
261 from lp.services.worlddata.interfaces.language import ILanguageSet
262@@ -23,7 +24,6 @@
263 from lp.translations.interfaces.translationfileformat import (
264 TranslationFileFormat)
265 from lp.translations.interfaces.translationmessage import TranslationConflict
266-from lp.translations.interfaces.translationsperson import ITranslationsPerson
267 from lp.translations.model.translationmessage import (
268 DummyTranslationMessage)
269
270@@ -675,41 +675,20 @@
271 self.assertNotEqual(None, current_shared)
272 self.assertEqual(None, current_shared.potemplate)
273
274- def test_setTranslationCreditsToTranslated_permissions(self):
275- # Even if POFile.owner has not agreed to the Launchpad
276- # translations licensing policy, automated credit
277- # messages are still created.
278- sr_pofile = self.factory.makePOFile('sr', self.devel_potemplate)
279- owner = sr_pofile.owner
280- ITranslationsPerson(owner).translations_relicensing_agreement = False
281- credits_potmsgset = self.factory.makePOTMsgSet(
282- self.devel_potemplate, singular=u'translator-credits')
283- current = credits_potmsgset.getCurrentTranslationMessage(
284- self.devel_potemplate, sr_pofile.language)
285- self.assertEqual(owner, current.submitter)
286-
287- def test_setTranslationCreditsToTranslated_submitter_lasttranslator(self):
288- # Submitter on the automated translation message is set to
289- # POFile.lasttranslator if it's defined.
290- sr_pofile = self.factory.makePOFile('sr', self.devel_potemplate)
291- last_translator = self.factory.makePerson()
292- sr_pofile.lasttranslator = last_translator
293- credits_potmsgset = self.factory.makePOTMsgSet(
294- self.devel_potemplate, singular=u'translator-credits')
295- current = credits_potmsgset.getCurrentTranslationMessage(
296- self.devel_potemplate, sr_pofile.language)
297- self.assertEqual(last_translator, current.submitter)
298-
299- def test_setTranslationCreditsToTranslated_submitter_owner(self):
300- # Submitter on the automated translation message is set to
301- # POFile.owner if POFile.lasttranslator is not defined.
302- sr_pofile = self.factory.makePOFile('sr', self.devel_potemplate)
303- self.assertEqual(None, sr_pofile.lasttranslator)
304- credits_potmsgset = self.factory.makePOTMsgSet(
305- self.devel_potemplate, singular=u'translator-credits')
306- current = credits_potmsgset.getCurrentTranslationMessage(
307- self.devel_potemplate, sr_pofile.language)
308- self.assertEqual(sr_pofile.owner, current.submitter)
309+ def test_setTranslationCreditsToTranslated_submitter(self):
310+ # Submitter on the automated translation message is always
311+ # the rosetta_experts team.
312+ sr_pofile = self.factory.makePOFile('sr', self.devel_potemplate)
313+ translator = self.factory.makePerson()
314+ sr_pofile.lasttranslator = translator
315+ sr_pofile.owner = translator
316+ credits_potmsgset = self.factory.makePOTMsgSet(
317+ self.devel_potemplate, singular=u'translator-credits')
318+ current = credits_potmsgset.getCurrentTranslationMessage(
319+ self.devel_potemplate, sr_pofile.language)
320+
321+ rosetta_experts = getUtility(ILaunchpadCelebrities).rosetta_experts
322+ self.assertEqual(rosetta_experts, current.submitter)
323
324
325 class TestPOTMsgSetSuggestions(TestCaseWithFactory):