Added unit tests for new course team API

cms/djangoapps/contentstore/tests/test_users.py

+import json
+from .utils import CourseTestCase
+from django.contrib.auth.models import User, Group
+from django.core.urlresolvers import reverse
+from auth.authz import get_course_groupname_for_role
+
+
+class UsersTestCase(CourseTestCase):
+    def setUp(self):
+        super(UsersTestCase, self).setUp()
+        self.ext_user = User.objects.create_user(
+            "joe", "joe@comedycentral.com", "haha")
+        self.ext_user.is_active = True
+        self.ext_user.is_staff = False
+        self.ext_user.save()
+        self.inactive_user = User.objects.create_user(
+            "carl", "carl@comedycentral.com", "haha")
+        self.inactive_user.is_active = False
+        self.inactive_user.is_staff = False
+        self.inactive_user.save()
+
+        self.index_url = reverse("manage_users", kwargs={
+            "org": self.course.location.org,
+            "course": self.course.location.course,
+            "name": self.course.location.name,
+        })
+        self.detail_url = reverse("course_team_user", kwargs={
+            "org": self.course.location.org,
+            "course": self.course.location.course,
+            "name": self.course.location.name,
+            "email": self.ext_user.email,
+        })
+        self.inactive_detail_url = reverse("course_team_user", kwargs={
+            "org": self.course.location.org,
+            "course": self.course.location.course,
+            "name": self.course.location.name,
+            "email": self.inactive_user.email,
+        })
+        self.invalid_detail_url = reverse("course_team_user", kwargs={
+            "org": self.course.location.org,
+            "course": self.course.location.course,
+            "name": self.course.location.name,
+            "email": "nonexistent@user.com",
+        })
+        self.staff_groupname = get_course_groupname_for_role(self.course.location, "staff")
+        self.inst_groupname = get_course_groupname_for_role(self.course.location, "instructor")
+
+    def test_index(self):
+        resp = self.client.get(self.index_url)
+        self.assertNotContains(resp, self.ext_user.email)
+
+    def test_detail(self):
+        resp = self.client.get(self.detail_url)
+        self.assertEqual(resp.status_code, 200)
+        result = json.loads(resp.content)
+        self.assertEqual(result["role"], None)
+        self.assertTrue(result["active"])
+
+    def test_detail_inactive(self):
+        resp = self.client.get(self.inactive_detail_url)
+        self.assert2XX(resp.status_code)
+        result = json.loads(resp.content)
+        self.assertFalse(result["active"])
+
+    def test_detail_invalid(self):
+        resp = self.client.get(self.invalid_detail_url)
+        self.assert4XX(resp.status_code)
+        result = json.loads(resp.content)
+        self.assertIn("error", result)
+
+    def test_detail_post(self):
+        resp = self.client.post(
+            self.detail_url,
+            data={"role": None},
+        )
+        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()]
+        # no content: should not be in any roles
+        self.assertNotIn(self.staff_groupname, groups)
+        self.assertNotIn(self.inst_groupname, groups)
+
+    def test_detail_post_staff(self):
+        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)
+
+    def test_detail_post_instructor(self):
+        resp = self.client.post(
+            self.detail_url,
+            data=json.dumps({"role": "instructor"}),
+            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.assertNotIn(self.staff_groupname, groups)
+        self.assertIn(self.inst_groupname, groups)
+
+    def test_detail_post_missing_role(self):
+        resp = self.client.post(
+            self.detail_url,
+            data=json.dumps({"toys": "fun"}),
+            content_type="application/json",
+            HTTP_ACCEPT="application/json",
+        )
+        self.assert4XX(resp.status_code)
+        result = json.loads(resp.content)
+        self.assertIn("error", result)
+
+    def test_detail_post_bad_json(self):
+        resp = self.client.post(
+            self.detail_url,
+            data="{foo}",
+            content_type="application/json",
+            HTTP_ACCEPT="application/json",
+        )
+        self.assert4XX(resp.status_code)
+        result = json.loads(resp.content)
+        self.assertIn("error", result)
+
+    def test_detail_post_no_json(self):
+        resp = self.client.post(
+            self.detail_url,
+            data={"role": "staff"},
+            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)
+
+    def test_detail_delete_staff(self):
+        group, _ = Group.objects.get_or_create(name=self.staff_groupname)
+        self.ext_user.groups.add(group)
+        self.ext_user.save()
+
+        resp = self.client.delete(
+            self.detail_url,
+            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.assertNotIn(self.staff_groupname, groups)
+
+    def test_detail_delete_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.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.assertNotIn(self.inst_groupname, groups)

cms/djangoapps/contentstore/views/user.py

@@ -111,20 +111,23 @@ def course_team_user(request, org, course, name, email):
         }
         return JsonResponse(msg, 404)
 
+    # role hierarchy: "instructor" has more permissions than "staff" (in a course)
+    roles = ["instructor", "staff"]
+
     if request.method == "GET":
         # just return info about the user
-        roles = set()
-        for group in user.groups.all():
-            if not "_" in group.name:
-                continue
-            role, coursename = group.name.split("_", 1)
-            if coursename in (location.course, location.course_id):
-                roles.add(role)
         msg = {
             "email": user.email,
             "active": user.is_active,
-            "roles": list(roles),
+            "role": None,
         }
+        # what's the highest role that this user has?
+        groupnames = set(g.name for g in user.groups.all())
+        for role in roles:
+            role_groupname = get_course_groupname_for_role(location, role)
+            if role_groupname in groupnames:
+                msg["role"] = role
+                break
         return JsonResponse(msg)
 
     # can't modify an inactive user
@@ -134,11 +137,14 @@ def course_team_user(request, org, course, name, email):
         }
         return JsonResponse(msg, 400)
 
-    # all other operations require the requesting user to specify a role --
-    # or if no role is specified, default to "staff"
-    if not request.body:
-        role = STAFF_ROLE_NAME
-    else:
+    if request.method == "DELETE":
+        # remove all roles in this course from this user
+        for role in roles:
+            remove_user_from_course_group(request.user, user, location, role)
+        return JsonResponse()
+
+    # all other operations require the requesting user to specify a role
+    if request.META.get("CONTENT_TYPE", "") == "application/json" and request.body:
         try:
             payload = json.loads(request.body)
         except:
@@ -147,15 +153,21 @@ def course_team_user(request, org, course, name, email):
             role = payload["role"]
         except KeyError:
             return JsonResponse({"error": "`role` is required"}, 400)
+    else:
+        if not "role" in request.POST:
+            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 = Group.objects.get_or_create(name=groupname)
+    Group.objects.get_or_create(name=groupname)
 
-    if request.method in ("POST", "PUT"):
+    if role == "instructor":
         add_user_to_course_group(request.user, user, location, role)
-        return JsonResponse()
-    elif request.method == "DELETE":
-        remove_user_from_course_group(request.user, user, location, role)
-        return JsonResponse()
+    elif role == "staff":
+        add_user_to_course_group(request.user, user, location, role)
+        remove_user_from_course_group(request.user, user, location, "instructor")
+    return JsonResponse()
 
 
 def _get_course_creator_status(user):

cms/templates/manage_users.html

@@ -140,9 +140,6 @@
         type: 'DELETE',
         dataType: 'json',
         contentType: 'application/json',
-        data: JSON.stringify({
-          role: 'staff',
-        }),
         complete: function() {
           location.reload();
         }
@@ -153,18 +150,18 @@
       e.preventDefault()
       var type;
       if($(this).hasClass("add-admin")) {
-        type = 'POST';
+        role = 'instructor';
       } else {
-        type = 'DELETE';
+        role = 'staff';
       }
       var url = $(this).closest("li").data("url");
       $.ajax({
         url: url,
-        type: type,
+        type: 'POST',
         dataType: 'json',
         contentType: 'application/json',
         data: JSON.stringify({
-          role: 'instructor',
+          role: role
         }),
         complete: function() {
           location.reload();