Merge pull request #143 from pmclanahan/add-accept-lang-subscribe-118…

…8047

Fix bug 1188047: Add accept_lang support to subscribe.

news/tests/test__utils.py

@@ -93,6 +93,48 @@ def test_invalid_lang(self):
             self.assert_response_error(response, 400, errors.BASKET_INVALID_LANGUAGE)
             mock_language_code_is_valid.assert_called_with('pt-BR')
 
+    @patch('news.utils.get_best_language')
+    def test_accept_lang(self, get_best_language_mock):
+        """If accept_lang param is provided, should set the lang in data."""
+        get_best_language_mock.return_value = 'pt'
+        request = self.factory.post('/')
+        data = {'email': 'dude@example.com', 'accept_lang': 'pt-pt,fr;q=0.8'}
+        after_data = {'email': 'dude@example.com', 'lang': 'pt'}
+
+        response = update_user_task(request, SUBSCRIBE, data, sync=False)
+        self.assert_response_ok(response)
+        self.update_user.delay.assert_called_with(after_data, 'dude@example.com',
+                                                  None, SUBSCRIBE, True)
+        self.assertFalse(self.lookup_subscriber.called)
+
+    def test_invalid_accept_lang(self):
+        """If accept_lang param is provided but invalid, return a 400."""
+        request = self.factory.post('/')
+        data = {'email': 'dude@example.com', 'accept_lang': 'the dude minds, man'}
+
+        response = update_user_task(request, SUBSCRIBE, data, sync=False)
+        self.assert_response_error(response, 400, errors.BASKET_INVALID_LANGUAGE)
+        self.assertFalse(self.update_user.delay.called)
+        self.assertFalse(self.lookup_subscriber.called)
+
+    @patch('news.utils.get_best_language')
+    def test_lang_overrides_accept_lang(self, get_best_language_mock):
+        """
+        If lang is provided it was from the user, and accept_lang isn't as reliable, so we should
+        prefer lang.
+        """
+        get_best_language_mock.return_value = 'pt-BR'
+        request = self.factory.post('/')
+        data = {'email': 'a@example.com',
+                'lang': 'de',
+                'accept_lang': 'pt-BR'}
+
+        response = update_user_task(request, SUBSCRIBE, data, sync=False)
+        self.assert_response_ok(response)
+        # basically asserts that the data['lang'] value wasn't changed.
+        self.update_user.delay.assert_called_with(data, 'a@example.com', None, SUBSCRIBE, True)
+        self.assertFalse(self.lookup_subscriber.called)
+
     def test_missing_email_and_sub(self):
         """
         If both the email and subscriber are missing, return a 400
@@ -248,6 +290,10 @@ def test_returns_first_good_lang(self):
         self._test(['zh-TW', 'es', 'de', 'en'], 'es')
         self._test(['pt-PT', 'zh-TW', 'pt-BR', 'en'], 'pt-BR')
 
+    def test_returns_first_good_lang_2_letter(self):
+        """Should return first 2 letter prefix language in the list that a newsletter supports."""
+        self._test(['pt-PT', 'zh-TW', 'es-AR', 'ar'], 'es')
+
     def test_returns_first_lang_no_good(self):
         """Should return the first in the list if no supported are found."""
         self._test(['pt-PT', 'zh-TW', 'zh-CN', 'ar'], 'pt-PT')

news/utils.py

@@ -1,7 +1,8 @@
-from functools import wraps
 import json
 import re
 from datetime import date
+from functools import wraps
+from itertools import chain
 
 from django.conf import settings
 from django.core.cache import get_cache
@@ -230,12 +231,24 @@ def update_user_task(request, api_call_type, data=None, optin=True, sync=False):
                     'code': errors.BASKET_INVALID_NEWSLETTER,
                 }, 400)
 
-    if 'lang' in data and not language_code_is_valid(data['lang']):
-        return HttpResponseJSON({
-            'status': 'error',
-            'desc': 'invalid language',
-            'code': errors.BASKET_INVALID_LANGUAGE,
-        }, 400)
+    if 'lang' in data:
+        if not language_code_is_valid(data['lang']):
+            return HttpResponseJSON({
+                'status': 'error',
+                'desc': 'invalid language',
+                'code': errors.BASKET_INVALID_LANGUAGE,
+            }, 400)
+    elif 'accept_lang' in data:
+        lang = get_best_language(get_accept_languages(data['accept_lang']))
+        if lang:
+            data['lang'] = lang
+            del data['accept_lang']
+        else:
+            return HttpResponseJSON({
+                'status': 'error',
+                'desc': 'invalid language',
+                'code': errors.BASKET_INVALID_LANGUAGE,
+            }, 400)
 
     email = data.get('email', sub.email if sub else None)
     if not email:
@@ -485,8 +498,11 @@ def get_best_language(languages):
     if not languages:
         return None
 
+    # try again with 2 letter languages
+    languages_2l = [lang[:2] for lang in languages]
     supported_langs = newsletter_languages()
-    for lang in languages:
+
+    for lang in chain(languages, languages_2l):
         if lang in supported_langs:
             return lang
 

news/views.py

@@ -13,7 +13,6 @@
 from ratelimit.decorators import ratelimit
 from ratelimit.exceptions import Ratelimited
 from ratelimit.utils import is_ratelimited
-from raven.contrib.django.raven_compat.models import client
 
 from news.models import Newsletter, Subscriber, Interest
 from news.newsletters import get_sms_messages, newsletter_slugs
@@ -128,7 +127,6 @@ def fxa_register(request):
             'code': errors.BASKET_USAGE_ERROR,
         }, 401)
     if 'accept_lang' not in data:
-        client.captureMessage(message='accept_lang missing from fxa-register request', data=data)
         return HttpResponseJSON({
             'status': 'error',
             'desc': 'fxa-register requires accept_lang',
@@ -137,7 +135,6 @@ def fxa_register(request):
 
     lang = get_best_language(get_accept_languages(data['accept_lang']))
     if lang is None:
-        client.captureMessage(message='invalid accept_lang value', data=data)
         return HttpResponseJSON({
             'status': 'error',
             'desc': 'invalid language',