Bug 863066: Fix subscription data returned

There was a bug in how we translated from what Exact Target gives us
to our preferred internal representation of what newsletters the user
is subscribed to. The symptom was that when a user confirmed, we
thought they hadn't subscribed to anything and so didn't send any
welcome messages.

Author: dpoirier

apps/news/newsletters.py

@@ -70,11 +70,19 @@ def newsletter_name(field):
         return None
 
 
-def newsletter_names():
-    """Get a list of all the available newsletters"""
+def newsletter_slugs():
+    """
+    Get a list of all the available newsletters.
+    Returns a list of their slugs.
+    """
     return _newsletters()['by_name'].keys()
 
 
+def slug_to_vendor_id(slug):
+    """Given a newsletter's slug, return its vendor_id"""
+    return _newsletters()['by_name'][slug].vendor_id
+
+
 def newsletter_fields():
     """Get a list of all the newsletter backend-specific fields"""
     return _newsletters()['by_vendor_id'].keys()

apps/news/tasks.py

@@ -13,7 +13,7 @@
 
 from .backends.exacttarget import (ExactTarget, ExactTargetDataExt)
 from .models import Newsletter
-from .newsletters import newsletter_field, newsletter_names
+from .newsletters import newsletter_field, newsletter_slugs
 
 
 log = logging.getLogger(__name__)
@@ -176,7 +176,7 @@ def parse_newsletters(record, type, newsletters, cur_newsletters):
                 unsubs = cur_newsletters - set(newsletters)
             else:
                 subs = set(newsletters)
-                all = set(newsletter_names())
+                all = set(newsletter_slugs())
                 unsubs = all - subs
         else:  # type == UNSUBSCRIBE
             # unsubscribe from the specified newsletters

apps/news/tests.py

@@ -12,6 +12,7 @@
 
 from news import models, tasks, views
 from news.backends.exacttarget import NewsletterException
+from news.models import Newsletter
 from news.newsletters import newsletter_fields, newsletter_languages
 from news.tasks import (FFAY_VENDOR_ID, FFOS_VENDOR_ID,
                         SET, SUBSCRIBE, UNSUBSCRIBE,
@@ -20,7 +21,7 @@
                         UU_MUST_CONFIRM_NEW, UU_MUST_CONFIRM_PENDING,
                         RetryTask,
                         confirm_user, update_user)
-from news.views import get_user_data, language_code_is_valid
+from news.views import get_user_data, language_code_is_valid, look_for_user
 
 
 class SubscriberTest(TestCase):
@@ -1278,6 +1279,41 @@ def test_cache_clear_on_delete(self):
         self.assertEqual([], vendor_ids)
 
 
+class TestLookForUser(TestCase):
+    def test_subscriptions(self):
+        """
+        look_for_user returns the right list of subscribed newsletters
+        """
+        Newsletter.objects.create(slug='n1', vendor_id='NEWSLETTER1')
+        Newsletter.objects.create(slug='n2', vendor_id='NEWSLETTER2')
+        fields = ['NEWSLETTER1_FLG', 'NEWSLETTER2_FLG']
+        with patch('news.views.ExactTargetDataExt') as et_ext:
+            data_ext = et_ext()
+            data_ext.get_record.return_value = {
+                'EMAIL_ADDRESS_': 'dude@example.com',
+                'EMAIL_FORMAT_': 'HTML',
+                'COUNTRY_': 'us',
+                'LANGUAGE_ISO2': 'en',
+                'TOKEN': 'asdf',
+                'CREATED_DATE_': 'Yesterday',
+                'NEWSLETTER1_FLG': 'Y',
+                'NEWSLETTER2_FLG': 'Y',
+            }
+            result = look_for_user(database='DUMMY', email='EMAIL',
+                                   token='TOKEN', fields=fields)
+        expected_result = {
+            'newsletters': [u'n1', u'n2'],
+            'status': 'ok',
+            'country': 'us',
+            'lang': 'en',
+            'token': 'asdf',
+            'created-date': 'Yesterday',
+            'email': 'dude@example.com',
+            'format': 'HTML',
+        }
+        self.assertEqual(expected_result, result)
+
+
 class TestGetUserData(TestCase):
 
     def generic_test(self,

apps/news/views.py

@@ -22,7 +22,7 @@
     update_user,
 )
 from .newsletters import (newsletter_fields, newsletter_languages,
-                          newsletter_names)
+                          newsletter_slugs, slug_to_vendor_id)
 
 
 ## Utility functions
@@ -146,7 +146,7 @@ def update_user_task(request, type, data=None, optin=True):
 
     newsletters = data.get('newsletters', None)
     if newsletters:
-        all_newsletters = newsletter_names()
+        all_newsletters = newsletter_slugs()
         for nl in [x.strip() for x in newsletters.split(',')]:
             if nl not in all_newsletters:
                 return HttpResponseJSON({
@@ -202,6 +202,12 @@ def look_for_user(database, email, token, fields):
         return None
     if database == settings.EXACTTARGET_CONFIRMATION:
         return True
+    newsletters = []
+    for slug in newsletter_slugs():
+        vendor_id = slug_to_vendor_id(slug)
+        flag = "%s_FLG" % vendor_id
+        if user.get(flag, 'N') == 'Y':
+            newsletters.append(slug)
     user_data = {
         'status': 'ok',
         'email': user['EMAIL_ADDRESS_'],
@@ -210,8 +216,7 @@ def look_for_user(database, email, token, fields):
         'lang': user['LANGUAGE_ISO2'],
         'token': user['TOKEN'],
         'created-date': user['CREATED_DATE_'],
-        'newsletters': [nl for nl in newsletter_names()
-                        if user.get('%s_FLG' % nl, False) == 'Y'],
+        'newsletters': newsletters,
     }
     return user_data
 
@@ -398,7 +403,7 @@ def unsubscribe(request, token):
     data = request.POST.copy()
 
     if data.get('optout', 'N') == 'Y':
-        data['newsletters'] = ','.join(newsletter_names())
+        data['newsletters'] = ','.join(newsletter_slugs())
 
     return update_user_task(request, UNSUBSCRIBE, data)