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 e6ccbe1ba8..6054eca9fd 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -1493,7 +1493,7 @@ class TestAccountRetirementUpdate(RetirementTestCase): self.test_user = self.retirement.user self.test_superuser = SuperuserFactory() self.headers = self.build_jwt_headers(self.test_superuser) - self.headers['content_type'] = "application/merge-patch+json" + self.headers['content_type'] = "application/json" self.url = reverse('accounts_retirement_update') def update_and_assert_status(self, data, expected_status=status.HTTP_204_NO_CONTENT): diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index a3a8c1d029..2101823d00 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -444,7 +444,7 @@ class DeactivateLogoutView(APIView): except AuthFailedError as err: return Response(text_type(err), status=status.HTTP_403_FORBIDDEN) except Exception as err: # pylint: disable=broad-except - return Response(u"Could not verify user password", status=status.HTTP_400_BAD_REQUEST) + return Response(u"Could not verify user password: {}".format(err), status=status.HTTP_400_BAD_REQUEST) def _check_excessive_login_attempts(self, user): """ @@ -480,7 +480,7 @@ class AccountRetirementStatusView(ViewSet): """ authentication_classes = (JwtAuthentication,) permission_classes = (permissions.IsAuthenticated, CanRetireUser,) - parser_classes = (MergePatchParser,) + parser_classes = (JSONParser,) serializer_class = UserRetirementStatusSerializer def retirement_queue(self, request): @@ -554,13 +554,14 @@ class AccountRetirementStatusView(ViewSet): Updates the RetirementStatus row for the given user to the new status, and append any messages to the message log. - Note that this implementation is the "merge patch" implementation proposed in - https://tools.ietf.org/html/rfc7396. The content_type must be "application/merge-patch+json" or - else an error response with status code 415 will be returned. + Note that this implementation DOES NOT use the "merge patch" + implementation seen in AccountViewSet. Slumber, the project + we use to power edx-rest-api-client, does not currently support + it. The content type for this request is 'application/json'. """ try: username = request.data['username'] - retirement = UserRetirementStatus.objects.get(user__username=username) + retirement = UserRetirementStatus.objects.get(original_username=username) retirement.update_state(request.data) return Response(status=status.HTTP_204_NO_CONTENT) except UserRetirementStatus.DoesNotExist: diff --git a/openedx/core/djangoapps/user_api/management/commands/populate_retirement_states.py b/openedx/core/djangoapps/user_api/management/commands/populate_retirement_states.py index 1cd10b98d0..69439bed27 100644 --- a/openedx/core/djangoapps/user_api/management/commands/populate_retirement_states.py +++ b/openedx/core/djangoapps/user_api/management/commands/populate_retirement_states.py @@ -66,6 +66,11 @@ class Command(BaseCommand): '{}'.format(REQ_STR) ) + def _check_users_in_states_to_delete(self, states_to_delete): + if UserRetirementStatus.objects.filter(current_state__state_name__in=states_to_delete).exists(): + raise CommandError('Users exist in a state that is marked for deletion! States to delete' + 'are: {}'.format(states_to_delete)) + def _delete_old_states_and_create_new(self, new_states): """ Wipes the RetirementState table and creates new entries based on new_states @@ -75,32 +80,44 @@ class Command(BaseCommand): """ # Save off old states before - current_states = RetirementState.objects.all().values_list('state_name', flat=True) - - # Delete all existing rows, easier than messing with the ordering - RetirementState.objects.all().delete() - - # Add new rows, with space in between to manually insert stages via Django admin if necessary - curr_sort_order = 1 - for state in new_states: - row = { - 'state_name': state, - 'state_execution_order': curr_sort_order, - 'is_dead_end_state': state in END_STATES, - 'required': state in REQUIRED_STATES - } - - RetirementState.objects.create(**row) - curr_sort_order += 10 + current_state_names = RetirementState.objects.all().values_list('state_name', flat=True) # Generate the diff - set_current_states = set(current_states) + set_current_states = set(current_state_names) set_new_states = set(new_states) states_to_create = set_new_states - set_current_states states_remaining = set_current_states.intersection(set_new_states) states_to_delete = set_current_states - set_new_states + # In theory this should not happen, this would have failed _check_current_users + # if the state was not required, and failed _validate_new_states if we're trying + # to remove a required state, but playing it extra safe here since a state delete + # will cascade and remove the UserRetirementState row as well if something slips + # through. + if states_to_delete: + self._check_users_in_states_to_delete(states_to_delete) + + # Delete states slated for removal + RetirementState.objects.filter(state_name__in=states_to_delete).delete() + + # Add new rows, with space in between to manually insert stages via Django admin if necessary + curr_sort_order = 1 + for state in new_states: + if state not in current_state_names: + row = { + 'state_name': state, + 'state_execution_order': curr_sort_order, + 'is_dead_end_state': state in END_STATES, + 'required': state in REQUIRED_STATES + } + + RetirementState.objects.create(**row) + else: + RetirementState.objects.filter(state_name=state).update(state_execution_order=curr_sort_order) + + curr_sort_order += 10 + return states_to_create, states_remaining, states_to_delete def handle(self, *args, **options): diff --git a/openedx/core/djangoapps/user_api/models.py b/openedx/core/djangoapps/user_api/models.py index b51f312118..d73e92458a 100644 --- a/openedx/core/djangoapps/user_api/models.py +++ b/openedx/core/djangoapps/user_api/models.py @@ -167,16 +167,6 @@ class RetirementState(models.Model): return cls.objects.all().values_list('state_name', flat=True) -@receiver(pre_delete, sender=RetirementState) -def retirementstate_pre_delete_callback(_, **kwargs): - """ - Event changes to user preferences. - """ - state = kwargs["instance"] - if state.required: - raise Exception('Required RetirementStates cannot be deleted.') - - class UserRetirementStatus(TimeStampedModel): """ Tracks the progress of a user's retirement request