From 3bda3c4a774ff69df5823037d0e3b06393b7d501 Mon Sep 17 00:00:00 2001 From: Simon Chen Date: Wed, 4 Jan 2017 14:48:19 -0500 Subject: [PATCH] Remove Sailthru enrollment related signal handlers because they are being invoked from the ECOM service already. ECOM-6581 --- lms/djangoapps/email_marketing/signals.py | 49 +--- lms/djangoapps/email_marketing/tasks.py | 226 ------------------ .../email_marketing/tests/test_signals.py | 219 +---------------- 3 files changed, 5 insertions(+), 489 deletions(-) diff --git a/lms/djangoapps/email_marketing/signals.py b/lms/djangoapps/email_marketing/signals.py index 27b36abe0c..300be26b5c 100644 --- a/lms/djangoapps/email_marketing/signals.py +++ b/lms/djangoapps/email_marketing/signals.py @@ -6,16 +6,14 @@ import datetime import logging from django.conf import settings -from django.core.urlresolvers import reverse from django.dispatch import receiver -from student.models import ENROLL_STATUS_CHANGE from student.cookies import CREATE_LOGON_COOKIE from student.views import REGISTER_USER from email_marketing.models import EmailMarketingConfiguration from util.model_utils import USER_FIELD_CHANGED from lms.djangoapps.email_marketing.tasks import ( - update_user, update_user_email, update_course_enrollment + update_user, update_user_email ) from sailthru.sailthru_client import SailthruClient @@ -29,51 +27,6 @@ CHANGED_FIELDNAMES = ['username', 'is_active', 'name', 'gender', 'education', 'country'] -@receiver(ENROLL_STATUS_CHANGE) -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 - """ - email_config = EmailMarketingConfiguration.current() - 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_string, email_config) - - # pass event to email_marketing.tasks - update_course_enrollment.delay(user.email, course_url, event, mode, - course_id=course_id_string, - message_id=request.COOKIES.get('sailthru_bid')) - - -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: - """ - 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) def add_email_marketing_cookies(sender, response=None, user=None, **kwargs): # pylint: disable=unused-argument diff --git a/lms/djangoapps/email_marketing/tasks.py b/lms/djangoapps/email_marketing/tasks.py index aa4c0e885b..bd7a229663 100644 --- a/lms/djangoapps/email_marketing/tasks.py +++ b/lms/djangoapps/email_marketing/tasks.py @@ -9,8 +9,6 @@ from django.core.cache import cache from django.conf import settings from email_marketing.models import EmailMarketingConfiguration -from student.models import EnrollStatusChange - from sailthru.sailthru_client import SailthruClient from sailthru.sailthru_error import SailthruClientError @@ -131,175 +129,6 @@ def _create_email_user_param(sailthru_vars, sailthru_client, email, new_user, em return sailthru_user -# pylint: disable=not-callable -@task(bind=True, default_retry_delay=3600, max_retries=24) -def update_course_enrollment(self, email, course_url, event, mode, - 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(str): enroll mode (audit, verification, ...) - unit_cost: cost if purchase event - course_id(str): course run id - currency(str): currency if purchase event - currently ignored since Sailthru only supports USD - Returns: - None - - - The event can be one of the following: - EnrollStatusChange.enroll - A free enroll (mode=audit or honor) - EnrollStatusChange.unenroll - An unenroll - EnrollStatusChange.upgrade_start - A paid upgrade added to cart - ignored - EnrollStatusChange.upgrade_complete - A paid upgrade purchase complete - ignored - EnrollStatusChange.paid_start - A non-free course added to cart - ignored - EnrollStatusChange.paid_complete - A non-free course purchase complete - ignored - """ - - email_config = EmailMarketingConfiguration.current() - if not email_config.enabled: - return - - # Use event type to figure out processing required - unenroll = False - send_template = None - cost_in_cents = 0 - - if event == EnrollStatusChange.enroll: - send_template = email_config.sailthru_enroll_template - # 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 - - 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 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 - course_data = {} - if email_config.sailthru_get_tags_from_sailthru: - course_data = _get_course_content(course_url, sailthru_client, email_config) - - # build item description - item = _build_purchase_item(course_id, course_url, cost_in_cents, mode, course_data) - - # build purchase api options list - options = {} - - # add appropriate send template - if send_template: - options['send_template'] = send_template - - 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): - """ - Build Sailthru purchase item object - :return: item - """ - - # build item description - item = { - 'id': "{}-{}".format(course_id_string, mode), - 'url': course_url, - 'price': cost_in_cents, - 'qty': 1, - } - - # make up title if we don't already have it from Sailthru - if 'title' in course_data: - item['title'] = course_data['title'] - else: - item['title'] = 'Course {} mode: {}'.format(course_id_string, mode) - - if 'tags' in course_data: - item['tags'] = course_data['tags'] - - # add vars to item - item['vars'] = dict(course_data.get('vars', {}), mode=mode, course_run_id=course_id_string) - - return item - - -def _record_purchase(sailthru_client, email, item, message_id, options): - """ - Record a purchase in Sailthru - :param sailthru_client: - :param email: - :param item: - :param incomplete: - :param message_id: - :param options: - :return: False it retryable error - """ - try: - sailthru_response = sailthru_client.purchase(email, [item], - message_id=message_id, - options=options) - - if not sailthru_response.is_ok(): - error = sailthru_response.get_error() - log.error("Error attempting to record purchase in Sailthru: %s", error.get_message()) - return not _retryable_sailthru_error(error) - - except SailthruClientError as exc: - log.error("Exception attempting to record purchase for %s in Sailthru - %s", email, unicode(exc)) - return False - - return True - - -def _get_course_content(course_url, sailthru_client, email_config): - """ - Get course information using the Sailthru content api. - - If there is an error, just return with an empty response. - :param course_url: - :param sailthru_client: - :return: dict with course information - """ - # check cache first - response = cache.get(course_url) - if not response: - try: - sailthru_response = sailthru_client.api_get("content", {"id": course_url}) - - if not sailthru_response.is_ok(): - return {} - - response = sailthru_response.json - cache.set(course_url, response, email_config.sailthru_content_cache_age) - - except SailthruClientError: - response = {} - - return response - - def _get_or_create_user_list_for_site(sailthru_client, site=None, default_list_name=None): """ Get the user list name from cache if exists else create one and return the name, @@ -393,61 +222,6 @@ def _create_user_list(sailthru_client, list_name): return True -def _update_unenrolled_list(sailthru_client, email, course_url, unenroll): - """ - Maintain a list of courses the user has unenrolled from in the Sailthru user record - :param sailthru_client: - :param email: - :param email_config: - :param course_url: - :param unenroll: - :return: False if retryable error, else True - """ - try: - # get the user 'vars' values from sailthru - sailthru_response = sailthru_client.api_get("user", {"id": email, "fields": {"vars": 1}}) - if not sailthru_response.is_ok(): - error = sailthru_response.get_error() - log.info("Error attempting to read user record from Sailthru: %s", error.get_message()) - return not _retryable_sailthru_error(error) - - response_json = sailthru_response.json - - unenroll_list = [] - if response_json and "vars" in response_json and response_json["vars"] \ - and "unenrolled" in response_json["vars"]: - unenroll_list = response_json["vars"]["unenrolled"] - - changed = False - # if unenrolling, add course to unenroll list - if unenroll: - if course_url not in unenroll_list: - unenroll_list.append(course_url) - changed = True - - # if enrolling, remove course from unenroll list - elif course_url in unenroll_list: - unenroll_list.remove(course_url) - changed = True - - if changed: - # write user record back - sailthru_response = sailthru_client.api_post( - "user", {'id': email, 'key': 'email', "vars": {"unenrolled": unenroll_list}}) - - if not sailthru_response.is_ok(): - error = sailthru_response.get_error() - log.info("Error attempting to update user record in Sailthru: %s", error.get_message()) - return not _retryable_sailthru_error(error) - - # everything worked - return True - - except SailthruClientError as exc: - log.error("Exception attempting to update user record for %s in Sailthru - %s", email, unicode(exc)) - return False - - def _retryable_sailthru_error(error): """ Return True if error should be retried. diff --git a/lms/djangoapps/email_marketing/tests/test_signals.py b/lms/djangoapps/email_marketing/tests/test_signals.py index f953edfa60..7982e4a45b 100644 --- a/lms/djangoapps/email_marketing/tests/test_signals.py +++ b/lms/djangoapps/email_marketing/tests/test_signals.py @@ -8,19 +8,17 @@ from django.contrib.sites.models import Site from mock import patch, ANY from util.json_request import JsonResponse -from email_marketing.signals import handle_enroll_status_change, \ - email_marketing_register_user, \ +from email_marketing.signals import email_marketing_register_user, \ email_marketing_user_field_changed, \ add_email_marketing_cookies from email_marketing.tasks import ( - update_user, update_user_email, update_course_enrollment, - _get_course_content, _update_unenrolled_list, _get_or_create_user_list, - _get_list_from_email_marketing_provider, _create_user_list) + update_user, update_user_email, _get_or_create_user_list, + _get_list_from_email_marketing_provider, _create_user_list +) from email_marketing.models import EmailMarketingConfiguration from django.test.client import RequestFactory from student.tests.factories import UserFactory, UserProfileFactory -from student.models import EnrollStatusChange from opaque_keys.edx.keys import CourseKey from sailthru.sailthru_response import SailthruResponse @@ -246,11 +244,6 @@ class EmailMarketingTests(TestCase): self.assertFalse(mock_log_error.called) self.assertFalse(mock_sailthru.called) - update_course_enrollment.delay(self.user.username, TEST_EMAIL, 'http://course', - EnrollStatusChange.enroll, 'audit') - self.assertFalse(mock_log_error.called) - self.assertFalse(mock_sailthru.called) - update_email_marketing_config(enabled=True) @patch('email_marketing.signals.log.error') @@ -260,9 +253,6 @@ class EmailMarketingTests(TestCase): """ update_email_marketing_config(enabled=False) - handle_enroll_status_change(None) - self.assertFalse(mock_log_error.called) - add_email_marketing_cookies(None) self.assertFalse(mock_log_error.called) @@ -279,55 +269,6 @@ 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): - """ - Test that the enroll status change signal handler properly calls the task routine - """ - # should just return if no current request found - mock_get_current_request.return_value = None - handle_enroll_status_change(None) - self.assertFalse(mock_update_course_enrollment.called) - - # now test with current request - mock_get_current_request.return_value = self.request - 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') - - # 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): """ @@ -341,158 +282,6 @@ class EmailMarketingTests(TestCase): self.assertEquals(userparms['id'], "old@edx.org") self.assertEquals(userparms['keys']['email'], TEST_EMAIL) - @patch('email_marketing.tasks.log.error') - @patch('email_marketing.tasks.log.info') - @patch('email_marketing.tasks.SailthruClient.purchase') - @patch('email_marketing.tasks.SailthruClient.api_get') - @patch('email_marketing.tasks.SailthruClient.api_post') - def test_update_course_enrollment(self, mock_sailthru_api_post, - mock_sailthru_api_get, mock_sailthru_purchase, mock_log_info, mock_log_error): - """ - test async method in task posts enrolls and purchases - """ - - 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 - update_course_enrollment.delay(TEST_EMAIL, - self.course_url, - EnrollStatusChange.enroll, - 'audit', - 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'}, - message_id='cookie_bid') - - # test unenroll - update_course_enrollment.delay(TEST_EMAIL, - self.course_url, - EnrollStatusChange.unenroll, - 'audit', - 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'}, - 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.enroll, - 'verified', - 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.enroll, - 'verified', - 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_info.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_info.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_info.called) - - @patch('email_marketing.tasks.SailthruClient') - def test_get_course_content(self, mock_sailthru_client): - """ - test routine which fetches data from Sailthru content api - """ - mock_sailthru_client.api_get.return_value = SailthruResponse(JsonResponse({"title": "The title"})) - response_json = _get_course_content('course:123', mock_sailthru_client, EmailMarketingConfiguration.current()) - self.assertEquals(response_json, {"title": "The title"}) - mock_sailthru_client.api_get.assert_called_with('content', {'id': 'course:123'}) - - # test second call uses cache - response_json = _get_course_content('course:123', mock_sailthru_client, EmailMarketingConfiguration.current()) - self.assertEquals(response_json, {"title": "The title"}) - mock_sailthru_client.api_get.assert_not_called() - - # test error from Sailthru - mock_sailthru_client.api_get.return_value = \ - SailthruResponse(JsonResponse({'error': 100, 'errormsg': 'Got an error'})) - self.assertEquals(_get_course_content('course:124', mock_sailthru_client, EmailMarketingConfiguration.current()), {}) - - # test exception - mock_sailthru_client.api_get.side_effect = SailthruClientError - self.assertEquals(_get_course_content('course:125', mock_sailthru_client, EmailMarketingConfiguration.current()), {}) - - @patch('email_marketing.tasks.SailthruClient') - def test_update_unenrolled_list(self, mock_sailthru_client): - """ - test routine which updates the unenrolled list in Sailthru - """ - - # test a new unenroll - mock_sailthru_client.api_get.return_value = \ - SailthruResponse(JsonResponse({'vars': {'unenrolled': ['course_u1']}})) - self.assertTrue(_update_unenrolled_list(mock_sailthru_client, TEST_EMAIL, - self.course_url, True)) - mock_sailthru_client.api_get.assert_called_with("user", {"id": TEST_EMAIL, "fields": {"vars": 1}}) - mock_sailthru_client.api_post.assert_called_with('user', - {'vars': {'unenrolled': ['course_u1', self.course_url]}, - 'id': TEST_EMAIL, 'key': 'email'}) - - # test an enroll of a previously unenrolled course - mock_sailthru_client.api_get.return_value = \ - SailthruResponse(JsonResponse({'vars': {'unenrolled': [self.course_url]}})) - self.assertTrue(_update_unenrolled_list(mock_sailthru_client, TEST_EMAIL, - self.course_url, False)) - mock_sailthru_client.api_post.assert_called_with('user', - {'vars': {'unenrolled': []}, - 'id': TEST_EMAIL, 'key': 'email'}) - - # test get error from Sailthru - mock_sailthru_client.api_get.return_value = \ - SailthruResponse(JsonResponse({'error': 43, 'errormsg': 'Got an error'})) - self.assertFalse(_update_unenrolled_list(mock_sailthru_client, TEST_EMAIL, - self.course_url, False)) - - # test post error from Sailthru - mock_sailthru_client.api_post.return_value = \ - SailthruResponse(JsonResponse({'error': 43, 'errormsg': 'Got an error'})) - mock_sailthru_client.api_get.return_value = \ - SailthruResponse(JsonResponse({'vars': {'unenrolled': [self.course_url]}})) - self.assertFalse(_update_unenrolled_list(mock_sailthru_client, TEST_EMAIL, - self.course_url, False)) - - # test exception - mock_sailthru_client.api_get.side_effect = SailthruClientError - self.assertFalse(_update_unenrolled_list(mock_sailthru_client, TEST_EMAIL, - self.course_url, False)) - @patch('email_marketing.tasks.SailthruClient') def test_get_or_create_sailthru_list(self, mock_sailthru_client): """