From 74b07c3ff84d24eb0bd4f59bef8f410b10df6774 Mon Sep 17 00:00:00 2001 From: bmedx Date: Fri, 13 Jul 2018 15:08:31 -0400 Subject: [PATCH 1/2] PLAT-2186 - allow retirement states to be forced by driver script When the optional "force" param is sent in this skips the checks for moving from dead end state and moving to earlier state, allowing us to bulk reset errored users or run users through new states. --- .../accounts/tests/test_retirement_views.py | 51 +++++++++---------- openedx/core/djangoapps/user_api/models.py | 9 +++- 2 files changed, 31 insertions(+), 29 deletions(-) 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 0802c3cd3a..e5f24f2d82 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 @@ -8,6 +8,9 @@ import datetime import json import unittest +import ddt +import pytz +import mock from consent.models import DataSharingConsent from django.conf import settings from django.contrib.auth.models import User @@ -25,9 +28,7 @@ from enterprise.models import ( from integrated_channels.sap_success_factors.models import ( SapSuccessFactorsLearnerDataTransmissionAudit ) -import mock from opaque_keys.edx.keys import CourseKey -import pytz from rest_framework import status from six import iteritems, text_type from social_django.models import UserSocialAuth @@ -51,7 +52,6 @@ from openedx.core.djangoapps.user_api.models import ( UserRetirementPartnerReportingStatus, UserOrgTag ) -from openedx.core.djangoapps.user_api.accounts.tests.retirement_helpers import fake_retirement from openedx.core.djangoapps.user_api.accounts.views import AccountRetirementPartnerReportView from openedx.core.lib.token_utils import JwtBuilder from student.models import ( @@ -881,6 +881,7 @@ class TestAccountRetirementRetrieve(RetirementTestCase): self.assert_status_and_user_data(values, username_to_find=original_username) +@ddt.ddt @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') class TestAccountRetirementUpdate(RetirementTestCase): """ @@ -994,35 +995,31 @@ class TestAccountRetirementUpdate(RetirementTestCase): data = {'new_state': 'LOCKING_ACCOUNT', 'response': 'this should fail', 'username': 'does not exist'} self.update_and_assert_status(data, status.HTTP_404_NOT_FOUND) - def test_move_from_dead_end(self): - """ - Confirm that trying to move from a dead end state to any other state fails - """ + @ddt.data( + # Test moving backward from intermediate state + ('LOCKING_ACCOUNT', 'PENDING', False, status.HTTP_400_BAD_REQUEST), + ('LOCKING_ACCOUNT', 'PENDING', True, status.HTTP_204_NO_CONTENT), + + # Test moving backward from dead end state + ('COMPLETE', 'PENDING', False, status.HTTP_400_BAD_REQUEST), + ('COMPLETE', 'PENDING', True, status.HTTP_204_NO_CONTENT), + + # Test moving to the same state + ('LOCKING_ACCOUNT', 'LOCKING_ACCOUNT', False, status.HTTP_400_BAD_REQUEST), + ('LOCKING_ACCOUNT', 'LOCKING_ACCOUNT', True, status.HTTP_204_NO_CONTENT), + ) + @ddt.unpack + def test_moves(self, start_state, move_to_state, force, expected_response_code): retirement = UserRetirementStatus.objects.get(id=self.retirement.id) - retirement.current_state = RetirementState.objects.filter(is_dead_end_state=True)[0] + retirement.current_state = RetirementState.objects.get(state_name=start_state) retirement.save() - data = {'new_state': 'LOCKING_ACCOUNT', 'response': 'this should fail'} - self.update_and_assert_status(data, status.HTTP_400_BAD_REQUEST) + data = {'new_state': move_to_state, 'response': 'foo'} - def test_move_backward(self): - """ - Confirm that trying to move to an earlier step in the process fails - """ - retirement = UserRetirementStatus.objects.get(id=self.retirement.id) - retirement.current_state = RetirementState.objects.get(state_name='COMPLETE') - retirement.save() + if force: + data['force'] = True - data = {'new_state': 'PENDING', 'response': 'this should fail'} - self.update_and_assert_status(data, status.HTTP_400_BAD_REQUEST) - - def test_move_same(self): - """ - Confirm that trying to move to the same step in the process fails - """ - # Should already be in 'PENDING' - data = {'new_state': 'PENDING', 'response': 'this should fail'} - self.update_and_assert_status(data, status.HTTP_400_BAD_REQUEST) + self.update_and_assert_status(data, expected_response_code) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') diff --git a/openedx/core/djangoapps/user_api/models.py b/openedx/core/djangoapps/user_api/models.py index 40c9a91146..dca0eb6aba 100644 --- a/openedx/core/djangoapps/user_api/models.py +++ b/openedx/core/djangoapps/user_api/models.py @@ -264,13 +264,15 @@ class UserRetirementStatus(TimeStampedModel): Confirm that the data passed in is properly formatted """ required_keys = ('username', 'new_state', 'response') + optional_keys = ('force', ) + known_keys = required_keys + optional_keys for required_key in required_keys: if required_key not in data: raise RetirementStateError('RetirementStatus: Required key {} missing from update'.format(required_key)) for key in data: - if key not in required_keys: + if key not in known_keys: raise RetirementStateError('RetirementStatus: Unknown key {} in update'.format(key)) @classmethod @@ -310,7 +312,10 @@ class UserRetirementStatus(TimeStampedModel): or throw a RetirementStateError with a useful error message """ self._validate_update_data(update) - self._validate_state_update(update['new_state']) + + force = update.get('force', False) + if not force: + self._validate_state_update(update['new_state']) old_state = self.current_state self.current_state = RetirementState.objects.get(state_name=update['new_state']) From 04fe4af9c0e18e503c7a8d20d0aa5680d0bc15a8 Mon Sep 17 00:00:00 2001 From: bmedx Date: Wed, 18 Jul 2018 13:52:55 -0400 Subject: [PATCH 2/2] Adding user_api call to get retirements by date range and current state - Also supports PLAT-2186 --- .../accounts/tests/test_retirement_views.py | 149 ++++++++++++++++++ .../djangoapps/user_api/accounts/views.py | 43 +++++ openedx/core/djangoapps/user_api/urls.py | 9 ++ 3 files changed, 201 insertions(+) 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 e5f24f2d82..57edcacf66 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 @@ -810,6 +810,155 @@ class TestAccountRetirementList(RetirementTestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) +@ddt.ddt +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') +class TestAccountRetirementsByStatusAndDate(RetirementTestCase): + """ + Tests the retirements_by_status_and_date endpoint + """ + + def setUp(self): + super(TestAccountRetirementsByStatusAndDate, self).setUp() + self.test_superuser = SuperuserFactory() + self.headers = build_jwt_headers(self.test_superuser) + self.url = reverse('accounts_retirements_by_status_and_date') + self.maxDiff = None + + def assert_status_and_user_list( + self, + expected_data, + expected_status=status.HTTP_200_OK, + state_to_request=None, + start_date=None, + end_date=None + ): + """ + Helper function for making a request to the endpoint, asserting the status, and + optionally asserting data returned. Will try to convert datetime start and end dates + to the correct string formatting. + """ + if state_to_request is None: + state_to_request = 'COMPLETE' + + if start_date is None: + start_date = datetime.datetime.now().date().strftime('%Y-%m-%d') + else: + start_date = start_date.date().strftime('%Y-%m-%d') + + if end_date is None: + end_date = datetime.datetime.now().date().strftime('%Y-%m-%d') + else: + end_date = end_date.date().strftime('%Y-%m-%d') + + data = {'start_date': start_date, 'end_date': end_date, 'state': state_to_request} + response = self.client.get(self.url, data, **self.headers) + + print(response.status_code) + print(response) + + self.assertEqual(response.status_code, expected_status) + response_data = response.json() + + if expected_data: + # These datetimes won't match up due to serialization, but they're inherited fields tested elsewhere + for data in (response_data, expected_data): + for retirement in data: + # These may have been deleted in a previous pass + try: + del retirement['created'] + del retirement['modified'] + except KeyError: + pass + + self.assertItemsEqual(response_data, expected_data) + + def test_empty(self): + """ + Verify that an empty array is returned if no users are awaiting retirement + """ + self.assert_status_and_user_list([]) + + def test_users_exist_none_in_correct_state(self): + """ + Verify that users in non-requested states are not returned + """ + state = RetirementState.objects.get(state_name='PENDING') + self._create_retirement(state=state) + self.assert_status_and_user_list([]) + + def test_users_exist(self): + """ + Verify correct user is returned when users in different states exist + """ + # Stores the user we expect to get back + retirement_values = None + for retirement in self._create_users_all_states(): + if retirement.current_state == 'COMPLETE': + retirement_values.append(self._retirement_to_dict(retirement)) + + self.assert_status_and_user_list(retirement_values) + + def test_bad_states(self): + """ + Check some bad inputs to make sure we get back the expected status + """ + self.assert_status_and_user_list(None, expected_status=status.HTTP_400_BAD_REQUEST, state_to_request='TACO') + + def test_date_filter(self): + """ + Verifies the functionality of the start and end date filters + """ + retirements = [] + complete_state = RetirementState.objects.get(state_name='COMPLETE') + + # Create retirements for the last 10 days + for days_back in range(0, 10): + create_datetime = datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=days_back) + ret = self._create_retirement(state=complete_state, create_datetime=create_datetime) + retirements.append(self._retirement_to_dict(ret)) + + # Go back in time adding days to the query, assert the correct retirements are present + end_date = datetime.datetime.now(pytz.UTC) + for days_back in range(1, 11): + retirement_dicts = retirements[:days_back] + start_date = end_date - datetime.timedelta(days=days_back - 1) + self.assert_status_and_user_list( + retirement_dicts, + start_date=start_date, + end_date=end_date + ) + + def test_bad_dates(self): + """ + Check some bad inputs to make sure we get back the expected status + """ + good_date = '2018-01-01' + for bad_param, good_param in (('start_date', 'end_date'), ('end_date', 'start_date')): + for bad_date in ('10/21/2001', '2118-01-01', '2018-14-25', 'toast', 5): + data = { + bad_param: bad_date, + good_param: good_date, + 'state': 'COMPLETE' + } + response = self.client.get(self.url, data, **self.headers) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + @ddt.data( + {}, + {'start_date': '2018-01-01'}, + {'end_date': '2018-01-01'}, + {'state': 'PENDING'}, + {'start_date': '2018-01-01', 'state': 'PENDING'}, + {'end_date': '2018-01-01', 'state': 'PENDING'}, + ) + def test_missing_params(self, request_data): + """ + All params are required, make sure that is enforced + """ + response = self.client.get(self.url, request_data, **self.headers) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') class TestAccountRetirementRetrieve(RetirementTestCase): """ diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index d0797ff3e7..c19fba8052 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -683,6 +683,49 @@ class AccountRetirementStatusView(ViewSet): except RetirementStateError as exc: return Response(text_type(exc), status=status.HTTP_400_BAD_REQUEST) + def retirements_by_status_and_date(self, request): + """ + GET /api/user/v1/accounts/retirements_by_status_and_date/ + ?start_date=2018-09-05&end_date=2018-09-07&state=COMPLETE + + Returns a list of UserRetirementStatusSerializer serialized + RetirementStatus rows in the given state that were created in the + retirement queue between the dates given. Date range is inclusive, + so to get one day you would set both dates to that day. + """ + try: + start_date = datetime.datetime.strptime(request.GET['start_date'], '%Y-%m-%d') + end_date = datetime.datetime.strptime(request.GET['end_date'], '%Y-%m-%d') + now = datetime.datetime.now() + if start_date > now or end_date > now or start_date > end_date: + raise RetirementStateError('Dates must be today or earlier, and start must be earlier than end.') + + # Add a day to make sure we get all the way to 23:59:59.999, this is compared "lt" in the query + # not "lte". + end_date += datetime.timedelta(days=1) + state = request.GET['state'] + + state_obj = RetirementState.objects.get(state_name=state) + + retirements = UserRetirementStatus.objects.select_related( + 'user', 'current_state', 'last_state' + ).filter( + current_state=state_obj, created__lt=end_date, created__gte=start_date + ).order_by( + 'id' + ) + serializer = UserRetirementStatusSerializer(retirements, many=True) + return Response(serializer.data) + # This should only occur on the datetime conversion of the start / end dates. + except ValueError as exc: + return Response('Invalid start or end date: {}'.format(text_type(exc)), status=status.HTTP_400_BAD_REQUEST) + except KeyError as exc: + return Response('Missing required parameter: {}'.format(text_type(exc)), status=status.HTTP_400_BAD_REQUEST) + except RetirementState.DoesNotExist: + return Response('Unknown retirement state.', status=status.HTTP_400_BAD_REQUEST) + except RetirementStateError as exc: + return Response(text_type(exc), status=status.HTTP_400_BAD_REQUEST) + def retrieve(self, request, username): # pylint: disable=unused-argument """ GET /api/user/v1/accounts/{username}/retirement_status/ diff --git a/openedx/core/djangoapps/user_api/urls.py b/openedx/core/djangoapps/user_api/urls.py index 494a7d3961..ee7cbd898f 100644 --- a/openedx/core/djangoapps/user_api/urls.py +++ b/openedx/core/djangoapps/user_api/urls.py @@ -43,6 +43,10 @@ RETIREMENT_QUEUE = AccountRetirementStatusView.as_view({ 'get': 'retirement_queue' }) +RETIREMENT_LIST_BY_STATUS_AND_DATE = AccountRetirementStatusView.as_view({ + 'get': 'retirements_by_status_and_date' +}) + RETIREMENT_RETRIEVE = AccountRetirementStatusView.as_view({ 'get': 'retrieve' }) @@ -115,6 +119,11 @@ urlpatterns = [ RETIREMENT_QUEUE, name='accounts_retirement_queue' ), + url( + r'^v1/accounts/retirements_by_status_and_date/$', + RETIREMENT_LIST_BY_STATUS_AND_DATE, + name='accounts_retirements_by_status_and_date' + ), url( r'^v1/accounts/retire/$', RETIREMENT_POST,