Code review comment for lp:~adiroiban/launchpad/bug-512307

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

« Back to merge proposal