From f438552b3b9dd13ad92ffec34971a7963915a91a Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Tue, 23 Jul 2013 15:27:33 -0400 Subject: [PATCH] Added unit tests for new course team API --- .../contentstore/tests/test_users.py | 175 ++++++++++++++++++ cms/djangoapps/contentstore/views/user.py | 52 ++++-- cms/templates/manage_users.html | 11 +- 3 files changed, 211 insertions(+), 27 deletions(-) create mode 100644 cms/djangoapps/contentstore/tests/test_users.py diff --git a/cms/djangoapps/contentstore/tests/test_users.py b/cms/djangoapps/contentstore/tests/test_users.py new file mode 100644 index 0000000000..82f511d6ab --- /dev/null +++ b/cms/djangoapps/contentstore/tests/test_users.py @@ -0,0 +1,175 @@ +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) diff --git a/cms/djangoapps/contentstore/views/user.py b/cms/djangoapps/contentstore/views/user.py index c5d642e207..2b2f170617 100644 --- a/cms/djangoapps/contentstore/views/user.py +++ b/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) - groupname = get_course_groupname_for_role(location, role) - group = Group.objects.get_or_create(name=groupname) + else: + if not "role" in request.POST: + return JsonResponse({"error": "`role` is required"}, 400) + role = request.POST["role"] - if request.method in ("POST", "PUT"): + # 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) - 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): diff --git a/cms/templates/manage_users.html b/cms/templates/manage_users.html index 69430cbbea..9a468664b5 100644 --- a/cms/templates/manage_users.html +++ b/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();