launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

Curtis Hovey has proposed merging lp:~sinzui/launchpad/dedup-preferred-launguages-1 into lp:launchpad/devel with lp:~sinzui/launchpad/dedup-preferred-launguages-0 as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers): code
Related bugs:
  #636453 move geoip code to lp.services
  https://bugs.launchpad.net/bugs/636453


This is my branch to prevent duplicate preferred languages.

    lp:~sinzui/launchpad/dedup-preferred-launguages-1
    Diff size: 48
    Launchpad bug:
          https://bugs.launchpad.net/bugs/159146
    Test command: ./bin/test -vv -t preferred-languages.txt
    Pre-implementation: no one
    Target release: 10.10


Prevent duplicate preferred languages
-------------------------------------

OOPS-668E1595 shows that a DB constraint was triggered setting the answer
contact for the ava-bzr project.

2 IntegrityError: ERROR: duplicate key violates unique constraint
 "personlanguage_person_key" INSERT INTO PersonLanguage (person, language)
 VALUES (1537687, 119

The problem is in the odd request header:
    HTTP_ACCEPT_LANGUAGE: en-US,en;q=0.9,de-CH;q=0.8,de;q=0.6,
                          de-DE;q=0.5,en-GB;q=0.4,en-us;q=0.3,en;q=0.1

RequestPreferredLanguages returns en twice. This is a really easy fix that
I should have made 2 years ago when I speculated on the origin of the
problem.


Rules
-----

    * Update RequestPreferredLanguages.getPreferredLanguages() to use
      a set instead of a list.


QA
--

    * Visit Launchpad and unset your preferred languages.
    * Reconfigure your browser to use the example HTTP_ACCEPT_LANGUAGE.
    * Visit a project and add yourself as an Answer Contact.
    * Verify your preferred languages are set to en, en-GB, de, de-CH


Lint
----

Linting changed files:
  lib/lp/services/geoip/model.py
  lib/lp/translations/doc/preferred-languages.txt


Test
----

    * lib/lp/translations/doc/preferred-languages.txt
      * Used the problematic HTTP_ACCEPT_LANGUAGE header from the oops
        request to verify 'en' does not appear twice in the list of languages.


Implementation
--------------

    * lib/lp/services/geoip/model.py
      * Switched the list of languages to a set to ensure they are unique.
-- 
https://code.launchpad.net/~sinzui/launchpad/dedup-preferred-launguages-1/+merge/35229
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/dedup-preferred-launguages-1 into lp:launchpad/devel.

=== modified file 'lib/lp/services/geoip/model.py'
--- lib/lp/services/geoip/model.py	2010-09-12 16:09:47 +0000
+++ lib/lp/services/geoip/model.py	2010-09-12 16:09:48 +0000
@@ -142,7 +142,7 @@
 
         codes = IUserPreferredLanguages(self.request).getPreferredLanguages()
         languageset = getUtility(ILanguageSet)
-        languages = []
+        languages = set()
 
         for code in codes:
             # We need to ensure that the code received contains only ASCII
@@ -162,7 +162,7 @@
                     continue
             code = languageset.canonicalise_language_code(code)
             try:
-                languages.append(languageset[code])
+                languages.add(languageset[code])
             except KeyError:
                 pass
 

=== modified file 'lib/lp/translations/doc/preferred-languages.txt'
--- lib/lp/translations/doc/preferred-languages.txt	2010-09-12 16:09:47 +0000
+++ lib/lp/translations/doc/preferred-languages.txt	2010-09-12 16:09:48 +0000
@@ -10,9 +10,20 @@
     >>> [l.code
     ...  for l in RequestPreferredLanguages(request).getPreferredLanguages()]
     [u'pt_BR']
-  
+
     >>> langs = {'HTTP_ACCEPT_LANGUAGE': u'pt_BR, Espa\xf1ol'}
     >>> request = LaunchpadTestRequest(**langs)
     >>> [l.code
     ...  for l in RequestPreferredLanguages(request).getPreferredLanguages()]
     [u'pt_BR']
+
+The getPreferredLanguages() method returns unique codes.
+
+    >>> langs = {
+    ...     'HTTP_ACCEPT_LANGUAGE': 'en-US,en;q=0.9,de-CH;q=0.8,de;q=0.6,'
+    ...                             'en-GB;q=0.4,en-us;q=0.3,en;q=0.1'
+    ...     }
+    >>> request = LaunchpadTestRequest(**langs)
+    >>> [l.code
+    ...  for l in RequestPreferredLanguages(request).getPreferredLanguages()]
+    [u'en', u'en_GB', u'de']