Convert course team management to new URL scheme.

Move RESTful URLs into our "done" section.

CHANGELOG.rst

@@ -5,6 +5,10 @@ These are notable changes in edx-platform.  This is a rolling list of changes,
 in roughly chronological order, most recent first.  Add your entries at or near
 the top.  Include a label indicating the component affected.
 
+Studio: Change course overview page, checklists, and course staff management
+page URLs to a RESTful interface. Also removed "\listing", which duplicated
+"\index".
+
 Blades: When start time and end time are specified for a video, a visual range
 will be shown on the time slider to highlight the place in the video that will
 be played.

cms/djangoapps/contentstore/tests/test_checklists.py

@@ -42,7 +42,7 @@ class ChecklistTestCase(CourseTestCase):
         response = self.client.get(self.checklists_url)
         self.assertContains(response, "Getting Started With Studio")
         # Verify expansion of action URL happened.
-        self.assertContains(response, '/mitX/333/team/Checklists_Course')
+        self.assertContains(response, 'course_team/mitX.333.Checklists_Course')
         # Verify persisted checklist does NOT have expanded URL.
         checklist_0 = self.get_persisted_checklists()[0]
         self.assertEqual('ManageUsers', get_action_url(checklist_0, 0))
@@ -137,7 +137,7 @@ class ChecklistTestCase(CourseTestCase):
             # Verify no side effect in the original list.
             self.assertEqual(get_action_url(checklist, index), stored)
 
-        test_expansion(self.course.checklists[0], 0, 'ManageUsers', '/mitX/333/team/Checklists_Course')
+        test_expansion(self.course.checklists[0], 0, 'ManageUsers', '/course_team/mitX.333.Checklists_Course/branch/draft/block/Checklists_Course')
         test_expansion(self.course.checklists[1], 1, 'CourseOutline', '/course/mitX.333.Checklists_Course/branch/draft/block/Checklists_Course')
         test_expansion(self.course.checklists[2], 0, 'http://help.edge.edx.org/', 'http://help.edge.edx.org/')
 

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -1585,6 +1585,8 @@ class ContentStoreTest(ModuleStoreTestCase):
         """
         import_from_xml(modulestore('direct'), 'common/test/data/', ['simple'])
         loc = Location(['i4x', 'edX', 'simple', 'course', '2012_Fall', None])
+        new_location = loc_mapper().translate_location(loc.course_id, loc, False, True)
+
         resp = self._show_course_overview(loc)
         self.assertEqual(resp.status_code, 200)
         self.assertContains(resp, 'Chapter 2')
@@ -1605,11 +1607,9 @@ class ContentStoreTest(ModuleStoreTestCase):
                                                'name': loc.name}))
         self.assertEqual(resp.status_code, 200)
 
-        # manage users
-        resp = self.client.get(reverse('manage_users',
-                                       kwargs={'org': loc.org,
-                                               'course': loc.course,
-                                               'name': loc.name}))
+        # course team
+        url = new_location.url_reverse('course_team/', '')
+        resp = self.client.get(url, HTTP_ACCEPT='text/html')
         self.assertEqual(resp.status_code, 200)
 
         # course info

cms/djangoapps/contentstore/tests/test_course_index.py

@@ -73,12 +73,9 @@ class TestCourseIndex(CourseTestCase):
         """
         course_staff_client, course_staff = self.createNonStaffAuthedUserClient()
         for course in [self.course, self.odd_course]:
-            permission_url = reverse("course_team_user", kwargs={
-                "org": course.location.org,
-                "course": course.location.course,
-                "name": course.location.name,
-                "email": course_staff.email,
-            })
+            new_location = loc_mapper().translate_location(course.location.course_id, course.location, False, True)
+            permission_url = new_location.url_reverse("course_team/", course_staff.email)
+
             self.client.post(
                 permission_url,
                 data=json.dumps({"role": "staff"}),

cms/djangoapps/contentstore/tests/test_users.py

@@ -4,9 +4,9 @@ Tests for contentstore/views/user.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
 from student.models import CourseEnrollment
+from xmodule.modulestore.django import loc_mapper
 
 
 class UsersTestCase(CourseTestCase):
@@ -23,34 +23,17 @@ class UsersTestCase(CourseTestCase):
         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.location = loc_mapper().translate_location(self.course.location.course_id, self.course.location, False, True)
+
+        self.index_url = self.location.url_reverse('course_team', '')
+        self.detail_url = self.location.url_reverse('course_team', self.ext_user.email)
+        self.inactive_detail_url = self.location.url_reverse('course_team', self.inactive_user.email)
+        self.invalid_detail_url = self.location.url_reverse('course_team', "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)
+        resp = self.client.get(self.index_url, HTTP_ACCEPT='text/html')
         # ext_user is not currently a member of the course team, and so should
         # not show up on the page.
         self.assertNotContains(resp, self.ext_user.email)
@@ -60,7 +43,7 @@ class UsersTestCase(CourseTestCase):
         self.ext_user.groups.add(group)
         self.ext_user.save()
 
-        resp = self.client.get(self.index_url)
+        resp = self.client.get(self.index_url, HTTP_ACCEPT='text/html')
         self.assertContains(resp, self.ext_user.email)
 
     def test_detail(self):
@@ -261,12 +244,7 @@ class UsersTestCase(CourseTestCase):
         self.user.is_staff = False
         self.user.save()
 
-        self_url = reverse("course_team_user", kwargs={
-            "org": self.course.location.org,
-            "course": self.course.location.course,
-            "name": self.course.location.name,
-            "email": self.user.email,
-        })
+        self_url = self.location.url_reverse('course_team', self.user.email)
 
         resp = self.client.post(
             self_url,
@@ -298,12 +276,7 @@ class UsersTestCase(CourseTestCase):
         self.user.is_staff = False
         self.user.save()
 
-        self_url = reverse("course_team_user", kwargs={
-            "org": self.course.location.org,
-            "course": self.course.location.course,
-            "name": self.course.location.name,
-            "email": self.user.email,
-        })
+        self_url = self.location.url_reverse('course_team', self.user.email)
 
         resp = self.client.delete(self_url)
         self.assertEqual(resp.status_code, 204)

cms/djangoapps/contentstore/views/checklist.py

@@ -113,20 +113,25 @@ def expand_checklist_action_url(course_module, checklist):
     The method does a copy of the input checklist and does not modify the input argument.
     """
     expanded_checklist = copy.deepcopy(checklist)
-    urlconf_map = {
-        "ManageUsers": "manage_users",
+    oldurlconf_map = {
         "SettingsDetails": "settings_details",
         "SettingsGrading": "settings_grading"
     }
 
+    urlconf_map = {
+        "ManageUsers": "course_team",
+        "CourseOutline": "course"
+    }
+
     for item in expanded_checklist.get('items'):
         action_url = item.get('action_url')
-        if action_url == "CourseOutline":
+        if action_url in urlconf_map:
+            url_prefix = urlconf_map[action_url]
             ctx_loc = course_module.location
             location = loc_mapper().translate_location(ctx_loc.course_id, ctx_loc, False, True)
-            item['action_url'] = location.url_reverse('course/', '')
-        elif action_url in urlconf_map:
-            urlconf_name = urlconf_map[action_url]
+            item['action_url'] = location.url_reverse(url_prefix, '')
+        elif action_url in oldurlconf_map:
+            urlconf_name = oldurlconf_map[action_url]
             item['action_url'] = reverse(urlconf_name, kwargs={
                 'org': course_module.location.org,
                 'course': course_module.location.course,

cms/djangoapps/contentstore/views/user.py

@@ -12,7 +12,6 @@ from mitxmako.shortcuts import render_to_response
 from django.core.context_processors import csrf
 
 from xmodule.modulestore.django import modulestore, loc_mapper
-from xmodule.modulestore import Location
 from xmodule.error_module import ErrorDescriptor
 from contentstore.utils import get_lms_link_for_item
 from util.json_request import JsonResponse
@@ -25,6 +24,11 @@ from course_creators.views import (
 from .access import has_access
 
 from student.models import CourseEnrollment
+from xmodule.modulestore.locator import BlockUsageLocator
+from django.http import HttpResponseNotFound
+
+
+__all__ = ['index', 'request_course_creator', 'course_team_handler']
 
 
 @login_required
@@ -65,7 +69,7 @@ def index(request):
     return render_to_response('index.html', {
         'courses': [format_course_for_view(c) for c in courses if not isinstance(c, ErrorDescriptor)],
         'user': request.user,
-        'request_course_creator_url': reverse('request_course_creator'),
+        'request_course_creator_url': reverse('contentstore.views.request_course_creator'),
         'course_creator_status': _get_course_creator_status(request.user),
         'csrf': csrf(request)['csrf_token']
     })
@@ -83,16 +87,42 @@ def request_course_creator(request):
 
 @login_required
 @ensure_csrf_cookie
-def manage_users(request, org, course, name):
-    '''
+@require_http_methods(("GET", "POST", "PUT", "DELETE"))
+def course_team_handler(request, tag=None, course_id=None, branch=None, version_guid=None, block=None, email=None):
+    """
+    The restful handler for course team users.
+
+    GET
+        html: return html page for managing course team
+        json: return json representation of a particular course team member (email is required).
+    POST or PUT
+        json: modify the permissions for a particular course team member (email is required, as well as role in the payload).
+    DELETE:
+        json: remove a particular course team member from the course team (email is required).
+    """
+    location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block)
+    if not has_access(request.user, location):
+        raise PermissionDenied()
+
+    if 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'):
+        return _course_team_user(request, location, email)
+    elif request.method == 'GET':  # assume html
+        return _manage_users(request, location)
+    else:
+        return HttpResponseNotFound()
+
+
+def _manage_users(request, location):
+    """
     This view will return all CMS users who are editors for the specified course
-    '''
-    location = Location('i4x', org, course, 'course', name)
+    """
+    old_location = loc_mapper().translate_locator_to_location(location)
+
     # 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()
 
-    course_module = modulestore().get_item(location)
+    course_module = modulestore().get_item(old_location)
 
     staff_groupname = get_course_groupname_for_role(location, "staff")
     staff_group, __ = Group.objects.get_or_create(name=staff_groupname)
@@ -107,11 +137,8 @@ def manage_users(request, org, course, name):
     })
 
 
-@login_required
-@ensure_csrf_cookie
-@require_http_methods(("GET", "POST", "PUT", "DELETE"))
-def course_team_user(request, org, course, name, email):
-    location = Location('i4x', org, course, 'course', name)
+def _course_team_user(request, location, email):
+    old_location = loc_mapper().translate_locator_to_location(location)
     # check that logged in user has permissions to this item
     if has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME):
         # instructors have full permissions
@@ -146,7 +173,7 @@ def course_team_user(request, org, course, name, email):
         # 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)
+            role_groupname = get_course_groupname_for_role(old_location, role)
             if role_groupname in groupnames:
                 msg["role"] = role
                 break
@@ -162,7 +189,7 @@ def course_team_user(request, org, course, name, email):
     # make sure that the role groups exist
     groups = {}
     for role in roles:
-        groupname = get_course_groupname_for_role(location, role)
+        groupname = get_course_groupname_for_role(old_location, role)
         group, __ = Group.objects.get_or_create(name=groupname)
         groups[role] = group
 
@@ -208,7 +235,7 @@ def course_team_user(request, org, course, name, email):
         user.groups.add(groups["instructor"])
         user.save()
         # auto-enroll the course creator in the course so that "View Live" will work.
-        CourseEnrollment.enroll(user, location.course_id)
+        CourseEnrollment.enroll(user, old_location.course_id)
     elif role == "staff":
         # 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.
@@ -223,7 +250,7 @@ def course_team_user(request, org, course, name, email):
         user.groups.add(groups["staff"])
         user.save()
         # auto-enroll the course creator in the course so that "View Live" will work.
-        CourseEnrollment.enroll(user, location.course_id)
+        CourseEnrollment.enroll(user, old_location.course_id)
 
     return JsonResponse()
 

cms/templates/manage_users.html

@@ -2,12 +2,13 @@
 <%! from django.core.urlresolvers import reverse %>
 <%! from auth.authz import is_user_in_course_group_role %>
 <%! import json %>
+<%! from xmodule.modulestore.django import loc_mapper %>
 <%inherit file="base.html" />
 <%block name="title">${_("Course Team Settings")}</%block>
 <%block name="bodyclass">is-signedin course users view-team</%block>
 
-
 <%block name="content">
+
 <div class="wrapper-mast wrapper">
   <header class="mast has-actions has-subtitle">
     <h1 class="page-header">
@@ -59,15 +60,10 @@
       %endif
 
       <ol class="user-list">
+        <% new_location = loc_mapper().translate_location(context_course.location.course_id, context_course.location, False, True) %>
         % for user in staff:
-          <% api_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,
-             ))
-          %>
-        <li class="user-item" data-email="${user.email}" data-url="${api_url}">
+
+        <li class="user-item" data-email="${user.email}" data-url="${new_location.url_reverse('course_team/', user.email) }">
 
         <% is_instuctor = is_user_in_course_group_role(user, context_course.location, 'instructor', check_staff=False) %>
           % if is_instuctor:
@@ -166,13 +162,11 @@ require(["jquery", "underscore", "gettext", "js/views/feedback_prompt"],
     function($, _, gettext, PromptView) {
 
   var staffEmails = ${json.dumps([user.email for user in staff])};
-  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@@",
-    ))}";
-  var unknownErrorMessage = gettext("Unknown")
+  var tplUserURL = "${loc_mapper().\
+      translate_location(context_course.location.course_id, context_course.location, False, True).\
+      url_reverse('course_team/', "@@EMAIL@@")}";
+
+  var unknownErrorMessage = gettext("Unknown");
 
   $(document).ready(function() {
     var $createUserForm = $('#create-user-form');

cms/templates/settings.html

@@ -284,13 +284,18 @@ require(["domReady!", "jquery", "js/models/settings/course_details", "js/views/s
 
      <div class="bit">
      % if context_course:
-      <% ctx_loc = context_course.location %>
+          <%! from xmodule.modulestore.django import loc_mapper %>
           <%! from django.core.urlresolvers import reverse %>
+          <%
+            ctx_loc = context_course.location
+            location = loc_mapper().translate_location(ctx_loc.course_id, ctx_loc, False, True)
+            course_team_url = location.url_reverse('course_team/', '')
+          %>
         <h3 class="title-3">${_("Other Course Settings")}</h3>
         <nav class="nav-related">
           <ul>
             <li class="nav-item"><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"><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"><a href="${course_team_url}">${_("Course Team")}</a></li>
             <li class="nav-item"><a href="${reverse('course_advanced_settings', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">${_("Advanced Settings")}</a></li>
           </ul>
         </nav>

cms/templates/settings_advanced.html

@@ -86,14 +86,19 @@ require(["domReady!", "jquery", "js/models/settings/advanced", "js/views/setting
 
       <div class="bit">
       % if context_course:
-      <% ctx_loc = context_course.location %>
+        <%! from xmodule.modulestore.django import loc_mapper %>
         <%! from django.core.urlresolvers import reverse %>
+        <%
+          ctx_loc = context_course.location
+          location = loc_mapper().translate_location(ctx_loc.course_id, ctx_loc, False, True)
+          course_team_url = location.url_reverse('course_team/', '')
+        %>
       <h3 class="title-3">${_("Other Course Settings")}</h3>
         <nav class="nav-related">
           <ul>
             <li class="nav-item"><a href="${reverse('contentstore.views.get_course_settings', kwargs=dict(org=ctx_loc.org, course=ctx_loc.course, name=ctx_loc.name))}">${_("Details &amp; Schedule")}</a></li>
             <li class="nav-item"><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"><a href="${reverse('manage_users', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">${_("Course Team")}</a></li>
+            <li class="nav-item"><a href="${course_team_url}">${_("Course Team")}</a></li>
           </ul>
         </nav>
       % endif

cms/templates/settings_graders.html

@@ -132,13 +132,18 @@ require(["domReady!", "jquery", "js/views/settings/grading", "js/models/settings
 
       <div class="bit">
       % if context_course:
-      <% ctx_loc = context_course.location %>
+        <%! from xmodule.modulestore.django import loc_mapper %>
         <%! from django.core.urlresolvers import reverse %>
+        <%
+          ctx_loc = context_course.location
+          location = loc_mapper().translate_location(ctx_loc.course_id, ctx_loc, False, True)
+          course_team_url = location.url_reverse('course_team/', '')
+        %>
         <h3 class="title-3">${_("Other Course Settings")}</h3>
         <nav class="nav-related">
           <ul>
             <li class="nav-item"><a href="${reverse('contentstore.views.get_course_settings', kwargs=dict(org=ctx_loc.org, course=ctx_loc.course, name=ctx_loc.name))}">${_("Details &amp; Schedule")}</a></li>
-            <li class="nav-item"><a href="${reverse('manage_users', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">${_("Course Team")}</a></li>
+            <li class="nav-item"><a href="${course_team_url}">${_("Course Team")}</a></li>
             <li class="nav-item"><a href="${reverse('course_advanced_settings', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">${_("Advanced Settings")}</a></li>
           </ul>
         </nav>

cms/templates/widgets/header.html

@@ -18,6 +18,7 @@
             location = loc_mapper().translate_location(ctx_loc.course_id, ctx_loc, False, True)
             index_url = location.url_reverse('course/', '')
             checklists_url = location.url_reverse('checklists/', '')
+            course_team_url = location.url_reverse('course_team/', '')
       %>
       <h2 class="info-course">
         <span class="sr">${_("Current Course:")}</span>
@@ -69,7 +70,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(org=ctx_loc.org, course=ctx_loc.course, name=ctx_loc.name))}">${_("Course Team")}</a>
+                    <a href="${course_team_url}">${_("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

@@ -13,8 +13,6 @@ admin.autodiscover()
 
 urlpatterns = patterns('',  # nopep8
     url(r'^$', 'contentstore.views.howitworks', name='homepage'),
-    url(r'^listing', 'contentstore.views.index', name='index'),
-    url(r'^request_course_creator$', 'contentstore.views.request_course_creator', name='request_course_creator'),
     url(r'^edit/(?P<location>.*?)$', 'contentstore.views.edit_unit', name='edit_unit'),
     url(r'^subsection/(?P<location>.*?)$', 'contentstore.views.edit_subsection', name='edit_subsection'),
     url(r'^preview_component/(?P<location>.*?)$', 'contentstore.views.preview_component', name='preview_component'),
@@ -51,12 +49,6 @@ urlpatterns = patterns('',  # 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'^(?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>.*)$',
@@ -130,17 +122,18 @@ urlpatterns += patterns(
     url(r'^login_post$', 'student.views.login_user', name='login_post'),
 
     url(r'^logout$', 'student.views.logout_user', name='logout'),
-
 )
 
 # restful api
 urlpatterns += patterns(
     'contentstore.views',
 
-    url(r'^course$', 'index'),
+    url(r'^course$', 'index', name='index'),
+    url(r'^request_course_creator$', 'request_course_creator'),
     # (?ix) == ignore case and verbose (multiline regex)
     url(r'(?ix)^course/{}$'.format(parsers.URL_RE_SOURCE), 'course_handler'),
     url(r'(?ix)^checklists/{}(/)?(?P<checklist_index>\d+)?$'.format(parsers.URL_RE_SOURCE), 'checklists_handler'),
+    url(r'(?ix)^course_team/{}(/)?(?P<email>.+)?$'.format(parsers.URL_RE_SOURCE), 'course_team_handler'),
     url(r'(?ix)^orphan/{}$'.format(parsers.URL_RE_SOURCE), 'orphan')
 )