launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-611179 into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers): code
Related bugs:
  #611179 DummyPOFile without language?
  https://bugs.launchpad.net/bugs/611179


= Bug 611179 =

This fixes an oops that we get a few of every other day, apparently because of users and bots making up invalid URLs.  They're properly "not found" errors, except they show up as exceptions because we have a custom navigation from a POTemplate down into a POFile.  If no POFile is found for the language code indicated by the URL, the navigation object makes up a DummyPOFile.

In order to do create a DummyPOFile, it first looks up the language.  But the language lookup quietly returns None if no matching language was found, and neither the navigation object, nor the DummyPOFile, nor the view itself objects to this.  The result: a confusing failure much further down the road.

This change makes the navigation object check that the language code indicates an actual language.  If not, it raises NotFound.

To test,
{{{
./bin/test -vvc -m lp.translations -t potemplate-navigation
}}}

To Q/A, try visiting any of the URLs that visited in the oopses listed in the bug.  To pick an arbitrary example: https://translations.launchpad.net/bleachbit/trunk/+pots/bleachbit/zh_tw/+translate

The page should show a "Page not found" error rather than a generic "stuff failed" oops.


No lint,

Jeroen
-- 
https://code.launchpad.net/~jtv/launchpad/bug-611179/+merge/33512
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-611179 into lp:launchpad/devel.

=== modified file 'lib/lp/translations/browser/potemplate.py'
--- lib/lp/translations/browser/potemplate.py	2010-08-20 20:31:18 +0000
+++ lib/lp/translations/browser/potemplate.py	2010-08-24 12:33:44 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 # pylint: disable-msg=F0401
 
@@ -109,8 +109,10 @@
             # It's just a query, get a fake one so we don't create new
             # POFiles just because someone is browsing the web.
             language = getUtility(ILanguageSet).getLanguageByCode(name)
-            return self.context.getDummyPOFile(language, requester=user,
-                                               check_for_existing=False)
+            if language is None:
+                raise NotFoundError(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

=== added file 'lib/lp/translations/stories/standalone/potemplate-navigation.txt'
--- lib/lp/translations/stories/standalone/potemplate-navigation.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/translations/stories/standalone/potemplate-navigation.txt	2010-08-24 12:33:44 +0000
@@ -0,0 +1,60 @@
+Translation URLs
+================
+
+A template's translations (i.e. a POTemplate's POFiles) are addressed in
+http as children of the template.
+
+    >>> def translation_to(potemplate, language_code):
+    ...     """Return URL for translating potemplate to language_code."""
+    ...     return "%s/%s/+translate" % (
+    ...         canonical_url(potemplate, rootsite='translations'),
+    ...         language_code)
+
+    >>> login(ANONYMOUS)
+    >>> template = factory.makePOTemplate()
+    >>> print translation_to(template, 'el')
+    http.../+pots/.../el/+translate
+    >>> logout()
+
+
+Accessing an existing translation
+---------------------------------
+
+A translation can represent an existing POFile.  This is
+straightforward.
+
+    >>> login(ANONYMOUS)
+    >>> pofile = factory.makePOFile('nl', potemplate=template)
+    >>> nl_url = translation_to(template, 'nl')
+    >>> logout()
+
+    >>> user_browser.open(nl_url)
+
+
+Accessing a new translation
+---------------------------
+
+It's also possible that the child represents a language for which the
+template has no translations yet.  In that case, the view substitutes a
+DummyPOFile which acts like a proper POFile.
+
+    >>> login(ANONYMOUS)
+    >>> de_url = translation_to(template, 'de')
+    >>> logout()
+
+    >>> user_browser.open(de_url)
+
+
+Translation to non-existent languages
+-------------------------------------
+
+Sometimes users (or crawlers) attempt to access translations for invalid
+language codes.  They get a NotFound error.
+
+    >>> login(ANONYMOUS)
+    >>> bogus_url = translation_to(template, 'ptbr')
+    >>> logout()
+    >>> user_browser.open(bogus_url)
+    Traceback (most recent call last):
+    ...
+    NotFound: ...