Skip to content

E
edx-platform
Commit d14e81cf by Christina Roberts
Options

Merge pull request #1467 from edx/christina/users 

Convert course team management to new URL scheme.
parents 1cec44e5 9a649492
Showing with 118 additions and 110 deletions
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
......@@ -6,9 +6,10 @@
<%!
from contentstore import utils
from django.utils.translation import ugettext as _
from xmodule.modulestore.django import loc_mapper
from django.core.urlresolvers import reverse
%>
<%block name="jsextra">
<link rel="stylesheet" type="text/css" href="${static.url('js/vendor/timepicker/jquery.timepicker.css')}" />
......@@ -284,13 +285,16 @@ require(["domReady!", "jquery", "js/models/settings/course_details", "js/views/s
<div class="bit">
% if context_course:
<% ctx_loc = context_course.location %>
<%! 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
......@@ -4,6 +4,8 @@
from django.core.urlresolvers import reverse
from django.utils.translation import ugettext as _
from contentstore import utils
from xmodule.modulestore.django import loc_mapper
from django.core.urlresolvers import reverse
%>
<%block name="title">${_("Advanced Settings")}</%block>
<%block name="bodyclass">is-signedin course advanced view-settings</%block>
......@@ -86,14 +88,17 @@ require(["domReady!", "jquery", "js/models/settings/advanced", "js/views/setting
<div class="bit">
% if context_course:
<% ctx_loc = context_course.location %>
<%! 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
......@@ -6,6 +6,8 @@
<%!
from contentstore import utils
from django.utils.translation import ugettext as _
from xmodule.modulestore.django import loc_mapper
from django.core.urlresolvers import reverse
%>
<%block name="header_extras">
......@@ -132,13 +134,16 @@ require(["domReady!", "jquery", "js/views/settings/grading", "js/models/settings
<div class="bit">
% if context_course:
<% ctx_loc = context_course.location %>
<%! 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')
)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment