Can't remove last instructor of a course

cms/djangoapps/contentstore/tests/test_users.py

@@ -105,6 +105,29 @@ class UsersTestCase(CourseTestCase):
         self.assertIn(self.staff_groupname, groups)
         self.assertNotIn(self.inst_groupname, groups)
 
+    def test_detail_post_staff_other_inst(self):
+        inst_group, _ = Group.objects.get_or_create(name=self.inst_groupname)
+        self.user.groups.add(inst_group)
+        self.user.save()
+
+        resp = self.client.post(
+            self.detail_url,
+            data=json.dumps({"role": "staff"}),
+            content_type="application/json",
+            HTTP_ACCEPT="application/json",
+        )
+        self.assert2XX(resp.status_code)
+        # reload user from DB
+        ext_user = User.objects.get(email=self.ext_user.email)
+        groups = [g.name for g in ext_user.groups.all()]
+        self.assertIn(self.staff_groupname, groups)
+        self.assertNotIn(self.inst_groupname, groups)
+        # check that other user is unchanged
+        user = User.objects.get(email=self.user.email)
+        groups = [g.name for g in user.groups.all()]
+        self.assertNotIn(self.staff_groupname, groups)
+        self.assertIn(self.inst_groupname, groups)
+
     def test_detail_post_instructor(self):
         resp = self.client.post(
             self.detail_url,
@@ -171,7 +194,9 @@ class UsersTestCase(CourseTestCase):
 
     def test_detail_delete_instructor(self):
         group, _ = Group.objects.get_or_create(name=self.inst_groupname)
+        self.user.groups.add(group)
         self.ext_user.groups.add(group)
+        self.user.save()
         self.ext_user.save()
 
         resp = self.client.delete(
@@ -183,3 +208,38 @@ class UsersTestCase(CourseTestCase):
         ext_user = User.objects.get(email=self.ext_user.email)
         groups = [g.name for g in ext_user.groups.all()]
         self.assertNotIn(self.inst_groupname, groups)
+
+    def test_delete_last_instructor(self):
+        group, _ = Group.objects.get_or_create(name=self.inst_groupname)
+        self.ext_user.groups.add(group)
+        self.ext_user.save()
+
+        resp = self.client.delete(
+            self.detail_url,
+            HTTP_ACCEPT="application/json",
+        )
+        self.assertEqual(resp.status_code, 400)
+        result = json.loads(resp.content)
+        self.assertIn("error", result)
+        # reload user from DB
+        ext_user = User.objects.get(email=self.ext_user.email)
+        groups = [g.name for g in ext_user.groups.all()]
+        self.assertIn(self.inst_groupname, groups)
+
+    def test_post_last_instructor(self):
+        group, _ = Group.objects.get_or_create(name=self.inst_groupname)
+        self.ext_user.groups.add(group)
+        self.ext_user.save()
+
+        resp = self.client.post(
+            self.detail_url,
+            data={"role": "staff"},
+            HTTP_ACCEPT="application/json",
+        )
+        self.assertEqual(resp.status_code, 400)
+        result = json.loads(resp.content)
+        self.assertIn("error", result)
+        # reload user from DB
+        ext_user = User.objects.get(email=self.ext_user.email)
+        groups = [g.name for g in ext_user.groups.all()]
+        self.assertIn(self.inst_groupname, groups)

cms/djangoapps/contentstore/views/user.py

@@ -87,9 +87,15 @@ def manage_users(request, org, course, name):
 
     course_module = modulestore().get_item(location)
 
+    staff_groupname = get_course_groupname_for_role(location, "staff")
+    staff_group, __ = Group.objects.get_or_create(name=staff_groupname)
+    inst_groupname = get_course_groupname_for_role(location, "instructor")
+    inst_group, __ = Group.objects.get_or_create(name=inst_groupname)
+
     return render_to_response('manage_users.html', {
         'context_course': course_module,
-        'staff': get_users_in_course_group_by_role(location, STAFF_ROLE_NAME),
+        'staff': staff_group.user_set.all(),
+        'instructors': inst_group.user_set.all(),
         'allow_actions': has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME),
     })
 
@@ -137,8 +143,22 @@ def course_team_user(request, org, course, name, email):
         }
         return JsonResponse(msg, 400)
 
+    # make sure that the role groups exist
+    staff_groupname = get_course_groupname_for_role(location, "staff")
+    staff_group, __ = Group.objects.get_or_create(name=staff_groupname)
+    inst_groupname = get_course_groupname_for_role(location, "instructor")
+    inst_group, __ = Group.objects.get_or_create(name=inst_groupname)
+
     if request.method == "DELETE":
-        # remove all roles in this course from this user
+        # remove all roles in this course from this user: but fail if the user
+        # is the last instructor in the course team
+        instructors = set(inst_group.user_set.all())
+        if user in instructors and len(instructors) == 1:
+            msg = {
+                "error": _("You may not remove the last instructor from a course")
+            }
+            return JsonResponse(msg, 400)
+
         for role in roles:
             remove_user_from_course_group(request.user, user, location, role)
         return JsonResponse()
@@ -152,24 +172,26 @@ def course_team_user(request, org, course, name, email):
         try:
             role = payload["role"]
         except KeyError:
-            return JsonResponse({"error": "`role` is required"}, 400)
+            return JsonResponse({"error": _("`role` is required")}, 400)
     else:
         if not "role" in request.POST:
-            return JsonResponse({"error": "`role` is required"}, 400)
+            return JsonResponse({"error": _("`role` is required")}, 400)
         role = request.POST["role"]
 
-    # make sure that the role group exists
-    groupname = get_course_groupname_for_role(location, role)
-    Group.objects.get_or_create(name=groupname)
-
     if role == "instructor":
         add_user_to_course_group(request.user, user, location, role)
     elif role == "staff":
-        add_user_to_course_group(request.user, user, location, role)
-        # should *not* be an instructor
-        inst_groupname = get_course_groupname_for_role(location, "instructor")
-        if any(g.name == inst_groupname for g in user.groups.all()):
+        # if we're trying to downgrade a user from "instructor" to "staff",
+        # make sure we have at least one other instructor in the course team.
+        instructors = set(inst_group.user_set.all())
+        if user in instructors:
+            if len(instructors) == 1:
+                msg = {
+                    "error": _("You may not remove the last instructor from a course")
+                }
+                return JsonResponse(msg, 400)
             remove_user_from_course_group(request.user, user, location, "instructor")
+        add_user_to_course_group(request.user, user, location, role)
     return JsonResponse()
 
 

cms/templates/manage_users.html

@@ -39,9 +39,9 @@
       <form class="new-user-form">
         <div id="result"></div>
         <div class="form-elements">
-          <label>email: </label><input type="text" id="email" class="email-input" autocomplete="off" placeholder="email@example.com">
-          <input type="submit" value="Add User" id="add_user" class="add-button" />
-          <input type="button" value="Cancel" class="cancel-button" />
+          <label>${_("email: ")}</label><input type="text" id="email" class="email-input" autocomplete="off" placeholder="email@example.com">
+          <input type="submit" value="${_("Add User")}" id="add_user" class="add-button" />
+          <input type="button" value="${_("Cancel")}" class="cancel-button" />
         </div>
       </form>
       %endif
@@ -58,15 +58,13 @@
             <span class="user-email">${user.email}</span>
             % if allow_actions:
             <div class="item-actions">
-              % if request.user.id != user.id:
-                % if is_user_in_course_group_role(user, context_course.location, 'instructor', check_staff=False):
-                     <% admin_class = "remove-admin" %>
-                     <% admin_text = "Remove Admin" %>
-                % else:
-                     <% admin_class = "add-admin" %>
-                     <% admin_text = "Add Admin" %>
-                % endif
-              <a href="#" class="toggle-admin ${admin_class}">${admin_text}</a>
+              <% is_instuctor = is_user_in_course_group_role(user, context_course.location, 'instructor', check_staff=False) %>
+              % if is_instuctor and len(instructors) == 1:
+                <span class="admin-role">Admin</span>
+              % else:
+                <a href="#" class="admin-role toggle-admin-role ${'remove' if is_instuctor else 'add'}-admin-role">${_("Remove Admin") if is_instuctor else _("Add Admin")}</a>
+              % endif
+              % if request.user.id != user.id: ## can't remove yourself
               <a href="#" class="delete-button remove-user" data-id="${user.email}"><span class="delete-icon"></span></a>
               % endif
             </div>
@@ -146,10 +144,10 @@
       });
     });
 
-    $(".toggle-admin").click(function(e) {
+    $(".toggle-admin-role").click(function(e) {
       e.preventDefault()
       var type;
-      if($(this).hasClass("add-admin")) {
+      if($(this).hasClass("add-admin-role")) {
         role = 'instructor';
       } else {
         role = 'staff';