diff --git a/lms/djangoapps/email_marketing/signals.py b/lms/djangoapps/email_marketing/signals.py index d8b5e297a7..13cecaf21b 100644 --- a/lms/djangoapps/email_marketing/signals.py +++ b/lms/djangoapps/email_marketing/signals.py @@ -273,31 +273,46 @@ def force_unsubscribe_all(sender, **kwargs): # pylint: disable=unused-argument of success or failure. Args: - user(User): Django model of type returned from get_user_model() + email: Email address to unsubscribe + new_email (optional): Email address to change 3rd party services to for this user (used in retirement to clear + personal information from the service) Returns: None """ - user = kwargs.get('user', None) + email = kwargs.get('email', None) + new_email = kwargs.get('new_email', None) - if not user: - raise TypeError('Expected a User type, but received None.') + if not email: + raise TypeError('Expected an email address to unsubscribe, but received None.') email_config = EmailMarketingConfiguration.current() if not email_config.enabled: return - sailthru_parms = {"id": user.email, "keys": {"optout_email": "all"}} + sailthru_parms = { + "id": email, + "optout_email": "all", + "fields": {"optout_email": 1} + } + + # If we have a new email address to change to, do that as well + if new_email: + sailthru_parms["keys"] = { + "email": new_email + } + sailthru_parms["fields"]["keys"] = 1 + sailthru_parms["keysconflict"] = "merge" 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: - error_msg = "Exception attempting to opt-out user %s from Sailthru - %s" % (user.email, text_type(exc)) + error_msg = "Exception attempting to opt-out user {} from Sailthru - {}".format(email, text_type(exc)) log.error(error_msg) raise Exception(error_msg) if not sailthru_response.is_ok(): error = sailthru_response.get_error() - error_msg = "Error attempting to opt-out user %s from Sailthru - %s" % (user.email, error.get_message()) + error_msg = "Error attempting to opt-out user {} from Sailthru - {}".format(email, error.get_message()) log.error(error_msg) raise Exception(error_msg) diff --git a/lms/djangoapps/email_marketing/tests/test_signals.py b/lms/djangoapps/email_marketing/tests/test_signals.py index 7f28e6115c..18cd64b4db 100644 --- a/lms/djangoapps/email_marketing/tests/test_signals.py +++ b/lms/djangoapps/email_marketing/tests/test_signals.py @@ -587,7 +587,7 @@ class EmailMarketingTests(TestCase): Ensure that a successful unsubscribe does not trigger an error log """ mock_sailthru.return_value = SailthruResponse(JsonResponse({'ok': True})) - force_unsubscribe_all(sender=self.__class__, user=self.user) + force_unsubscribe_all(sender=self.__class__, email=self.user.email) self.assertTrue(mock_sailthru.called) self.assertFalse(mock_log_error.called) @@ -596,7 +596,7 @@ class EmailMarketingTests(TestCase): def test_sailthru_on_connection_error(self, mock_sailthru, mock_log_error): mock_sailthru.side_effect = SailthruClientError with self.assertRaises(Exception): - force_unsubscribe_all(sender=self.__class__, user=self.user) + force_unsubscribe_all(sender=self.__class__, email=self.user.email) self.assertTrue(mock_log_error.called) @patch('email_marketing.tasks.log.error') @@ -604,10 +604,29 @@ class EmailMarketingTests(TestCase): def test_sailthru_on_bad_response(self, mock_sailthru, mock_log_error): mock_sailthru.return_value = SailthruResponse(JsonResponse({'error': 100, 'errormsg': 'Got an error'})) with self.assertRaises(Exception): - force_unsubscribe_all(sender=self.__class__, user=self.user) + force_unsubscribe_all(sender=self.__class__, email=self.user.email) self.assertTrue(mock_sailthru.called) self.assertTrue(mock_log_error.called) + @patch('email_marketing.tasks.log.error') + @patch('email_marketing.tasks.SailthruClient.api_post') + def test_sailthru_with_email_changed(self, mock_sailthru, mock_log_error): + mock_sailthru.return_value = SailthruResponse(JsonResponse({'ok': True})) + force_unsubscribe_all(sender=self.__class__, email=self.user.email, new_email='email@somewhere.invalid') + mock_sailthru.assert_called_with("user", { + "id": self.user.email, + "optout_email": "all", + "keys": { + "email": "email@somewhere.invalid" + }, + "fields": { + "optout_email": 1, + "keys": 1 + }, + "keysconflict": "merge" + }) + self.assertFalse(mock_log_error.called) + class MockSailthruResponse(object): """ diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 2101823d00..eb26d21f26 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -342,7 +342,12 @@ class AccountRetireMailingsView(APIView): # This signal allows lms' email_marketing and other 3rd party email # providers to unsubscribe the user as well - USER_RETIRE_MAILINGS.send(sender=self.__class__, user=retirement.user) + USER_RETIRE_MAILINGS.send( + sender=self.__class__, + email=retirement.original_email, + new_email=retirement.retired_email, + user=retirement.user + ) return Response(status=status.HTTP_204_NO_CONTENT) except UserRetirementStatus.DoesNotExist: