← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~danilo/launchpad/pass-language-to-getdummy into lp:launchpad/devel

 

Данило Шеган has proposed merging lp:~danilo/launchpad/pass-language-to-getdummy into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


= Pass language directly to getDummyPOFile =

In most of our actual code where we use IPOTemplate.getDummyPOFile, we
already have an ILanguage and we pass in language.code to
getDummyPOFile.  Then getDummyPOFile refetches the language from the DB
thus doing a needless query.

Since this is an annoyance with what I am trying to do for
ITranslatedLanguage interface (make sure "pofiles" attribute always does
a single query), I've decided to split it out into separate branch and
fix it.

A few tests needed fixing, and I've added minimal documentation for the
method, and a minimal test.

I am only fixing getDummyPOFile, but other methods like newPOFile and
getPOFileByLang could use with a similar fix. For some other time,
though.

= Tests =

bin/test -cvvt getDummyPOFile -t doc.potemplate.txt

= Lint =

There's a million complaints from Curtis' new linter ("uses moin
headers" for every touched doctest, and a million line checks).  I am
not bothering with them right now because that should be *another*
branch, and I've already got side-tracked twice with pre-requisite
branches for what I am actually working on.
-- 
https://code.launchpad.net/~danilo/launchpad/pass-language-to-getdummy/+merge/30718
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/launchpad/pass-language-to-getdummy into lp:launchpad/devel.
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-07-21 14:41:26 +0000
+++ lib/lp/testing/factory.py	2010-07-22 20:44:51 +0000
@@ -1613,6 +1613,15 @@
         syncUpdate(series)
         return series
 
+    def makeLanguage(self, language_code=None, name=None):
+        if language_code is None:
+            language_code = self.getUniqueString('lang')
+        if name is None:
+            name = "Language %s" % language_code
+
+        language_set = getUtility(ILanguageSet)
+        return language_set.createLanguage(language_code, name)
+
     def makeLibraryFileAlias(self, filename=None, content=None,
                              content_type='text/plain', restricted=False,
                              expires=None):

=== modified file 'lib/lp/translations/browser/potemplate.py'
--- lib/lp/translations/browser/potemplate.py	2010-07-15 09:41:27 +0000
+++ lib/lp/translations/browser/potemplate.py	2010-07-22 20:44:51 +0000
@@ -55,6 +55,7 @@
     ITranslationImporter)
 from lp.translations.interfaces.translationimportqueue import (
     ITranslationImportQueue)
+from lp.services.worlddata.interfaces.language import ILanguageSet
 from canonical.launchpad.webapp import (
     action, canonical_url, enabled_with_permission, GetitemNavigation,
     LaunchpadView, LaunchpadEditFormView, Link, Navigation, NavigationMenu,
@@ -90,7 +91,9 @@
         elif self.request.method in ['GET', 'HEAD']:
             # It's just a query, get a fake one so we don't create new
             # POFiles just because someone is browsing the web.
-            return self.context.getDummyPOFile(name, requester=user)
+            language = getUtility(ILanguageSet).getLanguageByCode(name)
+            return self.context.getDummyPOFile(language, requester=user,
+                                               check_for_existing=False)
         else:
             # It's a POST.
             # XXX CarlosPerelloMarin 2006-04-20 bug=40275: We should

=== modified file 'lib/lp/translations/browser/productseries.py'
--- lib/lp/translations/browser/productseries.py	2010-07-19 13:30:29 +0000
+++ lib/lp/translations/browser/productseries.py	2010-07-22 20:44:51 +0000
@@ -366,7 +366,7 @@
                         pot = self.context.getCurrentTranslationTemplates()[0]
                         pofile = pot.getPOFileByLang(lang.code)
                         if pofile is None:
-                            pofile = pot.getDummyPOFile(lang.code)
+                            pofile = pot.getDummyPOFile(lang)
                         productserieslang = (
                             productserieslangset.getProductSeriesLanguage(
                                 self.context, lang, pofile=pofile))

=== modified file 'lib/lp/translations/browser/tests/pofile-views.txt'
--- lib/lp/translations/browser/tests/pofile-views.txt	2010-01-20 22:41:54 +0000
+++ lib/lp/translations/browser/tests/pofile-views.txt	2010-07-22 20:44:51 +0000
@@ -15,6 +15,7 @@
     >>> from lp.registry.interfaces.distribution import IDistributionSet
     >>> from lp.registry.interfaces.sourcepackagename import (
     ...     ISourcePackageNameSet)
+    >>> from lp.services.worlddata.interfaces.language import ILanguageSet
     >>> from canonical.launchpad.layers import TranslationsLayer
 
 All the tests will be submitted as coming from the No Privilege person.
@@ -57,7 +58,8 @@
 This time, we are going to see what happens if we get an IPOFile without
 the plural form information.
 
-    >>> pofile_tlh = potemplate.getDummyPOFile('tlh')
+    >>> language_tlh = getUtility(ILanguageSet).getLanguageByCode('tlh')
+    >>> pofile_tlh = potemplate.getDummyPOFile(language_tlh)
     >>> form = {'show': 'all' }
     >>> pofile_view = create_view(
     ...     pofile_tlh, '+translate', form=form, layer=TranslationsLayer)

=== modified file 'lib/lp/translations/browser/tests/translationmessage-views.txt'
--- lib/lp/translations/browser/tests/translationmessage-views.txt	2010-03-22 17:18:28 +0000
+++ lib/lp/translations/browser/tests/translationmessage-views.txt	2010-07-22 20:44:51 +0000
@@ -7,6 +7,7 @@
     >>> from lp.translations.model.pofile import POFile
     >>> from lp.translations.model.translationmessage import (
     ...     TranslationMessage)
+    >>> from lp.services.worlddata.interfaces.language import ILanguageSet
     >>> from canonical.launchpad.layers import TranslationsLayer
     >>> from canonical.launchpad.webapp import canonical_url
 
@@ -23,8 +24,8 @@
 
     >>> translationmessage = TranslationMessage.get(1)
     >>> pofile = POFile.get(1)
-    >>> pofile_tlh = pofile.potemplate.getDummyPOFile(
-    ...     'tlh')
+    >>> language_tlh = getUtility(ILanguageSet).getLanguageByCode('tlh')
+    >>> pofile_tlh = pofile.potemplate.getDummyPOFile(language_tlh)
     >>> potmsgset = pofile_tlh.potemplate.getPOTMsgSetByMsgIDText(
     ...     u'evolution addressbook')
     >>> current_translationmessage = (

=== modified file 'lib/lp/translations/doc/pofile.txt'
--- lib/lp/translations/doc/pofile.txt	2010-02-01 15:06:20 +0000
+++ lib/lp/translations/doc/pofile.txt	2010-07-22 20:44:51 +0000
@@ -25,7 +25,9 @@
 Get Xhosa translation
 
     >>> pofile = potemplate.getPOFileByLang('xh')
-    >>> dummy_pofile = potemplate.getDummyPOFile('pt_BR')
+    >>> language_pt_BR = getUtility(
+    ...     ILanguageSet).getLanguageByCode('pt_BR')
+    >>> dummy_pofile = potemplate.getDummyPOFile(language_pt_BR)
 
 Both implement the IPOFile interface:
 
@@ -97,7 +99,8 @@
     >>> print potemplate.getPOFileByLang('es').getFullLanguageCode()
     es
 
-    >>> print potemplate.getDummyPOFile('sr', variant=u'Latn'
+    >>> language_sr = getUtility(ILanguageSet).getLanguageByCode('sr')
+    >>> print potemplate.getDummyPOFile(language_sr, variant=u'Latn'
     ...     ).getFullLanguageCode()
     sr@Latn
 
@@ -110,7 +113,7 @@
     >>> print potemplate.getPOFileByLang('es').getFullLanguageName()
     Spanish
 
-    >>> print potemplate.getDummyPOFile('sr', variant=u'Latn'
+    >>> print potemplate.getDummyPOFile(language_sr, variant=u'Latn'
     ...     ).getFullLanguageName()
     Serbian ("Latn" variant)
 
@@ -439,7 +442,8 @@
 
 Now, we get an IPOFile that does not have a translation team assigned.
 
-    >>> pofile_cy = potemplate.getDummyPOFile('cy')
+    >>> language_cy = getUtility(ILanguageSet).getLanguageByCode('cy')
+    >>> pofile_cy = potemplate.getDummyPOFile(language_cy)
 
 Valentina Commissari is not a translator for this language and does not
 have permissions.
@@ -608,7 +612,7 @@
     >>> serbian = getUtility(ILanguageSet)['sr']
     >>> serbian.pluralforms
     3
-    >>> evolution_sr = evolution_pot.getDummyPOFile(serbian.code)
+    >>> evolution_sr = evolution_pot.getDummyPOFile(serbian)
     >>> evolution_sr.plural_forms
     3
 
@@ -618,7 +622,7 @@
     >>> divehi = getUtility(ILanguageSet)['dv']
     >>> print divehi.pluralforms
     None
-    >>> evolution_dv = evolution_pot.getDummyPOFile(divehi.code)
+    >>> evolution_dv = evolution_pot.getDummyPOFile(divehi)
     >>> evolution_dv.plural_forms
     2
 

=== modified file 'lib/lp/translations/doc/potemplate.txt'
--- lib/lp/translations/doc/potemplate.txt	2010-04-26 16:00:31 +0000
+++ lib/lp/translations/doc/potemplate.txt	2010-07-22 20:44:51 +0000
@@ -228,13 +228,25 @@
 
 == getPOFileByPath ==
 
-We can get an IPOFile inside a templage based on its path.
+We can get an IPOFile inside a template based on its path.
 
     >>> pofile = potemplate.getPOFileByPath('es.po')
     >>> print pofile.title
     Spanish (es) translation of evolution-2.2 in Evolution trunk
 
 
+== getDummyPOFile ==
+
+To get an IPOFile object even for languages which don't have a translation
+of this template, we use the getDummyPOFile method, passing in the language.
+
+    >>> xx_language = factory.makeLanguage(
+    ...     'xx@test', name='Test language')
+    >>> xx_pofile = potemplate.getDummyPOFile(xx_language)
+    >>> print xx_pofile.title
+    Test language (xx@test) translation of evolution-2.2 in Evolution trunk
+
+
 == newPOFile ==
 
 The Portuguese translation has not been started yet; therefore,

=== modified file 'lib/lp/translations/doc/potmsgset.txt'
--- lib/lp/translations/doc/potmsgset.txt	2010-03-06 06:08:21 +0000
+++ lib/lp/translations/doc/potmsgset.txt	2010-07-22 20:44:51 +0000
@@ -445,7 +445,10 @@
 An empty translation does not need to exist in the database.  If not,
 a DummyPOFile is used instead.
 
-    >>> pt_BR_dummypofile = evolution_potemplate.getDummyPOFile('pt_BR')
+    >>> language_pt_BR = getUtility(
+    ...     ILanguageSet).getLanguageByCode('pt_BR')
+    >>> pt_BR_dummypofile = evolution_potemplate.getDummyPOFile(
+    ...     language_pt_BR)
 
 We get a POTMsgSet and verify it's a singular form:
 
@@ -475,7 +478,8 @@
 Using another dummy pofile we'll get a POTMsgset that's not a singular
 form:
 
-    >>> apa_dummypofile = evolution_potemplate.getDummyPOFile('apa')
+    >>> language_apa = getUtility(ILanguageSet).getLanguageByCode('apa')
+    >>> apa_dummypofile = evolution_potemplate.getDummyPOFile(language_apa)
     >>> plural_potmsgset = apa_dummypofile.potemplate.getPOTMsgSetByMsgIDText(
     ...     u'%d contact', u'%d contacts')
     >>> print apa_dummypofile.language.code
@@ -496,7 +500,8 @@
 
 We can guess the pluralforms for this language through ILanguage.pluralforms:
 
-    >>> ru_dummypofile = evolution_potemplate.getDummyPOFile('ru')
+    >>> language_ru = getUtility(ILanguageSet).getLanguageByCode('ru')
+    >>> ru_dummypofile = evolution_potemplate.getDummyPOFile(language_ru)
     >>> ru_dummy_current = plural_potmsgset.getCurrentDummyTranslationMessage(
     ...     evolution_potemplate, ru_dummypofile.language)
 
@@ -821,7 +826,8 @@
     ...     distroseries=ubuntu_hoary).getPOTemplateByName('man')
     >>> potmsgset_untranslated = pmount_man_template.getPOTMsgSetByMsgIDText(
     ...     'test man page')
-    >>> pofile = pmount_man_template.getDummyPOFile('es')
+    >>> language_es = getUtility(ILanguageSet).getLanguageByCode('es')
+    >>> pofile = pmount_man_template.getDummyPOFile(language_es)
     >>> print pofile.title
     Spanish (es) translation of man in Ubuntu Hoary package "pmount"
 

=== modified file 'lib/lp/translations/interfaces/potemplate.py'
--- lib/lp/translations/interfaces/potemplate.py	2010-07-20 12:33:43 +0000
+++ lib/lp/translations/interfaces/potemplate.py	2010-07-22 20:44:51 +0000
@@ -425,7 +425,8 @@
             loops when creating a new IPOTemplate.
         """
 
-    def getDummyPOFile(language_code, variant=None, requester=None):
+    def getDummyPOFile(language, variant=None, requester=None,
+                       check_for_existing=True):
         """Return a DummyPOFile if there isn't already a persistent `IPOFile`
 
         Raise `LanguageNotFound` if the language does not exist in the
@@ -435,7 +436,8 @@
         only create a POFile when you actually need to store data.
 
         We should not have already a POFile for the given language_code and
-        variant.
+        variant: if check_for_existing is set to False, no check will be
+        done for this.
         """
 
     def createPOTMsgSetFromMsgIDs(msgid_singular, msgid_plural=None,

=== modified file 'lib/lp/translations/model/potemplate.py'
--- lib/lp/translations/model/potemplate.py	2010-07-20 18:19:08 +0000
+++ lib/lp/translations/model/potemplate.py	2010-07-22 20:44:51 +0000
@@ -793,14 +793,15 @@
 
         return pofile
 
-    def getDummyPOFile(self, language_code, variant=None, requester=None):
+    def getDummyPOFile(self, language, variant=None, requester=None,
+                       check_for_existing=True):
         """See `IPOTemplate`."""
-        # see if a valid one exists.
-        existingpo = self.getPOFileByLang(language_code, variant)
-        assert existingpo is None, (
-            'There is already a valid IPOFile (%s)' % existingpo.title)
+        if check_for_existing:
+            # see if a valid one exists.
+            existingpo = self.getPOFileByLang(language.code, variant)
+            assert existingpo is None, (
+                'There is already a valid IPOFile (%s)' % existingpo.title)
 
-        language = self._lookupLanguage(language_code)
         return DummyPOFile(self, language, variant=variant, owner=requester)
 
     def createPOTMsgSetFromMsgIDs(self, msgid_singular, msgid_plural=None,

=== modified file 'lib/lp/translations/tests/test_potemplate.py'
--- lib/lp/translations/tests/test_potemplate.py	2010-07-21 14:02:28 +0000
+++ lib/lp/translations/tests/test_potemplate.py	2010-07-22 20:44:51 +0000
@@ -59,6 +59,31 @@
             "missing directory and language code. "
             "(Expected: '%s' Got: '%s')" % (expected, result))
 
+    def test_getDummyPOFile_no_existing_pofile(self):
+        # Test basic behaviour of getDummyPOFile.
+        language = self.factory.makeLanguage('sr@test')
+        dummy = self.potemplate.getDummyPOFile(language)
+        self.assertEquals(DummyPOFile, type(dummy))
+
+    def test_getDummyPOFile_with_existing_pofile(self):
+        # Test that getDummyPOFile fails when trying to get a DummyPOFile
+        # where a POFile already exists for that language.
+        language = self.factory.makeLanguage('sr@test')
+        pofile = self.potemplate.newPOFile(language.code)
+        self.assertRaises(
+            AssertionError, self.potemplate.getDummyPOFile, language)
+
+    def test_getDummyPOFile_with_existing_pofile_no_check(self):
+        # Test that getDummyPOFile succeeds when trying to get a DummyPOFile
+        # where a POFile already exists for that language when
+        # check_for_existing=False is passed in.
+        language = self.factory.makeLanguage('sr@test')
+        pofile = self.potemplate.newPOFile(language.code)
+        # This is just "assertNotRaises".
+        dummy = self.potemplate.getDummyPOFile(language,
+                                               check_for_existing=False)
+        self.assertEquals(DummyPOFile, type(dummy))
+
     def test_getTranslationCredits(self):
         # getTranslationCredits returns only translation credits.
         self.factory.makePOTMsgSet(self.potemplate, sequence=1)