Merge pull request #18175 from edx/bmedx/sailthru_retirement_fixes

Fix issues with Sailthru retirement

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)

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):
     """

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: