diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 43968d6716..9b3452eb50 100644 --- a/CHANGELOG.rst +++ b/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. diff --git a/cms/djangoapps/contentstore/tests/test_checklists.py b/cms/djangoapps/contentstore/tests/test_checklists.py index d7c9e78f62..4667e6e138 100644 --- a/cms/djangoapps/contentstore/tests/test_checklists.py +++ b/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/') diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index bf006e9c9d..91b9049584 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/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 diff --git a/cms/djangoapps/contentstore/tests/test_course_index.py b/cms/djangoapps/contentstore/tests/test_course_index.py index 42ca510c1f..44a115518c 100644 --- a/cms/djangoapps/contentstore/tests/test_course_index.py +++ b/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"}), diff --git a/cms/djangoapps/contentstore/tests/test_users.py b/cms/djangoapps/contentstore/tests/test_users.py index 80b2364c43..c10dec61bd 100644 --- a/cms/djangoapps/contentstore/tests/test_users.py +++ b/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) diff --git a/cms/djangoapps/contentstore/views/checklist.py b/cms/djangoapps/contentstore/views/checklist.py index 2f4d798a81..5643e5c044 100644 --- a/cms/djangoapps/contentstore/views/checklist.py +++ b/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, diff --git a/cms/djangoapps/contentstore/views/user.py b/cms/djangoapps/contentstore/views/user.py index 1d6804b564..67d26f1e5e 100644 --- a/cms/djangoapps/contentstore/views/user.py +++ b/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() diff --git a/cms/templates/manage_users.html b/cms/templates/manage_users.html index da4d255ecd..cfe715150e 100644 --- a/cms/templates/manage_users.html +++ b/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 name="bodyclass">is-signedin course users view-team - <%block name="content"> +

@@ -59,15 +60,10 @@ %endif
    + <% 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, - )) - %> -
  1. + +
  2. <% 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'); diff --git a/cms/templates/settings.html b/cms/templates/settings.html index f3eec1e195..10b9055979 100644 --- a/cms/templates/settings.html +++ b/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"> @@ -280,22 +281,25 @@ require(["domReady!", "jquery", "js/models/settings/course_details", "js/views/s

    ${_("Your course's schedule settings determine when students can enroll in and begin a course.")}

    ${_("Additionally, details provided on this page are also used in edX's catalog of courses, which new and returning students use to choose new courses to study.")}

    -

+ -
- % if context_course: - <% ctx_loc = context_course.location %> - <%! from django.core.urlresolvers import reverse %> +
+ % if context_course: + <% + 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/', '') + %>

${_("Other Course Settings")}

- % endif -
+ % endif +
diff --git a/cms/templates/settings_advanced.html b/cms/templates/settings_advanced.html index 6be114d714..2af3e43689 100644 --- a/cms/templates/settings_advanced.html +++ b/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 name="bodyclass">is-signedin course advanced view-settings @@ -86,14 +88,17 @@ require(["domReady!", "jquery", "js/models/settings/advanced", "js/views/setting
% 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/', '') + %>

${_("Other Course Settings")}

% endif diff --git a/cms/templates/settings_graders.html b/cms/templates/settings_graders.html index b14e3f71a4..8e43567491 100644 --- a/cms/templates/settings_graders.html +++ b/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
% 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/', '') + %>

${_("Other Course Settings")}

diff --git a/cms/templates/widgets/header.html b/cms/templates/widgets/header.html index 7c602eca3f..8b22ad4812 100644 --- a/cms/templates/widgets/header.html +++ b/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/', '') %>

${_("Current Course:")} @@ -69,7 +70,7 @@ ${_("Grading")}
  • - ${_("Course Team")} + ${_("Course Team")}
  • ${_("Advanced Settings")} diff --git a/cms/urls.py b/cms/urls.py index a9228eab3b..f40cc363f4 100644 --- a/cms/urls.py +++ b/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.*?)$', 'contentstore.views.edit_unit', name='edit_unit'), url(r'^subsection/(?P.*?)$', 'contentstore.views.edit_subsection', name='edit_subsection'), url(r'^preview_component/(?P.*?)$', 'contentstore.views.preview_component', name='preview_component'), @@ -51,12 +49,6 @@ urlpatterns = patterns('', # nopep8 url(r'^(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)/upload_asset$', 'contentstore.views.upload_asset', name='upload_asset'), - url(r'^(?P[^/]+)/(?P[^/]+)/team/(?P[^/]+)$', - 'contentstore.views.manage_users', name='manage_users'), - url(r'^(?P[^/]+)/(?P[^/]+)/team/(?P[^/]+)/(?P[^/]+)$', - 'contentstore.views.course_team_user', name='course_team_user'), - - url(r'^(?P[^/]+)/(?P[^/]+)/info/(?P[^/]+)$', 'contentstore.views.course_info', name='course_info'), url(r'^(?P[^/]+)/(?P[^/]+)/course_info/updates/(?P.*)$', @@ -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\d+)?$'.format(parsers.URL_RE_SOURCE), 'checklists_handler'), + url(r'(?ix)^course_team/{}(/)?(?P.+)?$'.format(parsers.URL_RE_SOURCE), 'course_team_handler'), url(r'(?ix)^orphan/{}$'.format(parsers.URL_RE_SOURCE), 'orphan') )