Reorganize URLs and views around course team

Match other views better, saner URLs, more RESTful style, extensible for other roles

cms/djangoapps/contentstore/views/user.py

+import json
 from django.conf import settings
 from django.core.exceptions import PermissionDenied
-from django.core.urlresolvers import reverse
+from django.contrib.auth.models import User
 from django.contrib.auth.decorators import login_required
+from django.views.decorators.http import require_http_methods
 from django.utils.translation import ugettext as _
 from django.views.decorators.http import require_POST
 from django_future.csrf import ensure_csrf_cookie
@@ -10,9 +12,9 @@ from django.core.context_processors import csrf
 
 from xmodule.modulestore.django import modulestore
 from contentstore.utils import get_url_reverse, get_lms_link_for_item
-from util.json_request import expect_json, JsonResponse
+from util.json_request import JsonResponse
 from auth.authz import STAFF_ROLE_NAME, INSTRUCTOR_ROLE_NAME, get_users_in_course_group_by_role
-from auth.authz import get_user_by_email, add_user_to_course_group, remove_user_from_course_group
+from auth.authz import add_user_to_course_group, remove_user_from_course_group
 from course_creators.views import get_course_creator_status, add_user_with_status_unrequested, user_requested_access
 
 from .access import has_access
@@ -60,10 +62,11 @@ def request_course_creator(request):
 
 @login_required
 @ensure_csrf_cookie
-def manage_users(request, location):
+def manage_users(request, org, course, name):
     '''
     This view will return all CMS users who are editors for the specified course
     '''
+    location = Location('i4x', org, course, 'course', name)
     # check that logged in user has permissions to this item
     if not has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME) and not has_access(request.user, location, role=STAFF_ROLE_NAME):
         raise PermissionDenied()
@@ -73,91 +76,72 @@ def manage_users(request, location):
     return render_to_response('manage_users.html', {
         'context_course': course_module,
         'staff': get_users_in_course_group_by_role(location, STAFF_ROLE_NAME),
-        'add_user_postback_url': reverse('add_user', args=[location]).rstrip('/'),
-        'remove_user_postback_url': reverse('remove_user', args=[location]).rstrip('/'),
         'allow_actions': has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME),
-        'request_user_id': request.user.id
     })
 
 
-@expect_json
 @login_required
 @ensure_csrf_cookie
-def add_user(request, location):
-    '''
-    This POST-back view will add a user - specified by email - to the list of editors for
-    the specified course
-    '''
-    email = request.POST.get("email")
-
-    if not email:
-        msg = {
-            'Status': 'Failed',
-            'ErrMsg': _('Please specify an email address.'),
-        }
-        return JsonResponse(msg, 400)
-
-    # remove leading/trailing whitespace if necessary
-    email = email.strip()
-
-    # check that logged in user has admin permissions to this course
-    if not has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME):
+@require_http_methods(("GET", "POST", "PUT", "DELETE"))
+def course_team_user(request, org, course, name, email):
+    location = Location('i4x', org, course, 'course', name)
+    # check that logged in user has permissions to this item
+    if not has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME) and not has_access(request.user, location, role=STAFF_ROLE_NAME):
         raise PermissionDenied()
 
-    user = get_user_by_email(email)
-
-    # user doesn't exist?!? Return error.
-    if user is None:
+    try:
+        user = User.objects.get(email=email)
+    except:
         msg = {
-            'Status': 'Failed',
-            'ErrMsg': _("Could not find user by email address '{email}'.").format(email=email),
+            "error": _("Could not find user by email address '{email}'.").format(email=email),
         }
         return JsonResponse(msg, 404)
 
-    # user exists, but hasn't activated account?!?
-    if not user.is_active:
+    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 = {
-            'Status': 'Failed',
-            'ErrMsg': _('User {email} has registered but has not yet activated his/her account.').format(email=email),
+            "email": user.email,
+            "active": user.is_active,
+            "roles": list(roles),
         }
-        return JsonResponse(msg, 400)
-
-    # ok, we're cool to add to the course group
-    add_user_to_course_group(request.user, user, location, STAFF_ROLE_NAME)
-
-    return JsonResponse({"Status": "OK"})
-
+        return JsonResponse(msg)
 
-@expect_json
-@login_required
-@ensure_csrf_cookie
-def remove_user(request, location):
-    '''
-    This POST-back view will remove a user - specified by email - from the list of editors for
-    the specified course
-    '''
-
-    email = request.POST["email"]
-
-    # check that logged in user has admin permissions on this course
-    if not has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME):
-        raise PermissionDenied()
-
-    user = get_user_by_email(email)
-    if user is None:
+    # can't modify an inactive user
+    if not user.is_active:
         msg = {
-            'Status': 'Failed',
-            'ErrMsg': _("Could not find user by email address '{email}'.").format(email=email),
+            "error": _('User {email} has registered but has not yet activated his/her account.').format(email=email),
         }
-        return JsonResponse(msg, 404)
-
-    # make sure we're not removing ourselves
-    if user.id == request.user.id:
-        raise PermissionDenied()
-
-    remove_user_from_course_group(request.user, user, location, STAFF_ROLE_NAME)
+        return JsonResponse(msg, 400)
 
-    return JsonResponse({"Status": "OK"})
+    # all other operations require the requesting user to specify a role --
+    # or if no role is specified, default to "staff"
+    if "role" in request.POST:
+        role = request.POST["role"]
+    elif request.body:
+        try:
+            payload = json.loads(request.body)
+        except:
+            return JsonResponse({"error": _("malformed JSON")}, 400)
+        try:
+            role = payload["role"]
+        except KeyError:
+            return JsonResponse({"error": "`role` is required"}, 400)
+    else:
+        role = STAFF_ROLE_NAME
+
+    if request.method in ("POST", "PUT"):
+        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()
 
 
 def _get_course_creator_status(user):

cms/templates/manage_users.html

 <%! from django.utils.translation import ugettext as _ %>
+<%! from django.core.urlresolvers import reverse %>
 <%inherit file="base.html" />
 <%block name="title">${_("Course Team Settings")}</%block>
 <%block name="bodyclass">is-signedin course users settings team</%block>
@@ -46,12 +47,17 @@
       <div>
         <ol class="user-list">
           % for user in staff:
-          <li>
+          <li data-url="${reverse('course_team_user', kwargs=dict(
+              org=context_course.location.org,
+              course=context_course.location.course,
+              name=context_course.location.name,
+              email=user.email,
+            ))}">
             <span class="user-name">${user.username}</span>
             <span class="user-email">${user.email}</span>
             %if allow_actions :
             <div class="item-actions">
-              %if request_user_id != user.id:
+              %if request.user.id != user.id:
               <a href="#" class="delete-button remove-user" data-id="${user.email}"><span class="delete-icon"></span></a>
               %endif
             </div>
@@ -67,20 +73,25 @@
 
 <%block name="jsextra">
 <script type="text/javascript">
-  var addUserPostbackUrl = "${add_user_postback_url}";
-  var removeUserPostbackUrl = "${remove_user_postback_url}";
+  var tplUserURL = "${reverse('course_team_user', kwargs=dict(
+      org=context_course.location.org,
+      course=context_course.location.course,
+      name=context_course.location.name,
+      email="@@EMAIL@@",
+    ))}"
 
   $(document).ready(function() {
     var $newUserForm = $('.new-user-form');
     $newUserForm.bind('submit', function(e) {
       e.preventDefault();
+      var url = tplUserURL.replace("@@EMAIL@@", $('#email').val())
       $.ajax({
-        url: addUserPostbackUrl,
+        url: url,
         type: 'POST',
         dataType: 'json',
         contentType: 'application/json',
         data: {
-          email: $('#email').val()
+          role: 'staff',
         },
         success: function(data) {
           location.reload();
@@ -88,7 +99,7 @@
         notifyOnError: false,
         error: function(jqXHR, textStatus, errorThrown) {
           data = JSON.parse(jqXHR.responseText);
-          $('#result').show().empty().append(data.ErrMsg);
+          $('#result').show().empty().append(data.error);
         }
       });
     });
@@ -114,13 +125,14 @@
     });
 
     $('.remove-user').click(function() {
+      var url = tplUserURL.replace("@@EMAIL@@", $(this).data('id'))
       $.ajax({
-        url: removeUserPostbackUrl,
-        type: 'POST',
+        url: url,
+        type: 'DELETE',
         dataType: 'json',
         contentType: 'application/json',
         data: {
-          email: $(this).data('id')
+          role: 'staff',
         },
         complete: function() {
           location.reload();

cms/templates/widgets/header.html

@@ -60,7 +60,7 @@
                     <a href="${reverse('contentstore.views.course_config_graders_page', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">${_("Grading")}</a>
                   </li>
                   <li class="nav-item nav-course-settings-team">
-                    <a href="${reverse('manage_users', kwargs=dict(location=ctx_loc))}">${_("Course Team")}</a>
+                    <a href="${reverse('manage_users', kwargs=dict(org=ctx_loc.org, course=ctx_loc.course, name=ctx_loc.name))}">${_("Course Team")}</a>
                   </li>
                   <li class="nav-item nav-course-settings-advanced">
                     <a href="${reverse('course_advanced_settings', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">${_("Advanced Settings")}</a>

cms/urls.py

@@ -40,14 +40,12 @@ urlpatterns = ('',  # nopep8
     url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/course/(?P<coursename>[^/]+)/upload_asset$',
         'contentstore.views.upload_asset', name='upload_asset'),
 
+    url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/team/(?P<name>[^/]+)$',
+        'contentstore.views.manage_users', name='manage_users'),
+    url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/team/(?P<name>[^/]+)/(?P<email>[^/]+)$',
+        'contentstore.views.course_team_user', name='course_team_user'),
+
 
-    url(r'^manage_users/(?P<location>.*?)$', 'contentstore.views.manage_users', name='manage_users'),
-    url(r'^add_user/(?P<location>.*?)$',
-        'contentstore.views.add_user', name='add_user'),
-    url(r'^remove_user/(?P<location>.*?)$',
-        'contentstore.views.remove_user', name='remove_user'),
-    url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/course/(?P<name>[^/]+)/remove_user$',
-        'contentstore.views.remove_user', name='remove_user'),
     url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/info/(?P<name>[^/]+)$',
         'contentstore.views.course_info', name='course_info'),
     url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/course_info/updates/(?P<provided_id>.*)$',