Merge pull request #137 from pmclanahan/add-admin-sms-ids

Fix bug 1173780: Add SMS message id admin.

news/admin.py

@@ -1,7 +1,13 @@
 from django.contrib import admin, messages
 
 from news.models import (APIUser, BlockedEmail, FailedTask, Interest, LocaleStewards, Newsletter,
-                         NewsletterGroup, Subscriber)
+                         NewsletterGroup, SMSMessage, Subscriber)
+
+
+class SMSMessageAdmin(admin.ModelAdmin):
+    fields = ('message_id', 'vendor_id', 'description')
+    list_display = ('message_id', 'vendor_id', 'description')
+    list_editable = ('vendor_id',)
 
 
 class BlockedEmailAdmin(admin.ModelAdmin):
@@ -87,6 +93,7 @@ def retry_task_action(self, request, queryset):
     retry_task_action.short_description = u"Retry task(s)"
 
 
+admin.site.register(SMSMessage, SMSMessageAdmin)
 admin.site.register(APIUser, APIUserAdmin)
 admin.site.register(BlockedEmail, BlockedEmailAdmin)
 admin.site.register(FailedTask, FailedTaskAdmin)

news/migrations/0014_auto__add_smsmessage.py

@@ -0,0 +1,103 @@
+# -*- coding: utf-8 -*-
+import datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Adding model 'SMSMessage'
+        db.create_table(u'news_smsmessage', (
+            ('message_id', self.gf('django.db.models.fields.SlugField')(max_length=50, primary_key=True)),
+            ('vendor_id', self.gf('django.db.models.fields.CharField')(max_length=50)),
+            ('description', self.gf('django.db.models.fields.CharField')(max_length=200, blank=True)),
+        ))
+        db.send_create_signal(u'news', ['SMSMessage'])
+
+
+    def backwards(self, orm):
+        # Deleting model 'SMSMessage'
+        db.delete_table(u'news_smsmessage')
+
+
+    models = {
+        u'news.apiuser': {
+            'Meta': {'object_name': 'APIUser'},
+            'api_key': ('django.db.models.fields.CharField', [], {'default': "'3155104b-4ddb-4c29-923d-2c5cb277df41'", 'max_length': '40', 'db_index': 'True'}),
+            'enabled': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '256'})
+        },
+        u'news.blockedemail': {
+            'Meta': {'object_name': 'BlockedEmail'},
+            'email_domain': ('django.db.models.fields.CharField', [], {'max_length': '50'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'})
+        },
+        u'news.failedtask': {
+            'Meta': {'object_name': 'FailedTask'},
+            'args': ('jsonfield.fields.JSONField', [], {'default': '[]'}),
+            'einfo': ('django.db.models.fields.TextField', [], {'default': 'None', 'null': 'True'}),
+            'exc': ('django.db.models.fields.TextField', [], {'default': 'None', 'null': 'True'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'kwargs': ('jsonfield.fields.JSONField', [], {'default': '{}'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'task_id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255'}),
+            'when': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'})
+        },
+        u'news.interest': {
+            'Meta': {'object_name': 'Interest'},
+            '_welcome_id': ('django.db.models.fields.CharField', [], {'max_length': '64', 'blank': 'True'}),
+            'default_steward_emails': ('news.fields.CommaSeparatedEmailField', [], {'blank': 'True'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'interest_id': ('django.db.models.fields.SlugField', [], {'unique': 'True', 'max_length': '50'}),
+            'title': ('django.db.models.fields.CharField', [], {'max_length': '128'})
+        },
+        u'news.localestewards': {
+            'Meta': {'unique_together': "(('interest', 'locale'),)", 'object_name': 'LocaleStewards'},
+            'emails': ('news.fields.CommaSeparatedEmailField', [], {}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'interest': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['news.Interest']"}),
+            'locale': ('news.fields.LocaleField', [], {'max_length': '32'})
+        },
+        u'news.newsletter': {
+            'Meta': {'ordering': "['order']", 'object_name': 'Newsletter'},
+            'active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'confirm_message': ('django.db.models.fields.CharField', [], {'max_length': '64', 'blank': 'True'}),
+            'description': ('django.db.models.fields.CharField', [], {'max_length': '256', 'blank': 'True'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'languages': ('django.db.models.fields.CharField', [], {'max_length': '200'}),
+            'order': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+            'requires_double_optin': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'show': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'slug': ('django.db.models.fields.SlugField', [], {'unique': 'True', 'max_length': '50'}),
+            'title': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+            'vendor_id': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+            'welcome': ('django.db.models.fields.CharField', [], {'max_length': '64', 'blank': 'True'})
+        },
+        u'news.newslettergroup': {
+            'Meta': {'object_name': 'NewsletterGroup'},
+            'active': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'description': ('django.db.models.fields.CharField', [], {'max_length': '256', 'blank': 'True'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'newsletters': ('django.db.models.fields.related.ManyToManyField', [], {'related_name': "'newsletter_groups'", 'symmetrical': 'False', 'to': u"orm['news.Newsletter']"}),
+            'show': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'slug': ('django.db.models.fields.SlugField', [], {'unique': 'True', 'max_length': '50'}),
+            'title': ('django.db.models.fields.CharField', [], {'max_length': '128'})
+        },
+        u'news.smsmessage': {
+            'Meta': {'object_name': 'SMSMessage'},
+            'description': ('django.db.models.fields.CharField', [], {'max_length': '200', 'blank': 'True'}),
+            'message_id': ('django.db.models.fields.SlugField', [], {'max_length': '50', 'primary_key': 'True'}),
+            'vendor_id': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+        },
+        u'news.subscriber': {
+            'Meta': {'object_name': 'Subscriber'},
+            'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'primary_key': 'True'}),
+            'fxa_id': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '100', 'null': 'True', 'blank': 'True'}),
+            'token': ('django.db.models.fields.CharField', [], {'default': "'1bffb7db-a262-4b3c-8131-dea52be0438e'", 'max_length': '40', 'db_index': 'True'})
+        }
+    }
+
+    complete_apps = ['news']

news/models.py

@@ -3,7 +3,6 @@
 from django.conf import settings
 from django.core.mail import send_mail
 from django.db import models
-from django.db.models.signals import post_delete, post_save
 from django.template.loader import render_to_string
 from django.utils.timezone import now
 
@@ -167,18 +166,6 @@ def newsletter_slugs(self):
         return [nl.slug for nl in self.newsletters.all()]
 
 
-def post_newsletter_change(*args, **kwargs):
-    # Cannot import earlier due to circular import
-    from news.newsletters import clear_newsletter_cache
-    clear_newsletter_cache()
-
-
-post_save.connect(post_newsletter_change, sender=Newsletter)
-post_delete.connect(post_newsletter_change, sender=Newsletter)
-post_save.connect(post_newsletter_change, sender=NewsletterGroup)
-post_delete.connect(post_newsletter_change, sender=NewsletterGroup)
-
-
 class APIUser(models.Model):
     """On some API calls, an API key must be passed that must
     exist in this table."""
@@ -340,3 +327,21 @@ def __unicode__(self):
             lang_code=self.locale,
             lang_name=product_details.languages[self.locale]['English'],
         )
+
+
+class SMSMessage(models.Model):
+    message_id = models.SlugField(
+        primary_key=True,
+        help_text='The ID for the message that will be used by clients',
+    )
+    vendor_id = models.CharField(
+        max_length=50,
+        help_text="The backend vendor's identifier for this message",
+    )
+    description = models.CharField(
+        max_length=200, blank=True,
+        help_text='Optional short description of this message'
+    )
+
+    class Meta:
+        verbose_name = "SMS message"

news/newsletters.py

@@ -4,16 +4,42 @@
 It's used to lookup the backend-specific newsletter name from a
 generic one passed by the user. This decouples the API from any
 specific email provider."""
+from django.db.models.signals import post_save
+from django.db.models.signals import post_delete
 from django.core.cache import cache
 
-from news.models import Newsletter, NewsletterGroup
+from news.models import Newsletter, NewsletterGroup, SMSMessage
 
 
-__all__ = ('clear_newsletter_cache', 'newsletter_field', 'newsletter_name',
-           'newsletter_fields')
+__all__ = ('clear_newsletter_cache', 'get_sms_messages', 'newsletter_field',
+           'newsletter_name', 'newsletter_fields')
 
 
 CACHE_KEY = "newsletters_cache_data"
+SMS_CACHE_KEY = "sms_messages_cache_data"
+# TODO remove after initial deployment. These values should be added to
+#   to the DB. This is so we don't miss any submissions.
+SMS_MESSAGES = {
+    'SMS_Android': 'MTo3ODow',
+}
+
+
+def get_sms_messages():
+    """
+    Returns a dict for which the keys are SMS message IDs that
+    basket clients will send, and the values are the message IDs
+    that our SMS vendor expects.
+    """
+    data = cache.get(SMS_CACHE_KEY)
+    if data is None:
+        # TODO have this be an empty dict when SMS_MESSAGES is removed.
+        data = SMS_MESSAGES.copy()
+        for msg in SMSMessage.objects.all():
+            data[msg.message_id] = msg.vendor_id
+
+        cache.set(SMS_CACHE_KEY, data)
+
+    return data
 
 
 def _newsletters():
@@ -139,5 +165,17 @@ def is_supported_newsletter_language(code):
     return code[:2].lower() in [lang[:2].lower() for lang in newsletter_languages()]
 
 
-def clear_newsletter_cache():
+def clear_newsletter_cache(*args, **kwargs):
     cache.delete(CACHE_KEY)
+
+
+def clear_sms_cache(*args, **kwargs):
+    cache.delete(SMS_CACHE_KEY)
+
+
+post_save.connect(clear_newsletter_cache, sender=Newsletter)
+post_delete.connect(clear_newsletter_cache, sender=Newsletter)
+post_save.connect(clear_newsletter_cache, sender=NewsletterGroup)
+post_delete.connect(clear_newsletter_cache, sender=NewsletterGroup)
+post_save.connect(clear_sms_cache, sender=SMSMessage)
+post_delete.connect(clear_sms_cache, sender=SMSMessage)

news/tasks.py

@@ -16,7 +16,7 @@
 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.newsletters import is_supported_newsletter_language
+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)
 
@@ -28,12 +28,6 @@
 # Base message ID for confirmation email
 CONFIRMATION_MESSAGE = "confirmation_email"
 
-SMS_MESSAGES = {
-    'SMS_Android': 'MTo3ODow',
-    'android': 'NTo3ODow',
-    'ios': 'Njo3ODow',
-    'mobile': 'Nzo3ODow',
-}
 PHONEBOOK_GROUPS = (
     'SYSTEMS_ADMINISTRATION',
     'BOOT2GECKO',
@@ -688,12 +682,13 @@ def confirm_user(token, user_data):
 
 @et_task
 def add_sms_user(send_name, mobile_number, optin):
-    if send_name not in SMS_MESSAGES:
+    messages = get_sms_messages()
+    if send_name not in messages:
         return
     et = ExactTargetRest()
 
     try:
-        et.send_sms([mobile_number], SMS_MESSAGES[send_name])
+        et.send_sms([mobile_number], messages[send_name])
     except ETRestError as error:
         return add_sms_user.retry(exc=error)
 

news/tests/test_newsletters.py

@@ -2,8 +2,30 @@
 
 from django.test import TestCase
 
+from mock import patch
+
 from news import newsletters, utils
-from news.models import Newsletter, NewsletterGroup
+from news.models import Newsletter, NewsletterGroup, SMSMessage
+
+
+@patch.object(newsletters, 'SMS_MESSAGES', {'the-dude': 'ABIDES',
+                                            'the-sheriff': 'REACTIONARY'})
+class TestSMSMessageCache(TestCase):
+    def setUp(self):
+        newsletters.clear_sms_cache()
+        SMSMessage.objects.create(message_id='the-dude', vendor_id='YOURE_NOT_WRONG_WALTER')
+        SMSMessage.objects.create(message_id='the-walrus', vendor_id='SHUTUP_DONNIE')
+
+    def test_messages_combined(self):
+        """Messages returned should be a combo of the DB and constants.
+
+        DB entries should override the contsants.
+        """
+        self.assertEqual(newsletters.get_sms_messages(), {
+            'the-sheriff': 'REACTIONARY',
+            'the-dude': 'YOURE_NOT_WRONG_WALTER',
+            'the-walrus': 'SHUTUP_DONNIE',
+        })
 
 
 class TestNewsletterUtils(TestCase):

news/tests/test_tasks.py

@@ -8,6 +8,7 @@
 
 from news.backends.exacttarget_rest import ETRestError, ExactTargetRest
 from news.models import FailedTask, Subscriber
+from news.newsletters import clear_sms_cache
 from news.tasks import (
     add_sms_user,
     et_task,
@@ -169,9 +170,10 @@ def self_test_success_no_token_create_user(self):
 
 
 @override_settings(ET_CLIENT_ID='client_id', ET_CLIENT_SECRET='client_secret')
-@patch('news.tasks.SMS_MESSAGES', {'foo': 'bar'})
+@patch('news.newsletters.SMS_MESSAGES', {'foo': 'bar'})
 class AddSMSUserTests(TestCase):
     def setUp(self):
+        clear_sms_cache()
         patcher = patch.object(ExactTargetRest, 'send_sms')
         self.send_sms = patcher.start()
         self.addCleanup(patcher.stop)

news/tests/test_views.py

@@ -262,7 +262,7 @@ def test_skip_welcome(self, fxa_mock):
                                                request_data['fxa_id'], skip_welcome=True)
 
 
-@patch.dict(views.SMS_MESSAGES, {'SMS_Android': 'My_Sherona'})
+@patch.dict('news.newsletters.SMS_MESSAGES', {'SMS_Android': 'My_Sherona'})
 class SubscribeSMSTests(TestCase):
     def setUp(self):
         cache.clear()

news/views.py

@@ -16,7 +16,7 @@
 from raven.contrib.django.raven_compat.models import client
 
 from news.models import Newsletter, Subscriber, Interest
-from news.newsletters import newsletter_slugs
+from news.newsletters import get_sms_messages, newsletter_slugs
 from news.tasks import (
     add_sms_user,
     confirm_user,
@@ -25,8 +25,7 @@
     update_fxa_info,
     update_get_involved,
     update_phonebook,
-    update_student_ambassadors,
-    SMS_MESSAGES)
+    update_student_ambassadors)
 from news.utils import (
     SET,
     SUBSCRIBE,
@@ -323,8 +322,9 @@ def subscribe_sms(request):
             'code': errors.BASKET_USAGE_ERROR,
         }, 400)
 
+    messages = get_sms_messages()
     msg_name = request.POST.get('msg_name', 'SMS_Android')
-    if msg_name not in SMS_MESSAGES:
+    if msg_name not in messages:
         return HttpResponseJSON({
             'status': 'error',
             'desc': 'Invalid msg_name',