From 98742c1c50d6b632908586f38cff7f053820b09f Mon Sep 17 00:00:00 2001 From: bmedx Date: Tue, 10 Apr 2018 17:13:03 -0400 Subject: [PATCH] Update retirement APIs to match spec --- .../djangoapps/enrollment/tests/test_views.py | 17 +- common/djangoapps/enrollment/views.py | 36 ++- .../discussion_api/tests/test_views.py | 37 ++-- lms/djangoapps/discussion_api/urls.py | 2 +- lms/djangoapps/discussion_api/views.py | 13 +- .../user_api/accounts/tests/test_views.py | 207 +++++++++--------- .../djangoapps/user_api/accounts/views.py | 43 ++-- openedx/core/djangoapps/user_api/models.py | 21 ++ openedx/core/djangoapps/user_api/urls.py | 2 +- 9 files changed, 201 insertions(+), 177 deletions(-) diff --git a/common/djangoapps/enrollment/tests/test_views.py b/common/djangoapps/enrollment/tests/test_views.py index a74dea421f..16902c930a 100644 --- a/common/djangoapps/enrollment/tests/test_views.py +++ b/common/djangoapps/enrollment/tests/test_views.py @@ -1279,11 +1279,19 @@ class UnenrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase): self._assert_active() def test_deactivate_enrollments_no_username(self): + self._assert_active() + response = self._submit_unenroll(self.superuser, None) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + data = json.loads(response.content) + self.assertEqual(data, u"Username not specified.") + self._assert_active() + + def test_deactivate_enrollments_empty_username(self): self._assert_active() response = self._submit_unenroll(self.superuser, "") self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) data = json.loads(response.content) - self.assertEqual(data['message'], 'The user was not specified.') + self.assertEqual(data, u'The user "" does not exist.') self._assert_active() def test_deactivate_enrollments_invalid_username(self): @@ -1291,7 +1299,7 @@ class UnenrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase): 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['message'], 'The user "a made up username" does not exist.') + self.assertEqual(data, u'The user "a made up username" does not exist.') self._assert_active() def test_deactivate_enrollments_called_twice(self): @@ -1315,7 +1323,10 @@ class UnenrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase): self.assertFalse(is_active) def _submit_unenroll(self, submitting_user, unenrolling_username): - data = {'user': unenrolling_username} + data = {} + if unenrolling_username is not None: + data['username'] = unenrolling_username + url = reverse('unenrollment') headers = self.build_jwt_headers(submitting_user) return self.client.post(url, json.dumps(data), content_type='application/json', **headers) diff --git a/common/djangoapps/enrollment/views.py b/common/djangoapps/enrollment/views.py index 096b576c29..83dc04dddd 100644 --- a/common/djangoapps/enrollment/views.py +++ b/common/djangoapps/enrollment/views.py @@ -5,6 +5,7 @@ consist primarily of authentication, request validation, and serialization. """ import logging +from django.contrib.auth import get_user_model from django.core.exceptions import ObjectDoesNotExist from django.utils.decorators import method_decorator from edx_rest_framework_extensions.authentication import JwtAuthentication @@ -23,6 +24,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, @@ -313,14 +315,14 @@ class UnenrollmentView(APIView): **Example Requests** POST /api/enrollment/v1/enrollment { - "user": "username12345" + "username": "username12345" } **POST Parameters** A POST request must include the following parameter. - * user: The username of the user being unenrolled. + * 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. @@ -342,31 +344,23 @@ class UnenrollmentView(APIView): """ Unenrolls the specified user from all courses. """ - # Get the User from the request. - username = request.data.get('user', None) - if not username: - return Response( - status=status.HTTP_404_NOT_FOUND, - data={ - 'message': u'The user was not specified.' - } - ) - try: - # make sure the specified user exists - User.objects.get(username=username) - except ObjectDoesNotExist: - return Response( - status=status.HTTP_404_NOT_FOUND, - data={ - 'message': u'The user "{}" does not exist.'.format(username) - } - ) + username = None + user_model = get_user_model() + try: + # Get the username from the request and check that it exists + username = request.data['username'] + user_model.objects.get(username=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(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 Exception as exc: # pylint: disable=broad-except return Response(text_type(exc), status=status.HTTP_500_INTERNAL_SERVER_ERROR) diff --git a/lms/djangoapps/discussion_api/tests/test_views.py b/lms/djangoapps/discussion_api/tests/test_views.py index 5695b6b389..1d1ff52f28 100644 --- a/lms/djangoapps/discussion_api/tests/test_views.py +++ b/lms/djangoapps/discussion_api/tests/test_views.py @@ -28,6 +28,7 @@ from discussion_api.tests.utils import ( ) from django_comment_client.tests.utils import ForumsEnableMixin from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_storage +from openedx.core.djangoapps.user_api.models import RetirementState, UserRetirementStatus from openedx.core.lib.token_utils import JwtBuilder from student.models import get_retired_username_by_username from student.tests.factories import CourseEnrollmentFactory, UserFactory, SuperuserFactory @@ -163,12 +164,16 @@ class RetireViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): """Tests for CourseView""" def setUp(self): super(RetireViewTest, self).setUp() + RetirementState.objects.create(state_name='PENDING', state_execution_order=1) + self.retire_forums_state = RetirementState.objects.create(state_name='RETIRE_FORUMS', state_execution_order=11) + + self.retirement = UserRetirementStatus.create_retirement(self.user) + self.retirement.current_state = self.retire_forums_state + self.retirement.save() + self.superuser = SuperuserFactory() self.retired_username = get_retired_username_by_username(self.user.username) - self.url = reverse( - "retire_discussion_user", - kwargs={"username": text_type(self.user.username)} - ) + self.url = reverse("retire_discussion_user") def assert_response_correct(self, response, expected_status, expected_content): """ @@ -193,24 +198,18 @@ class RetireViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): """ self.register_get_user_retire_response(self.user) headers = self.build_jwt_headers(self.superuser) - response = self.client.post(self.url, {'retired_username': self.retired_username}, **headers) + data = {'username': self.user.username} + response = self.client.post(self.url, data, **headers) self.assert_response_correct(response, 204, "") - def test_bad_hash(self): - """ - Check that we fail on a hash mismatch with an appropriate error - """ - headers = self.build_jwt_headers(self.superuser) - response = self.client.post(self.url, {'retired_username': "this will never match"}, **headers) - self.assert_response_correct(response, 500, '"Mismatched hashed_username, bad salt?"') - def test_downstream_forums_error(self): """ Check that we bubble up errors from the comments service """ self.register_get_user_retire_response(self.user, status=500, body="Server error") headers = self.build_jwt_headers(self.superuser) - response = self.client.post(self.url, {'retired_username': self.retired_username}, **headers) + data = {'username': self.user.username} + response = self.client.post(self.url, data, **headers) self.assert_response_correct(response, 500, '"Server error"') def test_nonexistent_user(self): @@ -218,19 +217,15 @@ class RetireViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): Check that we handle unknown users appropriately """ nonexistent_username = "nonexistent user" - self.url = reverse( - "retire_discussion_user", - kwargs={"username": nonexistent_username} - ) self.retired_username = get_retired_username_by_username(nonexistent_username) - + data = {'username': nonexistent_username} headers = self.build_jwt_headers(self.superuser) - response = self.client.post(self.url, {'retired_username': self.retired_username}, **headers) + response = self.client.post(self.url, data, **headers) self.assert_response_correct(response, 404, None) def test_not_authenticated(self): """ - Override the parent implementation of this, we auth differently + Override the parent implementation of this, we JWT auth for this API """ pass diff --git a/lms/djangoapps/discussion_api/urls.py b/lms/djangoapps/discussion_api/urls.py index 898dfdc921..a060f9c47b 100644 --- a/lms/djangoapps/discussion_api/urls.py +++ b/lms/djangoapps/discussion_api/urls.py @@ -17,7 +17,7 @@ urlpatterns = [ CourseView.as_view(), name="discussion_course" ), - url(r"^v1/users/{}".format(settings.USERNAME_PATTERN), RetireUserView.as_view(), name="retire_discussion_user"), + url(r"^v1/accounts/retire_forum", RetireUserView.as_view(), name="retire_discussion_user"), url( r"^v1/course_topics/{}".format(settings.COURSE_ID_PATTERN), CourseTopicsView.as_view(), diff --git a/lms/djangoapps/discussion_api/views.py b/lms/djangoapps/discussion_api/views.py index a264064c5e..c4ef9e3235 100644 --- a/lms/djangoapps/discussion_api/views.py +++ b/lms/djangoapps/discussion_api/views.py @@ -33,7 +33,7 @@ from discussion_api.forms import CommentGetForm, CommentListGetForm, ThreadListG from openedx.core.lib.api.parsers import MergePatchParser from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes from openedx.core.djangoapps.user_api.accounts.permissions import CanRetireUser -from student.models import get_potentially_retired_user_by_username_and_hash +from openedx.core.djangoapps.user_api.models import UserRetirementStatus from xmodule.modulestore.django import modulestore @@ -543,23 +543,22 @@ class RetireUserView(APIView): authentication_classes = (JwtAuthentication,) permission_classes = (permissions.IsAuthenticated, CanRetireUser) - def post(self, request, username): + def post(self, request): """ Implements the retirement endpoint. """ - user_model = get_user_model() - retired_username = request.data['retired_username'] + username = request.data['username'] try: - user = get_potentially_retired_user_by_username_and_hash(username, retired_username) - cc_user = comment_client.User.from_django_user(user) + retirement = UserRetirementStatus.get_retirement_for_retirement_action(username) + cc_user = comment_client.User.from_django_user(retirement.user) # We can't count on the LMS username being un-retired at this point, # so we pass the old username as a parameter to describe which # user to retire. This will either succeed or throw an error which # should be good to raise from here. cc_user.retire(username) - except user_model.DoesNotExist: + except UserRetirementStatus.DoesNotExist: return Response(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_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index e9557793d0..2ffd87f840 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -2,6 +2,8 @@ """ Test cases to cover Accounts-related behaviors of the User API application """ +from __future__ import print_function + import datetime import hashlib import json @@ -924,99 +926,6 @@ class TestAccountDeactivation(TestCase): ) -@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') -class TestAccountRetireMailings(TestCase): - """ - Tests the account retire mailings endpoint. - """ - - def setUp(self): - super(TestAccountRetireMailings, self).setUp() - self.test_user = UserFactory() - self.test_superuser = SuperuserFactory() - self.test_service_user = UserFactory() - - 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', kwargs={'username': self.test_user.username}) - - def build_post(self, user): - retired_username = get_retired_username_by_username(user.username) - return {'retired_username': retired_username} - - def build_jwt_headers(self, user): - """ - Helper function for creating headers for the JWT authentication. - """ - token = JwtBuilder(user).build_token([]) - headers = { - 'HTTP_AUTHORIZATION': 'JWT ' + token - } - return headers - - 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): - """ - Helper function for making a request to the retire subscriptions endpoint, and asserting the status. - """ - response = self.client.post(self.url, self.build_post(self.test_user), **headers) - 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) - - def test_superuser_retires_user_subscriptions(self): - """ - Verify a user's subscriptions are retired when a superuser posts to the retire subscriptions endpoint. - """ - headers = self.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 = self.build_jwt_headers(self.test_superuser) - self.assert_status_and_tag_count(headers, expected_tag_count=0) - - def test_unauthorized_rejection(self): - """ - Verify unauthorized users cannot retire subscriptions. - """ - headers = self.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") - - def test_signal_failure(self): - """ - Verify that if a signal fails the transaction is rolled back and a proper error message is returned. - """ - headers = self.build_jwt_headers(self.test_superuser) - - mock_handler = MagicMock() - mock_handler.side_effect = Exception("Tango") - - try: - USER_RETIRE_MAILINGS.connect(mock_handler) - - # User should still have 2 "True" subscriptions. - self.assert_status_and_tag_count( - headers, - expected_status=status.HTTP_500_INTERNAL_SERVER_ERROR, - expected_tag_value="True", - expected_content="Tango" - ) - finally: - USER_RETIRE_MAILINGS.disconnect(mock_handler) - - @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') class TestDeactivateLogout(TestCase): """ @@ -1052,7 +961,7 @@ class TestDeactivateLogout(TestCase): return headers def build_post(self, username): - return {'user': username} + return {'username': username} def test_superuser_deactivates_user(self): """ @@ -1095,8 +1004,13 @@ class RetirementTestCase(TestCase): """ Test case with a helper methods for retirement """ + @classmethod + def setUpClass(cls): + super(RetirementTestCase, cls).setUpClass() + cls.setup_states() - def setup_states(self): + @staticmethod + def setup_states(): """ Create basic states that mimic our current understanding of the retirement process """ @@ -1215,6 +1129,106 @@ class RetirementTestCase(TestCase): 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 TestAccountRetireMailings(RetirementTestCase): + """ + Tests the account retire mailings endpoint. + """ + def setUp(self): + super(TestAccountRetireMailings, self).setUp() + self.test_superuser = SuperuserFactory() + self.test_service_user = UserFactory() + + # Should be created in parent setUpClass + retiring_email_lists = RetirementState.objects.get(state_name='RETIRING_EMAIL_LISTS') + + 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_jwt_headers(self, user): + """ + Helper function for creating headers for the JWT authentication. + """ + token = JwtBuilder(user).build_token([]) + headers = { + 'HTTP_AUTHORIZATION': 'JWT ' + token + } + return headers + + 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): + """ + Helper function for making a request to the retire subscriptions endpoint, and asserting the status. + """ + response = self.client.post(self.url, self.build_post(self.test_user), **headers) + + if response.status_code != expected_status: + print(response) + + 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) + + def test_superuser_retires_user_subscriptions(self): + """ + Verify a user's subscriptions are retired when a superuser posts to the retire subscriptions endpoint. + """ + headers = self.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 = self.build_jwt_headers(self.test_superuser) + self.assert_status_and_tag_count(headers, expected_tag_count=0) + + def test_unauthorized_rejection(self): + """ + Verify unauthorized users cannot retire subscriptions. + """ + headers = self.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") + + def test_signal_failure(self): + """ + Verify that if a signal fails the transaction is rolled back and a proper error message is returned. + """ + headers = self.build_jwt_headers(self.test_superuser) + + mock_handler = MagicMock() + mock_handler.side_effect = Exception("Tango") + + try: + USER_RETIRE_MAILINGS.connect(mock_handler) + + # User should still have 2 "True" subscriptions. + self.assert_status_and_tag_count( + headers, + expected_status=status.HTTP_500_INTERNAL_SERVER_ERROR, + expected_tag_value="True", + expected_content="Tango" + ) + finally: + USER_RETIRE_MAILINGS.disconnect(mock_handler) + + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') class TestAccountRetirementList(RetirementTestCase): """ @@ -1227,7 +1241,6 @@ class TestAccountRetirementList(RetirementTestCase): self.headers = self.build_jwt_headers(self.test_superuser) self.url = reverse('accounts_retirement_queue') self.maxDiff = None - self.setup_states() def assert_status_and_user_list( self, @@ -1371,7 +1384,6 @@ class TestAccountRetirementRetrieve(RetirementTestCase): self.url = reverse('accounts_retirement_retrieve', kwargs={'username': self.test_user.username}) self.headers = self.build_jwt_headers(self.test_superuser) self.maxDiff = None - self.setup_states() def assert_status_and_user_data(self, expected_data, expected_status=status.HTTP_200_OK, username_to_find=None): """ @@ -1438,7 +1450,6 @@ class TestAccountRetirementUpdate(RetirementTestCase): """ def setUp(self): super(TestAccountRetirementUpdate, self).setUp() - self.setup_states() self.pending_state = RetirementState.objects.get(state_name='PENDING') self.locking_state = RetirementState.objects.get(state_name='LOCKING_ACCOUNT') diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 04a96b2a3c..8906c35528 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -271,7 +271,7 @@ class AccountRetireMailingsView(APIView): authentication_classes = (JwtAuthentication, ) permission_classes = (permissions.IsAuthenticated, CanRetireUser) - def post(self, request, username): + def post(self, request): """ POST /api/user/v1/accounts/{username}/retire_mailings/ @@ -280,40 +280,39 @@ class AccountRetireMailingsView(APIView): - Update UserOrgTags to opt the user out of org emails - Call Sailthru API to force opt-out the user from all email lists """ - user_model = get_user_model() - retired_username = request.data['retired_username'] + username = request.data['username'] try: - user = get_potentially_retired_user_by_username_and_hash(username, retired_username) + 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=user, key='email-optin'): - update_email_opt_in(user, preference.org, False) + 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(sender=self.__class__, user=user) - except user_model.DoesNotExist: + USER_RETIRE_MAILINGS.send(sender=self.__class__, user=retirement.user) + + return Response(status=status.HTTP_204_NO_CONTENT) + except UserRetirementStatus.DoesNotExist: return Response(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) - return Response(status=status.HTTP_204_NO_CONTENT) - class DeactivateLogoutView(APIView): """ POST /api/user/v1/accounts/deactivate_logout/ { - "user": "example_username", + "username": "example_username", } **POST Parameters** A POST request must include the following parameter. - * user: Required. The username of the user being deactivated. + * username: Required. The username of the user being deactivated. **POST Response Values** @@ -345,18 +344,11 @@ class DeactivateLogoutView(APIView): Marks the user as having no password set for deactivation purposes, and logs the user out. """ - username = request.data.get('user', None) - if not username: - return Response( - status=status.HTTP_404_NOT_FOUND, - data={ - 'message': u'The user was not specified.' - } - ) - + username = None user_model = get_user_model() try: - # make sure the specified user exists + # Get the username from the request and check that it exists + username = request.data['username'] user = user_model.objects.get(username=username) with transaction.atomic(): @@ -367,13 +359,14 @@ class DeactivateLogoutView(APIView): user.save() _set_unusable_password(user) # 3. Unlink social accounts & change password on each IDA, still to be implemented + return Response(status=status.HTTP_204_NO_CONTENT) + except KeyError: + return Response(u'Username not specified.', status=status.HTTP_404_NOT_FOUND) except user_model.DoesNotExist: - return Response(status=status.HTTP_404_NOT_FOUND) + return Response(u'The user "{}" does not exist.'.format(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) - return Response(status=status.HTTP_204_NO_CONTENT) - def _set_unusable_password(user): """ diff --git a/openedx/core/djangoapps/user_api/models.py b/openedx/core/djangoapps/user_api/models.py index 5b824139bb..a13b675ba0 100644 --- a/openedx/core/djangoapps/user_api/models.py +++ b/openedx/core/djangoapps/user_api/models.py @@ -268,5 +268,26 @@ class UserRetirementStatus(TimeStampedModel): self.responses += "\n Moved from {} to {}:\n{}\n".format(old_state, self.current_state, update['response']) self.save() + @classmethod + def get_retirement_for_retirement_action(cls, username): + """ + Convenience method to get a UseRetirementStatus for a particular user with some checking + to make sure they're in a state that is acceptable to be acted upon. The user should be + in a "working state" (not a dead end state, PENDING, or *_COMPLETE). This should help + a bit with situations like the retirement driver accidentally trying to act upon the + same user twice at the same time, or trying to take action on an errored user. + + Can raise UserRetirementStatus.DoesNotExist or RetirementStateError, otherwise should + return a UserRetirementStatus + """ + retirement = cls.objects.get(original_username=username) + state = retirement.current_state + + if state.required or state.state_name.endswith('_COMPLETE'): + raise RetirementStateError('{} is in {}, not a valid state to perform retirement ' + 'actions on.'.format(retirement, state.state_name)) + + return retirement + def __unicode__(self): return u'User: {} State: {} Last Updated: {}'.format(self.user.id, self.current_state, self.modified) diff --git a/openedx/core/djangoapps/user_api/urls.py b/openedx/core/djangoapps/user_api/urls.py index 2c9bb31a6f..9ea27ec077 100644 --- a/openedx/core/djangoapps/user_api/urls.py +++ b/openedx/core/djangoapps/user_api/urls.py @@ -70,7 +70,7 @@ urlpatterns = [ name='accounts_deactivation' ), url( - r'^v1/accounts/{}/retire_mailings/$'.format(settings.USERNAME_PATTERN), + r'^v1/accounts/retire_mailings/$', AccountRetireMailingsView.as_view(), name='accounts_retire_mailings' ),