launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02241
[Merge] lp:~henninge/launchpad/devel-bugjamming-2 into lp:launchpad
Henning Eggers has proposed merging lp:~henninge/launchpad/devel-bugjamming-2 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#440406 Translations overview should state language code
https://bugs.launchpad.net/bugs/440406
#560701 „Not showing suggestions from selected alternative language” warning should link to edit languages page
https://bugs.launchpad.net/bugs/560701
For more details, see:
https://code.launchpad.net/~henninge/launchpad/devel-bugjamming-2/+merge/44590
= Summary =
Fixes two bugs:
Bug 560701: "Not showing suggestions from selected alternative
language" warning should link to edit languages page
Bug 440406: Translations overview should state language code
== Proposed fix ==
Add a link to the user's +editlanguages page to the message.
List languages in person translation tables. The languages can be
extracted from the list of pofiles that is covered by the given
statistic.
== Pre-implementation notes ==
Trivial, no pre-imp.
== Implementation details ==
Pretty straightforward, not much to explain.
The _makePOFiles helper method in test_persontranslationview had to
be adapted to create files for multiple languages but in the same
POTemplate, so they can get aggretated into one.
In that file I also removed the references to the hard-coded language
(Dutch) and used generic languages, which is possible nowadays.
== Tests ==
bin/test -vvcm lp.translations \
-t xx-pofile-translate-alternative-language \
-t test_getTargetsForTranslation \
-t xx-translations-to-review
== Demo and Q/A ==
For bug 560701:
Go to the page listed in the bug and verify that the message
contains a link to +editlanguages.
For bug 440406:
Got to https://translations.launchpad.net/people/+me and verify that
the languages are listed on each line on the table.
= Launchpad lint =
I removed some lint. Yeah!
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/translations/browser/tests/test_persontranslationview.py
lib/lp/translations/stories/standalone/xx-pofile-translate-alternative-language.txt
lib/lp/translations/templates/person-translations-to-complete-table.pt
lib/lp/translations/templates/person-translations-to-review.pt
lib/lp/translations/browser/translationmessage.py
lib/lp/translations/templates/person-translations-to-review-table.pt
lib/lp/translations/browser/person.py
./lib/lp/translations/browser/translationmessage.py
337: E301 expected 1 blank line, found 2
365: E301 expected 1 blank line, found 2
509: E301 expected 1 blank line, found 2
744: E301 expected 1 blank line, found 2
827: E301 expected 1 blank line, found 2
1357: E301 expected 1 blank line, found 2
./lib/lp/translations/browser/person.py
363: E202 whitespace before ']'
--
https://code.launchpad.net/~henninge/launchpad/devel-bugjamming-2/+merge/44590
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~henninge/launchpad/devel-bugjamming-2 into lp:launchpad.
=== modified file 'lib/lp/translations/browser/person.py'
--- lib/lp/translations/browser/person.py 2010-11-23 23:22:27 +0000
+++ lib/lp/translations/browser/person.py 2010-12-23 16:07:17 +0000
@@ -67,6 +67,9 @@
"""See `TranslationLinksAggregator.describe`."""
strings_count = sum(
[self.countStrings(pofile) for pofile in covered_files])
+ languages = set(
+ [pofile.language.englishname for pofile in covered_files])
+ languages_list = ", ".join(sorted(list(languages)))
if strings_count == 1:
strings_wording = "%d string"
@@ -79,6 +82,7 @@
'count_wording': strings_wording % strings_count,
'is_product': not ISourcePackage.providedBy(target),
'link': link,
+ 'languages': languages_list,
}
=== modified file 'lib/lp/translations/browser/tests/test_persontranslationview.py'
--- lib/lp/translations/browser/tests/test_persontranslationview.py 2010-10-04 19:50:45 +0000
+++ lib/lp/translations/browser/tests/test_persontranslationview.py 2010-12-23 16:07:17 +0000
@@ -26,38 +26,52 @@
person = removeSecurityProxy(self.factory.makePerson())
self.view = PersonTranslationView(person, LaunchpadTestRequest())
self.translationgroup = None
- self.dutch = LanguageSet().getLanguageByCode('nl')
- self.view.context.addLanguage(self.dutch)
+ self.language = self.factory.makeLanguage()
+ self.view.context.addLanguage(self.language)
def _makeReviewer(self):
- """Set up the person we're looking at as a Dutch reviewer."""
+ """Set up the person we're looking at as a reviewer."""
owner = self.factory.makePerson()
self.translationgroup = self.factory.makeTranslationGroup(owner=owner)
TranslatorSet().new(
- translationgroup=self.translationgroup, language=self.dutch,
+ translationgroup=self.translationgroup, language=self.language,
translator=self.view.context)
- def _makePOFiles(self, count, previously_worked_on):
+ def _makePOFiles(self, count, previously_worked_on, languages=None):
"""Create `count` `POFile`s that the view's person can review.
:param count: Number of POFiles to create.
:param previously_worked_on: Whether these should be POFiles
that the person has already worked on.
+ :param languages: List of languages for each pofile. The length of
+ the list must be the same as count. If None, all files will be
+ created with self.language. Also, if languages is not None,
+ all files will be created for the same template.
"""
pofiles = []
+ if languages is not None:
+ potemplate = self.factory.makePOTemplate()
for counter in xrange(count):
- pofile = self.factory.makePOFile(language_code='nl')
+ if languages is None:
+ pofile = self.factory.makePOFile(language=self.language)
+ else:
+ pofile = self.factory.makePOFile(
+ potemplate=potemplate, language=languages[counter])
if self.translationgroup:
product = pofile.potemplate.productseries.product
product.translationgroup = self.translationgroup
if previously_worked_on:
+ if languages is not None:
+ sequence = counter+1
+ else:
+ sequence = 1
potmsgset = self.factory.makePOTMsgSet(
- potemplate=pofile.potemplate, singular='x', sequence=1)
+ potemplate=pofile.potemplate, sequence=sequence)
self.factory.makeTranslationMessage(
potmsgset=potmsgset, pofile=pofile,
- translator=self.view.context, translations=['y'])
+ translator=self.view.context)
removeSecurityProxy(pofile).unreviewed_count = 1
pofiles.append(pofile)
@@ -210,11 +224,12 @@
self.assertTrue(self.view.person_is_translator)
- def test_getTargetsForTranslation(self):
+ def test_getTargetsForTranslation_nothing(self):
# If there's nothing to translate, _getTargetsForTranslation
# returns nothing.
self.assertEqual([], self.view._getTargetsForTranslation())
+ def test_getTargetsForTranslation(self):
# If there's a translation that this person has worked on and
# is not a reviewer for, and it has untranslated strings, it
# shows up in _getTargetsForTranslation.
@@ -228,7 +243,26 @@
description = descriptions[0]
self.assertEqual(product, description['target'])
self.assertTrue(description['link'].startswith(canonical_url(pofile)))
-
+ self.assertEqual(
+ pofile.language.englishname, description['languages'])
+
+ def test_getTargetsForTranslation_multiple_languages(self):
+ # Translations in different languages are aggregated to one target
+ # but the language names are listed.
+ other_language = self.factory.makeLanguage()
+ pofiles = self._makePOFiles(
+ 2, previously_worked_on=True,
+ languages=[self.language, other_language])
+ for pofile in pofiles:
+ self._addUntranslatedMessages(pofile, 1)
+
+ descriptions = self.view._getTargetsForTranslation()
+ self.assertEqual(1, len(descriptions))
+ description = descriptions[0]
+ expected_languages = ', '.join(sorted([
+ self.language.englishname, other_language.englishname]))
+ self.assertContentEqual(expected_languages, description['languages'])
+
def test_getTargetsForTranslation_max_fetch(self):
# The max_fetch parameter limits how many POFiles are considered
# by _getTargetsForTranslation. This lets you get the target(s)
@@ -360,7 +394,7 @@
# But the answer is True if the person has no preferred
# languages.
- self.view.context.removeLanguage(self.dutch)
+ self.view.context.removeLanguage(self.language)
self.assertTrue(self.view.requires_preferred_languages)
=== modified file 'lib/lp/translations/browser/translationmessage.py'
--- lib/lp/translations/browser/translationmessage.py 2010-12-01 11:26:57 +0000
+++ lib/lp/translations/browser/translationmessage.py 2010-12-23 16:07:17 +0000
@@ -551,12 +551,18 @@
translations_person = ITranslationsPerson(user)
choices = set(translations_person.translatable_languages)
if choices and alternative_language not in choices:
- self.request.response.addInfoNotification(
+ editlanguages_url = canonical_url(
+ self.user, view_name="+editlanguages")
+ self.request.response.addInfoNotification(structured(
u"Not showing suggestions from selected alternative "
- "language %s. If you wish to see suggestions from "
- "this language, add it to your preferred languages "
- "first."
- % alternative_language.displayname)
+ "language %(alternative)s. If you wish to see "
+ "suggestions from this language, "
+ '<a href="%(editlanguages_url)s">'
+ "add it to your preferred languages</a> first."
+ % dict(
+ alternative=alternative_language.displayname,
+ editlanguages_url=editlanguages_url,
+ )))
alternative_language = None
second_lang_code = None
=== modified file 'lib/lp/translations/stories/standalone/xx-pofile-translate-alternative-language.txt'
--- lib/lp/translations/stories/standalone/xx-pofile-translate-alternative-language.txt 2010-12-01 11:26:57 +0000
+++ lib/lp/translations/stories/standalone/xx-pofile-translate-alternative-language.txt 2010-12-23 16:07:17 +0000
@@ -113,11 +113,16 @@
first.
>>> for message in get_feedback_messages(browser.contents):
- ... print message
+ ... print extract_text(message)
Not showing suggestions from selected alternative language Japanese (ja).
If you wish to see suggestions from this language, add it to your
preferred languages first.
+It even presents a link to where the user can set the preferred languages.
+
+ >>> print browser.getLink("add it to your preferred languages").url
+ http...~carlos/+editlanguages
+
This distinction between alternative languages from the user's preferred set
and other alternative languages does not exist, of course, if no preferred
languages are defined. Suggestions just work for anonymous users.
=== modified file 'lib/lp/translations/templates/person-translations-to-complete-table.pt'
--- lib/lp/translations/templates/person-translations-to-complete-table.pt 2009-09-09 09:51:14 +0000
+++ lib/lp/translations/templates/person-translations-to-complete-table.pt 2010-12-23 16:07:17 +0000
@@ -26,8 +26,10 @@
<tal:stringcount replace="target_info/count_wording">
1 string
</tal:stringcount>
- translated
- </a>
+ translated </a> in
+ <tal:languages replace="target_info/languages">
+ Spanish
+ </tal:languages>
</td>
</tr>
</table>
=== modified file 'lib/lp/translations/templates/person-translations-to-review-table.pt'
--- lib/lp/translations/templates/person-translations-to-review-table.pt 2009-09-09 09:51:14 +0000
+++ lib/lp/translations/templates/person-translations-to-review-table.pt 2010-12-23 16:07:17 +0000
@@ -26,8 +26,10 @@
<tal:stringcount replace="target_info/count_wording">
1 string
</tal:stringcount>
- reviewed
- </a>
+ reviewed </a> in
+ <tal:languages replace="target_info/languages">
+ Spanish
+ </tal:languages>
</td>
</tr>
</table>
=== modified file 'lib/lp/translations/templates/person-translations-to-review.pt'
--- lib/lp/translations/templates/person-translations-to-review.pt 2009-09-17 20:11:48 +0000
+++ lib/lp/translations/templates/person-translations-to-review.pt 2010-12-23 16:07:17 +0000
@@ -39,7 +39,10 @@
1 string
</tal:stringcount>
reviewed
- </a>
+ </a> in
+ <tal:languages replace="target_info/languages">
+ Spanish
+ </tal:languages>
</td>
</tr>
</table>
Follow ups
