More helpful UX in beta membership widget

LMS-2262

lms/djangoapps/instructor/tests/test_api.py

@@ -904,6 +904,38 @@ class TestInstructorAPILevelsAccess(ModuleStoreTestCase, LoginEnrollmentTestCase
         })
         self.assertEqual(response.status_code, 200)
 
+    def test_modify_access_with_fake_user(self):
+        url = reverse('modify_access', kwargs={'course_id': self.course.id})
+        response = self.client.get(url, {
+            'unique_student_identifier': 'GandalfTheGrey',
+            'rolename': 'staff',
+            'action': 'revoke',
+        })
+        self.assertEqual(response.status_code, 200)
+        expected = {
+            'unique_student_identifier': 'GandalfTheGrey',
+            'userDoesNotExist': True,
+        }
+        res_json = json.loads(response.content)
+        self.assertEqual(res_json, expected)
+
+    def test_modify_access_with_inactive_user(self):
+        self.other_user.is_active = False
+        self.other_user.save()  # pylint: disable=no-member
+        url = reverse('modify_access', kwargs={'course_id': self.course.id})
+        response = self.client.get(url, {
+            'unique_student_identifier': self.other_user.username,
+            'rolename': 'beta',
+            'action': 'allow',
+        })
+        self.assertEqual(response.status_code, 200)
+        expected = {
+            'unique_student_identifier': self.other_user.username,
+            'inactiveUser': True,
+        }
+        res_json = json.loads(response.content)
+        self.assertEqual(res_json, expected)
+
     def test_modify_access_revoke_not_allowed(self):
         """ Test revoking access that a user does not have. """
         url = reverse('modify_access', kwargs={'course_id': self.course.id})
@@ -924,7 +956,16 @@ class TestInstructorAPILevelsAccess(ModuleStoreTestCase, LoginEnrollmentTestCase
             'rolename': 'instructor',
             'action': 'revoke',
         })
-        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.status_code, 200)
+        # check response content
+        expected = {
+            'unique_student_identifier': self.instructor.username,
+            'rolename': 'instructor',
+            'action': 'revoke',
+            'removingSelfAsInstructor': True,
+        }
+        res_json = json.loads(response.content)
+        self.assertEqual(res_json, expected)
 
     def test_list_course_role_members_noparams(self):
         """ Test missing all query parameters. """

lms/djangoapps/instructor/views/api.py

@@ -395,8 +395,25 @@ def modify_access(request, course_id):
     course = get_course_with_access(
         request.user, course_id, 'instructor', depth=None
     )
+    try:
+        user = get_student_from_identifier(request.GET.get('unique_student_identifier'))
+    except User.DoesNotExist:
+        response_payload = {
+            'unique_student_identifier': request.GET.get('unique_student_identifier'),
+            'userDoesNotExist': True,
+        }
+        return JsonResponse(response_payload)
+
+    # Check that user is active, because add_users
+    # in common/djangoapps/student/roles.py fails
+    # silently when we try to add an inactive user.
+    if not user.is_active:
+        response_payload = {
+            'unique_student_identifier': user.username,
+            'inactiveUser': True,
+        }
+        return JsonResponse(response_payload)
 
-    user = get_student_from_identifier(request.GET.get('unique_student_identifier'))
     rolename = request.GET.get('rolename')
     action = request.GET.get('action')
 
@@ -407,9 +424,13 @@ def modify_access(request, course_id):
 
     # disallow instructors from removing their own instructor access.
     if rolename == 'instructor' and user == request.user and action != 'allow':
-        return HttpResponseBadRequest(
-            "An instructor cannot remove their own instructor access."
-        )
+        response_payload = {
+            'unique_student_identifier': user.username,
+            'rolename': rolename,
+            'action': action,
+            'removingSelfAsInstructor': True,
+        }
+        return JsonResponse(response_payload)
 
     if action == 'allow':
         allow_access(course, user, rolename)