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

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

I am not using a set, since Language.translators is already sorted by karma (done in a single SQL query).
Using a set and then resorting it at the end will require a new SQL query for each person.

KarmaCache can not be updated from the „browser” layer and we need a unit test for services.worlddata.model.language.Language.translators running in LaunchpadZopelessLayer.

I have extended the current Language.translators karma text code to check that translators for a language are sorted according to their karma value.

Here is the latest diff:

=== modified file 'lib/lp/services/worlddata/doc/language.txt'
--- lib/lp/services/worlddata/doc/language.txt 2010-04-15 11:53:11 +0000
+++ lib/lp/services/worlddata/doc/language.txt 2010-04-21 00:06:25 +0000
@@ -273,8 +273,14 @@
 To be considered a translator, they must have done some translations and
 have the language among their preferred languages.

- >>> translator = factory.makePerson(name=u'serbian-translator')
- >>> translator.addLanguage(sr)
+ >>> translator_10 = factory.makePerson(name=u'serbian-translator-karma-10')
+ >>> translator_10.addLanguage(sr)
+ >>> translator_20 = factory.makePerson(name=u'serbian-translator-karma-20')
+ >>> translator_20.addLanguage(sr)
+ >>> translator_30 = factory.makePerson(name=u'serbian-translator-karma-30')
+ >>> translator_30.addLanguage(sr)
+ >>> translator_40 = factory.makePerson(name=u'serbian-translator-karma-40')
+ >>> translator_40.addLanguage(sr)
   >>> from canonical.testing import LaunchpadZopelessLayer
   >>> LaunchpadZopelessLayer.commit()

@@ -283,13 +289,26 @@
   >>> LaunchpadZopelessLayer.switchDbUser('karma')
   >>> translations_category = KarmaCategory.selectOne(
   ... KarmaCategory.name=='translations')
- >>> karma = KarmaCache(person=translator,
- ... category=translations_category,
- ... karmavalue=1)
+ >>> karma = KarmaCache(person=translator_30,
+ ... category=translations_category,
+ ... karmavalue=30)
+ >>> karma = KarmaCache(person=translator_10,
+ ... category=translations_category,
+ ... karmavalue=10)
+ >>> karma = KarmaCache(person=translator_20,
+ ... category=translations_category,
+ ... karmavalue=20)
+ >>> karma = KarmaCache(person=translator_40,
+ ... category=translations_category,
+ ... karmavalue=40)
   >>> LaunchpadZopelessLayer.commit()
   >>> LaunchpadZopelessLayer.switchDbUser('launchpad')
- >>> [translator.name for translator in sr.translators]
- [u'serbian-translator']
+ >>> for translator in sr.translators:
+ ... print translator.name
+ serbian-translator-karma-40
+ serbian-translator-karma-30
+ serbian-translator-karma-20
+ serbian-translator-karma-10

 =========

=== modified file 'lib/lp/translations/browser/tests/language-views.txt'
--- lib/lp/translations/browser/tests/language-views.txt 2010-04-17 17:40:26 +0000
+++ lib/lp/translations/browser/tests/language-views.txt 2010-04-20 23:53:54 +0000
@@ -106,7 +106,7 @@
     1 : 2, 3, 4, 22, 23, 24...
     2 : 0, 5, 6, 7, 8, 9...

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

 Create some translators and a merged account.
@@ -169,6 +169,7 @@

     >>> Language.translators = translators_method

+
 View LanguageSet
 ------------------

« Back to merge proposal