Launchpad itself

Merge lp:~adiroiban/launchpad/bug-512307 into lp:launchpad

  1. bug-512307
  2. Merge into devel
Proposed by Adi Roiban
Status: Merged
Approved by: Graham Binns
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~adiroiban/launchpad/bug-512307
Merge into: lp:launchpad
Diff against target: 246 lines (+111/-19)
5 files modified
lib/lp/translations/browser/language.py (+26/-3)
lib/lp/translations/browser/tests/language-views.txt (+73/-7)
lib/lp/translations/stories/standalone/xx-pofile-details.txt (+4/-3)
lib/lp/translations/templates/language-portlet-top-contributors.pt (+2/-2)
lib/lp/translations/templates/pofile-details.pt (+6/-4)
To merge this branch: bzr merge lp:~adiroiban/launchpad/bug-512307
Reviewer Review Type Date Requested Status
Graham Binns (community) code Approve
Review via email: mp+20442@code.launchpad.net

Commit message

Filter merge accounts from overall language contributors.

To post a comment you must log in.
Revision history for this message
Adi Roiban (adiroiban) wrote :

= Bug 512307=
On https://translations.launchpad.net/+languages/st we can see that Leonardo Gregianin appears twice, one with this current account (leogregianin) and an old merged one, (leogregianin-merged.

We should not show merged accounts.

In bug 121520 , we fixed some placed where merged accounts were displayed, but missed language-portlet-top-contributors.pt

== Proposed fix ==
Only show top 20 contributors and if we have a merged account in the top, replace it with the target account.

== Pre-implementation notes ==
My initial proposed fix was to just filter the merge account, but since by doing this we will no longer have a top 20, Jeroen asked to replace merged account with they target.

== Implementation details ==
Top contributors for a language, depends on the full contributors list and this is generated by cronjobs that compute the KarmaCache tables. I talked with Danilo and for writing the tests he suggest to fake the generation of language contributors.

In lib/lp/translations/templates/pofile-details.pt I have done a small cleanup for my fix for bug 121520, since for merged account it generates empty <li> tags.

== Tests ==
lp-test -t language-views -t pofile-details

== Demo and Q/A ==
Since overall language statistics depends on KarmaCache and user preferences it is hard to test it on a local branch.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/translations/browser/language.py
  lib/lp/translations/browser/tests/language-views.txt
  lib/lp/translations/stories/standalone/xx-pofile-details.txt
  lib/lp/translations/templates/language-portlet-top-contributors.pt
  lib/lp/translations/templates/pofile-details.pt

Revision history for this message
Graham Binns (gmb) wrote :
Download full text (9.4 KiB)

Hi Adi,

Nice branch. I've got a couple of comments - just fixes needed in the doctest narrative - other than that this is fine. Please ping me when you've made the necessary changes and I'll EC2 it.

> === modified file 'lib/lp/translations/browser/language.py'
> --- lib/lp/translations/browser/language.py 2009-12-08 10:20:37 +0000
> +++ lib/lp/translations/browser/language.py 2010-03-02 11:04:22 +0000
> @@ -34,7 +34,6 @@
> ITranslationsPerson)
> from lp.translations.browser.translations import TranslationsMixin
> from lp.translations.utilities.pluralforms import make_friendly_plural_forms
> -from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
>
> from canonical.widgets import LabeledMultiCheckBoxWidget
>
> @@ -214,8 +213,31 @@
> })
> return translation_teams
>
> - def getTopContributors(self):
> - return self.context.translators[:20]
> + @property
> + def top_contributors(self):
> + """
> + Get top 20 contributors for a language.

s/Get/Get the

> +
> + Instead of merged account, show the merged target account.

This would read better as: "If an account has been merged, the account
into which it was merged will be returned."

> + """
> + translators = []
> + for translator in reversed(list(self.context.translators)):
> + # Get only the top 20 contributors
> + if (len(translators) >= 20):
> + break
> +
> + # For merged account add the target account
> + if translator.merged != None:
> + translator_target = translator.merged
> + else:
> + translator_target = translator
> +
> + # Add translator only if it was not previouly added as a
> + # merged account
> + if translator_target not in translators:
> + translators.append(translator_target)
> +
> + return translators
>
> @property
> def friendly_plural_forms(self):
>
> === modified file 'lib/lp/translations/browser/tests/language-views.txt'
> --- lib/lp/translations/browser/tests/language-views.txt 2009-12-03 15:14:55 +0000
> +++ lib/lp/translations/browser/tests/language-views.txt 2010-03-02 11:04:22 +0000
> @@ -1,8 +1,8 @@
> Language Views
> -==================
> +==============
>
> Admin Language
> -------------------
> +--------------
>
> >>> from zope.component import getUtility
> >>> from lp.services.worlddata.interfaces.language import ILanguageSet
> @@ -34,7 +34,7 @@
>
>
> Add Language
> -------------------
> +------------
>
> >>> language_add_view = create_view(language_set, '+add',
> ... layer=TranslationsLayer)
> @@ -77,13 +77,22 @@
>
>
> View Language
> -------------------
> -
> +-------------
> +
> +Translators list for each language is computed using KarmaCache table for

s/list/lists ; s/is computed/are computed ; s/table/tables.

> +users which have configured their prefered language. Since KarmaCache tables
> +are generated using nightly builds, we will change Langauge.translators to
> +use the list of translators generated by this test.
> +
> + >>> ...

Read more...

review: Approve (code)
Revision history for this message
Adi Roiban (adiroiban) wrote :

Thanks for the review.

I have fixed and pushed the pagetest comments.

Here is the diff.
=== modified file 'lib/lp/translations/browser/language.py'
--- lib/lp/translations/browser/language.py 2010-03-02 11:02:08 +0000
+++ lib/lp/translations/browser/language.py 2010-03-02 12:03:30 +0000
@@ -216,9 +216,10 @@
     @property
     def top_contributors(self):
         """
- Get top 20 contributors for a language.
+ Get the top 20 contributors for a language.

- Instead of merged account, show the merged target account.
+ If an account has been merged, the account into which it was
+ merged will be returned.
         """
         translators = []
         for translator in reversed(list(self.context.translators)):

=== modified file 'lib/lp/translations/browser/tests/language-views.txt'
--- lib/lp/translations/browser/tests/language-views.txt 2010-03-02 10:16:13 +0000
+++ lib/lp/translations/browser/tests/language-views.txt 2010-03-02 12:04:45 +0000
@@ -79,7 +79,7 @@
 View Language
 -------------

-Translators list for each language is computed using KarmaCache table for
+Translators lists for each language are computed using KarmaCache tables for
 users which have configured their prefered language. Since KarmaCache tables
 are generated using nightly builds, we will change Langauge.translators to
 use the list of translators generated by this test.
@@ -104,7 +104,7 @@
     1 : 2, 3, 4, 22, 23, 24...
     2 : 5, 6, 7, 8, 9, 10...

-Top 20 contributors are listed on the language page, and for merged account
+The top 20 contributors are listed on the language page, and for merged account
 we will see their targed account.

 Create some translators and a merged account.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/translations/browser/language.py'
2--- lib/lp/translations/browser/language.py 2009-12-08 10:20:37 +0000
3+++ lib/lp/translations/browser/language.py 2010-03-16 01:06:32 +0000
4@@ -34,7 +34,6 @@
5 ITranslationsPerson)
6 from lp.translations.browser.translations import TranslationsMixin
7 from lp.translations.utilities.pluralforms import make_friendly_plural_forms
8-from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
9
10 from canonical.widgets import LabeledMultiCheckBoxWidget
11
12@@ -214,8 +213,32 @@
13 })
14 return translation_teams
15
16- def getTopContributors(self):
17- return self.context.translators[:20]
18+ @property
19+ def top_contributors(self):
20+ """
21+ Get the top 20 contributors for a language.
22+
23+ If an account has been merged, the account into which it was
24+ merged will be returned.
25+ """
26+ translators = []
27+ for translator in reversed(list(self.context.translators)):
28+ # Get only the top 20 contributors
29+ if (len(translators) >= 20):
30+ break
31+
32+ # For merged account add the target account
33+ if translator.merged != None:
34+ translator_target = translator.merged
35+ else:
36+ translator_target = translator
37+
38+ # Add translator only if it was not previouly added as a
39+ # merged account
40+ if translator_target not in translators:
41+ translators.append(translator_target)
42+
43+ return translators
44
45 @property
46 def friendly_plural_forms(self):
47
48=== modified file 'lib/lp/translations/browser/tests/language-views.txt'
49--- lib/lp/translations/browser/tests/language-views.txt 2010-03-05 13:50:29 +0000
50+++ lib/lp/translations/browser/tests/language-views.txt 2010-03-16 01:06:32 +0000
51@@ -1,8 +1,8 @@
52 Language Views
53-==================
54+==============
55
56 Admin Language
57-------------------
58+--------------
59
60 >>> from zope.component import getUtility
61 >>> from lp.services.worlddata.interfaces.language import ILanguageSet
62@@ -34,7 +34,7 @@
63
64
65 Add Language
66-------------------
67+------------
68
69 >>> language_add_view = create_view(language_set, '+add',
70 ... layer=TranslationsLayer)
71@@ -77,13 +77,24 @@
72
73
74 View Language
75-------------------
76-
77+-------------
78+
79+Translators lists for each language are computed using KarmaCache tables for
80+users which have configured their prefered language. Since KarmaCache tables
81+are generated using nightly builds, we will change Langauge.translators to
82+use the list of translators generated by this test.
83+
84+Language.translators is deleted so that we can add our custom
85+implementation later.
86+
87+ >>> from lp.services.worlddata.model.language import Language
88+ >>> translators_method = Language.translators
89+ >>> del Language.translators
90 >>> serbian = language_set.getLanguageByCode('sr')
91 >>> language_view = create_initialized_view(serbian, '+index',
92 ... layer=TranslationsLayer)
93
94-The friendlypluralforms function shows us a list of plural forms and
95+The 'friendly_plural_forms' function shows us a list of plural forms and
96 a set of examples for each one, so one won't need to understand the plural
97 formula expression to see how it works.
98
99@@ -95,6 +106,61 @@
100 1 : 2, 3, 4, 22, 23, 24...
101 2 : 0, 5, 6, 7, 8, 9...
102
103+The top 20 contributors are listed on the language page, and for merged account
104+we will see their targed account.
105+
106+Create some translators and a merged account.
107+
108+ >>> from zope.security.proxy import removeSecurityProxy
109+ >>> translators = []
110+ >>> for translator_nr in range(22):
111+ ... translators.append(factory.makePerson(
112+ ... name='translator-' + str(translator_nr),
113+ ... displayname='Translator No.' + str(translator_nr)))
114+ >>> translator_main = factory.makePerson(
115+ ... name='translator-main',
116+ ... displayname='Translator Main')
117+ >>> translators.append(translator_main)
118+ >>> translator_merged = removeSecurityProxy(factory.makePerson(
119+ ... name='translator-merged',
120+ ... displayname='Translator Merged'))
121+ >>> translators.append(translator_merged)
122+ >>> translator_merged.merged = translator_main
123+
124+Create a product, a template with one msgset and a pofile
125+
126+ >>> product = factory.makeProduct(official_rosetta=True)
127+ >>> template = factory.makePOTemplate(
128+ ... productseries=product.getSeries('trunk'))
129+ >>> potmsgset = factory.makePOTMsgSet(template)
130+ >>> pofile = factory.makePOFile('sr', potemplate=template)
131+
132+Add a translation for each translator and one more for main and merged
133+accounts.
134+
135+ >>> for translator in translators:
136+ ... translation = factory.makeTranslationMessage(
137+ ... pofile=pofile, translator=translator, potmsgset=potmsgset)
138+ >>> translation = factory.makeTranslationMessage(
139+ ... pofile=pofile, translator=translator_merged, potmsgset=potmsgset)
140+ >>> translation = factory.makeTranslationMessage(
141+ ... pofile=pofile, translator=translator_main, potmsgset=potmsgset)
142+
143+Langauge.translators is Monkey-patched to avoid fetching KarmaCache.
144+
145+ >>> serbian.translators = translators
146+ >>> language_view = create_initialized_view(serbian, '+index',
147+ ... layer=TranslationsLayer)
148+ >>> top_contributors = language_view.top_contributors
149+ >>> translator_main in top_contributors
150+ True
151+ >>> translator_merged in top_contributors
152+ False
153+
154+In the end, the changes done to Language class are reverted.
155+
156+ >>> Language.translators = translators_method
157+
158 View LanguageSet
159 ------------------
160
161@@ -110,7 +176,7 @@
162 <a href=".../en" ...>English</a>,
163 <a href=".../es" ...>Spanish</a>
164
165-For a user without any preferred languages, English will be returned.
166+For a user without any preferred languages, English will be returned.
167
168 >>> person = factory.makePerson()
169 >>> print person.languages
170
171=== modified file 'lib/lp/translations/stories/standalone/xx-pofile-details.txt'
172--- lib/lp/translations/stories/standalone/xx-pofile-details.txt 2010-03-11 20:54:36 +0000
173+++ lib/lp/translations/stories/standalone/xx-pofile-details.txt 2010-03-16 01:06:32 +0000
174@@ -151,10 +151,8 @@
175 >>> language_code = 'es'
176 >>> pofile = factory.makePOFile(language_code, potemplate=template)
177 >>> potmsgset = factory.makePOTMsgSet(template)
178- >>> potmsgset.setSequence(template, 1)
179 >>> translation = factory.makeTranslationMessage(pofile=pofile,
180 ... translator=merged_translator, potmsgset=potmsgset)
181- >>> potmsgset.setSequence(template, 2)
182 >>> translation = factory.makeTranslationMessage(pofile=pofile,
183 ... translator=translator, potmsgset=potmsgset)
184 >>> from zope.security.proxy import removeSecurityProxy
185@@ -165,10 +163,13 @@
186 ... ("http://translations.launchpad.dev/"
187 ... "ubuntu/hoary/+source/%s/+pots/%s/%s/+details") % (
188 ... package.name, template.name, language_code))
189- >>> print extract_text(find_main_content(browser.contents))
190+ >>> main_text = extract_text(find_main_content(browser.contents))
191+ >>> print main_text
192 Details for ...
193 Contributors to this translation
194 The following people have made some contribution to this specific
195 translation:
196 Poly Glot (filter)
197
198+ >>> u'Mere Pere' in main_text
199+ False
200
201=== modified file 'lib/lp/translations/templates/language-portlet-top-contributors.pt'
202--- lib/lp/translations/templates/language-portlet-top-contributors.pt 2009-08-28 15:40:27 +0000
203+++ lib/lp/translations/templates/language-portlet-top-contributors.pt 2010-03-16 01:06:32 +0000
204@@ -3,7 +3,7 @@
205 xmlns:metal="http://xml.zope.org/namespaces/metal"
206 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
207 class="portlet" id="portlet-top-contributors"
208- tal:condition="view/getTopContributors">
209+ tal:condition="view/top_contributors">
210 <h2>Top contributors</h2>
211 <div class="portletBody portletContent">
212 <p>
213@@ -14,7 +14,7 @@
214 </tal:language_name>:
215 </p>
216 <ul>
217- <li tal:repeat="contributor view/getTopContributors">
218+ <li tal:repeat="contributor view/top_contributors">
219 <a tal:replace="structure contributor/fmt:link">Guilherme Salgado</a>
220 </li>
221 </ul>
222
223=== modified file 'lib/lp/translations/templates/pofile-details.pt'
224--- lib/lp/translations/templates/pofile-details.pt 2010-01-20 22:31:50 +0000
225+++ lib/lp/translations/templates/pofile-details.pt 2010-03-16 01:06:32 +0000
226@@ -41,14 +41,16 @@
227 </p>
228
229 <ul tal:condition="view/contributors">
230- <li tal:repeat="contributor view/contributors">
231- <tal:not-merged condition="not: contributor/merged">
232- <a tal:replace="structure contributor/fmt:link" />
233+ <tal:contributors-loop repeat="contributor view/contributors">
234+ <li tal:condition="not: contributor/merged">
235+ <a tal:replace="structure contributor/fmt:link">
236+ Mister Potato
237+ </a>
238 (<a tal:attributes="
239 href string:${context/fmt:url}/+filter?person=${contributor/name}"
240 >filter</a>)
241- </tal:not-merged>
242 </li>
243+ </tal:contributors-loop>
244 </ul>
245
246 <p