diff --git a/common/djangoapps/enrollment/tests/test_views.py b/common/djangoapps/enrollment/tests/test_views.py index ef50c57b1d0d580f6a47e5ecf3b7b3fed4dfea02..4bfd820a0ddef49a27c554506fc2476d48cd49fa 100644 --- a/common/djangoapps/enrollment/tests/test_views.py +++ b/common/djangoapps/enrollment/tests/test_views.py @@ -31,12 +31,20 @@ from enrollment.views import EnrollmentUserThrottle from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.embargo.models import Country, CountryAccessRule, RestrictedCourse from openedx.core.djangoapps.embargo.test_utils import restrict_course -from openedx.core.djangoapps.user_api.models import UserOrgTag +from openedx.core.djangoapps.user_api.models import ( + RetirementState, + UserRetirementStatus, + UserOrgTag +) from openedx.core.lib.django_test_client_utils import get_absolute_url from openedx.core.lib.token_utils import JwtBuilder from openedx.features.enterprise_support.tests import FAKE_ENTERPRISE_CUSTOMER from openedx.features.enterprise_support.tests.mixins.enterprise import EnterpriseServiceMockMixin -from student.models import CourseEnrollment +from student.models import ( + CourseEnrollment, + get_retired_username_by_username, + get_retired_email_by_email, +) from student.roles import CourseStaffRole from student.tests.factories import AdminFactory, UserFactory, SuperuserFactory from util.models import RateLimitConfiguration @@ -1288,6 +1296,20 @@ class UnenrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase): for course in self.courses: self.assert_enrollment_status(course_id=str(course.id), username=self.USERNAME, is_active=True) + def _create_test_retirement(self, user=None): + """ + Helper method to create a RetirementStatus with useful defaults + """ + pending_state = RetirementState.objects.create( + state_name='PENDING', + state_execution_order=1, + is_dead_end_state=False, + required=False + ) + if user is None: + user = UserFactory() + return UserRetirementStatus.create_retirement(user) + def build_jwt_headers(self, user): """ Helper function for creating headers for the JWT authentication. @@ -1299,6 +1321,7 @@ class UnenrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase): def test_deactivate_enrollments(self): self._assert_active() + self._create_test_retirement(self.user) response = self._submit_unenroll(self.superuser, self.user.username) self.assertEqual(response.status_code, status.HTTP_200_OK) data = json.loads(response.content) @@ -1306,6 +1329,11 @@ class UnenrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase): self.assertEqual(set(data), self.orgs) self._assert_inactive() + def test_deactivate_enrollments_no_retirement_status(self): + self._assert_active() + response = self._submit_unenroll(self.superuser, self.user.username) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + def test_deactivate_enrollments_unauthorized(self): self._assert_active() response = self._submit_unenroll(self.user, self.user.username) @@ -1322,26 +1350,25 @@ class UnenrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase): def test_deactivate_enrollments_empty_username(self): self._assert_active() + self._create_test_retirement(self.user) response = self._submit_unenroll(self.superuser, "") self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - data = json.loads(response.content) - self.assertEqual(data, u'The user "" does not exist.') self._assert_active() def test_deactivate_enrollments_invalid_username(self): self._assert_active() + self._create_test_retirement(self.user) response = self._submit_unenroll(self.superuser, "a made up username") self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - data = json.loads(response.content) - self.assertEqual(data, u'The user "a made up username" does not exist.') self._assert_active() def test_deactivate_enrollments_called_twice(self): self._assert_active() + self._create_test_retirement(self.user) response = self._submit_unenroll(self.superuser, self.user.username) self.assertEqual(response.status_code, status.HTTP_200_OK) response = self._submit_unenroll(self.superuser, self.user.username) - self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) self.assertEqual(response.content, "") self._assert_inactive() diff --git a/common/djangoapps/enrollment/views.py b/common/djangoapps/enrollment/views.py index 1b552b2ae9e5b4433f9b2fc3fff75469e79e7753..e45820c1cf8bcbefc082bfd7e24c5af067ddb515 100644 --- a/common/djangoapps/enrollment/views.py +++ b/common/djangoapps/enrollment/views.py @@ -19,6 +19,7 @@ from openedx.core.djangoapps.cors_csrf.authentication import SessionAuthenticati from openedx.core.djangoapps.cors_csrf.decorators import ensure_csrf_cookie_cross_domain from openedx.core.djangoapps.embargo import api as embargo_api from openedx.core.djangoapps.user_api.accounts.permissions import CanRetireUser +from openedx.core.djangoapps.user_api.models import UserRetirementStatus from openedx.core.djangoapps.user_api.preferences.api import update_email_opt_in from openedx.core.lib.api.authentication import ( OAuth2AuthenticationAllowInactiveUser, @@ -316,21 +317,23 @@ class UnenrollmentView(APIView): "username": "username12345" } - **POST Parameters** + **POST Parameters** - A POST request must include the following parameter. + A POST request must include the following parameter. - * username: The username of the user being unenrolled. - This will never match the username from the request, - since the request is issued as a privileged service user. + * username: The username of the user being unenrolled. + This will never match the username from the request, + since the request is issued as a privileged service user. **POST Response Values** - If the user does not exist, or the user is already unenrolled - from all courses, the request returns an HTTP 404 "Does Not Exist" - response. + If the user has not requested retirement and does not have a retirement + request status, the request returns an HTTP 404 "Does Not Exist" response. + + If the user is already unenrolled from all courses, the request returns + an HTTP 204 "No Content" response. - If an unexpected error occurs, the request returns an HTTP 500 response. + If an unexpected error occurs, the request returns an HTTP 500 response. If the request is successful, an HTTP 200 "OK" response is returned along with a list of all courses from which the user was unenrolled. @@ -342,23 +345,20 @@ class UnenrollmentView(APIView): """ Unenrolls the specified user from all courses. """ - username = None - user_model = get_user_model() - try: - # Get the username from the request and check that it exists + # Get the username from the request. username = request.data['username'] - user_model.objects.get(username=username) - + # Ensure that a retirement request status row exists for this username. + UserRetirementStatus.get_retirement_for_retirement_action(username) enrollments = api.get_enrollments(username) active_enrollments = [enrollment for enrollment in enrollments if enrollment['is_active']] if len(active_enrollments) < 1: - return Response(status=status.HTTP_200_OK) + return Response(status=status.HTTP_204_NO_CONTENT) return Response(api.unenroll_user_from_all_courses(username)) except KeyError: return Response(u'Username not specified.', status=status.HTTP_404_NOT_FOUND) - except user_model.DoesNotExist: - return Response(u'The user "{}" does not exist.'.format(username), status=status.HTTP_404_NOT_FOUND) + except UserRetirementStatus.DoesNotExist: + return Response(u'No retirement request status for username.', status=status.HTTP_404_NOT_FOUND) except Exception as exc: # pylint: disable=broad-except return Response(text_type(exc), status=status.HTTP_500_INTERNAL_SERVER_ERROR) 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 f3db944c631c513dfa067989af845a563c144d77..06f7390278f69a3f3f9fcc1ac6d316ec85ab4e4c 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 @@ -1177,14 +1177,6 @@ class TestAccountRetirementPost(RetirementTestCase): } self.assertEqual(expected_user_profile_pii, USER_PROFILE_PII) - def test_retire_user_where_user_does_not_exist(self): - path = 'openedx.core.djangoapps.user_api.accounts.views.is_username_retired' - with mock.patch(path, return_value=False) as mock_retired_username: - data = {'username': 'not_a_user'} - response = self.post_and_assert_status(data, status.HTTP_404_NOT_FOUND) - self.assertFalse(response.content) - mock_retired_username.assert_called_once_with('not_a_user') - def test_retire_user_server_error_is_raised(self): path = 'openedx.core.djangoapps.user_api.models.UserRetirementStatus.get_retirement_for_retirement_action' with mock.patch(path, side_effect=Exception('Unexpected Exception')) as mock_get_retirement: @@ -1197,9 +1189,9 @@ class TestAccountRetirementPost(RetirementTestCase): path = 'openedx.core.djangoapps.user_api.accounts.views.is_username_retired' with mock.patch(path, return_value=True) as mock_is_username_retired: data = {'username': self.test_user.username} - response = self.post_and_assert_status(data, status.HTTP_404_NOT_FOUND) + response = self.post_and_assert_status(data, status.HTTP_204_NO_CONTENT) self.assertFalse(response.content) - mock_is_username_retired.assert_called_once_with(self.original_username) + mock_is_username_retired.assert_not_called() def test_retire_user_where_username_not_provided(self): response = self.post_and_assert_status({}, status.HTTP_404_NOT_FOUND) @@ -1251,6 +1243,11 @@ class TestAccountRetirementPost(RetirementTestCase): self.assertFalse(CourseEnrollmentAllowed.objects.filter(email=self.original_email).exists()) self.assertFalse(UnregisteredLearnerCohortAssignments.objects.filter(email=self.original_email).exists()) + def test_retire_user_twice_idempotent(self): + data = {'username': self.original_username} + self.post_and_assert_status(data) + 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(): if value_to_assign == '': @@ -1476,3 +1473,10 @@ class TestLMSAccountRetirementPost(RetirementTestCase, ModuleStoreTestCase): 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) + 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 9338112f98fa296897909308fb849982cfe7f0f6..e364bfde224c89749b9b086e5ca50fe7cf9d57af 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -721,8 +721,6 @@ class LMSAccountRetirementView(ViewSet): """ username = request.data['username'] - if is_username_retired(username): - return Response(status=status.HTTP_404_NOT_FOUND) try: retirement = UserRetirementStatus.get_retirement_for_retirement_action(username) @@ -770,8 +768,6 @@ class AccountRetirementView(ViewSet): any other PII associated with this user. """ username = request.data['username'] - if is_username_retired(username): - return Response(status=status.HTTP_404_NOT_FOUND) try: retirement_status = UserRetirementStatus.get_retirement_for_retirement_action(username) @@ -801,9 +797,6 @@ class AccountRetirementView(ViewSet): CourseEnrollmentAllowed.delete_by_user_value(original_email, field='email') UnregisteredLearnerCohortAssignments.delete_by_user_value(original_email, field='email') - # TODO: Password Reset links - https://openedx.atlassian.net/browse/PLAT-2104 - # TODO: Delete OAuth2 records - https://openedx.atlassian.net/browse/EDUCATOR-2703 - user.first_name = '' user.last_name = '' user.is_active = False