diff --git a/cms/envs/common.py b/cms/envs/common.py index fcdc69951577a198c15aef1746446ba16128d7a1..fc4e314ec09c1cbc0d21f1a3006719a837d7cb6d 100644 --- a/cms/envs/common.py +++ b/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', diff --git a/lms/djangoapps/email_marketing/signals.py b/lms/djangoapps/email_marketing/signals.py index 13cecaf21b32da6aad66e350b4e439b68bbf56af..0362086622d9c7f3043b436c7bcf5682e24e0b50 100644 --- a/lms/djangoapps/email_marketing/signals.py +++ b/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. diff --git a/lms/djangoapps/survey/apps.py b/lms/djangoapps/survey/apps.py new file mode 100644 index 0000000000000000000000000000000000000000..1a9e20a2fc15aa619b60282cd3b0e79e4ca60ab2 --- /dev/null +++ b/lms/djangoapps/survey/apps.py @@ -0,0 +1,19 @@ +""" +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 diff --git a/lms/djangoapps/survey/signals.py b/lms/djangoapps/survey/signals.py new file mode 100644 index 0000000000000000000000000000000000000000..1370fa1622c5d40ae9b570b77aafc472dc3229ab --- /dev/null +++ b/lms/djangoapps/survey/signals.py @@ -0,0 +1,17 @@ +""" +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) diff --git a/lms/djangoapps/survey/tests/test_signals.py b/lms/djangoapps/survey/tests/test_signals.py new file mode 100644 index 0000000000000000000000000000000000000000..a78aefb0ae15da8d0bb471312eb8cbc033350be6 --- /dev/null +++ b/lms/djangoapps/survey/tests/test_signals.py @@ -0,0 +1,50 @@ +""" +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()) diff --git a/lms/djangoapps/verify_student/signals.py b/lms/djangoapps/verify_student/signals.py index b332a184f656e5e90e361c246afc2ad54fe7d3a7..5d4d2f4c8096a1ad8a88da96fb3cb81562832e99 100644 --- a/lms/djangoapps/verify_student/signals.py +++ b/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) diff --git a/lms/djangoapps/verify_student/tests/test_signals.py b/lms/djangoapps/verify_student/tests/test_signals.py index de1dc1d6367b96da3d707f94e48aac614ada7680..294cff801de2c2168a6fdbf0f2cc1785756fc5b6 100644 --- a/lms/djangoapps/verify_student/tests/test_signals.py +++ b/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)) diff --git a/lms/envs/common.py b/lms/envs/common.py index d9353f1326c3ecc79f3c8c8318e62c67f70220cb..a3f604a71217add2684f97cfec5837174ed1953a 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2204,7 +2204,7 @@ INSTALLED_APPS = [ 'social_django', # Surveys - 'survey', + 'survey.apps.SurveyConfig', 'lms.djangoapps.lms_xblock.apps.LMSXBlockConfig', diff --git a/openedx/core/djangoapps/user_api/accounts/signals.py b/openedx/core/djangoapps/user_api/accounts/signals.py index 6972c496c5223a6bc93a5fb436d6b9ac0106a2e2..107ccdf064e75e3bcce393ca22f5174c54a61bac 100644 --- a/openedx/core/djangoapps/user_api/accounts/signals.py +++ b/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"]) diff --git a/openedx/core/djangoapps/user_api/accounts/tests/retirement_helpers.py b/openedx/core/djangoapps/user_api/accounts/tests/retirement_helpers.py new file mode 100644 index 0000000000000000000000000000000000000000..0953892ea635b7d343ec6388c3145a8a93cfd1b7 --- /dev/null +++ b/openedx/core/djangoapps/user_api/accounts/tests/retirement_helpers.py @@ -0,0 +1,166 @@ +""" +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) diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py index f2addc20455479d9556058f00836cd455d6f746f..b2fe846067bb47f357c09868cfe9ced99f76d7ac 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py +++ b/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) diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index ec08a19ea5fbb7b96d9404a34db2f176c4e9f11f..959acb1ef45fee983b04a4241dacc3a4cf3875bb 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/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()