From 9d7a1d5b539b4f56b6e37170b7ef73a5d77e0d50 Mon Sep 17 00:00:00 2001 From: tasawernawaz Date: Mon, 11 Sep 2017 14:12:19 +0500 Subject: [PATCH] send welcome email to only activated users LEARNER-2474 --- lms/djangoapps/email_marketing/signals.py | 4 +-- lms/djangoapps/email_marketing/tasks.py | 7 ++-- .../email_marketing/tests/test_signals.py | 33 ++++++++++--------- 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/lms/djangoapps/email_marketing/signals.py b/lms/djangoapps/email_marketing/signals.py index d776260e87..8c2351deeb 100644 --- a/lms/djangoapps/email_marketing/signals.py +++ b/lms/djangoapps/email_marketing/signals.py @@ -148,9 +148,9 @@ def email_marketing_user_field_changed(sender, user=None, table=None, setting=No if not email_config.enabled: return - # perform update asynchronously + # set the activation flag when the user is marked as activated update_user.delay(_create_sailthru_user_vars(user, user.profile), user.email, site=_get_current_site(), - new_user=False) + new_user=False, activation=(setting == 'is_active') and new_value is True) elif setting == 'email': # email update is special case diff --git a/lms/djangoapps/email_marketing/tasks.py b/lms/djangoapps/email_marketing/tasks.py index 1f289c6a23..a8e72ef521 100644 --- a/lms/djangoapps/email_marketing/tasks.py +++ b/lms/djangoapps/email_marketing/tasks.py @@ -59,13 +59,14 @@ def get_email_cookies_via_sailthru(self, user_email, post_parms): # pylint: disable=not-callable @task(bind=True, default_retry_delay=3600, max_retries=24) -def update_user(self, sailthru_vars, email, site=None, new_user=False): +def update_user(self, sailthru_vars, email, site=None, new_user=False, activation=False): """ Adds/updates Sailthru profile information for a user. Args: 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 """ @@ -94,8 +95,8 @@ def update_user(self, sailthru_vars, email, site=None, new_user=False): max_retries=email_config.sailthru_max_retries) return - # if new user, send welcome email - if new_user and email_config.sailthru_welcome_template and is_default_site(site): + # if activating user, send welcome email + if activation and email_config.sailthru_welcome_template and is_default_site(site): scheduled_datetime = datetime.utcnow() + timedelta(seconds=email_config.welcome_email_send_delay) try: sailthru_response = sailthru_client.api_post( diff --git a/lms/djangoapps/email_marketing/tests/test_signals.py b/lms/djangoapps/email_marketing/tests/test_signals.py index 6fc23aa080..1f6474c39d 100644 --- a/lms/djangoapps/email_marketing/tests/test_signals.py +++ b/lms/djangoapps/email_marketing/tests/test_signals.py @@ -175,7 +175,7 @@ class EmailMarketingTests(TestCase): @patch('email_marketing.tasks.SailthruClient.api_get') def test_add_user(self, mock_sailthru_get, mock_sailthru_post, mock_log_error): """ - test async method in tasks that actually updates Sailthru and send Welcome template. + test async method in tasks that actually updates Sailthru """ site_dict = {'id': self.site.id, 'domain': self.site.domain, 'name': self.site.name} mock_sailthru_post.return_value = SailthruResponse(JsonResponse({'ok': True})) @@ -183,13 +183,15 @@ class EmailMarketingTests(TestCase): update_user.delay( {'gender': 'm', 'username': 'test', 'activated': 1}, TEST_EMAIL, site_dict, new_user=True ) - expected_schedule = datetime.datetime.utcnow() + datetime.timedelta(seconds=600) self.assertFalse(mock_log_error.called) - self.assertEquals(mock_sailthru_post.call_args[0][0], "send") + self.assertEquals(mock_sailthru_post.call_args[0][0], "user") userparms = mock_sailthru_post.call_args[0][1] - self.assertEquals(userparms['email'], TEST_EMAIL) - self.assertEquals(userparms['template'], "Welcome") - self.assertEquals(userparms['schedule_time'], expected_schedule.strftime('%Y-%m-%dT%H:%M:%SZ')) + self.assertEquals(userparms['key'], "email") + self.assertEquals(userparms['id'], TEST_EMAIL) + self.assertEquals(userparms['vars']['gender'], "m") + self.assertEquals(userparms['vars']['username'], "test") + self.assertEquals(userparms['vars']['activated'], 1) + self.assertEquals(userparms['lists']['new list'], 1) @patch('email_marketing.tasks.log.error') @patch('email_marketing.tasks.SailthruClient.api_post') @@ -214,8 +216,6 @@ class EmailMarketingTests(TestCase): """ test non existing domain name updates Sailthru user lists with default list """ - # Set template to empty string to disable 2nd post call to Sailthru - update_email_marketing_config(template='') existing_site = Site.objects.create(domain='testing.com', name='testing.com') site_dict = {'id': existing_site.id, 'domain': existing_site.domain, 'name': existing_site.name} mock_sailthru_post.return_value = SailthruResponse(JsonResponse({'ok': True})) @@ -233,16 +233,19 @@ class EmailMarketingTests(TestCase): @patch('email_marketing.tasks.SailthruClient.api_get') def test_user_activation(self, mock_sailthru_get, mock_sailthru_post): """ - Test that welcome template not sent if not new user. + Test send of welcome template """ mock_sailthru_post.return_value = SailthruResponse(JsonResponse({'ok': True})) mock_sailthru_get.return_value = SailthruResponse(JsonResponse({'lists': [{'name': 'new list'}], 'ok': True})) - update_user.delay({}, self.user.email, new_user=False) + expected_schedule = datetime.datetime.utcnow() + datetime.timedelta(seconds=600) + update_user.delay({}, self.user.email, new_user=True, activation=True) + # look for call args for 2nd call - self.assertEquals(mock_sailthru_post.call_args[0][0], "user") + self.assertEquals(mock_sailthru_post.call_args[0][0], "send") userparms = mock_sailthru_post.call_args[0][1] - self.assertIsNone(userparms.get('email')) - self.assertIsNone(userparms.get('template')) + self.assertEquals(userparms['email'], TEST_EMAIL) + self.assertEquals(userparms['template'], "Welcome") + self.assertEquals(userparms['schedule_time'], expected_schedule.strftime('%Y-%m-%dT%H:%M:%SZ')) @patch('email_marketing.tasks.log.error') @patch('email_marketing.tasks.SailthruClient.api_post') @@ -263,14 +266,14 @@ class EmailMarketingTests(TestCase): # force Sailthru API exception on 2nd call mock_log_error.reset_mock() mock_sailthru.side_effect = [SailthruResponse(JsonResponse({'ok': True})), SailthruClientError] - update_user.delay({}, self.user.email, new_user=True) + update_user.delay({}, self.user.email, activation=True) self.assertTrue(mock_log_error.called) # force Sailthru API error return on 2nd call 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, new_user=True) + update_user.delay({}, self.user.email, activation=True) self.assertTrue(mock_log_error.called) @patch('email_marketing.tasks.update_user.retry')