From 3173302f06ae6dffeea22dad1681da31b49e3b1e Mon Sep 17 00:00:00 2001 From: Paul McLanahan Date: Tue, 19 Aug 2014 17:49:50 -0400 Subject: [PATCH 1/2] Bug 1051806: Add /fxa-register endpoint. --- .../0008_auto__add_field_subscriber_fxa_id.py | 64 +++++++++++++++++ news/models.py | 2 + news/tasks.py | 54 +++++++++++++- news/tests/test_users.py | 54 ++++++++++++++ news/tests/test_views.py | 70 +++++++++++++++++++ news/urls.py | 3 +- news/views.py | 34 +++++++++ 7 files changed, 278 insertions(+), 3 deletions(-) create mode 100644 news/migrations/0008_auto__add_field_subscriber_fxa_id.py diff --git a/news/migrations/0008_auto__add_field_subscriber_fxa_id.py b/news/migrations/0008_auto__add_field_subscriber_fxa_id.py new file mode 100644 index 0000000..6687e90 --- /dev/null +++ b/news/migrations/0008_auto__add_field_subscriber_fxa_id.py @@ -0,0 +1,64 @@ +# -*- 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 field 'Subscriber.fxa_id' + db.add_column(u'news_subscriber', 'fxa_id', + self.gf('django.db.models.fields.CharField')(db_index=True, max_length=100, null=True, blank=True), + keep_default=False) + + + def backwards(self, orm): + # Deleting field 'Subscriber.fxa_id' + db.delete_column(u'news_subscriber', 'fxa_id') + + + models = { + u'news.apiuser': { + 'Meta': {'object_name': 'APIUser'}, + 'api_key': ('django.db.models.fields.CharField', [], {'default': "'c17bac3d-1abd-4d6c-801e-866671c77dfd'", '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.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.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.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': "'b498d69d-441a-46fa-818d-faa447a5acd1'", 'max_length': '40', 'db_index': 'True'}) + } + } + + complete_apps = ['news'] \ No newline at end of file diff --git a/news/models.py b/news/models.py index 4ca0275..54e60b4 100644 --- a/news/models.py +++ b/news/models.py @@ -34,6 +34,8 @@ class Subscriber(models.Model): email = models.EmailField(primary_key=True) token = models.CharField(max_length=40, default=lambda: str(uuid4()), db_index=True) + fxa_id = models.CharField(max_length=100, null=True, blank=True, + db_index=True) objects = SubscriberManager() diff --git a/news/tasks.py b/news/tasks.py index 3f6ca88..62ce30e 100644 --- a/news/tasks.py +++ b/news/tasks.py @@ -13,9 +13,9 @@ from celery.task import Task, task -from .backends.common import NewsletterException +from .backends.common import NewsletterException, NewsletterNoResultsException from .backends.exacttarget import (ExactTarget, ExactTargetDataExt) -from .models import FailedTask, Newsletter +from .models import FailedTask, Newsletter, Subscriber from .newsletters import (is_supported_newsletter_language, newsletter_field, newsletter_slugs) @@ -253,6 +253,56 @@ def parse_newsletters(record, type, newsletters, cur_newsletters): return to_subscribe, to_unsubscribe +def get_external_user_data(email=None, token=None, fields=None, database=None): + database = database or settings.EXACTTARGET_DATA + fields = fields or [ + 'EMAIL_ADDRESS_', + 'EMAIL_FORMAT_', + 'COUNTRY_', + 'LANGUAGE_ISO2', + 'TOKEN', + ] + ext = ExactTargetDataExt(settings.EXACTTARGET_USER, + settings.EXACTTARGET_PASS) + try: + user = ext.get_record(database, token or email, fields, + 'TOKEN' if token else 'EMAIL_ADDRESS_') + except NewsletterNoResultsException: + return None + + return user + + +@et_task +def update_fxa_info(email, fxa_id): + created = False + try: + sub = Subscriber.objects.get(email=email) + except Subscriber.DoesNotExist: + kwargs = {'email': email, 'fxa_id': fxa_id} + user = get_external_user_data(email=email) + if user is not None and 'TOKEN' in user: + kwargs['token'] = user['TOKEN'] + else: + created = True + + sub = Subscriber.objects.create(**kwargs) + else: + sub.fxa_id = fxa_id + sub.save() + + record = { + 'EMAIL_ADDRESS_': email, + 'TOKEN': sub.token, + 'FXA_ID': fxa_id, + 'MODIFIED_DATE_': gmttime(), + } + if created: + record['SOURCE_URL'] = 'https://accounts.firefox.com' + + apply_updates(settings.EXACTTARGET_DATA, record) + + @et_task def update_phonebook(data, email, token): record = { diff --git a/news/tests/test_users.py b/news/tests/test_users.py index 2d3d77c..c8cc878 100644 --- a/news/tests/test_users.py +++ b/news/tests/test_users.py @@ -14,6 +14,60 @@ from news.views import look_for_user, get_user_data +@patch.object(tasks, 'apply_updates') +@patch.object(tasks, 'get_external_user_data') +class UpdateFxAInfoTest(TestCase): + def test_new_user(self, get_mock, apply_mock): + """Adding a new user to the DB should add fxa_id.""" + get_mock.return_value = None + email = 'dude@example.com' + fxa_id = 'the fxa abides' + tasks.update_fxa_info(email, fxa_id) + sub = models.Subscriber.objects.get(email=email) + self.assertEqual(sub.fxa_id, fxa_id) + call_data = apply_mock.call_args[0][1] + self.assertEqual(call_data['EMAIL_ADDRESS_'], email) + self.assertEqual(call_data['TOKEN'], sub.token) + self.assertEqual(call_data['FXA_ID'], fxa_id) + self.assertEqual(call_data['SOURCE_URL'], 'https://accounts.firefox.com') + get_mock.assert_called_with(email=email) + + def test_existing_user_not_in_basket(self, get_mock, apply_mock): + """Adding a user in ET but not basket should preserve token.""" + email = 'dude@example.com' + fxa_id = 'the fxa abides' + token = 'hehe... you said **token**.' + get_mock.return_value = { + 'EMAIL_ADDRESS_': email, + 'TOKEN': token, + } + tasks.update_fxa_info(email, fxa_id) + sub = models.Subscriber.objects.get(email=email) + self.assertEqual(sub.fxa_id, fxa_id) + self.assertEqual(sub.token, token) + call_data = apply_mock.call_args[0][1] + self.assertEqual(call_data['EMAIL_ADDRESS_'], email) + self.assertEqual(call_data['TOKEN'], token) + self.assertEqual(call_data['FXA_ID'], fxa_id) + self.assertNotIn('SOURCE_URL', call_data) + get_mock.assert_called_with(email=email) + + def test_existing_user(self, get_mock, apply_mock): + """Adding a fxa_id to an existing user shouldn't modify other things.""" + email = 'dude@example.com' + fxa_id = 'the fxa abides' + old_sub = models.Subscriber.objects.create(email=email) + tasks.update_fxa_info(email, fxa_id) + sub = models.Subscriber.objects.get(email=email) + self.assertEqual(sub.fxa_id, fxa_id) + call_data = apply_mock.call_args[0][1] + self.assertEqual(call_data['EMAIL_ADDRESS_'], email) + self.assertEqual(call_data['TOKEN'], old_sub.token) + self.assertEqual(call_data['FXA_ID'], fxa_id) + self.assertNotIn('SOURCE_URL', call_data) + self.assertFalse(get_mock.called) + + class DebugUserTest(TestCase): def setUp(self): self.sub = models.Subscriber.objects.create(email='dude@example.com') diff --git a/news/tests/test_views.py b/news/tests/test_views.py index 07ee2d5..f7c023a 100644 --- a/news/tests/test_views.py +++ b/news/tests/test_views.py @@ -16,6 +16,76 @@ none_mock = Mock(return_value=None) +@patch('news.views.update_fxa_info') +class FxAccountsTest(TestCase): + def ssl_post(self, url, params=None, **extra): + """Fake a post that used SSL""" + extra['wsgi.url_scheme'] = 'https' + params = params or {} + return self.client.post(url, data=params, **extra) + + def test_requires_ssl(self, fxa_mock): + """fxa-register requires SSL""" + resp = self.client.post('/news/fxa-register/', { + 'email': 'dude@example.com', + 'fxa_id': 'the dude has a Fx account.' + }) + self.assertEqual(resp.status_code, 401, resp.content) + data = json.loads(resp.content) + self.assertEqual(errors.BASKET_SSL_REQUIRED, data['code']) + self.assertFalse(fxa_mock.delay.called) + + def test_requires_api_key(self, fxa_mock): + """fxa-register requires API key""" + # Use SSL but no API key + resp = self.ssl_post('/news/fxa-register/', { + 'email': 'dude@example.com', + 'fxa_id': 'the dude has a Fx account.' + }) + self.assertEqual(resp.status_code, 401, resp.content) + data = json.loads(resp.content) + self.assertEqual(errors.BASKET_AUTH_ERROR, data['code']) + self.assertFalse(fxa_mock.delay.called) + + def test_requires_fxa_id(self, fxa_mock): + """fxa-register requires Firefox Account ID""" + auth = APIUser.objects.create(name="test") + resp = self.ssl_post('/news/fxa-register/', { + 'email': 'dude@example.com', + 'api-key': auth.api_key, + }) + self.assertEqual(resp.status_code, 401, resp.content) + data = json.loads(resp.content) + self.assertEqual(errors.BASKET_USAGE_ERROR, data['code']) + self.assertFalse(fxa_mock.delay.called) + + def test_requires_email(self, fxa_mock): + """fxa-register requires email address""" + auth = APIUser.objects.create(name="test") + resp = self.ssl_post('/news/fxa-register/', { + 'fxa_id': 'the dude has a Fx account.', + 'api-key': auth.api_key, + }) + self.assertEqual(resp.status_code, 401, resp.content) + data = json.loads(resp.content) + self.assertEqual(errors.BASKET_USAGE_ERROR, data['code']) + self.assertFalse(fxa_mock.delay.called) + + def test_with_ssl_and_api_key(self, fxa_mock): + """fxa-register should succeed with SSL, API Key, and data.""" + auth = APIUser.objects.create(name="test") + request_data = { + 'email': 'dude@example.com', + 'fxa_id': 'the dude has a Fx account.', + 'api-key': auth.api_key, + } + resp = self.ssl_post('/news/fxa-register/', request_data) + self.assertEqual(resp.status_code, 200, resp.content) + data = json.loads(resp.content) + self.assertEqual('ok', data['status']) + fxa_mock.delay.assert_called_once_with(request_data['email'], request_data['fxa_id']) + + @patch('news.views.validate_email', none_mock) @patch('news.views.update_user_task') class FxOSMalformedPOSTTest(TestCase): diff --git a/news/urls.py b/news/urls.py index 93f5c1c..9c7a659 100644 --- a/news/urls.py +++ b/news/urls.py @@ -2,12 +2,13 @@ from .views import (confirm, custom_unsub_reason, custom_update_phonebook, custom_update_student_ambassadors, debug_user, - list_newsletters, lookup_user, newsletters, + fxa_register, list_newsletters, lookup_user, newsletters, send_recovery_message, subscribe, subscribe_sms, unsubscribe, user) urlpatterns = patterns('', # noqa + url('^fxa-register/$', fxa_register), url('^subscribe/$', subscribe), url('^subscribe_sms/$', subscribe_sms), url('^unsubscribe/(.*)/$', unsubscribe), diff --git a/news/views.py b/news/views.py index 727caf1..b79b5a0 100644 --- a/news/views.py +++ b/news/views.py @@ -25,6 +25,7 @@ confirm_user, send_recovery_message_task, update_custom_unsub, + update_fxa_info, update_phonebook, update_student_ambassadors, update_user, @@ -395,6 +396,39 @@ def confirm(request, token): return HttpResponseJSON({'status': 'ok'}) +@require_POST +@csrf_exempt +def fxa_register(request): + if not request.is_secure(): + return HttpResponseJSON({ + 'status': 'error', + 'desc': 'fxa-register requires SSL', + 'code': errors.BASKET_SSL_REQUIRED, + }, 401) + if not has_valid_api_key(request): + return HttpResponseJSON({ + 'status': 'error', + 'desc': 'fxa-register requires a valid API-key', + 'code': errors.BASKET_AUTH_ERROR, + }, 401) + + data = request.POST.dict() + if 'email' not in data: + return HttpResponseJSON({ + 'status': 'error', + 'desc': 'fxa-register requires an email address', + 'code': errors.BASKET_USAGE_ERROR, + }, 401) + if 'fxa_id' not in data: + return HttpResponseJSON({ + 'status': 'error', + 'desc': 'fxa-register requires a Firefox Account ID', + 'code': errors.BASKET_USAGE_ERROR, + }, 401) + update_fxa_info.delay(data['email'], data['fxa_id']) + return HttpResponseJSON({'status': 'ok'}) + + @require_POST @csrf_exempt def subscribe(request): From 92f1d35b2963179c2cf3017297a751ee6bd4764a Mon Sep 17 00:00:00 2001 From: Paul McLanahan Date: Wed, 20 Aug 2014 16:25:17 -0400 Subject: [PATCH 2/2] Bug 1054316: Trigger welcome email for fxa registeration. --- news/models.py | 15 ++-- news/tasks.py | 53 +++++++----- news/tests/test_users.py | 169 +++++++++++++++++++++++++++++++-------- news/tests/test_views.py | 36 ++++++++- news/views.py | 17 +++- 5 files changed, 226 insertions(+), 64 deletions(-) diff --git a/news/models.py b/news/models.py index 54e60b4..0461f67 100644 --- a/news/models.py +++ b/news/models.py @@ -12,17 +12,20 @@ class SubscriberManager(models.Manager): - def get_and_sync(self, email, token): + 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. """ - sub, created = self.get_or_create( - email=email, - defaults={'token': token}, - ) - if not created and sub.token != token: + 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 diff --git a/news/tasks.py b/news/tasks.py index 62ce30e..3e5dd55 100644 --- a/news/tasks.py +++ b/news/tasks.py @@ -77,6 +77,7 @@ # This is prefixed with the 2-letter language code + _ before sending, # e.g. 'en_recovery_message', and '_T' if text, e.g. 'en_recovery_message_T'. RECOVERY_MESSAGE_ID = 'recovery_message' +FXACCOUNT_WELCOME = 'FxAccounts_Welcome' # Vendor IDs for Firefox OS and Firefox & You: FFOS_VENDOR_ID = 'FIREFOX_OS' @@ -270,36 +271,44 @@ def get_external_user_data(email=None, token=None, fields=None, database=None): except NewsletterNoResultsException: return None - return user + user_data = { + 'email': user['EMAIL_ADDRESS_'], + 'format': user['EMAIL_FORMAT_'] or 'H', + 'country': user['COUNTRY_'] or '', + 'lang': user['LANGUAGE_ISO2'] or '', # Never None + 'token': user['TOKEN'], + } + return user_data @et_task -def update_fxa_info(email, fxa_id): - created = False - try: - sub = Subscriber.objects.get(email=email) - except Subscriber.DoesNotExist: - kwargs = {'email': email, 'fxa_id': fxa_id} - user = get_external_user_data(email=email) - if user is not None and 'TOKEN' in user: - kwargs['token'] = user['TOKEN'] - else: - created = True - - sub = Subscriber.objects.create(**kwargs) - else: - sub.fxa_id = fxa_id - sub.save() - +def update_fxa_info(email, lang, fxa_id, source_url=None): + user = get_external_user_data(email=email) record = { 'EMAIL_ADDRESS_': email, - 'TOKEN': sub.token, 'FXA_ID': fxa_id, 'MODIFIED_DATE_': gmttime(), } - if created: - record['SOURCE_URL'] = 'https://accounts.firefox.com' - + if user: + format = user['format'] + token = user['token'] + Subscriber.objects.get_and_sync(email, token, fxa_id) + record['TOKEN'] = token + if not user['lang']: + record['LANGUAGE_ISO2'] = lang + 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() + format = 'H' + token = sub.token + record['TOKEN'] = token + record['LANGUAGE_ISO2'] = lang + record['SOURCE_URL'] = source_url or 'https://accounts.firefox.com' + + welcome = mogrify_message_id(FXACCOUNT_WELCOME, lang, format) + send_message(welcome, email, token, format) apply_updates(settings.EXACTTARGET_DATA, record) diff --git a/news/tests/test_users.py b/news/tests/test_users.py index c8cc878..1b6bd62 100644 --- a/news/tests/test_users.py +++ b/news/tests/test_users.py @@ -14,58 +14,161 @@ from news.views import look_for_user, get_user_data -@patch.object(tasks, 'apply_updates') -@patch.object(tasks, 'get_external_user_data') class UpdateFxAInfoTest(TestCase): - def test_new_user(self, get_mock, apply_mock): + def setUp(self): + patcher = patch.object(tasks, 'send_message') + self.addCleanup(patcher.stop) + self.send_message = patcher.start() + + patcher = patch.object(tasks, 'apply_updates') + self.addCleanup(patcher.stop) + self.apply_updates = patcher.start() + + patcher = patch.object(tasks, 'get_external_user_data') + self.addCleanup(patcher.stop) + self.get_external_user_data = patcher.start() + + def test_new_user(self): """Adding a new user to the DB should add fxa_id.""" - get_mock.return_value = None + self.get_external_user_data.return_value = None email = 'dude@example.com' fxa_id = 'the fxa abides' - tasks.update_fxa_info(email, fxa_id) + + tasks.update_fxa_info(email, 'de', fxa_id) sub = models.Subscriber.objects.get(email=email) self.assertEqual(sub.fxa_id, fxa_id) - call_data = apply_mock.call_args[0][1] - self.assertEqual(call_data['EMAIL_ADDRESS_'], email) - self.assertEqual(call_data['TOKEN'], sub.token) - self.assertEqual(call_data['FXA_ID'], fxa_id) - self.assertEqual(call_data['SOURCE_URL'], 'https://accounts.firefox.com') - get_mock.assert_called_with(email=email) - - def test_existing_user_not_in_basket(self, get_mock, apply_mock): - """Adding a user in ET but not basket should preserve token.""" + + self.apply_updates.assert_called_once_with(settings.EXACTTARGET_DATA, { + 'EMAIL_ADDRESS_': email, + 'TOKEN': sub.token, + 'FXA_ID': fxa_id, + 'LANGUAGE_ISO2': 'de', + 'SOURCE_URL': 'https://accounts.firefox.com', + 'MODIFIED_DATE_': ANY, + }) + self.get_external_user_data.assert_called_with(email=email) + self.send_message.assert_called_with('de_{0}'.format(tasks.FXACCOUNT_WELCOME), + email, sub.token, 'H') + + def test_user_in_et_not_basket(self): + """A user could exist in basket but not ET, should still work.""" + self.get_external_user_data.return_value = None email = 'dude@example.com' fxa_id = 'the fxa abides' - token = 'hehe... you said **token**.' - get_mock.return_value = { + + models.Subscriber.objects.create(email=email) + tasks.update_fxa_info(email, 'de', fxa_id) + sub = models.Subscriber.objects.get(email=email) + self.assertEqual(sub.fxa_id, fxa_id) + + self.apply_updates.assert_called_once_with(settings.EXACTTARGET_DATA, { 'EMAIL_ADDRESS_': email, - 'TOKEN': token, + 'TOKEN': sub.token, + 'FXA_ID': fxa_id, + 'LANGUAGE_ISO2': 'de', + 'SOURCE_URL': 'https://accounts.firefox.com', + 'MODIFIED_DATE_': ANY, + }) + self.get_external_user_data.assert_called_with(email=email) + self.send_message.assert_called_with('de_{0}'.format(tasks.FXACCOUNT_WELCOME), + email, sub.token, 'H') + + def test_existing_user_not_in_basket(self): + """Adding a user already in ET but not basket should preserve token.""" + email = 'dude@example.com' + fxa_id = 'the fxa abides' + token = 'hehe... you said **token**.' + self.get_external_user_data.return_value = { + 'email': email, + 'token': token, + 'lang': 'de', + 'format': '', } - tasks.update_fxa_info(email, fxa_id) + tasks.update_fxa_info(email, 'de', fxa_id) sub = models.Subscriber.objects.get(email=email) self.assertEqual(sub.fxa_id, fxa_id) self.assertEqual(sub.token, token) - call_data = apply_mock.call_args[0][1] - self.assertEqual(call_data['EMAIL_ADDRESS_'], email) - self.assertEqual(call_data['TOKEN'], token) - self.assertEqual(call_data['FXA_ID'], fxa_id) - self.assertNotIn('SOURCE_URL', call_data) - get_mock.assert_called_with(email=email) - - def test_existing_user(self, get_mock, apply_mock): + + self.apply_updates.assert_called_once_with(settings.EXACTTARGET_DATA, { + 'EMAIL_ADDRESS_': email, + 'TOKEN': token, + 'FXA_ID': fxa_id, + 'MODIFIED_DATE_': ANY, + }) + self.get_external_user_data.assert_called_with(email=email) + self.send_message.assert_called_with('de_{0}'.format(tasks.FXACCOUNT_WELCOME), + email, sub.token, '') + + def test_existing_user(self): """Adding a fxa_id to an existing user shouldn't modify other things.""" email = 'dude@example.com' fxa_id = 'the fxa abides' old_sub = models.Subscriber.objects.create(email=email) - tasks.update_fxa_info(email, fxa_id) + self.get_external_user_data.return_value = { + 'email': email, + 'token': old_sub.token, + 'lang': 'de', + 'format': 'T', + } + tasks.update_fxa_info(email, 'de', fxa_id) sub = models.Subscriber.objects.get(email=email) self.assertEqual(sub.fxa_id, fxa_id) - call_data = apply_mock.call_args[0][1] - self.assertEqual(call_data['EMAIL_ADDRESS_'], email) - self.assertEqual(call_data['TOKEN'], old_sub.token) - self.assertEqual(call_data['FXA_ID'], fxa_id) - self.assertNotIn('SOURCE_URL', call_data) - self.assertFalse(get_mock.called) + + self.apply_updates.assert_called_once_with(settings.EXACTTARGET_DATA, { + 'EMAIL_ADDRESS_': email, + 'TOKEN': old_sub.token, + 'FXA_ID': fxa_id, + 'MODIFIED_DATE_': ANY, + }) + self.send_message.assert_called_with('de_{0}_T'.format(tasks.FXACCOUNT_WELCOME), + email, sub.token, 'T') + + def test_existing_user_no_lang(self): + """Adding a fxa_id to an existing user should update lang only if not set.""" + email = 'dude@example.com' + fxa_id = 'the fxa abides' + old_sub = models.Subscriber.objects.create(email=email) + self.get_external_user_data.return_value = { + 'email': email, + 'token': old_sub.token, + 'lang': '', + 'format': '', + } + tasks.update_fxa_info(email, 'de', fxa_id) + sub = models.Subscriber.objects.get(email=email) + self.assertEqual(sub.fxa_id, fxa_id) + + self.apply_updates.assert_called_once_with(settings.EXACTTARGET_DATA, { + 'EMAIL_ADDRESS_': email, + 'TOKEN': old_sub.token, + 'FXA_ID': fxa_id, + 'LANGUAGE_ISO2': 'de', + 'MODIFIED_DATE_': ANY, + }) + self.send_message.assert_called_with('de_{0}'.format(tasks.FXACCOUNT_WELCOME), + email, sub.token, '') + + self.apply_updates.reset_mock() + self.send_message.reset_mock() + self.get_external_user_data.reset_mock() + self.get_external_user_data.return_value = { + 'email': email, + 'token': old_sub.token, + 'lang': 'es', + 'format': '', + } + tasks.update_fxa_info(email, 'de', fxa_id) + sub = models.Subscriber.objects.get(email=email) + self.assertEqual(sub.fxa_id, fxa_id) + + self.apply_updates.assert_called_once_with(settings.EXACTTARGET_DATA, { + 'EMAIL_ADDRESS_': email, + 'TOKEN': old_sub.token, + 'FXA_ID': fxa_id, + 'MODIFIED_DATE_': ANY, + }) + self.send_message.assert_called_with('de_{0}'.format(tasks.FXACCOUNT_WELCOME), + email, sub.token, '') class DebugUserTest(TestCase): diff --git a/news/tests/test_views.py b/news/tests/test_views.py index f7c023a..9904096 100644 --- a/news/tests/test_views.py +++ b/news/tests/test_views.py @@ -28,7 +28,8 @@ def test_requires_ssl(self, fxa_mock): """fxa-register requires SSL""" resp = self.client.post('/news/fxa-register/', { 'email': 'dude@example.com', - 'fxa_id': 'the dude has a Fx account.' + 'fxa_id': 'the dude has a Fx account.', + 'lang': 'de', }) self.assertEqual(resp.status_code, 401, resp.content) data = json.loads(resp.content) @@ -52,6 +53,7 @@ def test_requires_fxa_id(self, fxa_mock): auth = APIUser.objects.create(name="test") resp = self.ssl_post('/news/fxa-register/', { 'email': 'dude@example.com', + 'lang': 'de', 'api-key': auth.api_key, }) self.assertEqual(resp.status_code, 401, resp.content) @@ -63,6 +65,20 @@ def test_requires_email(self, fxa_mock): """fxa-register requires email address""" auth = APIUser.objects.create(name="test") resp = self.ssl_post('/news/fxa-register/', { + 'fxa_id': 'the dude has a Fx account.', + 'lang': 'de', + 'api-key': auth.api_key, + }) + self.assertEqual(resp.status_code, 401, resp.content) + data = json.loads(resp.content) + self.assertEqual(errors.BASKET_USAGE_ERROR, data['code']) + self.assertFalse(fxa_mock.delay.called) + + def test_requires_lang(self, fxa_mock): + """fxa-register requires language""" + auth = APIUser.objects.create(name="test") + resp = self.ssl_post('/news/fxa-register/', { + 'email': 'dude@example.com', 'fxa_id': 'the dude has a Fx account.', 'api-key': auth.api_key, }) @@ -71,6 +87,20 @@ def test_requires_email(self, fxa_mock): self.assertEqual(errors.BASKET_USAGE_ERROR, data['code']) self.assertFalse(fxa_mock.delay.called) + def test_requires_valid_lang(self, fxa_mock): + """fxa-register requires language""" + auth = APIUser.objects.create(name="test") + resp = self.ssl_post('/news/fxa-register/', { + 'email': 'dude@example.com', + 'fxa_id': 'the dude has a Fx account.', + 'lang': 'Phones ringing Dude.', + 'api-key': auth.api_key, + }) + self.assertEqual(resp.status_code, 400, resp.content) + data = json.loads(resp.content) + self.assertEqual(errors.BASKET_INVALID_LANGUAGE, data['code']) + self.assertFalse(fxa_mock.delay.called) + def test_with_ssl_and_api_key(self, fxa_mock): """fxa-register should succeed with SSL, API Key, and data.""" auth = APIUser.objects.create(name="test") @@ -78,12 +108,14 @@ def test_with_ssl_and_api_key(self, fxa_mock): 'email': 'dude@example.com', 'fxa_id': 'the dude has a Fx account.', 'api-key': auth.api_key, + 'lang': 'de', } resp = self.ssl_post('/news/fxa-register/', request_data) self.assertEqual(resp.status_code, 200, resp.content) data = json.loads(resp.content) self.assertEqual('ok', data['status']) - fxa_mock.delay.assert_called_once_with(request_data['email'], request_data['fxa_id']) + fxa_mock.delay.assert_called_once_with(request_data['email'], 'de', + request_data['fxa_id']) @patch('news.views.validate_email', none_mock) diff --git a/news/views.py b/news/views.py index b79b5a0..5727676 100644 --- a/news/views.py +++ b/news/views.py @@ -425,7 +425,22 @@ def fxa_register(request): 'desc': 'fxa-register requires a Firefox Account ID', 'code': errors.BASKET_USAGE_ERROR, }, 401) - update_fxa_info.delay(data['email'], data['fxa_id']) + if 'lang' not in data: + return HttpResponseJSON({ + 'status': 'error', + 'desc': 'fxa-register requires a language', + 'code': errors.BASKET_USAGE_ERROR, + }, 401) + if not language_code_is_valid(data['lang']): + return HttpResponseJSON({ + 'status': 'error', + 'desc': 'invalid language', + 'code': errors.BASKET_INVALID_LANGUAGE, + }, 400) + args = [data['email'], data['lang'], data['fxa_id']] + if 'source_url' in data: + args.append(data['source_url']) + update_fxa_info.delay(*args) return HttpResponseJSON({'status': 'ok'})