Fix bug 1065624: Remove subscriber data from db.

.coveragerc

@@ -1,3 +1,5 @@
 [run]
 source = news
-omit = news/tests/*
+omit =
+  news/tests/*
+  news/migrations/*

.travis.yml

@@ -6,14 +6,14 @@ before_script:
   - flake8 news
   - mysql -e 'create database basket;'
   - python manage.py migrate --noinput
-script: coverage run manage.py test news
+script: py.test --cov news
 before_install:
   - git submodule update --init --recursive
 install:
   - pip install -r requirements/compiled.txt -r requirements/dev.txt
+  - pip install coveralls
 after_success:
   # Report coverage results to coveralls.io
-  - pip install coveralls
   - coveralls
 notifications:
   irc:

manage.py

@@ -6,7 +6,10 @@
 
 ROOT = os.path.dirname(os.path.abspath(__file__))
 
-path = lambda *a: os.path.join(ROOT, *a)
+
+def path(*a):
+    return os.path.join(ROOT, *a)
+
 
 prev_sys_path = list(sys.path)
 
@@ -21,9 +24,7 @@
         sys.path.remove(item)
 sys.path[:0] = new_sys_path
 
-# No third-party imports until we've added all our sitedirs!
-from django.core.management import execute_from_command_line
-
 if __name__ == "__main__":
     os.environ.setdefault("DJANGO_SETTINGS_MODULE", "settings")
+    from django.core.management import execute_from_command_line
     execute_from_command_line(sys.argv)

news/admin.py

@@ -1,7 +1,7 @@
 from django.contrib import admin, messages
 
 from news.models import (APIUser, BlockedEmail, FailedTask, Interest, LocaleStewards, Newsletter,
-                         NewsletterGroup, SMSMessage, Subscriber)
+                         NewsletterGroup, SMSMessage)
 
 
 class SMSMessageAdmin(admin.ModelAdmin):
@@ -39,12 +39,6 @@ class APIUserAdmin(admin.ModelAdmin):
     list_display = ('name', 'enabled')
 
 
-class SubscriberAdmin(admin.ModelAdmin):
-    fields = ('email', 'token', 'fxa_id')
-    list_display = ('email', 'token', 'fxa_id')
-    search_fields = ('email', 'token', 'fxa_id')
-
-
 class NewsletterAdmin(admin.ModelAdmin):
     fields = ('title', 'slug', 'vendor_id', 'welcome', 'confirm_message',
               'description', 'languages', 'show', 'order', 'active',
@@ -100,4 +94,3 @@ def retry_task_action(self, request, queryset):
 admin.site.register(Interest, InterestAdmin)
 admin.site.register(Newsletter, NewsletterAdmin)
 admin.site.register(NewsletterGroup, NewsletterGroupAdmin)
-admin.site.register(Subscriber, SubscriberAdmin)

news/migrations/0002_delete_subscriber.py

@@ -0,0 +1,17 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import models, migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('news', '0001_initial'),
+    ]
+
+    operations = [
+        migrations.DeleteModel(
+            name='Subscriber',
+        ),
+    ]

news/models.py

@@ -22,38 +22,6 @@ class BlockedEmail(models.Model):
     email_domain = models.CharField(max_length=50)
 
 
-class SubscriberManager(models.Manager):
-    def get_and_sync(self, email, token, fxa_id=None):
-        """
-        Get the subscriber for the email and token and ensure that such a
-        subscriber exists.
-        """
-        defaults = {'token': token}
-        if fxa_id:
-            defaults['fxa_id'] = fxa_id
-
-        sub, created = self.get_or_create(email=email, defaults=defaults)
-        if not created:
-            sub.token = token
-            if fxa_id:
-                sub.fxa_id = fxa_id
-            sub.save()
-            # FIXME: this could mean there's another record in Exact Target
-            # with the other token
-
-        return sub
-
-
-class Subscriber(models.Model):
-    email = models.EmailField(primary_key=True)
-    token = models.CharField(max_length=40, default=get_uuid,
-                             db_index=True)
-    fxa_id = models.CharField(max_length=100, null=True, blank=True,
-                              db_index=True)
-
-    objects = SubscriberManager()
-
-
 class Newsletter(models.Model):
     slug = models.SlugField(
         unique=True,

news/tasks.py

@@ -15,10 +15,10 @@
 from news.backends.common import NewsletterException, NewsletterNoResultsException
 from news.backends.exacttarget import ExactTarget, ExactTargetDataExt
 from news.backends.exacttarget_rest import ETRestError, ExactTargetRest
-from news.models import FailedTask, Newsletter, Subscriber, Interest
+from news.models import FailedTask, Newsletter, Interest
 from news.newsletters import get_sms_messages, is_supported_newsletter_language
-from news.utils import (get_user_data, lookup_subscriber, MSG_USER_NOT_FOUND,
-                        SUBSCRIBE, parse_newsletters)
+from news.utils import (generate_token, get_user_data, MSG_USER_NOT_FOUND,
+                        parse_newsletters, SUBSCRIBE)
 
 
 log = logging.getLogger(__name__)
@@ -211,14 +211,9 @@ def update_fxa_info(email, lang, fxa_id, source_url=None, skip_welcome=False):
     if user:
         welcome_format = user['format']
         token = user['token']
-        Subscriber.objects.get_and_sync(email, token, fxa_id)
     else:
-        sub, created = Subscriber.objects.get_or_create(email=email, defaults={'fxa_id': fxa_id})
-        if not created:
-            sub.fxa_id = fxa_id
-            sub.save()
         welcome_format = 'H'
-        token = sub.token
+        token = generate_token()
         # only want source url for first contact
         record['SOURCE_URL'] = source_url or 'https://accounts.firefox.com'
 
@@ -255,12 +250,10 @@ def update_get_involved(interest_id, lang, name, email, country, email_format,
     }
     if user:
         token = user['token']
-        Subscriber.objects.get_and_sync(email, token)
         if 'get-involved' not in user.get('newsletters', []):
             record['GET_INVOLVED_DATE'] = gmttime()
     else:
-        sub, created = Subscriber.objects.get_or_create(email=email)
-        token = sub.token
+        token = generate_token()
         record['EMAIL_FORMAT_'] = email_format
         record['GET_INVOLVED_DATE'] = gmttime()
         # only want source url for first contact
@@ -305,9 +298,14 @@ def update_get_involved(interest_id, lang, name, email, country, email_format,
 
 
 @et_task
-def update_phonebook(data, email, token):
+def update_phonebook(data, token):
+    user_data = get_user_data(token=token)
+    if not user_data:
+        # no user with that token
+        return
+
     record = {
-        'EMAIL_ADDRESS': email,
+        'EMAIL_ADDRESS': user_data['email'],
         'TOKEN': token,
     }
     if 'city' in data:
@@ -322,8 +320,13 @@ def update_phonebook(data, email, token):
 
 
 @et_task
-def update_student_ambassadors(data, email, token):
-    data['EMAIL_ADDRESS'] = email
+def update_student_ambassadors(data, token):
+    user_data = get_user_data(token=token)
+    if not user_data:
+        # no user with that token
+        return
+
+    data['EMAIL_ADDRESS'] = user_data['email']
     data['TOKEN'] = token
     et = ExactTarget(settings.EXACTTARGET_USER, settings.EXACTTARGET_PASS)
     et.data_ext().add_record('Student_Ambassadors', data.keys(), data.values())
@@ -358,10 +361,23 @@ def update_user(data, email, token, api_call_type, optin):
     :raises: NewsletterException if there are any errors that would be
         worth retrying. Our task wrapper will retry in that case.
     """
-    # If token is missing, find it or generate it.
-    if not token:
-        sub, user_data, created = lookup_subscriber(email=email)
-        token = sub.token
+    # Get the user's current settings from ET, if any
+    user_data = get_user_data(email=email, token=token)
+    # If we don't find the user, get_user_data returns None. Create
+    # a minimal dictionary to use going forward. This will happen
+    # often due to new people signing up.
+    if user_data is None:
+        user_data = {
+            'email': email,
+            'token': token or generate_token(),
+            'master': False,
+            'pending': False,
+            'confirmed': False,
+            'lang': '',
+            'status': 'ok',
+        }
+    else:
+        token = user_data['token']
 
     # Parse the parameters
     # `record` will contain the data we send to ET in the format they want.
@@ -385,22 +401,6 @@ def update_user(data, email, token, api_call_type, optin):
 
     lang = record.get('LANGUAGE_ISO2', '') or ''
 
-    # Get the user's current settings from ET, if any
-    user_data = get_user_data(token=token)
-    # If we don't find the user, get_user_data returns None. Create
-    # a minimal dictionary to use going forward. This will happen
-    # often due to new people signing up.
-    if user_data is None:
-        user_data = {
-            'email': email,
-            'token': token,
-            'master': False,
-            'pending': False,
-            'confirmed': False,
-            'lang': lang,
-            'status': 'ok',
-        }
-
     if lang:
         # User asked for a language change. Use the new language from
         # here on.
@@ -640,7 +640,7 @@ def send_welcomes(user_data, newsletter_slugs, format):
 
 
 @et_task
-def confirm_user(token, user_data):
+def confirm_user(token, user_data=None):
     """
     Confirm any pending subscriptions for the user with this token.
 
@@ -654,13 +654,11 @@ def confirm_user(token, user_data):
     :raises: BasketError for fatal errors, NewsletterException for retryable
         errors.
     """
-    # Get user data if we don't already have it
     if user_data is None:
-        from .utils import get_user_data   # Avoid circular import
         user_data = get_user_data(token=token)
 
-    if user_data is None:
-        raise BasketError(MSG_USER_NOT_FOUND)
+        if user_data is None:
+            raise BasketError(MSG_USER_NOT_FOUND)
 
     if user_data['confirmed']:
         log.info('In confirm_user, user with token %s '
@@ -737,7 +735,7 @@ def send_recovery_message_task(email):
     # We should check ET so we can get format and lang if they exist.
     # If they don't exist, then we can create a basket subscriber.
 
-    user_data = get_user_data(email=email, sync_data=True)
+    user_data = get_user_data(email=email)
     if not user_data:
         log.warn("In send_recovery_message_task, email not known: %s" % email)
         return