From ab242145cbf24dc8d6398e2ae52dfaa9b99141e6 Mon Sep 17 00:00:00 2001 From: PaulWattenberger Date: Fri, 15 Jul 2016 08:43:21 -0400 Subject: [PATCH] Fix 500 error unenrolling when running Sailthru integration on Stage ECOM-4960 --- .../migrations/0003_auto_20160715_1145.py | 39 +++ lms/djangoapps/email_marketing/models.py | 16 +- lms/djangoapps/email_marketing/signals.py | 58 ++-- lms/djangoapps/email_marketing/tasks.py | 171 ++++-------- .../email_marketing/tests/test_signals.py | 248 +++++++----------- 5 files changed, 240 insertions(+), 292 deletions(-) create mode 100644 lms/djangoapps/email_marketing/migrations/0003_auto_20160715_1145.py diff --git a/lms/djangoapps/email_marketing/migrations/0003_auto_20160715_1145.py b/lms/djangoapps/email_marketing/migrations/0003_auto_20160715_1145.py new file mode 100644 index 0000000000..56dcb69267 --- /dev/null +++ b/lms/djangoapps/email_marketing/migrations/0003_auto_20160715_1145.py @@ -0,0 +1,39 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('email_marketing', '0002_auto_20160623_1656'), + ] + + operations = [ + migrations.AddField( + model_name='emailmarketingconfiguration', + name='sailthru_lms_url_override', + field=models.CharField(help_text='Optional lms url scheme + host used to construct urls for content library, e.g. https://courses.edx.org.', max_length=80, blank=True), + ), + migrations.AlterField( + model_name='emailmarketingconfiguration', + name='sailthru_abandoned_cart_delay', + field=models.IntegerField(default=60, help_text='Sailthru minutes to wait before sending abandoned cart message. Deprecated.'), + ), + migrations.AlterField( + model_name='emailmarketingconfiguration', + name='sailthru_abandoned_cart_template', + field=models.CharField(help_text='Sailthru template to use on abandoned cart reminder. Deprecated.', max_length=20, blank=True), + ), + migrations.AlterField( + model_name='emailmarketingconfiguration', + name='sailthru_purchase_template', + field=models.CharField(help_text='Sailthru send template to use on purchasing a course seat. Deprecated ', max_length=20, blank=True), + ), + migrations.AlterField( + model_name='emailmarketingconfiguration', + name='sailthru_upgrade_template', + field=models.CharField(help_text='Sailthru send template to use on upgrading a course. Deprecated ', max_length=20, blank=True), + ), + ] diff --git a/lms/djangoapps/email_marketing/models.py b/lms/djangoapps/email_marketing/models.py index d31b71f73b..04d7285eeb 100644 --- a/lms/djangoapps/email_marketing/models.py +++ b/lms/djangoapps/email_marketing/models.py @@ -60,14 +60,14 @@ class EmailMarketingConfiguration(ConfigurationModel): max_length=20, blank=True, help_text=_( - "Sailthru template to use on abandoned cart reminder. " + "Sailthru template to use on abandoned cart reminder. Deprecated." ) ) sailthru_abandoned_cart_delay = models.fields.IntegerField( default=60, help_text=_( - "Sailthru minutes to wait before sending abandoned cart message." + "Sailthru minutes to wait before sending abandoned cart message. Deprecated." ) ) @@ -83,7 +83,7 @@ class EmailMarketingConfiguration(ConfigurationModel): max_length=20, blank=True, help_text=_( - "Sailthru send template to use on upgrading a course. " + "Sailthru send template to use on upgrading a course. Deprecated " ) ) @@ -91,7 +91,7 @@ class EmailMarketingConfiguration(ConfigurationModel): max_length=20, blank=True, help_text=_( - "Sailthru send template to use on purchasing a course seat. " + "Sailthru send template to use on purchasing a course seat. Deprecated " ) ) @@ -119,6 +119,14 @@ class EmailMarketingConfiguration(ConfigurationModel): ) ) + sailthru_lms_url_override = models.fields.CharField( + max_length=80, + blank=True, + help_text=_( + "Optional lms url scheme + host used to construct urls for content library, e.g. https://courses.edx.org." + ) + ) + def __unicode__(self): return u"Email marketing configuration: New user list %s, Activation template: %s" % \ (self.sailthru_new_user_list, self.sailthru_activation_template) diff --git a/lms/djangoapps/email_marketing/signals.py b/lms/djangoapps/email_marketing/signals.py index b5979ddec0..235b246072 100644 --- a/lms/djangoapps/email_marketing/signals.py +++ b/lms/djangoapps/email_marketing/signals.py @@ -6,6 +6,7 @@ import datetime import crum from django.dispatch import receiver +from django.core.urlresolvers import reverse from student.models import ENROLL_STATUS_CHANGE from student.cookies import CREATE_LOGON_COOKIE @@ -26,7 +27,7 @@ CHANGED_FIELDNAMES = ['username', 'is_active', 'name', 'gender', 'education', @receiver(ENROLL_STATUS_CHANGE) -def handle_enroll_status_change(sender, event=None, user=None, mode=None, course_id=None, cost=None, currency=None, +def handle_enroll_status_change(sender, event=None, user=None, mode=None, course_id=None, **kwargs): # pylint: disable=unused-argument """ Signal receiver for enroll/unenroll/purchase events @@ -35,35 +36,39 @@ def handle_enroll_status_change(sender, event=None, user=None, mode=None, course if not email_config.enabled or not event or not user or not mode or not course_id: return + # skip tracking (un)enrolls if simulated cost=0 + if email_config.sailthru_enroll_cost == 0: + return + request = crum.get_current_request() if not request: return + # get string course_id serializable to send through celery + course_id_string = unicode(course_id) + # figure out course url - course_url = _build_course_url(request, course_id.to_deprecated_string()) + course_url = _build_course_url(request, course_id_string, email_config) # pass event to email_marketing.tasks update_course_enrollment.delay(user.email, course_url, event, mode, - unit_cost=cost, course_id=course_id, currency=currency, + course_id=course_id_string, message_id=request.COOKIES.get('sailthru_bid')) -def _build_course_url(request, course_id): +def _build_course_url(request, course_id, email_config): """ Build a course url from a course id and the host from the current request + or use override in config :param request: :param course_id: :return: """ - host = request.get_host() - # hack for integration testing since Sailthru rejects urls with localhost - if host.startswith('localhost'): - host = 'courses.edx.org' - return '{scheme}://{host}/courses/{course}/info'.format( - scheme=request.scheme, - host=host, - course=course_id - ) + path = reverse('info', kwargs={'course_id': course_id}) + if email_config.sailthru_lms_url_override: + return '{}{}'.format(email_config.sailthru_lms_url_override, path) + else: + return '{}://{}{}'.format(request.scheme, request.get_host(), path) @receiver(CREATE_LOGON_COOKIE) @@ -142,7 +147,7 @@ def email_marketing_register_user(sender, user=None, profile=None, return # perform update asynchronously - update_user.delay(user.username, new_user=True) + update_user.delay(_create_sailthru_user_vars(user, user.profile), user.email, new_user=True) @receiver(USER_FIELD_CHANGED) @@ -179,7 +184,8 @@ def email_marketing_user_field_changed(sender, user=None, table=None, setting=No if not email_config.enabled: return # perform update asynchronously, flag if activation - update_user.delay(user.username, new_user=False, + update_user.delay(_create_sailthru_user_vars(user, user.profile), user.email, + new_user=False, activation=(setting == 'is_active') and new_value is True) elif setting == 'email': @@ -187,4 +193,24 @@ def email_marketing_user_field_changed(sender, user=None, table=None, setting=No email_config = EmailMarketingConfiguration.current() if not email_config.enabled: return - update_user_email.delay(user.username, old_value) + update_user_email.delay(user.email, old_value) + + +def _create_sailthru_user_vars(user, profile): + """ + Create sailthru user create/update vars from user + profile. + """ + sailthru_vars = {'username': user.username, + 'activated': int(user.is_active), + 'joined_date': user.date_joined.strftime("%Y-%m-%d")} + + if profile: + sailthru_vars['fullname'] = profile.name + sailthru_vars['gender'] = profile.gender + sailthru_vars['education'] = profile.level_of_education + + if profile.year_of_birth: + sailthru_vars['year_of_birth'] = profile.year_of_birth + sailthru_vars['country'] = unicode(profile.country.code) + + return sailthru_vars diff --git a/lms/djangoapps/email_marketing/tasks.py b/lms/djangoapps/email_marketing/tasks.py index 4c9c5723c9..aa8f9a5b56 100644 --- a/lms/djangoapps/email_marketing/tasks.py +++ b/lms/djangoapps/email_marketing/tasks.py @@ -5,13 +5,9 @@ import logging import time from celery import task -from django.contrib.auth.models import User -from django.http import Http404 from django.core.cache import cache from email_marketing.models import EmailMarketingConfiguration -from course_modes.models import CourseMode -from courseware.courses import get_course_by_id from student.models import EnrollStatusChange from sailthru.sailthru_client import SailthruClient @@ -22,11 +18,14 @@ log = logging.getLogger(__name__) # pylint: disable=not-callable @task(bind=True, default_retry_delay=3600, max_retries=24) -def update_user(self, username, new_user=False, activation=False): +def update_user(self, sailthru_vars, email, new_user=False, activation=False): """ Adds/updates Sailthru profile information for a user. Args: - username(str): A string representation of user identifier + sailthru_vars(dict): User profile information to pass as 'vars' to Sailthru + email(str): User email address + new_user(boolean): True if new registration + activation(boolean): True if activation request Returns: None """ @@ -34,22 +33,14 @@ def update_user(self, username, new_user=False, activation=False): if not email_config.enabled: return - # get user - try: - user = User.objects.select_related('profile').get(username=username) - except User.DoesNotExist: - log.error("User not found during Sailthru update %s", username) - return - - # get profile - profile = user.profile - sailthru_client = SailthruClient(email_config.sailthru_key, email_config.sailthru_secret) try: sailthru_response = sailthru_client.api_post("user", - _create_sailthru_user_parm(user, profile, new_user, email_config)) + _create_sailthru_user_parm(sailthru_vars, email, + new_user, email_config)) + except SailthruClientError as exc: - log.error("Exception attempting to add/update user %s in Sailthru - %s", username, unicode(exc)) + log.error("Exception attempting to add/update user %s in Sailthru - %s", email, unicode(exc)) raise self.retry(exc=exc, countdown=email_config.sailthru_retry_interval, max_retries=email_config.sailthru_max_retries) @@ -65,23 +56,23 @@ def update_user(self, username, new_user=False, activation=False): if activation and email_config.sailthru_activation_template: try: sailthru_response = sailthru_client.api_post("send", - {"email": user.email, + {"email": email, "template": email_config.sailthru_activation_template}) except SailthruClientError as exc: - log.error("Exception attempting to send welcome email to user %s in Sailthru - %s", username, unicode(exc)) + log.error("Exception attempting to send welcome email to user %s in Sailthru - %s", email, unicode(exc)) raise self.retry(exc=exc, countdown=email_config.sailthru_retry_interval, max_retries=email_config.sailthru_max_retries) if not sailthru_response.is_ok(): error = sailthru_response.get_error() - # probably an invalid template name, just put out error + # probably a disabled template, just put out error message log.error("Error attempting to send welcome email to user in Sailthru: %s", error.get_message()) # pylint: disable=not-callable @task(bind=True, default_retry_delay=3600, max_retries=24) -def update_user_email(self, username, old_email): +def update_user_email(self, new_email, old_email): """ Adds/updates Sailthru when a user email address is changed Args: @@ -94,24 +85,17 @@ def update_user_email(self, username, old_email): if not email_config.enabled: return - # get user - try: - user = User.objects.get(username=username) - except User.DoesNotExist: - log.error("User not found duing Sailthru update %s", username) - return - # ignore if email not changed - if user.email == old_email: + if new_email == old_email: return - sailthru_parms = {"id": old_email, "key": "email", "keysconflict": "merge", "keys": {"email": user.email}} + sailthru_parms = {"id": old_email, "key": "email", "keysconflict": "merge", "keys": {"email": new_email}} try: sailthru_client = SailthruClient(email_config.sailthru_key, email_config.sailthru_secret) sailthru_response = sailthru_client.api_post("user", sailthru_parms) except SailthruClientError as exc: - log.error("Exception attempting to update email for %s in Sailthru - %s", username, unicode(exc)) + log.error("Exception attempting to update email for %s in Sailthru - %s", old_email, unicode(exc)) raise self.retry(exc=exc, countdown=email_config.sailthru_retry_interval, max_retries=email_config.sailthru_max_retries) @@ -123,26 +107,12 @@ def update_user_email(self, username, old_email): max_retries=email_config.sailthru_max_retries) -def _create_sailthru_user_parm(user, profile, new_user, email_config): +def _create_sailthru_user_parm(sailthru_vars, email, new_user, email_config): """ - Create sailthru user create/update parms from user + profile. + Create sailthru user create/update parms """ - sailthru_user = {'id': user.email, 'key': 'email'} - sailthru_vars = {'username': user.username, - 'activated': int(user.is_active), - 'joined_date': user.date_joined.strftime("%Y-%m-%d")} - sailthru_user['vars'] = sailthru_vars - sailthru_vars['last_changed_time'] = int(time.time()) - - if profile: - sailthru_vars['fullname'] = profile.name - sailthru_vars['gender'] = profile.gender - sailthru_vars['education'] = profile.level_of_education - # age is not useful since it is not automatically updated - #sailthru_vars['age'] = profile.age or -1 - if profile.year_of_birth: - sailthru_vars['year_of_birth'] = profile.year_of_birth - sailthru_vars['country'] = unicode(profile.country.code) + sailthru_user = {'id': email, 'key': 'email'} + sailthru_user['vars'] = dict(sailthru_vars, last_changed_time=int(time.time())) # if new user add to list if new_user and email_config.sailthru_new_user_list: @@ -154,17 +124,16 @@ def _create_sailthru_user_parm(user, profile, new_user, email_config): # pylint: disable=not-callable @task(bind=True, default_retry_delay=3600, max_retries=24) def update_course_enrollment(self, email, course_url, event, mode, - unit_cost=None, course_id=None, - currency=None, message_id=None): # pylint: disable=unused-argument + course_id=None, message_id=None): # pylint: disable=unused-argument """ Adds/updates Sailthru when a user enrolls/unenrolls/adds to cart/purchases/upgrades a course Args: email(str): The user's email address course_url(str): Course home page url event(str): event type - mode(object): enroll mode (audit, verification, ...) + mode(str): enroll mode (audit, verification, ...) unit_cost: cost if purchase event - course_id(CourseKey): course id + course_id(str): course run id currency(str): currency if purchase event - currently ignored since Sailthru only supports USD Returns: None @@ -172,98 +141,72 @@ def update_course_enrollment(self, email, course_url, event, mode, The event can be one of the following: EnrollStatusChange.enroll - A free enroll (mode=audit) + A free enroll (mode=audit or honor) EnrollStatusChange.unenroll An unenroll EnrollStatusChange.upgrade_start - A paid upgrade added to cart + A paid upgrade added to cart - ignored EnrollStatusChange.upgrade_complete - A paid upgrade purchase complete + A paid upgrade purchase complete - ignored EnrollStatusChange.paid_start - A non-free course added to cart + A non-free course added to cart - ignored EnrollStatusChange.paid_complete - A non-free course purchase complete + A non-free course purchase complete - ignored """ email_config = EmailMarketingConfiguration.current() if not email_config.enabled: return - course_id_string = course_id.to_deprecated_string() - # Use event type to figure out processing required - new_enroll = unenroll = fetch_tags = False - incomplete = send_template = None - if unit_cost: - cost_in_cents = unit_cost * 100 + unenroll = False + send_template = None + cost_in_cents = 0 if event == EnrollStatusChange.enroll: - # new enroll for audit (no cost) - new_enroll = True - fetch_tags = True send_template = email_config.sailthru_enroll_template - # set cost of $1 so that Sailthru recognizes the event + # set cost so that Sailthru recognizes the event cost_in_cents = email_config.sailthru_enroll_cost elif event == EnrollStatusChange.unenroll: # unenroll - need to update list of unenrolled courses for user in Sailthru unenroll = True - elif event == EnrollStatusChange.upgrade_start: - # add upgrade to cart - incomplete = 1 - - elif event == EnrollStatusChange.paid_start: - # add course purchase (probably 'honor') to cart - incomplete = 1 - - elif event == EnrollStatusChange.upgrade_complete: - # upgrade complete - fetch_tags = True - send_template = email_config.sailthru_upgrade_template - - elif event == EnrollStatusChange.paid_complete: - # paid course purchase complete - new_enroll = True - fetch_tags = True - send_template = email_config.sailthru_purchase_template + else: + # All purchase events should be handled by ecommerce, so ignore + return sailthru_client = SailthruClient(email_config.sailthru_key, email_config.sailthru_secret) - # update the "unenrolled" course array in the user record on Sailthru if new enroll or unenroll - if new_enroll or unenroll: - if not _update_unenrolled_list(sailthru_client, email, course_url, unenroll): - raise self.retry(countdown=email_config.sailthru_retry_interval, - max_retries=email_config.sailthru_max_retries) + # update the "unenrolled" course array in the user record on Sailthru + if not _update_unenrolled_list(sailthru_client, email, course_url, unenroll): + raise self.retry(countdown=email_config.sailthru_retry_interval, + max_retries=email_config.sailthru_max_retries) # if there is a cost, call Sailthru purchase api to record if cost_in_cents: # get course information if configured and appropriate event - if fetch_tags and email_config.sailthru_get_tags_from_sailthru: + course_data = {} + if email_config.sailthru_get_tags_from_sailthru: course_data = _get_course_content(course_url, sailthru_client, email_config) - else: - course_data = {} # build item description - item = _build_purchase_item(course_id_string, course_url, cost_in_cents, mode, course_data, course_id) + item = _build_purchase_item(course_id, course_url, cost_in_cents, mode, course_data) # build purchase api options list options = {} - if incomplete and email_config.sailthru_abandoned_cart_template: - options['reminder_template'] = email_config.sailthru_abandoned_cart_template - options['reminder_time'] = "+{} minutes".format(email_config.sailthru_abandoned_cart_delay) # add appropriate send template if send_template: options['send_template'] = send_template - if not _record_purchase(sailthru_client, email, item, incomplete, message_id, options): + if not _record_purchase(sailthru_client, email, item, message_id, options): raise self.retry(countdown=email_config.sailthru_retry_interval, max_retries=email_config.sailthru_max_retries) -def _build_purchase_item(course_id_string, course_url, cost_in_cents, mode, course_data, course_id): +def _build_purchase_item(course_id_string, course_url, cost_in_cents, mode, course_data): """ Build Sailthru purchase item object :return: item @@ -277,38 +220,22 @@ def _build_purchase_item(course_id_string, course_url, cost_in_cents, mode, cour 'qty': 1, } - # get title from course info if we don't already have it from Sailthru + # make up title if we don't already have it from Sailthru if 'title' in course_data: item['title'] = course_data['title'] else: - try: - course = get_course_by_id(course_id) - item['title'] = course.display_name - except Http404: - # can't find, just invent title - item['title'] = 'Course {} mode: {}'.format(course_id_string, mode) + item['title'] = 'Course {} mode: {}'.format(course_id_string, mode) if 'tags' in course_data: item['tags'] = course_data['tags'] # add vars to item - sailthru_vars = {} - if 'vars' in course_data: - sailthru_vars = course_data['vars'] - sailthru_vars['mode'] = mode - sailthru_vars['course_run_id'] = course_id_string - item['vars'] = sailthru_vars - - # get list of modes for course and add upgrade deadlines for verified modes - for mode_entry in CourseMode.modes_for_course(course_id): - if mode_entry.expiration_datetime is not None and CourseMode.is_verified_slug(mode_entry.slug): - sailthru_vars['upgrade_deadline_{}'.format(mode_entry.slug)] = \ - mode_entry.expiration_datetime.strftime("%Y-%m-%d") + item['vars'] = dict(course_data.get('vars', {}), mode=mode, course_run_id=course_id_string) return item -def _record_purchase(sailthru_client, email, item, incomplete, message_id, options): +def _record_purchase(sailthru_client, email, item, message_id, options): """ Record a purchase in Sailthru :param sailthru_client: @@ -321,7 +248,7 @@ def _record_purchase(sailthru_client, email, item, incomplete, message_id, optio """ try: sailthru_response = sailthru_client.purchase(email, [item], - incomplete=incomplete, message_id=message_id, + message_id=message_id, options=options) if not sailthru_response.is_ok(): diff --git a/lms/djangoapps/email_marketing/tests/test_signals.py b/lms/djangoapps/email_marketing/tests/test_signals.py index 3adef8d7a3..9fcfa658f7 100644 --- a/lms/djangoapps/email_marketing/tests/test_signals.py +++ b/lms/djangoapps/email_marketing/tests/test_signals.py @@ -1,13 +1,11 @@ """Tests of email marketing signal handlers.""" import logging import ddt -import datetime from django.test import TestCase from django.contrib.auth.models import AnonymousUser from mock import patch, ANY from util.json_request import JsonResponse -from django.http import Http404 from email_marketing.signals import handle_enroll_status_change, \ email_marketing_register_user, \ @@ -18,13 +16,9 @@ from email_marketing.tasks import update_user, update_user_email, update_course_ from email_marketing.models import EmailMarketingConfiguration from django.test.client import RequestFactory from student.tests.factories import UserFactory, UserProfileFactory -from request_cache.middleware import RequestCache from student.models import EnrollStatusChange from opaque_keys.edx.keys import CourseKey -from course_modes.models import CourseMode -from xmodule.modulestore.tests.factories import CourseFactory -from sailthru.sailthru_client import SailthruClient from sailthru.sailthru_response import SailthruResponse from sailthru.sailthru_error import SailthruClientError @@ -33,8 +27,8 @@ log = logging.getLogger(__name__) TEST_EMAIL = "test@edx.org" -def update_email_marketing_config(enabled=False, key='badkey', secret='badsecret', new_user_list='new list', - template='Activation'): +def update_email_marketing_config(enabled=True, key='badkey', secret='badsecret', new_user_list='new list', + template='Activation', enroll_cost=100, lms_url_override='http://testserver'): """ Enable / Disable Sailthru integration """ @@ -45,10 +39,10 @@ def update_email_marketing_config(enabled=False, key='badkey', secret='badsecret sailthru_new_user_list=new_user_list, sailthru_activation_template=template, sailthru_enroll_template='enroll_template', - sailthru_upgrade_template='upgrade_template', - sailthru_purchase_template='purchase_template', - sailthru_abandoned_cart_template='abandoned_template', - sailthru_get_tags_from_sailthru=False + sailthru_lms_url_override=lms_url_override, + sailthru_get_tags_from_sailthru=False, + sailthru_enroll_cost=enroll_cost, + sailthru_max_retries=0, ) @@ -59,9 +53,13 @@ class EmailMarketingTests(TestCase): """ def setUp(self): + update_email_marketing_config(enabled=False) self.request_factory = RequestFactory() self.user = UserFactory.create(username='test', email=TEST_EMAIL) self.profile = self.user.profile + self.profile.year_of_birth = 1980 + self.profile.save() + self.request = self.request_factory.get("foo") update_email_marketing_config(enabled=True) @@ -121,7 +119,7 @@ class EmailMarketingTests(TestCase): test async method in tasks that actually updates Sailthru """ mock_sailthru.return_value = SailthruResponse(JsonResponse({'ok': True})) - update_user.delay(self.user.username, new_user=True) + update_user.delay({'gender': 'm', 'username': 'test', 'activated': 1}, TEST_EMAIL, new_user=True) self.assertFalse(mock_log_error.called) self.assertEquals(mock_sailthru.call_args[0][0], "user") userparms = mock_sailthru.call_args[0][1] @@ -138,7 +136,7 @@ class EmailMarketingTests(TestCase): test send of activation template """ mock_sailthru.return_value = SailthruResponse(JsonResponse({'ok': True})) - update_user.delay(self.user.username, new_user=True, activation=True) + update_user.delay({}, self.user.email, new_user=True, activation=True) # look for call args for 2nd call self.assertEquals(mock_sailthru.call_args[0][0], "send") userparms = mock_sailthru.call_args[0][1] @@ -152,41 +150,28 @@ class EmailMarketingTests(TestCase): Ensure that error returned from Sailthru api is logged """ mock_sailthru.return_value = SailthruResponse(JsonResponse({'error': 100, 'errormsg': 'Got an error'})) - update_user.delay(self.user.username) + update_user.delay({}, self.user.email) self.assertTrue(mock_log_error.called) # force Sailthru API exception + mock_log_error.reset_mock() mock_sailthru.side_effect = SailthruClientError - update_user.delay(self.user.username) + update_user.delay({}, self.user.email) self.assertTrue(mock_log_error.called) # force Sailthru API exception on 2nd call - mock_sailthru.side_effect = [None, SailthruClientError] - mock_sailthru.return_value = SailthruResponse(JsonResponse({'ok': True})) - update_user.delay(self.user.username, new_user=True) + mock_log_error.reset_mock() + mock_sailthru.side_effect = [SailthruResponse(JsonResponse({'ok': True})), SailthruClientError] + update_user.delay({}, self.user.email, activation=True) self.assertTrue(mock_log_error.called) # force Sailthru API error return on 2nd call - mock_sailthru.side_effect = None - mock_sailthru.return_value = [SailthruResponse(JsonResponse({'ok': True})), - SailthruResponse(JsonResponse({'error': 100, 'errormsg': 'Got an error'}))] - update_user.delay(self.user.username, new_user=True) + mock_log_error.reset_mock() + mock_sailthru.side_effect = [SailthruResponse(JsonResponse({'ok': True})), + SailthruResponse(JsonResponse({'error': 100, 'errormsg': 'Got an error'}))] + update_user.delay({}, self.user.email, activation=True) self.assertTrue(mock_log_error.called) - @patch('email_marketing.tasks.log.error') - @patch('email_marketing.tasks.SailthruClient.api_post') - def test_update_user_error_logging_bad_user(self, mock_sailthru, mock_log_error): - """ - Test update_user with invalid user - """ - update_user.delay('baduser') - self.assertTrue(mock_log_error.called) - self.assertFalse(mock_sailthru.called) - - update_user_email.delay('baduser', 'aa@bb.com') - self.assertTrue(mock_log_error.called) - self.assertFalse(mock_sailthru.called) - @patch('email_marketing.tasks.log.error') @patch('email_marketing.tasks.SailthruClient.api_post') def test_just_return_tasks(self, mock_sailthru, mock_log_error): @@ -236,6 +221,13 @@ class EmailMarketingTests(TestCase): email_marketing_user_field_changed(None, user=anon) self.assertFalse(mock_log_error.called) + # make sure enroll ignored when cost = 0 + update_email_marketing_config(enroll_cost=0) + handle_enroll_status_change(None, event=EnrollStatusChange.enroll, + user=self.user, + mode='audit', course_id=self.course_id) + self.assertFalse(mock_log_error.called) + @patch('email_marketing.signals.crum.get_current_request') @patch('lms.djangoapps.email_marketing.tasks.update_course_enrollment.delay') def test_handle_enroll_status_change(self, mock_update_course_enrollment, mock_get_current_request): @@ -259,10 +251,24 @@ class EmailMarketingTests(TestCase): self.course_url, EnrollStatusChange.enroll, 'audit', - course_id=self.course_id, - currency=None, - message_id='cookie_bid', - unit_cost=None) + course_id=self.course_id_string, + message_id='cookie_bid') + + # now test with current request constructing url from request + mock_get_current_request.return_value = self.request + update_email_marketing_config(lms_url_override='') + self.request.COOKIES['sailthru_bid'] = 'cookie_bid' + handle_enroll_status_change(None, event=EnrollStatusChange.enroll, + user=self.user, + mode='audit', course_id=self.course_id, + cost=None, currency=None) + self.assertTrue(mock_update_course_enrollment.called) + mock_update_course_enrollment.assert_called_with(TEST_EMAIL, + self.course_url, + EnrollStatusChange.enroll, + 'audit', + course_id=self.course_id_string, + message_id='cookie_bid') @patch('email_marketing.tasks.SailthruClient.api_post') def test_change_email(self, mock_sailthru): @@ -270,7 +276,7 @@ class EmailMarketingTests(TestCase): test async method in task that changes email in Sailthru """ mock_sailthru.return_value = SailthruResponse(JsonResponse({'ok': True})) - update_user_email.delay(self.user.username, "old@edx.org") + update_user_email.delay(TEST_EMAIL, "old@edx.org") self.assertEquals(mock_sailthru.call_args[0][0], "user") userparms = mock_sailthru.call_args[0][1] self.assertEquals(userparms['key'], "email") @@ -281,155 +287,83 @@ class EmailMarketingTests(TestCase): @patch('email_marketing.tasks.SailthruClient.purchase') @patch('email_marketing.tasks.SailthruClient.api_get') @patch('email_marketing.tasks.SailthruClient.api_post') - @patch('email_marketing.tasks.get_course_by_id') - def test_update_course_enrollment(self, mock_get_course, mock_sailthru_api_post, + def test_update_course_enrollment(self, mock_sailthru_api_post, mock_sailthru_api_get, mock_sailthru_purchase, mock_log_error): """ test async method in task posts enrolls and purchases """ - CourseMode.objects.create( - course_id=self.course_id, - mode_slug=CourseMode.AUDIT, - mode_display_name=CourseMode.AUDIT - ) - CourseMode.objects.create( - course_id=self.course_id, - mode_slug=CourseMode.VERIFIED, - mode_display_name=CourseMode.VERIFIED, - min_price=49, - expiration_datetime=datetime.date(2020, 3, 12) - ) - mock_get_course.return_value = {'display_name': "Test Title"} mock_sailthru_api_post.return_value = SailthruResponse(JsonResponse({'ok': True})) mock_sailthru_api_get.return_value = SailthruResponse(JsonResponse({'vars': {'unenrolled': ['course_u1']}})) mock_sailthru_purchase.return_value = SailthruResponse(JsonResponse({'ok': True})) # test enroll - mock_get_course.side_effect = Http404 update_course_enrollment.delay(TEST_EMAIL, self.course_url, EnrollStatusChange.enroll, 'audit', - course_id=self.course_id, - currency='USD', - message_id='cookie_bid', - unit_cost=0) - mock_sailthru_purchase.assert_called_with(TEST_EMAIL, [{'vars': {'course_run_id': self.course_id_string, 'mode': 'audit', - 'upgrade_deadline_verified': '2020-03-12'}, + course_id=self.course_id_string, + message_id='cookie_bid') + mock_sailthru_purchase.assert_called_with(TEST_EMAIL, [{'vars': {'course_run_id': self.course_id_string, 'mode': 'audit'}, 'title': 'Course ' + self.course_id_string + ' mode: audit', 'url': self.course_url, 'price': 100, 'qty': 1, 'id': self.course_id_string + '-audit'}], options={'send_template': 'enroll_template'}, - incomplete=None, message_id='cookie_bid') + message_id='cookie_bid') # test unenroll update_course_enrollment.delay(TEST_EMAIL, self.course_url, EnrollStatusChange.unenroll, 'audit', - course_id=self.course_id, - currency='USD', - message_id='cookie_bid', - unit_cost=0) - mock_sailthru_purchase.assert_called_with(TEST_EMAIL, [{'vars': {'course_run_id': self.course_id_string, 'mode': 'audit', - 'upgrade_deadline_verified': '2020-03-12'}, + course_id=self.course_id_string, + message_id='cookie_bid') + mock_sailthru_purchase.assert_called_with(TEST_EMAIL, [{'vars': {'course_run_id': self.course_id_string, 'mode': 'audit'}, 'title': 'Course ' + self.course_id_string + ' mode: audit', 'url': self.course_url, 'price': 100, 'qty': 1, 'id': self.course_id_string + '-audit'}], options={'send_template': 'enroll_template'}, - incomplete=None, message_id='cookie_bid') - - # test add upgrade to cart - update_course_enrollment.delay(TEST_EMAIL, - self.course_url, - EnrollStatusChange.upgrade_start, - 'verified', - course_id=self.course_id, - currency='USD', - message_id='cookie_bid', - unit_cost=49) - mock_sailthru_purchase.assert_called_with(TEST_EMAIL, [{'vars': {'course_run_id': self.course_id_string, 'mode': 'verified', - 'upgrade_deadline_verified': '2020-03-12'}, - 'title': 'Course ' + self.course_id_string + ' mode: verified', - 'url': self.course_url, - 'price': 4900, 'qty': 1, 'id': self.course_id_string + '-verified'}], - options={'reminder_template': 'abandoned_template', 'reminder_time': '+60 minutes'}, - incomplete=1, message_id='cookie_bid') - - # test add purchase to cart - update_course_enrollment.delay(TEST_EMAIL, - self.course_url, - EnrollStatusChange.paid_start, - 'honor', - course_id=self.course_id, - currency='USD', - message_id='cookie_bid', - unit_cost=49) - mock_sailthru_purchase.assert_called_with(TEST_EMAIL, [{'vars': {'course_run_id': self.course_id_string, 'mode': 'honor', - 'upgrade_deadline_verified': '2020-03-12'}, - 'title': 'Course ' + self.course_id_string + ' mode: honor', - 'url': self.course_url, - 'price': 4900, 'qty': 1, 'id': self.course_id_string + '-honor'}], - options={'reminder_template': 'abandoned_template', 'reminder_time': '+60 minutes'}, - incomplete=1, message_id='cookie_bid') - - # test purchase complete - update_course_enrollment.delay(TEST_EMAIL, - self.course_url, - EnrollStatusChange.paid_complete, - 'honor', - course_id=self.course_id, - currency='USD', - message_id='cookie_bid', - unit_cost=99) - mock_sailthru_purchase.assert_called_with(TEST_EMAIL, [{'vars': {'course_run_id': self.course_id_string, 'mode': 'honor', - 'upgrade_deadline_verified': '2020-03-12'}, - 'title': 'Course ' + self.course_id_string + ' mode: honor', - 'url': self.course_url, - 'price': 9900, 'qty': 1, 'id': self.course_id_string + '-honor'}], - options={'send_template': 'purchase_template'}, - incomplete=None, message_id='cookie_bid') - - # test upgrade complete - update_course_enrollment.delay(TEST_EMAIL, - self.course_url, - EnrollStatusChange.upgrade_complete, - 'verified', - course_id=self.course_id, - currency='USD', - message_id='cookie_bid', - unit_cost=99) - mock_sailthru_purchase.assert_called_with(TEST_EMAIL, [{'vars': {'course_run_id': self.course_id_string, 'mode': 'verified', - 'upgrade_deadline_verified': '2020-03-12'}, - 'title': 'Course ' + self.course_id_string + ' mode: verified', - 'url': self.course_url, - 'price': 9900, 'qty': 1, 'id': self.course_id_string + '-verified'}], - options={'send_template': 'upgrade_template'}, - incomplete=None, message_id='cookie_bid') + message_id='cookie_bid') # test purchase API error mock_sailthru_purchase.return_value = SailthruResponse(JsonResponse({'error': 100, 'errormsg': 'Got an error'})) update_course_enrollment.delay(TEST_EMAIL, self.course_url, - EnrollStatusChange.upgrade_complete, + EnrollStatusChange.enroll, 'verified', - course_id=self.course_id, - currency='USD', - message_id='cookie_bid', - unit_cost=99) + course_id=self.course_id_string, + message_id='cookie_bid') self.assertTrue(mock_log_error.called) # test purchase API exception mock_sailthru_purchase.side_effect = SailthruClientError update_course_enrollment.delay(TEST_EMAIL, self.course_url, - EnrollStatusChange.upgrade_complete, + EnrollStatusChange.enroll, 'verified', - course_id=self.course_id, - currency='USD', - message_id='cookie_bid', - unit_cost=99) + course_id=self.course_id_string, + message_id='cookie_bid') + self.assertTrue(mock_log_error.called) + + # test unsupported event + mock_sailthru_purchase.side_effect = SailthruClientError + mock_log_error.reset_mock() + update_course_enrollment.delay(TEST_EMAIL, + self.course_url, + EnrollStatusChange.upgrade_start, + 'verified', + course_id=self.course_id_string, + message_id='cookie_bid') + self.assertFalse(mock_log_error.called) + + # test error updating user + mock_sailthru_api_get.return_value = SailthruResponse(JsonResponse({'error': 100, 'errormsg': 'Got an error'})) + update_course_enrollment.delay(TEST_EMAIL, + self.course_url, + EnrollStatusChange.enroll, + 'verified', + course_id=self.course_id_string, + message_id='cookie_bid') self.assertTrue(mock_log_error.called) @patch('email_marketing.tasks.SailthruClient') @@ -533,3 +467,17 @@ class EmailMarketingTests(TestCase): """ email_marketing_user_field_changed(None, self.user, table=table, setting=setting, new_value=value) self.assertEqual(mock_update_user.called, result) + + @patch('lms.djangoapps.email_marketing.tasks.update_user_email.delay') + def test_modify_email(self, mock_update_user): + """ + Test that change to email calls update_user_email + """ + email_marketing_user_field_changed(None, self.user, table='auth_user', setting='email', old_value='new@a.com') + mock_update_user.assert_called_with(self.user.email, 'new@a.com') + + # make sure nothing called if disabled + mock_update_user.reset_mock() + update_email_marketing_config(enabled=False) + email_marketing_user_field_changed(None, self.user, table='auth_user', setting='email', old_value='new@a.com') + self.assertFalse(mock_update_user.called)