Merge pull request #18423 from edx/bmedx/retirement_endpoint_refactor

Refactor retirement endpoints to isolate Sailthru and respect boundaries

cms/envs/common.py

@@ -1098,7 +1098,7 @@ INSTALLED_APPS = [
     # by installed apps.
     'oauth_provider',
     'courseware',
-    'survey',
+    'survey.apps.SurveyConfig',
     'lms.djangoapps.verify_student.apps.VerifyStudentConfig',
     'completion',
 

lms/djangoapps/email_marketing/signals.py

@@ -16,7 +16,7 @@ from six import text_type
 import third_party_auth
 from course_modes.models import CourseMode
 from email_marketing.models import EmailMarketingConfiguration
-from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_MAILINGS
+from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_THIRD_PARTY_MAILINGS
 from openedx.core.djangoapps.waffle_utils import WaffleSwitchNamespace
 from lms.djangoapps.email_marketing.tasks import update_user, update_user_email, get_email_cookies_via_sailthru
 from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY
@@ -263,7 +263,7 @@ def _log_sailthru_api_call_time(time_before_call):
              delta_sailthru_api_call_time.microseconds / 1000)
 
 
-@receiver(USER_RETIRE_MAILINGS)
+@receiver(USER_RETIRE_THIRD_PARTY_MAILINGS)
 def force_unsubscribe_all(sender, **kwargs):  # pylint: disable=unused-argument
     """
     Synchronously(!) unsubscribes the given user from all Sailthru email lists.

lms/djangoapps/survey/apps.py

+"""
+Survey Application Configuration
+"""
+
+from django.apps import AppConfig
+
+
+class SurveyConfig(AppConfig):
+    """
+    Application Configuration for survey.
+    """
+    name = 'survey'
+    verbose_name = 'Student Surveys'
+
+    def ready(self):
+        """
+        Connect signal handlers.
+        """
+        from . import signals  # pylint: disable=unused-variable

lms/djangoapps/survey/signals.py

+"""
+Signal handlers for the survey app
+"""
+from django.dispatch.dispatcher import receiver
+
+from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_LMS_MISC
+
+from survey.models import SurveyAnswer
+
+
+@receiver(USER_RETIRE_LMS_MISC)
+def _listen_for_lms_retire(sender, **kwargs):  # pylint: disable=unused-argument
+    """
+    Listener for the USER_RETIRE_LMS_MISC signal, just does the SurveyAnswer retirement
+    """
+    user = kwargs.get('user')
+    SurveyAnswer.retire_user(user.id)

lms/djangoapps/survey/tests/test_signals.py

+"""
+Test signal handlers for the survey app
+"""
+
+from openedx.core.djangoapps.user_api.accounts.tests.retirement_helpers import fake_retirement
+from student.tests.factories import UserFactory
+from survey.models import SurveyAnswer
+from survey.tests.factories import SurveyAnswerFactory
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+
+from lms.djangoapps.survey.signals import _listen_for_lms_retire
+
+
+class SurveyRetireSignalTests(ModuleStoreTestCase):
+    """
+    Test the _listen_for_lms_retire signal
+    """
+    shard = 4
+
+    def test_success_answers_exist(self):
+        """
+        Basic success path for users that have answers in the table
+        """
+        answer = SurveyAnswerFactory(field_value="test value")
+
+        _listen_for_lms_retire(sender=self.__class__, user=answer.user)
+
+        # All values for this user should now be empty string
+        self.assertFalse(SurveyAnswer.objects.filter(user=answer.user).exclude(field_value='').exists())
+
+    def test_success_no_answers(self):
+        """
+        Basic success path for users who have no answers, should simply not error
+        """
+        user = UserFactory()
+        _listen_for_lms_retire(sender=self.__class__, user=user)
+
+    def test_idempotent(self):
+        """
+        Tests that re-running a retirement multiple times does not throw an error
+        """
+        answer = SurveyAnswerFactory(field_value="test value")
+
+        # Run twice to make sure no errors are raised
+        _listen_for_lms_retire(sender=self.__class__, user=answer.user)
+        fake_retirement(answer.user)
+        _listen_for_lms_retire(sender=self.__class__, user=answer.user)
+
+        # All values for this user should still be here and just be an empty string
+        self.assertFalse(SurveyAnswer.objects.filter(user=answer.user).exclude(field_value='').exists())

lms/djangoapps/verify_student/signals.py

@@ -4,9 +4,10 @@ Signal handler for setting default course verification dates
 from django.core.exceptions import ObjectDoesNotExist
 from django.dispatch.dispatcher import receiver
 
+from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_LMS_CRITICAL
 from xmodule.modulestore.django import SignalHandler, modulestore
 
-from .models import VerificationDeadline
+from .models import SoftwareSecurePhotoVerification, VerificationDeadline
 
 
 @receiver(SignalHandler.course_published)
@@ -23,3 +24,9 @@ def _listen_for_course_publish(sender, course_key, **kwargs):  # pylint: disable
                 VerificationDeadline.set_deadline(course_key, course.end)
         except ObjectDoesNotExist:
             VerificationDeadline.set_deadline(course_key, course.end)
+
+
+@receiver(USER_RETIRE_LMS_CRITICAL)
+def _listen_for_lms_retire(sender, **kwargs):  # pylint: disable=unused-argument
+    user = kwargs.get('user')
+    SoftwareSecurePhotoVerification.retire_user(user.id)

lms/djangoapps/verify_student/tests/test_signals.py

@@ -6,8 +6,11 @@ from datetime import datetime, timedelta
 
 from pytz import UTC
 
-from lms.djangoapps.verify_student.models import VerificationDeadline
-from lms.djangoapps.verify_student.signals import _listen_for_course_publish
+from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification, VerificationDeadline
+from lms.djangoapps.verify_student.signals import _listen_for_course_publish, _listen_for_lms_retire
+from lms.djangoapps.verify_student.tests.factories import SoftwareSecurePhotoVerificationFactory
+from openedx.core.djangoapps.user_api.accounts.tests.retirement_helpers import fake_retirement
+from student.tests.factories import UserFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 
@@ -48,3 +51,55 @@ class VerificationDeadlineSignalTest(ModuleStoreTestCase):
         actual_deadline = VerificationDeadline.deadline_for_course(self.course.id)
         self.assertNotEqual(actual_deadline, self.course.end)
         self.assertEqual(actual_deadline, deadline)
+
+
+class RetirementSignalTest(ModuleStoreTestCase):
+    """
+    Tests for the VerificationDeadline signal
+    """
+    shard = 4
+
+    def _create_entry(self):
+        """
+        Helper method to create and return a SoftwareSecurePhotoVerification with appropriate data
+        """
+        name = 'Test Name'
+        face_url = 'https://test.invalid'
+        id_url = 'https://test2.invalid'
+        key = 'test+key'
+        user = UserFactory()
+        return SoftwareSecurePhotoVerificationFactory(
+            user=user,
+            name=name,
+            face_image_url=face_url,
+            photo_id_image_url=id_url,
+            photo_id_key=key
+        )
+
+    def test_retire_success(self):
+        verification = self._create_entry()
+        _listen_for_lms_retire(sender=self.__class__, user=verification.user)
+
+        ver_obj = SoftwareSecurePhotoVerification.objects.get(user=verification.user)
+
+        # All values for this user should now be empty string
+        for field in ('name', 'face_image_url', 'photo_id_image_url', 'photo_id_key'):
+            self.assertEqual('', getattr(ver_obj, field))
+
+    def test_retire_success_no_entries(self):
+        user = UserFactory()
+        _listen_for_lms_retire(sender=self.__class__, user=user)
+
+    def test_idempotent(self):
+        verification = self._create_entry()
+
+        # Run this twice to make sure there are no errors raised 2nd time through
+        _listen_for_lms_retire(sender=self.__class__, user=verification.user)
+        fake_retirement(verification.user)
+        _listen_for_lms_retire(sender=self.__class__, user=verification.user)
+
+        ver_obj = SoftwareSecurePhotoVerification.objects.get(user=verification.user)
+
+        # All values for this user should now be empty string
+        for field in ('name', 'face_image_url', 'photo_id_image_url', 'photo_id_key'):
+            self.assertEqual('', getattr(ver_obj, field))

lms/envs/common.py

@@ -2204,7 +2204,7 @@ INSTALLED_APPS = [
     'social_django',
 
     # Surveys
-    'survey',
+    'survey.apps.SurveyConfig',
 
     'lms.djangoapps.lms_xblock.apps.LMSXBlockConfig',
 

openedx/core/djangoapps/user_api/accounts/signals.py

@@ -4,4 +4,14 @@ Django Signal related functionality for user_api accounts
 
 from django.dispatch import Signal
 
+# Signal to retire a user from third party mailing services, such as Sailthru.
+USER_RETIRE_THIRD_PARTY_MAILINGS = Signal(providing_args=["user"])
+
+# Signal to retire a user from LMS-initiated mailings (course mailings, etc)
 USER_RETIRE_MAILINGS = Signal(providing_args=["user"])
+
+# Signal to retire LMS critical information
+USER_RETIRE_LMS_CRITICAL = Signal(providing_args=["user"])
+
+# Signal to retire LMS misc information
+USER_RETIRE_LMS_MISC = Signal(providing_args=["user"])

openedx/core/djangoapps/user_api/accounts/tests/retirement_helpers.py

+"""
+Helpers for testing retirement functionality
+"""
+import datetime
+
+import pytz
+from django.test import TestCase
+from social_django.models import UserSocialAuth
+
+from enrollment import api
+from openedx.core.djangoapps.user_api.models import (
+    RetirementState,
+    UserRetirementStatus
+)
+from student.models import (
+    get_retired_username_by_username,
+    get_retired_email_by_email,
+)
+from student.tests.factories import UserFactory
+
+from ..views import AccountRetirementView
+
+
+class RetirementTestCase(TestCase):
+    """
+    Test case with a helper methods for retirement
+    """
+    @classmethod
+    def setUpClass(cls):
+        super(RetirementTestCase, cls).setUpClass()
+        cls.setup_states()
+
+    @staticmethod
+    def setup_states():
+        """
+        Create basic states that mimic our current understanding of the retirement process
+        """
+        default_states = [
+            ('PENDING', 1, False, True),
+            ('LOCKING_ACCOUNT', 20, False, False),
+            ('LOCKING_COMPLETE', 30, False, False),
+            ('RETIRING_CREDENTIALS', 40, False, False),
+            ('CREDENTIALS_COMPLETE', 50, False, False),
+            ('RETIRING_ECOM', 60, False, False),
+            ('ECOM_COMPLETE', 70, False, False),
+            ('RETIRING_FORUMS', 80, False, False),
+            ('FORUMS_COMPLETE', 90, False, False),
+            ('RETIRING_EMAIL_LISTS', 100, False, False),
+            ('EMAIL_LISTS_COMPLETE', 110, False, False),
+            ('RETIRING_ENROLLMENTS', 120, False, False),
+            ('ENROLLMENTS_COMPLETE', 130, False, False),
+            ('RETIRING_NOTES', 140, False, False),
+            ('NOTES_COMPLETE', 150, False, False),
+            ('RETIRING_LMS', 160, False, False),
+            ('LMS_COMPLETE', 170, False, False),
+            ('ADDING_TO_PARTNER_QUEUE', 180, False, False),
+            ('PARTNER_QUEUE_COMPLETE', 190, False, False),
+            ('ERRORED', 200, True, True),
+            ('ABORTED', 210, True, True),
+            ('COMPLETE', 220, True, True),
+        ]
+
+        for name, ex, dead, req in default_states:
+            RetirementState.objects.create(
+                state_name=name,
+                state_execution_order=ex,
+                is_dead_end_state=dead,
+                required=req
+            )
+
+    def _create_retirement(self, state, create_datetime=None):
+        """
+        Helper method to create a RetirementStatus with useful defaults
+        """
+        if create_datetime is None:
+            create_datetime = datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=8)
+
+        user = UserFactory()
+        return UserRetirementStatus.objects.create(
+            user=user,
+            original_username=user.username,
+            original_email=user.email,
+            original_name=user.profile.name,
+            retired_username=get_retired_username_by_username(user.username),
+            retired_email=get_retired_email_by_email(user.email),
+            current_state=state,
+            last_state=state,
+            responses="",
+            created=create_datetime,
+            modified=create_datetime
+        )
+
+    def _retirement_to_dict(self, retirement, all_fields=False):
+        """
+        Return a dict format of this model to a consistent format for serialization, removing the long text field
+        `responses` for performance reasons.
+        """
+        retirement_dict = {
+            u'id': retirement.id,
+            u'user': {
+                u'id': retirement.user.id,
+                u'username': retirement.user.username,
+                u'email': retirement.user.email,
+                u'profile': {
+                    u'id': retirement.user.profile.id,
+                    u'name': retirement.user.profile.name
+                },
+            },
+            u'original_username': retirement.original_username,
+            u'original_email': retirement.original_email,
+            u'original_name': retirement.original_name,
+            u'retired_username': retirement.retired_username,
+            u'retired_email': retirement.retired_email,
+            u'current_state': {
+                u'id': retirement.current_state.id,
+                u'state_name': retirement.current_state.state_name,
+                u'state_execution_order': retirement.current_state.state_execution_order,
+            },
+            u'last_state': {
+                u'id': retirement.last_state.id,
+                u'state_name': retirement.last_state.state_name,
+                u'state_execution_order': retirement.last_state.state_execution_order,
+            },
+            u'created': retirement.created,
+            u'modified': retirement.modified
+        }
+
+        if all_fields:
+            retirement_dict['responses'] = retirement.responses
+
+        return retirement_dict
+
+    def _create_users_all_states(self):
+        return [self._create_retirement(state) for state in RetirementState.objects.all()]
+
+    def _get_non_dead_end_states(self):
+        return [state for state in RetirementState.objects.filter(is_dead_end_state=False)]
+
+    def _get_dead_end_states(self):
+        return [state for state in RetirementState.objects.filter(is_dead_end_state=True)]
+
+
+def fake_retirement(user):
+    """
+    Makes an attempt to put user for the given user into a "COMPLETED"
+    retirement state by faking important parts of retirement.
+
+    Use to test idempotency for retirement API calls. Since there are many
+    configurable retirement steps this is only a "best guess" and may need
+    additional changes added to more accurately reflect post-retirement state.
+    """
+    # Deactivate / logout and hash username & email
+    UserSocialAuth.objects.filter(user_id=user.id).delete()
+    user.first_name = ''
+    user.last_name = ''
+    user.is_active = False
+    user.username = get_retired_username_by_username(user.username)
+    user.email = get_retired_email_by_email(user.email)
+    user.set_unusable_password()
+    user.save()
+
+    # Clear profile
+    AccountRetirementView.clear_pii_from_userprofile(user)
+
+    # Unenroll from all courses
+    api.unenroll_user_from_all_courses(user.username)

openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py

@@ -29,7 +29,7 @@ import mock
 from opaque_keys.edx.keys import CourseKey
 import pytz
 from rest_framework import status
-from six import text_type
+from six import iteritems, text_type
 from social_django.models import UserSocialAuth
 from wiki.models import ArticleRevision, Article
 from wiki.models.pluginbase import RevisionPluginRevision, RevisionPlugin
@@ -38,14 +38,13 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 
 from entitlements.models import CourseEntitlementSupportDetail
 from entitlements.tests.factories import CourseEntitlementFactory
-from lms.djangoapps.verify_student.tests.factories import SoftwareSecurePhotoVerificationFactory
 from openedx.core.djangoapps.api_admin.models import ApiAccessRequest
 from openedx.core.djangoapps.credit.models import (
     CreditRequirementStatus, CreditRequest, CreditCourse, CreditProvider, CreditRequirement
 )
 from openedx.core.djangoapps.course_groups.models import CourseUserGroup, UnregisteredLearnerCohortAssignments
 from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory
-from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_MAILINGS
+from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_THIRD_PARTY_MAILINGS
 from openedx.core.djangoapps.user_api.models import (
     RetirementState,
     UserRetirementStatus,
@@ -74,10 +73,10 @@ from student.tests.factories import (
     SuperuserFactory,
     UserFactory
 )
-from survey.models import SurveyAnswer
 
 from ..views import AccountRetirementView, USER_PROFILE_PII
 from ...tests.factories import UserOrgTagFactory
+from .retirement_helpers import RetirementTestCase, fake_retirement
 
 
 def build_jwt_headers(user):
@@ -163,125 +162,6 @@ class TestAccountDeactivation(TestCase):
         )
 
 
-class RetirementTestCase(TestCase):
-    """
-    Test case with a helper methods for retirement
-    """
-    @classmethod
-    def setUpClass(cls):
-        super(RetirementTestCase, cls).setUpClass()
-        cls.setup_states()
-
-    @staticmethod
-    def setup_states():
-        """
-        Create basic states that mimic our current understanding of the retirement process
-        """
-        default_states = [
-            ('PENDING', 1, False, True),
-            ('LOCKING_ACCOUNT', 20, False, False),
-            ('LOCKING_COMPLETE', 30, False, False),
-            ('RETIRING_CREDENTIALS', 40, False, False),
-            ('CREDENTIALS_COMPLETE', 50, False, False),
-            ('RETIRING_ECOM', 60, False, False),
-            ('ECOM_COMPLETE', 70, False, False),
-            ('RETIRING_FORUMS', 80, False, False),
-            ('FORUMS_COMPLETE', 90, False, False),
-            ('RETIRING_EMAIL_LISTS', 100, False, False),
-            ('EMAIL_LISTS_COMPLETE', 110, False, False),
-            ('RETIRING_ENROLLMENTS', 120, False, False),
-            ('ENROLLMENTS_COMPLETE', 130, False, False),
-            ('RETIRING_NOTES', 140, False, False),
-            ('NOTES_COMPLETE', 150, False, False),
-            ('RETIRING_LMS', 160, False, False),
-            ('LMS_COMPLETE', 170, False, False),
-            ('ADDING_TO_PARTNER_QUEUE', 180, False, False),
-            ('PARTNER_QUEUE_COMPLETE', 190, False, False),
-            ('ERRORED', 200, True, True),
-            ('ABORTED', 210, True, True),
-            ('COMPLETE', 220, True, True),
-        ]
-
-        for name, ex, dead, req in default_states:
-            RetirementState.objects.create(
-                state_name=name,
-                state_execution_order=ex,
-                is_dead_end_state=dead,
-                required=req
-            )
-
-    def _create_retirement(self, state, create_datetime=None):
-        """
-        Helper method to create a RetirementStatus with useful defaults
-        """
-        if create_datetime is None:
-            create_datetime = datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=8)
-
-        user = UserFactory()
-        return UserRetirementStatus.objects.create(
-            user=user,
-            original_username=user.username,
-            original_email=user.email,
-            original_name=user.profile.name,
-            retired_username=get_retired_username_by_username(user.username),
-            retired_email=get_retired_email_by_email(user.email),
-            current_state=state,
-            last_state=state,
-            responses="",
-            created=create_datetime,
-            modified=create_datetime
-        )
-
-    def _retirement_to_dict(self, retirement, all_fields=False):
-        """
-        Return a dict format of this model to a consistent format for serialization, removing the long text field
-        `responses` for performance reasons.
-        """
-        retirement_dict = {
-            u'id': retirement.id,
-            u'user': {
-                u'id': retirement.user.id,
-                u'username': retirement.user.username,
-                u'email': retirement.user.email,
-                u'profile': {
-                    u'id': retirement.user.profile.id,
-                    u'name': retirement.user.profile.name
-                },
-            },
-            u'original_username': retirement.original_username,
-            u'original_email': retirement.original_email,
-            u'original_name': retirement.original_name,
-            u'retired_username': retirement.retired_username,
-            u'retired_email': retirement.retired_email,
-            u'current_state': {
-                u'id': retirement.current_state.id,
-                u'state_name': retirement.current_state.state_name,
-                u'state_execution_order': retirement.current_state.state_execution_order,
-            },
-            u'last_state': {
-                u'id': retirement.last_state.id,
-                u'state_name': retirement.last_state.state_name,
-                u'state_execution_order': retirement.last_state.state_execution_order,
-            },
-            u'created': retirement.created,
-            u'modified': retirement.modified
-        }
-
-        if all_fields:
-            retirement_dict['responses'] = retirement.responses
-
-        return retirement_dict
-
-    def _create_users_all_states(self):
-        return [self._create_retirement(state) for state in RetirementState.objects.all()]
-
-    def _get_non_dead_end_states(self):
-        return [state for state in RetirementState.objects.filter(is_dead_end_state=False)]
-
-    def _get_dead_end_states(self):
-        return [state for state in RetirementState.objects.filter(is_dead_end_state=True)]
-
-
 @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS')
 class TestDeactivateLogout(RetirementTestCase):
     """
@@ -377,16 +257,12 @@ class TestAccountRetireMailings(RetirementTestCase):
         self.retirement = self._create_retirement(retiring_email_lists)
         self.test_user = self.retirement.user
 
-        UserOrgTag.objects.create(user=self.test_user, key='email-optin', org="foo", value="True")
-        UserOrgTag.objects.create(user=self.test_user, key='email-optin', org="bar", value="True")
-
         self.url = reverse('accounts_retire_mailings')
 
     def build_post(self, user):
         return {'username': user.username}
 
-    def assert_status_and_tag_count(self, headers, expected_status=status.HTTP_204_NO_CONTENT, expected_tag_count=2,
-                                    expected_tag_value="False", expected_content=None):
+    def assert_status(self, headers, expected_status=status.HTTP_204_NO_CONTENT, expected_content=None):
         """
         Helper function for making a request to the retire subscriptions endpoint, and asserting the status.
         """
@@ -394,10 +270,6 @@ class TestAccountRetireMailings(RetirementTestCase):
 
         self.assertEqual(response.status_code, expected_status)
 
-        # Check that the expected number of tags with the correct value exist
-        tag_count = UserOrgTag.objects.filter(user=self.test_user, value=expected_tag_value).count()
-        self.assertEqual(tag_count, expected_tag_count)
-
         if expected_content:
             self.assertEqual(response.content.strip('"'), expected_content)
 
@@ -406,15 +278,7 @@ class TestAccountRetireMailings(RetirementTestCase):
         Verify a user's subscriptions are retired when a superuser posts to the retire subscriptions endpoint.
         """
         headers = build_jwt_headers(self.test_superuser)
-        self.assert_status_and_tag_count(headers)
-
-    def test_superuser_retires_user_subscriptions_no_orgtags(self):
-        """
-        Verify the call succeeds when the user doesn't have any org tags.
-        """
-        UserOrgTag.objects.all().delete()
-        headers = build_jwt_headers(self.test_superuser)
-        self.assert_status_and_tag_count(headers, expected_tag_count=0)
+        self.assert_status(headers)
 
     def test_unauthorized_rejection(self):
         """
@@ -423,7 +287,7 @@ class TestAccountRetireMailings(RetirementTestCase):
         headers = build_jwt_headers(self.test_user)
 
         # User should still have 2 "True" subscriptions.
-        self.assert_status_and_tag_count(headers, expected_status=status.HTTP_403_FORBIDDEN, expected_tag_value="True")
+        self.assert_status(headers, expected_status=status.HTTP_403_FORBIDDEN)
 
     def test_signal_failure(self):
         """
@@ -435,17 +299,16 @@ class TestAccountRetireMailings(RetirementTestCase):
         mock_handler.side_effect = Exception("Tango")
 
         try:
-            USER_RETIRE_MAILINGS.connect(mock_handler)
+            USER_RETIRE_THIRD_PARTY_MAILINGS.connect(mock_handler)
 
             # User should still have 2 "True" subscriptions.
-            self.assert_status_and_tag_count(
+            self.assert_status(
                 headers,
                 expected_status=status.HTTP_500_INTERNAL_SERVER_ERROR,
-                expected_tag_value="True",
                 expected_content="Tango"
             )
         finally:
-            USER_RETIRE_MAILINGS.disconnect(mock_handler)
+            USER_RETIRE_THIRD_PARTY_MAILINGS.disconnect(mock_handler)
 
 
 @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS')
@@ -1193,7 +1056,6 @@ class TestAccountRetirementPost(RetirementTestCase):
         )
 
         # Misc. setup
-        self.photo_verification = SoftwareSecurePhotoVerificationFactory.create(user=self.test_user)
         PendingEmailChangeFactory.create(user=self.test_user)
         UserOrgTagFactory.create(user=self.test_user, key='foo', value='bar')
         UserOrgTagFactory.create(user=self.test_user, key='cat', value='dog')
@@ -1275,10 +1137,10 @@ class TestAccountRetirementPost(RetirementTestCase):
             'is_active': False,
             'username': self.retired_username,
         }
-        for field, expected_value in expected_user_values.iteritems():
+        for field, expected_value in iteritems(expected_user_values):
             self.assertEqual(expected_value, getattr(self.test_user, field))
 
-        for field, expected_value in USER_PROFILE_PII.iteritems():
+        for field, expected_value in iteritems(USER_PROFILE_PII):
             self.assertEqual(expected_value, getattr(self.test_user.profile, field))
 
         self.assertIsNone(self.test_user.profile.profile_image_uploaded_at)
@@ -1298,7 +1160,6 @@ class TestAccountRetirementPost(RetirementTestCase):
         self._pending_enterprise_customer_user_assertions()
         self._entitlement_support_detail_assertions()
 
-        self._photo_verification_assertions()
         self.assertFalse(PendingEmailChange.objects.filter(user=self.test_user).exists())
         self.assertFalse(UserOrgTag.objects.filter(user=self.test_user).exists())
 
@@ -1308,10 +1169,11 @@ class TestAccountRetirementPost(RetirementTestCase):
     def test_retire_user_twice_idempotent(self):
         data = {'username': self.original_username}
         self.post_and_assert_status(data)
+        fake_retirement(self.test_user)
         self.post_and_assert_status(data)
 
     def test_deletes_pii_from_user_profile(self):
-        for model_field, value_to_assign in USER_PROFILE_PII.iteritems():
+        for model_field, value_to_assign in iteritems(USER_PROFILE_PII):
             if value_to_assign == '':
                 value = 'foo'
             else:
@@ -1320,7 +1182,7 @@ class TestAccountRetirementPost(RetirementTestCase):
 
         AccountRetirementView.clear_pii_from_userprofile(self.test_user)
 
-        for model_field, value_to_assign in USER_PROFILE_PII.iteritems():
+        for model_field, value_to_assign in iteritems(USER_PROFILE_PII):
             self.assertEqual(value_to_assign, getattr(self.test_user.profile, model_field))
 
         social_links = SocialLink.objects.filter(
@@ -1406,15 +1268,6 @@ class TestAccountRetirementPost(RetirementTestCase):
         self.entitlement_support_detail.refresh_from_db()
         self.assertEqual('', self.entitlement_support_detail.comments)
 
-    def _photo_verification_assertions(self):
-        """
-        Helper method for asserting that ``SoftwareSecurePhotoVerification`` objects are retired.
-        """
-        self.photo_verification.refresh_from_db()
-        self.assertEqual(self.test_user, self.photo_verification.user)
-        for field in ('name', 'face_image_url', 'photo_id_image_url', 'photo_id_key'):
-            self.assertEqual('', getattr(self.photo_verification, field))
-
 
 @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS')
 class TestLMSAccountRetirementPost(RetirementTestCase, ModuleStoreTestCase):
@@ -1478,9 +1331,6 @@ class TestLMSAccountRetirementPost(RetirementTestCase, ModuleStoreTestCase):
             reason=self.pii_standin,
         )
 
-        # SurveyAnswer setup
-        SurveyAnswer.objects.create(user=self.test_user, field_value=self.pii_standin, form_id=0)
-
         # other setup
         PendingNameChange.objects.create(user=self.test_user, new_name=self.pii_standin, rationale=self.pii_standin)
         PasswordHistory.objects.create(user=self.test_user, password=self.pii_standin)
@@ -1534,11 +1384,10 @@ class TestLMSAccountRetirementPost(RetirementTestCase, ModuleStoreTestCase):
         self.assertEqual(retired_api_access_request.company_address, '')
         self.assertEqual(retired_api_access_request.company_name, '')
         self.assertEqual(retired_api_access_request.reason, '')
-        self.assertEqual(SurveyAnswer.objects.get(user=self.test_user).field_value, '')
 
     def test_retire_user_twice_idempotent(self):
         # check that a second call to the retire_misc endpoint will work
-        UserRetirementStatus.get_retirement_for_retirement_action(self.test_user.username)
         data = {'username': self.original_username}
         self.post_and_assert_status(data)
+        fake_retirement(self.test_user)
         self.post_and_assert_status(data)

openedx/core/djangoapps/user_api/accounts/views.py

@@ -27,27 +27,24 @@ from rest_framework.parsers import JSONParser
 from rest_framework.response import Response
 from rest_framework.views import APIView
 from rest_framework.viewsets import ViewSet
-from six import text_type
+from six import iteritems, text_type
 from social_django.models import UserSocialAuth
 from wiki.models import ArticleRevision
 from wiki.models.pluginbase import RevisionPluginRevision
 
 from entitlements.models import CourseEntitlement
-from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification
 from openedx.core.djangoapps.ace_common.template_context import get_base_template_context
 from openedx.core.djangoapps.api_admin.models import ApiAccessRequest
 from openedx.core.djangoapps.credit.models import CreditRequirementStatus, CreditRequest
 from openedx.core.djangoapps.course_groups.models import UnregisteredLearnerCohortAssignments
 from openedx.core.djangoapps.profile_images.images import remove_profile_images
 from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_names, set_has_profile_image
-from openedx.core.djangoapps.user_api.preferences.api import update_email_opt_in
 from openedx.core.djangolib.oauth2_retirement_utils import retire_dot_oauth2_models, retire_dop_oauth2_models
 from openedx.core.lib.api.authentication import (
     OAuth2AuthenticationAllowInactiveUser,
     SessionAuthenticationAllowInactiveUser
 )
 from openedx.core.lib.api.parsers import MergePatchParser
-from survey.models import SurveyAnswer
 from student.models import (
     CourseEnrollment,
     ManualEnrollmentAudit,
@@ -76,7 +73,12 @@ from ..models import (
 from .api import get_account_settings, update_account_settings
 from .permissions import CanDeactivateUser, CanRetireUser
 from .serializers import UserRetirementPartnerReportSerializer, UserRetirementStatusSerializer
-from .signals import USER_RETIRE_MAILINGS
+from .signals import (
+    USER_RETIRE_LMS_CRITICAL,
+    USER_RETIRE_LMS_MISC,
+    USER_RETIRE_MAILINGS,
+    USER_RETIRE_THIRD_PARTY_MAILINGS
+)
 from ..message_types import DeletionNotificationMessage
 
 log = logging.getLogger(__name__)
@@ -338,7 +340,8 @@ class AccountDeactivationView(APIView):
 class AccountRetireMailingsView(APIView):
     """
     Part of the retirement API, accepts POSTs to unsubscribe a user
-    from all email lists.
+    from all EXTERNAL email lists (ex: Sailthru). LMS email subscriptions
+    are handled in the LMS retirement endpoints.
     """
     authentication_classes = (JwtAuthentication, )
     permission_classes = (permissions.IsAuthenticated, CanRetireUser)
@@ -347,10 +350,9 @@ class AccountRetireMailingsView(APIView):
         """
         POST /api/user/v1/accounts/{username}/retire_mailings/
 
-        Allows an administrative user to take the following actions
-        on behalf of an LMS user:
-        -  Update UserOrgTags to opt the user out of org emails
-        -  Call Sailthru API to force opt-out the user from all email lists
+        Fires the USER_RETIRE_THIRD_PARTY_MAILINGS signal, currently the
+        only receiver is email_marketing to force opt-out the user from
+        externally managed email lists.
         """
         username = request.data['username']
 
@@ -358,13 +360,9 @@ class AccountRetireMailingsView(APIView):
             retirement = UserRetirementStatus.get_retirement_for_retirement_action(username)
 
             with transaction.atomic():
-                # Take care of org emails first, using the existing API for consistency
-                for preference in UserOrgTag.objects.filter(user=retirement.user, key='email-optin'):
-                    update_email_opt_in(retirement.user, preference.org, False)
-
                 # This signal allows lms' email_marketing and other 3rd party email
-                # providers to unsubscribe the user as well
-                USER_RETIRE_MAILINGS.send(
+                # providers to unsubscribe the user
+                USER_RETIRE_THIRD_PARTY_MAILINGS.send(
                     sender=self.__class__,
                     email=retirement.original_email,
                     new_email=retirement.retired_email,
@@ -766,8 +764,18 @@ class LMSAccountRetirementView(ViewSet):
             CreditRequest.retire_user(retirement.original_username, retirement.retired_username)
             ApiAccessRequest.retire_user(retirement.user)
             CreditRequirementStatus.retire_user(retirement.user.username)
-            SurveyAnswer.retire_user(retirement.user.id)
 
+            # This signal allows code in higher points of LMS to retire the user as necessary
+            USER_RETIRE_LMS_MISC.send(sender=self.__class__, user=retirement.user)
+
+            # This signal allows code in higher points of LMS to unsubscribe the user
+            # from various types of mailings.
+            USER_RETIRE_MAILINGS.send(
+                sender=self.__class__,
+                email=retirement.original_email,
+                new_email=retirement.retired_email,
+                user=retirement.user
+            )
         except UserRetirementStatus.DoesNotExist:
             return Response(status=status.HTTP_404_NOT_FOUND)
         except RetirementStateError as exc:
@@ -796,7 +804,7 @@ class AccountRetirementView(ViewSet):
         }
 
         Retires the user with the given username.  This includes
-        retiring this username, the associates email address, and
+        retiring this username, the associated email address, and
         any other PII associated with this user.
         """
         username = request.data['username']
@@ -821,7 +829,6 @@ class AccountRetirementView(ViewSet):
             self.retire_entitlement_support_detail(user)
 
             # Retire misc. models that may contain PII of this user
-            SoftwareSecurePhotoVerification.retire_user(user.id)
             PendingEmailChange.delete_by_user_value(user, field='user')
             UserOrgTag.delete_by_user_value(user, field='user')
 
@@ -829,6 +836,9 @@ class AccountRetirementView(ViewSet):
             CourseEnrollmentAllowed.delete_by_user_value(original_email, field='email')
             UnregisteredLearnerCohortAssignments.delete_by_user_value(original_email, field='email')
 
+            # This signal allows code in higher points of LMS to retire the user as necessary
+            USER_RETIRE_LMS_CRITICAL.send(sender=self.__class__, user=user)
+
             user.first_name = ''
             user.last_name = ''
             user.is_active = False
@@ -849,7 +859,7 @@ class AccountRetirementView(ViewSet):
         For the given user, sets all of the user's profile fields to some retired value.
         This also deletes all ``SocialLink`` objects associated with this user's profile.
         """
-        for model_field, value_to_assign in USER_PROFILE_PII.iteritems():
+        for model_field, value_to_assign in iteritems(USER_PROFILE_PII):
             setattr(user.profile, model_field, value_to_assign)
 
         user.profile.save()