Merge pull request #18146 from edx/bmedx/fix_populate_retirement_states

Fix issues with retirement state population and update view

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):

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:

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):

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