diff --git a/common/djangoapps/contentserver/middleware.py b/common/djangoapps/contentserver/middleware.py
index b9c14cd537aa44b4cfe75e6eb6be6f6bed35b88a..056a1ce45983b46b142b5903c9c1aade11c16c2d 100644
--- a/common/djangoapps/contentserver/middleware.py
+++ b/common/djangoapps/contentserver/middleware.py
@@ -4,10 +4,12 @@ from student.models import CourseEnrollment
from xmodule.contentstore.django import contentstore
from xmodule.contentstore.content import StaticContent, XASSET_LOCATION_TAG
-from xmodule.modulestore import InvalidLocationError
+from xmodule.modulestore import InvalidLocationError, InvalidKeyError
from cache_toolbox.core import get_cached_content, set_cached_content
from xmodule.exceptions import NotFoundError
+# TODO: Soon as we have a reasonable way to serialize/deserialize AssetKeys, we need
+# to change this file so instead of using course_id_partial, we're just using asset keys
class StaticContentServer(object):
def process_request(self, request):
@@ -15,7 +17,7 @@ class StaticContentServer(object):
if request.path.startswith('/' + XASSET_LOCATION_TAG + '/'):
try:
loc = StaticContent.get_location_from_path(request.path)
- except InvalidLocationError:
+ except (InvalidLocationError, InvalidKeyError):
# return a 'Bad Request' to browser as we have a malformed Location
response = HttpResponse()
response.status_code = 400
@@ -47,9 +49,9 @@ class StaticContentServer(object):
if getattr(content, "locked", False):
if not hasattr(request, "user") or not request.user.is_authenticated():
return HttpResponseForbidden('Unauthorized')
- course_partial_id = "/".join([loc.org, loc.course])
if not request.user.is_staff and not CourseEnrollment.is_enrolled_by_partial(
- request.user, course_partial_id):
+ request.user, loc.course_key
+ ):
return HttpResponseForbidden('Unauthorized')
# convert over the DB persistent last modified timestamp to a HTTP compatible
diff --git a/common/djangoapps/contentserver/tests/test.py b/common/djangoapps/contentserver/tests/test.py
index 334b10ca00c0eeaee249c2fd571d13060ca572c0..21666b6f2330b1f2f5d9c67d2ede3c04b2cbbdaf 100644
--- a/common/djangoapps/contentserver/tests/test.py
+++ b/common/djangoapps/contentserver/tests/test.py
@@ -15,9 +15,9 @@ from django.test.utils import override_settings
from student.models import CourseEnrollment
from xmodule.contentstore.django import contentstore, _CONTENTSTORE
-from xmodule.modulestore import Location
from xmodule.contentstore.content import StaticContent
from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
from xmodule.modulestore.tests.django_utils import (studio_store_config,
ModuleStoreTestCase)
from xmodule.modulestore.xml_importer import import_from_xml
@@ -47,18 +47,20 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
self.client = Client()
self.contentstore = contentstore()
- # A locked asset
- self.loc_locked = Location('c4x', 'edX', 'toy', 'asset', 'sample_static.txt')
- self.url_locked = StaticContent.get_url_path_from_location(self.loc_locked)
-
- # An unlocked asset
- self.loc_unlocked = Location('c4x', 'edX', 'toy', 'asset', 'another_static.txt')
- self.url_unlocked = StaticContent.get_url_path_from_location(self.loc_unlocked)
+ self.course_key = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')
import_from_xml(modulestore('direct'), 'common/test/data/', ['toy'],
static_content_store=self.contentstore, verbose=True)
- self.contentstore.set_attr(self.loc_locked, 'locked', True)
+ # A locked asset
+ self.locked_asset = self.course_key.make_asset_key('asset', 'sample_static.txt')
+ self.url_locked = self.locked_asset.to_deprecated_string()
+
+ # An unlocked asset
+ self.unlocked_asset = self.course_key.make_asset_key('asset', 'another_static.txt')
+ self.url_unlocked = self.unlocked_asset.to_deprecated_string()
+
+ self.contentstore.set_attr(self.locked_asset, 'locked', True)
# Create user
self.usr = 'testuser'
@@ -114,10 +116,8 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
Test that locked assets behave appropriately in case user is logged in
and registered for the course.
"""
- # pylint: disable=E1101
- course_id = "/".join([self.loc_locked.org, self.loc_locked.course, '2012_Fall'])
- CourseEnrollment.enroll(self.user, course_id)
- self.assertTrue(CourseEnrollment.is_enrolled(self.user, course_id))
+ CourseEnrollment.enroll(self.user, self.course_key)
+ self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course_key))
self.client.login(username=self.usr, password=self.pwd)
resp = self.client.get(self.url_locked)
@@ -127,9 +127,6 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
"""
Test that locked assets behave appropriately in case user is staff.
"""
- # pylint: disable=E1101
- course_id = "/".join([self.loc_locked.org, self.loc_locked.course, '2012_Fall'])
-
self.client.login(username=self.staff_usr, password=self.staff_pwd)
resp = self.client.get(self.url_locked)
self.assertEqual(resp.status_code, 200) # pylint: disable=E1103
diff --git a/common/djangoapps/course_groups/cohorts.py b/common/djangoapps/course_groups/cohorts.py
index 7e6925879ff6ae465ad16881e7440e4201b5e9e9..380bc30c5f3860934f64ce8a8d525a412f31b299 100644
--- a/common/djangoapps/course_groups/cohorts.py
+++ b/common/djangoapps/course_groups/cohorts.py
@@ -32,30 +32,30 @@ def local_random():
return _local_random
-def is_course_cohorted(course_id):
+def is_course_cohorted(course_key):
"""
- Given a course id, return a boolean for whether or not the course is
+ Given a course key, return a boolean for whether or not the course is
cohorted.
Raises:
Http404 if the course doesn't exist.
"""
- return courses.get_course_by_id(course_id).is_cohorted
+ return courses.get_course_by_id(course_key).is_cohorted
-def get_cohort_id(user, course_id):
+def get_cohort_id(user, course_key):
"""
- Given a course id and a user, return the id of the cohort that user is
+ Given a course key and a user, return the id of the cohort that user is
assigned to in that course. If they don't have a cohort, return None.
"""
- cohort = get_cohort(user, course_id)
+ cohort = get_cohort(user, course_key)
return None if cohort is None else cohort.id
-def is_commentable_cohorted(course_id, commentable_id):
+def is_commentable_cohorted(course_key, commentable_id):
"""
Args:
- course_id: string
+ course_key: CourseKey
commentable_id: string
Returns:
@@ -64,7 +64,7 @@ def is_commentable_cohorted(course_id, commentable_id):
Raises:
Http404 if the course doesn't exist.
"""
- course = courses.get_course_by_id(course_id)
+ course = courses.get_course_by_id(course_key)
if not course.is_cohorted:
# this is the easy case :)
@@ -77,18 +77,18 @@ def is_commentable_cohorted(course_id, commentable_id):
# inline discussions are cohorted by default
ans = True
- log.debug(u"is_commentable_cohorted({0}, {1}) = {2}".format(course_id,
- commentable_id,
- ans))
+ log.debug(u"is_commentable_cohorted({0}, {1}) = {2}".format(
+ course_key, commentable_id, ans
+ ))
return ans
-def get_cohorted_commentables(course_id):
+def get_cohorted_commentables(course_key):
"""
- Given a course_id return a list of strings representing cohorted commentables
+ Given a course_key return a list of strings representing cohorted commentables
"""
- course = courses.get_course_by_id(course_id)
+ course = courses.get_course_by_id(course_key)
if not course.is_cohorted:
# this is the easy case :)
@@ -99,34 +99,34 @@ def get_cohorted_commentables(course_id):
return ans
-def get_cohort(user, course_id):
+def get_cohort(user, course_key):
"""
- Given a django User and a course_id, return the user's cohort in that
+ Given a django User and a CourseKey, return the user's cohort in that
cohort.
Arguments:
user: a Django User object.
- course_id: string in the format 'org/course/run'
+ course_key: CourseKey
Returns:
A CourseUserGroup object if the course is cohorted and the User has a
cohort, else None.
Raises:
- ValueError if the course_id doesn't exist.
+ ValueError if the CourseKey doesn't exist.
"""
# First check whether the course is cohorted (users shouldn't be in a cohort
# in non-cohorted courses, but settings can change after course starts)
try:
- course = courses.get_course_by_id(course_id)
+ course = courses.get_course_by_id(course_key)
except Http404:
- raise ValueError("Invalid course_id")
+ raise ValueError("Invalid course_key")
if not course.is_cohorted:
return None
try:
- return CourseUserGroup.objects.get(course_id=course_id,
+ return CourseUserGroup.objects.get(course_id=course_key,
group_type=CourseUserGroup.COHORT,
users__id=user.id)
except CourseUserGroup.DoesNotExist:
@@ -142,72 +142,81 @@ def get_cohort(user, course_id):
# Nowhere to put user
log.warning("Course %s is auto-cohorted, but there are no"
" auto_cohort_groups specified",
- course_id)
+ course_key)
return None
# Put user in a random group, creating it if needed
group_name = local_random().choice(choices)
group, created = CourseUserGroup.objects.get_or_create(
- course_id=course_id,
+ course_id=course_key,
group_type=CourseUserGroup.COHORT,
- name=group_name)
+ name=group_name
+ )
user.course_groups.add(group)
return group
-def get_course_cohorts(course_id):
+def get_course_cohorts(course_key):
"""
Get a list of all the cohorts in the given course.
Arguments:
- course_id: string in the format 'org/course/run'
+ course_key: CourseKey
Returns:
A list of CourseUserGroup objects. Empty if there are no cohorts. Does
not check whether the course is cohorted.
"""
- return list(CourseUserGroup.objects.filter(course_id=course_id,
- group_type=CourseUserGroup.COHORT))
+ return list(CourseUserGroup.objects.filter(
+ course_id=course_key,
+ group_type=CourseUserGroup.COHORT
+ ))
### Helpers for cohort management views
-def get_cohort_by_name(course_id, name):
+def get_cohort_by_name(course_key, name):
"""
Return the CourseUserGroup object for the given cohort. Raises DoesNotExist
it isn't present.
"""
- return CourseUserGroup.objects.get(course_id=course_id,
- group_type=CourseUserGroup.COHORT,
- name=name)
+ return CourseUserGroup.objects.get(
+ course_id=course_key,
+ group_type=CourseUserGroup.COHORT,
+ name=name
+ )
-def get_cohort_by_id(course_id, cohort_id):
+def get_cohort_by_id(course_key, cohort_id):
"""
Return the CourseUserGroup object for the given cohort. Raises DoesNotExist
- it isn't present. Uses the course_id for extra validation...
+ it isn't present. Uses the course_key for extra validation...
"""
- return CourseUserGroup.objects.get(course_id=course_id,
- group_type=CourseUserGroup.COHORT,
- id=cohort_id)
+ return CourseUserGroup.objects.get(
+ course_id=course_key,
+ group_type=CourseUserGroup.COHORT,
+ id=cohort_id
+ )
-def add_cohort(course_id, name):
+def add_cohort(course_key, name):
"""
Add a cohort to a course. Raises ValueError if a cohort of the same name already
exists.
"""
- log.debug("Adding cohort %s to %s", name, course_id)
- if CourseUserGroup.objects.filter(course_id=course_id,
+ log.debug("Adding cohort %s to %s", name, course_key)
+ if CourseUserGroup.objects.filter(course_id=course_key,
group_type=CourseUserGroup.COHORT,
name=name).exists():
raise ValueError("Can't create two cohorts with the same name")
- return CourseUserGroup.objects.create(course_id=course_id,
- group_type=CourseUserGroup.COHORT,
- name=name)
+ return CourseUserGroup.objects.create(
+ course_id=course_key,
+ group_type=CourseUserGroup.COHORT,
+ name=name
+ )
class CohortConflict(Exception):
@@ -237,9 +246,10 @@ def add_user_to_cohort(cohort, username_or_email):
previous_cohort = None
course_cohorts = CourseUserGroup.objects.filter(
- course_id=cohort.course_id,
+ course_id=cohort.course_key,
users__id=user.id,
- group_type=CourseUserGroup.COHORT)
+ group_type=CourseUserGroup.COHORT
+ )
if course_cohorts.exists():
if course_cohorts[0] == cohort:
raise ValueError("User {0} already present in cohort {1}".format(
@@ -253,21 +263,21 @@ def add_user_to_cohort(cohort, username_or_email):
return (user, previous_cohort)
-def get_course_cohort_names(course_id):
+def get_course_cohort_names(course_key):
"""
Return a list of the cohort names in a course.
"""
- return [c.name for c in get_course_cohorts(course_id)]
+ return [c.name for c in get_course_cohorts(course_key)]
-def delete_empty_cohort(course_id, name):
+def delete_empty_cohort(course_key, name):
"""
Remove an empty cohort. Raise ValueError if cohort is not empty.
"""
- cohort = get_cohort_by_name(course_id, name)
+ cohort = get_cohort_by_name(course_key, name)
if cohort.users.exists():
raise ValueError(
"Can't delete non-empty cohort {0} in course {1}".format(
- name, course_id))
+ name, course_key))
cohort.delete()
diff --git a/common/djangoapps/course_groups/models.py b/common/djangoapps/course_groups/models.py
index 8bab17493be0e8150e7a962b9c87abfeab80f719..52c22b6e9ac3be6bd2a44862b7e4d8eb4b0a9ff9 100644
--- a/common/djangoapps/course_groups/models.py
+++ b/common/djangoapps/course_groups/models.py
@@ -2,6 +2,7 @@ import logging
from django.contrib.auth.models import User
from django.db import models
+from xmodule_django.models import CourseKeyField
log = logging.getLogger(__name__)
@@ -23,7 +24,8 @@ class CourseUserGroup(models.Model):
# Note: groups associated with particular runs of a course. E.g. Fall 2012 and Spring
# 2013 versions of 6.00x will have separate groups.
- course_id = models.CharField(max_length=255, db_index=True,
+ # TODO change field name to course_key
+ course_id = CourseKeyField(max_length=255, db_index=True,
help_text="Which course is this group associated with?")
# For now, only have group type 'cohort', but adding a type field to support
diff --git a/common/djangoapps/course_groups/tests/test_cohorts.py b/common/djangoapps/course_groups/tests/test_cohorts.py
index a17df56a714dcf920403e6c0dadb1d36ef0fa147..4901bdc94a43e93af9e55578271241a459b02963 100644
--- a/common/djangoapps/course_groups/tests/test_cohorts.py
+++ b/common/djangoapps/course_groups/tests/test_cohorts.py
@@ -9,6 +9,7 @@ from course_groups.cohorts import (get_cohort, get_course_cohorts,
is_commentable_cohorted, get_cohort_by_name)
from xmodule.modulestore.django import modulestore, clear_existing_modulestores
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
from xmodule.modulestore.tests.django_utils import mixed_store_config
@@ -84,13 +85,14 @@ class TestCohorts(django.test.TestCase):
Make sure that course is reloaded every time--clear out the modulestore.
"""
clear_existing_modulestores()
+ self.toy_course_key = SlashSeparatedCourseKey("edX", "toy", "2012_Fall")
def test_get_cohort(self):
"""
Make sure get_cohort() does the right thing when the course is cohorted
"""
- course = modulestore().get_course("edX/toy/2012_Fall")
- self.assertEqual(course.id, "edX/toy/2012_Fall")
+ course = modulestore().get_course(self.toy_course_key)
+ self.assertEqual(course.id, self.toy_course_key)
self.assertFalse(course.is_cohorted)
user = User.objects.create(username="test", email="a@b.com")
@@ -120,8 +122,7 @@ class TestCohorts(django.test.TestCase):
"""
Make sure get_cohort() does the right thing when the course is auto_cohorted
"""
- course = modulestore().get_course("edX/toy/2012_Fall")
- self.assertEqual(course.id, "edX/toy/2012_Fall")
+ course = modulestore().get_course(self.toy_course_key)
self.assertFalse(course.is_cohorted)
user1 = User.objects.create(username="test", email="a@b.com")
@@ -168,8 +169,7 @@ class TestCohorts(django.test.TestCase):
"""
Make sure get_cohort() randomizes properly.
"""
- course = modulestore().get_course("edX/toy/2012_Fall")
- self.assertEqual(course.id, "edX/toy/2012_Fall")
+ course = modulestore().get_course(self.toy_course_key)
self.assertFalse(course.is_cohorted)
groups = ["group_{0}".format(n) for n in range(5)]
@@ -194,26 +194,26 @@ class TestCohorts(django.test.TestCase):
self.assertLess(num_users, 50)
def test_get_course_cohorts(self):
- course1_id = 'a/b/c'
- course2_id = 'e/f/g'
+ course1_key = SlashSeparatedCourseKey('a', 'b', 'c')
+ course2_key = SlashSeparatedCourseKey('e', 'f', 'g')
# add some cohorts to course 1
cohort = CourseUserGroup.objects.create(name="TestCohort",
- course_id=course1_id,
+ course_id=course1_key,
group_type=CourseUserGroup.COHORT)
cohort = CourseUserGroup.objects.create(name="TestCohort2",
- course_id=course1_id,
+ course_id=course1_key,
group_type=CourseUserGroup.COHORT)
# second course should have no cohorts
- self.assertEqual(get_course_cohorts(course2_id), [])
+ self.assertEqual(get_course_cohorts(course2_key), [])
- cohorts = sorted([c.name for c in get_course_cohorts(course1_id)])
+ cohorts = sorted([c.name for c in get_course_cohorts(course1_key)])
self.assertEqual(cohorts, ['TestCohort', 'TestCohort2'])
def test_is_commentable_cohorted(self):
- course = modulestore().get_course("edX/toy/2012_Fall")
+ course = modulestore().get_course(self.toy_course_key)
self.assertFalse(course.is_cohorted)
def to_id(name):
diff --git a/common/djangoapps/course_groups/views.py b/common/djangoapps/course_groups/views.py
index 9dc9cf523c0918ce592cf52c3c8a294a26c0b3f7..a9a058a539f82a9a3a70833c91f989c886d44035 100644
--- a/common/djangoapps/course_groups/views.py
+++ b/common/djangoapps/course_groups/views.py
@@ -33,25 +33,25 @@ def split_by_comma_and_whitespace(s):
@ensure_csrf_cookie
-def list_cohorts(request, course_id):
+def list_cohorts(request, course_key):
"""
Return json dump of dict:
{'success': True,
'cohorts': [{'name': name, 'id': id}, ...]}
"""
- get_course_with_access(request.user, course_id, 'staff')
+ get_course_with_access(request.user, 'staff', course_key)
all_cohorts = [{'name': c.name, 'id': c.id}
- for c in cohorts.get_course_cohorts(course_id)]
+ for c in cohorts.get_course_cohorts(course_key)]
return json_http_response({'success': True,
- 'cohorts': all_cohorts})
+ 'cohorts': all_cohorts})
@ensure_csrf_cookie
@require_POST
-def add_cohort(request, course_id):
+def add_cohort(request, course_key):
"""
Return json of dict:
{'success': True,
@@ -63,7 +63,7 @@ def add_cohort(request, course_id):
{'success': False,
'msg': error_msg} if there's an error
"""
- get_course_with_access(request.user, course_id, 'staff')
+ get_course_with_access(request.user, 'staff', course_key)
name = request.POST.get("name")
if not name:
@@ -71,7 +71,7 @@ def add_cohort(request, course_id):
'msg': "No name specified"})
try:
- cohort = cohorts.add_cohort(course_id, name)
+ cohort = cohorts.add_cohort(course_key, name)
except ValueError as err:
return json_http_response({'success': False,
'msg': str(err)})
@@ -84,7 +84,7 @@ def add_cohort(request, course_id):
@ensure_csrf_cookie
-def users_in_cohort(request, course_id, cohort_id):
+def users_in_cohort(request, course_key, cohort_id):
"""
Return users in the cohort. Show up to 100 per page, and page
using the 'page' GET attribute in the call. Format:
@@ -97,11 +97,11 @@ def users_in_cohort(request, course_id, cohort_id):
'users': [{'username': ..., 'email': ..., 'name': ...}]
}
"""
- get_course_with_access(request.user, course_id, 'staff')
+ get_course_with_access(request.user, 'staff', course_key)
# this will error if called with a non-int cohort_id. That's ok--it
# shoudn't happen for valid clients.
- cohort = cohorts.get_cohort_by_id(course_id, int(cohort_id))
+ cohort = cohorts.get_cohort_by_id(course_key, int(cohort_id))
paginator = Paginator(cohort.users.all(), 100)
page = request.GET.get('page')
@@ -119,17 +119,17 @@ def users_in_cohort(request, course_id, cohort_id):
user_info = [{'username': u.username,
'email': u.email,
'name': '{0} {1}'.format(u.first_name, u.last_name)}
- for u in users]
+ for u in users]
return json_http_response({'success': True,
- 'page': page,
- 'num_pages': paginator.num_pages,
- 'users': user_info})
+ 'page': page,
+ 'num_pages': paginator.num_pages,
+ 'users': user_info})
@ensure_csrf_cookie
@require_POST
-def add_users_to_cohort(request, course_id, cohort_id):
+def add_users_to_cohort(request, course_key, cohort_id):
"""
Return json dict of:
@@ -144,9 +144,9 @@ def add_users_to_cohort(request, course_id, cohort_id):
'present': [str1, str2, ...], # already there
'unknown': [str1, str2, ...]}
"""
- get_course_with_access(request.user, course_id, 'staff')
+ get_course_with_access(request.user, 'staff', course_key)
- cohort = cohorts.get_cohort_by_id(course_id, cohort_id)
+ cohort = cohorts.get_cohort_by_id(course_key, cohort_id)
users = request.POST.get('users', '')
added = []
@@ -175,15 +175,15 @@ def add_users_to_cohort(request, course_id, cohort_id):
unknown.append(username_or_email)
return json_http_response({'success': True,
- 'added': added,
- 'changed': changed,
- 'present': present,
- 'unknown': unknown})
+ 'added': added,
+ 'changed': changed,
+ 'present': present,
+ 'unknown': unknown})
@ensure_csrf_cookie
@require_POST
-def remove_user_from_cohort(request, course_id, cohort_id):
+def remove_user_from_cohort(request, course_key, cohort_id):
"""
Expects 'username': username in POST data.
@@ -193,14 +193,14 @@ def remove_user_from_cohort(request, course_id, cohort_id):
{'success': False,
'msg': error_msg}
"""
- get_course_with_access(request.user, course_id, 'staff')
+ get_course_with_access(request.user, 'staff', course_key)
username = request.POST.get('username')
if username is None:
return json_http_response({'success': False,
- 'msg': 'No username specified'})
+ 'msg': 'No username specified'})
- cohort = cohorts.get_cohort_by_id(course_id, cohort_id)
+ cohort = cohorts.get_cohort_by_id(course_key, cohort_id)
try:
user = User.objects.get(username=username)
cohort.users.remove(user)
@@ -208,16 +208,18 @@ def remove_user_from_cohort(request, course_id, cohort_id):
except User.DoesNotExist:
log.debug('no user')
return json_http_response({'success': False,
- 'msg': "No user '{0}'".format(username)})
+ 'msg': "No user '{0}'".format(username)})
-def debug_cohort_mgmt(request, course_id):
+def debug_cohort_mgmt(request, course_key):
"""
Debugging view for dev.
"""
# add staff check to make sure it's safe if it's accidentally deployed.
- get_course_with_access(request.user, course_id, 'staff')
+ get_course_with_access(request.user, 'staff', course_key)
- context = {'cohorts_ajax_url': reverse('cohorts',
- kwargs={'course_id': course_id})}
+ context = {'cohorts_ajax_url': reverse(
+ 'cohorts',
+ kwargs={'course_id': course_key.to_deprecated_string()}
+ )}
return render_to_response('/course_groups/debug.html', context)
diff --git a/common/djangoapps/course_modes/models.py b/common/djangoapps/course_modes/models.py
index ae9fac5ed429985297a2eab4581368a79f108979..2657f877f63b291a32a3b5082971821a207bd14c 100644
--- a/common/djangoapps/course_modes/models.py
+++ b/common/djangoapps/course_modes/models.py
@@ -9,6 +9,8 @@ from collections import namedtuple
from django.utils.translation import ugettext as _
from django.db.models import Q
+from xmodule_django.models import CourseKeyField
+
Mode = namedtuple('Mode', ['slug', 'name', 'min_price', 'suggested_prices', 'currency', 'expiration_datetime'])
class CourseMode(models.Model):
@@ -17,7 +19,7 @@ class CourseMode(models.Model):
"""
# the course that this mode is attached to
- course_id = models.CharField(max_length=255, db_index=True)
+ course_id = CourseKeyField(max_length=255, db_index=True)
# the reference to this mode that can be used by Enrollments to generate
# similar behavior for the same slug across courses
diff --git a/common/djangoapps/course_modes/tests/test_models.py b/common/djangoapps/course_modes/tests/test_models.py
index 5336b3e5fab74a5ad66b893b112017049ed94b40..c29a9e7ec3d368c84ae59d4d56e1e51916992842 100644
--- a/common/djangoapps/course_modes/tests/test_models.py
+++ b/common/djangoapps/course_modes/tests/test_models.py
@@ -8,6 +8,7 @@ Replace this with more appropriate tests for your application.
from datetime import datetime, timedelta
import pytz
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
from django.test import TestCase
from course_modes.models import CourseMode, Mode
@@ -18,7 +19,7 @@ class CourseModeModelTest(TestCase):
"""
def setUp(self):
- self.course_id = 'TestCourse'
+ self.course_key = SlashSeparatedCourseKey('Test', 'TestCourse', 'TestCourseRun')
CourseMode.objects.all().delete()
def create_mode(self, mode_slug, mode_name, min_price=0, suggested_prices='', currency='usd'):
@@ -26,7 +27,7 @@ class CourseModeModelTest(TestCase):
Create a new course mode
"""
return CourseMode.objects.get_or_create(
- course_id=self.course_id,
+ course_id=self.course_key,
mode_display_name=mode_name,
mode_slug=mode_slug,
min_price=min_price,
@@ -39,7 +40,7 @@ class CourseModeModelTest(TestCase):
If we can't find any modes, we should get back the default mode
"""
# shouldn't be able to find a corresponding course
- modes = CourseMode.modes_for_course(self.course_id)
+ modes = CourseMode.modes_for_course(self.course_key)
self.assertEqual([CourseMode.DEFAULT_MODE], modes)
def test_nodes_for_course_single(self):
@@ -48,13 +49,13 @@ class CourseModeModelTest(TestCase):
"""
self.create_mode('verified', 'Verified Certificate')
- modes = CourseMode.modes_for_course(self.course_id)
+ modes = CourseMode.modes_for_course(self.course_key)
mode = Mode(u'verified', u'Verified Certificate', 0, '', 'usd', None)
self.assertEqual([mode], modes)
- modes_dict = CourseMode.modes_for_course_dict(self.course_id)
+ modes_dict = CourseMode.modes_for_course_dict(self.course_key)
self.assertEqual(modes_dict['verified'], mode)
- self.assertEqual(CourseMode.mode_for_course(self.course_id, 'verified'),
+ self.assertEqual(CourseMode.mode_for_course(self.course_key, 'verified'),
mode)
def test_modes_for_course_multiple(self):
@@ -67,18 +68,18 @@ class CourseModeModelTest(TestCase):
for mode in set_modes:
self.create_mode(mode.slug, mode.name, mode.min_price, mode.suggested_prices)
- modes = CourseMode.modes_for_course(self.course_id)
+ modes = CourseMode.modes_for_course(self.course_key)
self.assertEqual(modes, set_modes)
- self.assertEqual(mode1, CourseMode.mode_for_course(self.course_id, u'honor'))
- self.assertEqual(mode2, CourseMode.mode_for_course(self.course_id, u'verified'))
- self.assertIsNone(CourseMode.mode_for_course(self.course_id, 'DNE'))
+ self.assertEqual(mode1, CourseMode.mode_for_course(self.course_key, u'honor'))
+ self.assertEqual(mode2, CourseMode.mode_for_course(self.course_key, u'verified'))
+ self.assertIsNone(CourseMode.mode_for_course(self.course_key, 'DNE'))
def test_min_course_price_for_currency(self):
"""
Get the min course price for a course according to currency
"""
# no modes, should get 0
- self.assertEqual(0, CourseMode.min_course_price_for_currency(self.course_id, 'usd'))
+ self.assertEqual(0, CourseMode.min_course_price_for_currency(self.course_key, 'usd'))
# create some modes
mode1 = Mode(u'honor', u'Honor Code Certificate', 10, '', 'usd', None)
@@ -88,27 +89,27 @@ class CourseModeModelTest(TestCase):
for mode in set_modes:
self.create_mode(mode.slug, mode.name, mode.min_price, mode.suggested_prices, mode.currency)
- self.assertEqual(10, CourseMode.min_course_price_for_currency(self.course_id, 'usd'))
- self.assertEqual(80, CourseMode.min_course_price_for_currency(self.course_id, 'cny'))
+ self.assertEqual(10, CourseMode.min_course_price_for_currency(self.course_key, 'usd'))
+ self.assertEqual(80, CourseMode.min_course_price_for_currency(self.course_key, 'cny'))
def test_modes_for_course_expired(self):
expired_mode, _status = self.create_mode('verified', 'Verified Certificate')
expired_mode.expiration_datetime = datetime.now(pytz.UTC) + timedelta(days=-1)
expired_mode.save()
- modes = CourseMode.modes_for_course(self.course_id)
+ modes = CourseMode.modes_for_course(self.course_key)
self.assertEqual([CourseMode.DEFAULT_MODE], modes)
mode1 = Mode(u'honor', u'Honor Code Certificate', 0, '', 'usd', None)
self.create_mode(mode1.slug, mode1.name, mode1.min_price, mode1.suggested_prices)
- modes = CourseMode.modes_for_course(self.course_id)
+ modes = CourseMode.modes_for_course(self.course_key)
self.assertEqual([mode1], modes)
expiration_datetime = datetime.now(pytz.UTC) + timedelta(days=1)
expired_mode.expiration_datetime = expiration_datetime
expired_mode.save()
expired_mode_value = Mode(u'verified', u'Verified Certificate', 0, '', 'usd', expiration_datetime)
- modes = CourseMode.modes_for_course(self.course_id)
+ modes = CourseMode.modes_for_course(self.course_key)
self.assertEqual([expired_mode_value, mode1], modes)
- modes = CourseMode.modes_for_course('second_test_course')
+ modes = CourseMode.modes_for_course(SlashSeparatedCourseKey('TestOrg', 'TestCourse', 'TestRun'))
self.assertEqual([CourseMode.DEFAULT_MODE], modes)
diff --git a/common/djangoapps/course_modes/views.py b/common/djangoapps/course_modes/views.py
index 8ca43a17bb514556ee6190d49ef3d79cd32b9409..14bbac303699fd12432c2cd8f0c6d64ef8b7601f 100644
--- a/common/djangoapps/course_modes/views.py
+++ b/common/djangoapps/course_modes/views.py
@@ -20,6 +20,7 @@ from courseware.access import has_access
from student.models import CourseEnrollment
from student.views import course_from_id
from verify_student.models import SoftwareSecurePhotoVerification
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
class ChooseModeView(View):
@@ -35,7 +36,9 @@ class ChooseModeView(View):
def get(self, request, course_id, error=None):
""" Displays the course mode choice page """
- enrollment_mode = CourseEnrollment.enrollment_mode_for_user(request.user, course_id)
+ course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
+
+ enrollment_mode = CourseEnrollment.enrollment_mode_for_user(request.user, course_key)
upgrade = request.GET.get('upgrade', False)
request.session['attempting_upgrade'] = upgrade
@@ -47,13 +50,13 @@ class ChooseModeView(View):
if enrollment_mode is not None and upgrade is False:
return redirect(reverse('dashboard'))
- modes = CourseMode.modes_for_course_dict(course_id)
+ modes = CourseMode.modes_for_course_dict(course_key)
donation_for_course = request.session.get("donation_for_course", {})
- chosen_price = donation_for_course.get(course_id, None)
+ chosen_price = donation_for_course.get(course_key, None)
- course = course_from_id(course_id)
+ course = course_from_id(course_key)
context = {
- "course_id": course_id,
+ "course_modes_choose_url": reverse("course_modes_choose", kwargs={'course_id': course_key.to_deprecated_string()}),
"modes": modes,
"course_name": course.display_name_with_default,
"course_org": course.display_org_with_default,
@@ -72,25 +75,26 @@ class ChooseModeView(View):
@method_decorator(login_required)
def post(self, request, course_id):
""" Takes the form submission from the page and parses it """
+ course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
user = request.user
# This is a bit redundant with logic in student.views.change_enrollement,
# but I don't really have the time to refactor it more nicely and test.
- course = course_from_id(course_id)
- if not has_access(user, course, 'enroll'):
+ course = course_from_id(course_key)
+ if not has_access(user, 'enroll', course):
error_msg = _("Enrollment is closed")
- return self.get(request, course_id, error=error_msg)
+ return self.get(request, course_key, error=error_msg)
upgrade = request.GET.get('upgrade', False)
requested_mode = self.get_requested_mode(request.POST)
- allowed_modes = CourseMode.modes_for_course_dict(course_id)
+ allowed_modes = CourseMode.modes_for_course_dict(course_key)
if requested_mode not in allowed_modes:
return HttpResponseBadRequest(_("Enrollment mode not supported"))
if requested_mode in ("audit", "honor"):
- CourseEnrollment.enroll(user, course_id, requested_mode)
+ CourseEnrollment.enroll(user, course_key, requested_mode)
return redirect('dashboard')
mode_info = allowed_modes[requested_mode]
@@ -104,25 +108,25 @@ class ChooseModeView(View):
amount_value = decimal.Decimal(amount).quantize(decimal.Decimal('.01'), rounding=decimal.ROUND_DOWN)
except decimal.InvalidOperation:
error_msg = _("Invalid amount selected.")
- return self.get(request, course_id, error=error_msg)
+ return self.get(request, course_key, error=error_msg)
# Check for minimum pricing
if amount_value < mode_info.min_price:
error_msg = _("No selected price or selected price is too low.")
- return self.get(request, course_id, error=error_msg)
+ return self.get(request, course_key, error=error_msg)
donation_for_course = request.session.get("donation_for_course", {})
- donation_for_course[course_id] = amount_value
+ donation_for_course[course_key] = amount_value
request.session["donation_for_course"] = donation_for_course
if SoftwareSecurePhotoVerification.user_has_valid_or_pending(request.user):
return redirect(
reverse('verify_student_verified',
- kwargs={'course_id': course_id}) + "?upgrade={}".format(upgrade)
+ kwargs={'course_id': course_key.to_deprecated_string()}) + "?upgrade={}".format(upgrade)
)
return redirect(
reverse('verify_student_show_requirements',
- kwargs={'course_id': course_id}) + "?upgrade={}".format(upgrade))
+ kwargs={'course_id': course_key.to_deprecated_string()}) + "?upgrade={}".format(upgrade))
def get_requested_mode(self, request_dict):
"""
diff --git a/common/djangoapps/django_comment_common/models.py b/common/djangoapps/django_comment_common/models.py
index 0479b7ab28cd10c113e81e6fb443eec40bed6c7e..67e33fb9ebb64a39bfb3e3f504c4341445409fd9 100644
--- a/common/djangoapps/django_comment_common/models.py
+++ b/common/djangoapps/django_comment_common/models.py
@@ -9,7 +9,8 @@ from django.utils.translation import ugettext_noop
from student.models import CourseEnrollment
from xmodule.modulestore.django import modulestore
-from xmodule.course_module import CourseDescriptor
+from xmodule.modulestore.exceptions import ItemNotFoundError
+from xmodule_django.models import CourseKeyField, NoneToEmptyManager
FORUM_ROLE_ADMINISTRATOR = ugettext_noop('Administrator')
FORUM_ROLE_MODERATOR = ugettext_noop('Moderator')
@@ -48,16 +49,20 @@ def assign_default_role(course_id, user):
class Role(models.Model):
+
+ objects = NoneToEmptyManager()
+
name = models.CharField(max_length=30, null=False, blank=False)
users = models.ManyToManyField(User, related_name="roles")
- course_id = models.CharField(max_length=255, blank=True, db_index=True)
+ course_id = CourseKeyField(max_length=255, blank=True, db_index=True)
class Meta:
# use existing table that was originally created from django_comment_client app
db_table = 'django_comment_client_role'
def __unicode__(self):
- return self.name + " for " + (self.course_id if self.course_id else "all courses")
+ # pylint: disable=no-member
+ return self.name + " for " + (self.course_id.to_deprecated_string() if self.course_id else "all courses")
def inherit_permissions(self, role): # TODO the name of this method is a little bit confusing,
# since it's one-off and doesn't handle inheritance later
@@ -71,8 +76,9 @@ class Role(models.Model):
self.permissions.add(Permission.objects.get_or_create(name=permission)[0])
def has_permission(self, permission):
- course_loc = CourseDescriptor.id_to_location(self.course_id)
- course = modulestore().get_instance(self.course_id, course_loc)
+ course = modulestore().get_course(self.course_id)
+ if course is None:
+ raise ItemNotFoundError(self.course_id)
if self.name == FORUM_ROLE_STUDENT and \
(permission.startswith('edit') or permission.startswith('update') or permission.startswith('create')) and \
(not course.forum_posts_allowed):
diff --git a/common/djangoapps/django_comment_common/tests.py b/common/djangoapps/django_comment_common/tests.py
index fd776c75d322731a54ad4406e8b7a59699f3a598..308a1c93906c31bececda87d50bb57c81c395b7d 100644
--- a/common/djangoapps/django_comment_common/tests.py
+++ b/common/djangoapps/django_comment_common/tests.py
@@ -1,5 +1,6 @@
from django.test import TestCase
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
from django_comment_common.models import Role
from student.models import CourseEnrollment, User
@@ -21,13 +22,13 @@ class RoleAssignmentTest(TestCase):
"hacky",
"hacky@fake.edx.org"
)
- self.course_id = "edX/Fake101/2012"
- CourseEnrollment.enroll(self.staff_user, self.course_id)
- CourseEnrollment.enroll(self.student_user, self.course_id)
+ self.course_key = SlashSeparatedCourseKey("edX", "Fake101", "2012")
+ CourseEnrollment.enroll(self.staff_user, self.course_key)
+ CourseEnrollment.enroll(self.student_user, self.course_key)
def test_enrollment_auto_role_creation(self):
student_role = Role.objects.get(
- course_id=self.course_id,
+ course_id=self.course_key,
name="Student"
)
diff --git a/common/djangoapps/django_comment_common/utils.py b/common/djangoapps/django_comment_common/utils.py
index 75da2453dc6bc244d8eb2ea4993ba71da8d5cc6c..9650401b65ab95dbdf9a2c6b5c82765ae765ed28 100644
--- a/common/djangoapps/django_comment_common/utils.py
+++ b/common/djangoapps/django_comment_common/utils.py
@@ -10,27 +10,27 @@ _MODERATOR_ROLE_PERMISSIONS = ["edit_content", "delete_thread", "openclose_threa
_ADMINISTRATOR_ROLE_PERMISSIONS = ["manage_moderator"]
-def _save_forum_role(course_id, name):
+def _save_forum_role(course_key, name):
"""
- Save and Update 'course_id' for all roles which are already created to keep course_id same
- as actual passed course id
+ Save and Update 'course_key' for all roles which are already created to keep course_id same
+ as actual passed course key
"""
- role, created = Role.objects.get_or_create(name=name, course_id=course_id)
+ role, created = Role.objects.get_or_create(name=name, course_id=course_key)
if created is False:
- role.course_id = course_id
+ role.course_id = course_key
role.save()
return role
-def seed_permissions_roles(course_id):
+def seed_permissions_roles(course_key):
"""
Create and assign permissions for forum roles
"""
- administrator_role = _save_forum_role(course_id, "Administrator")
- moderator_role = _save_forum_role(course_id, "Moderator")
- community_ta_role = _save_forum_role(course_id, "Community TA")
- student_role = _save_forum_role(course_id, "Student")
+ administrator_role = _save_forum_role(course_key, "Administrator")
+ moderator_role = _save_forum_role(course_key, "Moderator")
+ community_ta_role = _save_forum_role(course_key, "Community TA")
+ student_role = _save_forum_role(course_key, "Student")
for per in _STUDENT_ROLE_PERMISSIONS:
student_role.add_permission(per)
diff --git a/common/djangoapps/embargo/forms.py b/common/djangoapps/embargo/forms.py
index 09ea11445be7a6b89177e79dd7f4add7599b01aa..ce7a6a00da09d89e179ec620bb3e8720aaac6834 100644
--- a/common/djangoapps/embargo/forms.py
+++ b/common/djangoapps/embargo/forms.py
@@ -10,6 +10,8 @@ from embargo.fixtures.country_codes import COUNTRY_CODES
import socket
from xmodule.modulestore.django import modulestore
+from opaque_keys import InvalidKeyError
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
class EmbargoedCourseForm(forms.ModelForm): # pylint: disable=incomplete-protocol
@@ -20,19 +22,29 @@ class EmbargoedCourseForm(forms.ModelForm): # pylint: disable=incomplete-protoc
def clean_course_id(self):
"""Validate the course id"""
- course_id = self.cleaned_data["course_id"]
+
+ cleaned_id = self.cleaned_data["course_id"]
+
+ try:
+ course_id = SlashSeparatedCourseKey.from_deprecated_string(cleaned_id)
+
+ except InvalidKeyError:
+ msg = 'COURSE NOT FOUND'
+ msg += u' --- Entered course id was: "{0}". '.format(cleaned_id)
+ msg += 'Please recheck that you have supplied a valid course id.'
+ raise forms.ValidationError(msg)
# Try to get the course. If this returns None, it's not a real course
try:
course = modulestore().get_course(course_id)
except ValueError:
msg = 'COURSE NOT FOUND'
- msg += u' --- Entered course id was: "{0}". '.format(course_id)
+ msg += u' --- Entered course id was: "{0}". '.format(course_id.to_deprecated_string())
msg += 'Please recheck that you have supplied a valid course id.'
raise forms.ValidationError(msg)
if not course:
msg = 'COURSE NOT FOUND'
- msg += u' --- Entered course id was: "{0}". '.format(course_id)
+ msg += u' --- Entered course id was: "{0}". '.format(course_id.to_deprecated_string())
msg += 'Please recheck that you have supplied a valid course id.'
raise forms.ValidationError(msg)
diff --git a/common/djangoapps/embargo/models.py b/common/djangoapps/embargo/models.py
index 4ee66138590b19c559fefc2686731b6ae2b7cd18..fb85c34713ff125bf7d868721bad85a6ccd82e07 100644
--- a/common/djangoapps/embargo/models.py
+++ b/common/djangoapps/embargo/models.py
@@ -13,14 +13,17 @@ file and check it in at the same time as your model changes. To do that,
from django.db import models
from config_models.models import ConfigurationModel
+from xmodule_django.models import CourseKeyField, NoneToEmptyManager
class EmbargoedCourse(models.Model):
"""
Enable course embargo on a course-by-course basis.
"""
+ objects = NoneToEmptyManager()
+
# The course to embargo
- course_id = models.CharField(max_length=255, db_index=True, unique=True)
+ course_id = CourseKeyField(max_length=255, db_index=True, unique=True)
# Whether or not to embargo
embargoed = models.BooleanField(default=False)
@@ -42,7 +45,8 @@ class EmbargoedCourse(models.Model):
not_em = "Not "
if self.embargoed:
not_em = ""
- return u"Course '{}' is {}Embargoed".format(self.course_id, not_em)
+ # pylint: disable=no-member
+ return u"Course '{}' is {}Embargoed".format(self.course_id.to_deprecated_string(), not_em)
class EmbargoedState(ConfigurationModel):
diff --git a/common/djangoapps/embargo/tests/test_forms.py b/common/djangoapps/embargo/tests/test_forms.py
index cea030c23d607243ceac36f2f897129622bf12e3..8dc5dbbb806a7c876535250cc2f717d84502a80d 100644
--- a/common/djangoapps/embargo/tests/test_forms.py
+++ b/common/djangoapps/embargo/tests/test_forms.py
@@ -22,8 +22,8 @@ class EmbargoCourseFormTest(ModuleStoreTestCase):
def setUp(self):
self.course = CourseFactory.create()
- self.true_form_data = {'course_id': self.course.id, 'embargoed': True}
- self.false_form_data = {'course_id': self.course.id, 'embargoed': False}
+ self.true_form_data = {'course_id': self.course.id.to_deprecated_string(), 'embargoed': True}
+ self.false_form_data = {'course_id': self.course.id.to_deprecated_string(), 'embargoed': False}
def test_embargo_course(self):
self.assertFalse(EmbargoedCourse.is_embargoed(self.course.id))
@@ -62,7 +62,7 @@ class EmbargoCourseFormTest(ModuleStoreTestCase):
def test_form_typo(self):
# Munge course id
- bad_id = self.course.id + '_typo'
+ bad_id = self.course.id.to_deprecated_string() + '_typo'
form_data = {'course_id': bad_id, 'embargoed': True}
form = EmbargoedCourseForm(data=form_data)
@@ -79,7 +79,7 @@ class EmbargoCourseFormTest(ModuleStoreTestCase):
def test_invalid_location(self):
# Munge course id
- bad_id = self.course.id.split('/')[-1]
+ bad_id = self.course.id.to_deprecated_string().split('/')[-1]
form_data = {'course_id': bad_id, 'embargoed': True}
form = EmbargoedCourseForm(data=form_data)
diff --git a/common/djangoapps/embargo/tests/test_middleware.py b/common/djangoapps/embargo/tests/test_middleware.py
index c3f418c9057a2a4deface6069f55aa0c188e59c3..cd09b2cbd9376f59924d01383c40f1c9184a52f6 100644
--- a/common/djangoapps/embargo/tests/test_middleware.py
+++ b/common/djangoapps/embargo/tests/test_middleware.py
@@ -32,8 +32,8 @@ class EmbargoMiddlewareTests(TestCase):
self.embargo_course.save()
self.regular_course = CourseFactory.create(org="Regular")
self.regular_course.save()
- self.embargoed_page = '/courses/' + self.embargo_course.id + '/info'
- self.regular_page = '/courses/' + self.regular_course.id + '/info'
+ self.embargoed_page = '/courses/' + self.embargo_course.id.to_deprecated_string() + '/info'
+ self.regular_page = '/courses/' + self.regular_course.id.to_deprecated_string() + '/info'
EmbargoedCourse(course_id=self.embargo_course.id, embargoed=True).save()
EmbargoedState(
embargoed_countries="cu, ir, Sy, SD",
diff --git a/common/djangoapps/embargo/tests/test_models.py b/common/djangoapps/embargo/tests/test_models.py
index 12c66295b8a15c7dae896f8ec56cad0fa5d8fbb0..37a8d48a2eceab04fc15200e9b4b88f82d8f234f 100644
--- a/common/djangoapps/embargo/tests/test_models.py
+++ b/common/djangoapps/embargo/tests/test_models.py
@@ -1,13 +1,14 @@
"""Test of models for embargo middleware app"""
from django.test import TestCase
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
from embargo.models import EmbargoedCourse, EmbargoedState, IPFilter
class EmbargoModelsTest(TestCase):
"""Test each of the 3 models in embargo.models"""
def test_course_embargo(self):
- course_id = 'abc/123/doremi'
+ course_id = SlashSeparatedCourseKey('abc', '123', 'doremi')
# Test that course is not authorized by default
self.assertFalse(EmbargoedCourse.is_embargoed(course_id))
diff --git a/common/djangoapps/external_auth/tests/test_shib.py b/common/djangoapps/external_auth/tests/test_shib.py
index c8f38563de3541c178a572ece67bf2bd800ffdbd..0250ecc1a00ef464e23363719d09e44dd33061ae 100644
--- a/common/djangoapps/external_auth/tests/test_shib.py
+++ b/common/djangoapps/external_auth/tests/test_shib.py
@@ -19,6 +19,7 @@ from xmodule.modulestore.tests.factories import CourseFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, mixed_store_config
from xmodule.modulestore.inheritance import own_metadata
from xmodule.modulestore.django import editable_modulestore
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
from external_auth.models import ExternalAuthMap
from external_auth.views import shib_login, course_specific_login, course_specific_register, _flatten_to_ascii
@@ -340,8 +341,8 @@ class ShibSPTest(ModuleStoreTestCase):
'?course_id=MITx/999/course/Robot_Super_Course' +
'&enrollment_action=enroll')
- login_response = course_specific_login(login_request, 'MITx/999/Robot_Super_Course')
- reg_response = course_specific_register(login_request, 'MITx/999/Robot_Super_Course')
+ login_response = course_specific_login(login_request, SlashSeparatedCourseKey('MITx', '999', 'Robot_Super_Course'))
+ reg_response = course_specific_register(login_request, SlashSeparatedCourseKey('MITx', '999', 'Robot_Super_Course'))
if "shib" in domain:
self.assertIsInstance(login_response, HttpResponseRedirect)
@@ -375,8 +376,8 @@ class ShibSPTest(ModuleStoreTestCase):
'?course_id=DNE/DNE/DNE/Robot_Super_Course' +
'&enrollment_action=enroll')
- login_response = course_specific_login(login_request, 'DNE/DNE/DNE')
- reg_response = course_specific_register(login_request, 'DNE/DNE/DNE')
+ login_response = course_specific_login(login_request, SlashSeparatedCourseKey('DNE', 'DNE', 'DNE'))
+ reg_response = course_specific_register(login_request, SlashSeparatedCourseKey('DNE', 'DNE', 'DNE'))
self.assertIsInstance(login_response, HttpResponseRedirect)
self.assertEqual(login_response['Location'],
@@ -436,7 +437,7 @@ class ShibSPTest(ModuleStoreTestCase):
for student in [shib_student, other_ext_student, int_student]:
request = self.request_factory.post('/change_enrollment')
request.POST.update({'enrollment_action': 'enroll',
- 'course_id': course.id})
+ 'course_id': course.id.to_deprecated_string()})
request.user = student
response = change_enrollment(request)
# If course is not limited or student has correct shib extauth then enrollment should be allowed
@@ -476,7 +477,7 @@ class ShibSPTest(ModuleStoreTestCase):
self.assertFalse(CourseEnrollment.is_enrolled(student, course.id))
self.client.logout()
request_kwargs = {'path': '/shib-login/',
- 'data': {'enrollment_action': 'enroll', 'course_id': course.id, 'next': '/testredirect'},
+ 'data': {'enrollment_action': 'enroll', 'course_id': course.id.to_deprecated_string(), 'next': '/testredirect'},
'follow': False,
'REMOTE_USER': 'testuser@stanford.edu',
'Shib-Identity-Provider': 'https://idp.stanford.edu/'}
diff --git a/common/djangoapps/external_auth/tests/test_ssl.py b/common/djangoapps/external_auth/tests/test_ssl.py
index cd9940bf7e6d2a7b1baa991fb1dd4f2429930af2..24d4dc3bd6ff0ed2ba686b959c46dc89f75e1a88 100644
--- a/common/djangoapps/external_auth/tests/test_ssl.py
+++ b/common/djangoapps/external_auth/tests/test_ssl.py
@@ -3,8 +3,6 @@ Provides unit tests for SSL based authentication portions
of the external_auth app.
"""
-import logging
-import StringIO
import unittest
from django.conf import settings
@@ -22,7 +20,7 @@ from edxmako.middleware import MakoMiddleware
from external_auth.models import ExternalAuthMap
import external_auth.views
from student.tests.factories import UserFactory
-from xmodule.modulestore.exceptions import InsufficientSpecificationError
+from opaque_keys import InvalidKeyError
FEATURES_WITH_SSL_AUTH = settings.FEATURES.copy()
FEATURES_WITH_SSL_AUTH['AUTH_USE_CERTIFICATES'] = True
@@ -193,18 +191,23 @@ class SSLClientTest(TestCase):
This tests to make sure when immediate signup is on that
the user doesn't get presented with the registration page.
"""
- # Expect a NotImplementError from course page as we don't have anything else built
- with self.assertRaisesRegexp(InsufficientSpecificationError,
- 'Must provide one of url, version_guid, package_id'):
+ # Expect an InvalidKeyError from course page as we don't have anything else built
+ with self.assertRaisesRegexp(
+ InvalidKeyError,
+ "<class 'xmodule.modulestore.keys.CourseKey'>: None"
+ ):
self.client.get(
reverse('signup'), follow=True,
- SSL_CLIENT_S_DN=self.AUTH_DN.format(self.USER_NAME, self.USER_EMAIL))
+ SSL_CLIENT_S_DN=self.AUTH_DN.format(self.USER_NAME, self.USER_EMAIL)
+ )
# assert that we are logged in
self.assertIn(SESSION_KEY, self.client.session)
# Now that we are logged in, make sure we don't see the registration page
- with self.assertRaisesRegexp(InsufficientSpecificationError,
- 'Must provide one of url, version_guid, package_id'):
+ with self.assertRaisesRegexp(
+ InvalidKeyError,
+ "<class 'xmodule.modulestore.keys.CourseKey'>: None"
+ ):
self.client.get(reverse('signup'), follow=True)
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
@@ -228,7 +231,6 @@ class SSLClientTest(TestCase):
self.assertIn(reverse('dashboard'), response['location'])
self.assertIn(SESSION_KEY, self.client.session)
-
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
@override_settings(FEATURES=FEATURES_WITH_SSL_AUTH_IMMEDIATE_SIGNUP)
def test_ssl_bad_eamap(self):
diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py
index 0f3baf7b9226136193cf407abe3b76eab48d0389..5a88a5a0191cbb5a0d0a35cc08bdd25c8e69fc9a 100644
--- a/common/djangoapps/external_auth/views.py
+++ b/common/djangoapps/external_auth/views.py
@@ -576,9 +576,8 @@ def course_specific_login(request, course_id):
Dispatcher function for selecting the specific login method
required by the course
"""
- try:
- course = course_from_id(course_id)
- except ItemNotFoundError:
+ course = student.views.course_from_id(course_id)
+ if not course:
# couldn't find the course, will just return vanilla signin page
return _redirect_with_get_querydict('signin_user', request.GET)
@@ -595,9 +594,9 @@ def course_specific_register(request, course_id):
Dispatcher function for selecting the specific registration method
required by the course
"""
- try:
- course = course_from_id(course_id)
- except ItemNotFoundError:
+ course = student.views.course_from_id(course_id)
+
+ if not course:
# couldn't find the course, will just return vanilla registration page
return _redirect_with_get_querydict('register_user', request.GET)
@@ -934,9 +933,3 @@ def provider_xrds(request):
# custom XRDS header necessary for discovery process
response['X-XRDS-Location'] = get_xrds_url('xrds', request)
return response
-
-
-def course_from_id(course_id):
- """Return the CourseDescriptor corresponding to this course_id"""
- course_loc = CourseDescriptor.id_to_location(course_id)
- return modulestore().get_instance(course_id, course_loc)
diff --git a/common/djangoapps/heartbeat/views.py b/common/djangoapps/heartbeat/views.py
index 0cee7116b467418793785f02368e26232abdfd20..9825436e7e51919e0199d62beaa672dca4a5b6e5 100644
--- a/common/djangoapps/heartbeat/views.py
+++ b/common/djangoapps/heartbeat/views.py
@@ -13,6 +13,6 @@ def heartbeat(request):
"""
output = {
'date': datetime.now(UTC).isoformat(),
- 'courses': [course.location.url() for course in modulestore().get_courses()],
+ 'courses': [course.location.to_deprecated_string() for course in modulestore().get_courses()],
}
return HttpResponse(json.dumps(output, indent=4))
diff --git a/common/djangoapps/reverification/models.py b/common/djangoapps/reverification/models.py
index 53b2b659c94fcde0a1d4fdf1017ae25be6cd1712..c7a07ee19aa2e267c6dddd06adc66790f754bdec 100644
--- a/common/djangoapps/reverification/models.py
+++ b/common/djangoapps/reverification/models.py
@@ -7,6 +7,7 @@ import pytz
from django.core.exceptions import ValidationError
from django.db import models
from util.validate_on_save import ValidateOnSaveMixin
+from xmodule_django.models import CourseKeyField
class MidcourseReverificationWindow(ValidateOnSaveMixin, models.Model):
@@ -17,7 +18,7 @@ class MidcourseReverificationWindow(ValidateOnSaveMixin, models.Model):
overlapping time ranges. This is enforced by this class's clean() method.
"""
# the course that this window is attached to
- course_id = models.CharField(max_length=255, db_index=True)
+ course_id = CourseKeyField(max_length=255, db_index=True)
start_date = models.DateTimeField(default=None, null=True, blank=True)
end_date = models.DateTimeField(default=None, null=True, blank=True)
diff --git a/common/djangoapps/reverification/tests/factories.py b/common/djangoapps/reverification/tests/factories.py
index 5a0452b7f7074c6784323913fcc70707a4e5e26f..65669c6b0852c794418dd25ee0784aa37846344b 100644
--- a/common/djangoapps/reverification/tests/factories.py
+++ b/common/djangoapps/reverification/tests/factories.py
@@ -5,6 +5,7 @@ from reverification.models import MidcourseReverificationWindow
from factory.django import DjangoModelFactory
import pytz
from datetime import timedelta, datetime
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
# Factories don't have __init__ methods, and are self documenting
@@ -13,7 +14,7 @@ class MidcourseReverificationWindowFactory(DjangoModelFactory):
""" Creates a generic MidcourseReverificationWindow. """
FACTORY_FOR = MidcourseReverificationWindow
- course_id = u'MITx/999/Robot_Super_Course'
+ course_id = SlashSeparatedCourseKey.from_deprecated_string(u'MITx/999/Robot_Super_Course')
# By default this factory creates a window that is currently open
start_date = datetime.now(pytz.UTC) - timedelta(days=100)
end_date = datetime.now(pytz.UTC) + timedelta(days=100)
diff --git a/common/djangoapps/static_replace/__init__.py b/common/djangoapps/static_replace/__init__.py
index 29dcf7455b030ac19fc58d8d598753a54202c4c2..56f31b2e36728bde6d51e11540607999d8016492 100644
--- a/common/djangoapps/static_replace/__init__.py
+++ b/common/djangoapps/static_replace/__init__.py
@@ -72,7 +72,7 @@ def replace_jump_to_id_urls(text, course_id, jump_to_id_base_url):
return re.sub(_url_replace_regex('/jump_to_id/'), replace_jump_to_id_url, text)
-def replace_course_urls(text, course_id):
+def replace_course_urls(text, course_key):
"""
Replace /course/$stuff urls with /courses/$course_id/$stuff urls
@@ -82,6 +82,8 @@ def replace_course_urls(text, course_id):
returns: text with the links replaced
"""
+ course_id = course_key.to_deprecated_string()
+
def replace_course_url(match):
quote = match.group('quote')
rest = match.group('rest')
diff --git a/common/djangoapps/static_replace/test/test_static_replace.py b/common/djangoapps/static_replace/test/test_static_replace.py
index 1e7521c3a932ac41de63392b8010acd02a93651d..d631b1ffe84990d50e07e1a75376c8c0de96ea6e 100644
--- a/common/djangoapps/static_replace/test/test_static_replace.py
+++ b/common/djangoapps/static_replace/test/test_static_replace.py
@@ -4,12 +4,13 @@ from nose.tools import assert_equals, assert_true, assert_false # pylint: disab
from static_replace import (replace_static_urls, replace_course_urls,
_url_replace_regex)
from mock import patch, Mock
-from xmodule.modulestore import Location
+
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
from xmodule.modulestore.mongo import MongoModuleStore
from xmodule.modulestore.xml import XMLModuleStore
DATA_DIRECTORY = 'data_dir'
-COURSE_ID = 'org/course/run'
+COURSE_KEY = SlashSeparatedCourseKey('org', 'course', 'run')
STATIC_SOURCE = '"/static/file.png"'
@@ -21,8 +22,8 @@ def test_multi_replace():
replace_static_urls(replace_static_urls(STATIC_SOURCE, DATA_DIRECTORY), DATA_DIRECTORY)
)
assert_equals(
- replace_course_urls(course_source, COURSE_ID),
- replace_course_urls(replace_course_urls(course_source, COURSE_ID), COURSE_ID)
+ replace_course_urls(course_source, COURSE_KEY),
+ replace_course_urls(replace_course_urls(course_source, COURSE_KEY), COURSE_KEY)
)
@@ -59,10 +60,10 @@ def test_mongo_filestore(mock_modulestore, mock_static_content):
# Namespace => content url
assert_equals(
'"' + mock_static_content.convert_legacy_static_url_with_course_id.return_value + '"',
- replace_static_urls(STATIC_SOURCE, DATA_DIRECTORY, course_id=COURSE_ID)
+ replace_static_urls(STATIC_SOURCE, DATA_DIRECTORY, course_id=COURSE_KEY)
)
- mock_static_content.convert_legacy_static_url_with_course_id.assert_called_once_with('file.png', COURSE_ID)
+ mock_static_content.convert_legacy_static_url_with_course_id.assert_called_once_with('file.png', COURSE_KEY)
@patch('static_replace.settings')
@@ -101,7 +102,7 @@ def test_static_url_with_query(mock_modulestore, mock_storage):
pre_text = 'EMBED src ="/static/LAlec04_controller.swf?csConfigFile=/c4x/org/course/asset/LAlec04_config.xml"'
post_text = 'EMBED src ="/c4x/org/course/asset/LAlec04_controller.swf?csConfigFile=/c4x/org/course/asset/LAlec04_config.xml"'
- assert_equals(post_text, replace_static_urls(pre_text, DATA_DIRECTORY, COURSE_ID))
+ assert_equals(post_text, replace_static_urls(pre_text, DATA_DIRECTORY, COURSE_KEY))
def test_regex():
diff --git a/common/djangoapps/student/auth.py b/common/djangoapps/student/auth.py
index 97583f69dc300af65906cfd4565549cecf78dccf..505ad9d4dad9e687b8218ab4dd56c31e937dd8f6 100644
--- a/common/djangoapps/student/auth.py
+++ b/common/djangoapps/student/auth.py
@@ -35,7 +35,7 @@ def has_access(user, role):
return True
# if not, then check inferred permissions
if (isinstance(role, (CourseStaffRole, CourseBetaTesterRole)) and
- CourseInstructorRole(role.location).has_user(user)):
+ CourseInstructorRole(role.course_key).has_user(user)):
return True
return False
@@ -81,6 +81,6 @@ def _check_caller_authority(caller, role):
if isinstance(role, (GlobalStaff, CourseCreatorRole)):
raise PermissionDenied
elif isinstance(role, CourseRole): # instructors can change the roles w/in their course
- if not has_access(caller, CourseInstructorRole(role.location)):
+ if not has_access(caller, CourseInstructorRole(role.course_key)):
raise PermissionDenied
diff --git a/common/djangoapps/student/management/commands/anonymized_id_mapping.py b/common/djangoapps/student/management/commands/anonymized_id_mapping.py
index d27c306d8da5aec0bc680492daa8e281dbeee89d..75b61e45ba82c88665158e54a6d5154eb4ea59b8 100644
--- a/common/djangoapps/student/management/commands/anonymized_id_mapping.py
+++ b/common/djangoapps/student/management/commands/anonymized_id_mapping.py
@@ -59,7 +59,7 @@ class Command(BaseCommand):
for student in students:
csv_writer.writerow((
student.id,
- anonymous_id_for_user(student, ''),
+ anonymous_id_for_user(student, None),
anonymous_id_for_user(student, course_id)
))
except IOError:
diff --git a/common/djangoapps/student/management/commands/create_user.py b/common/djangoapps/student/management/commands/create_user.py
index bf848004ce98603195d1619caa5d5de19af529ae..9ba00cabdaf8eb4a82a5dff28750a719fd60f089 100644
--- a/common/djangoapps/student/management/commands/create_user.py
+++ b/common/djangoapps/student/management/commands/create_user.py
@@ -5,6 +5,9 @@ from django.contrib.auth.models import User
from django.core.management.base import BaseCommand
from django.utils import translation
+from opaque_keys import InvalidKeyError
+from xmodule.modulestore.keys import CourseKey
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
from student.models import CourseEnrollment, Registration, create_comments_service_user
from student.views import _do_create_account, AccountValidationError
from track.management.tracked_command import TrackedCommand
@@ -68,6 +71,15 @@ class Command(TrackedCommand):
if not name:
name = options['email'].split('@')[0]
+ # parse out the course into a coursekey
+ if options['course']:
+ try:
+ course = CourseKey.from_string(options['course'])
+ # if it's not a new-style course key, parse it from an old-style
+ # course key
+ except InvalidKeyError:
+ course = SlashSeparatedCourseKey.from_deprecated_string(options['course'])
+
post_data = {
'username': username,
'email': options['email'],
@@ -93,5 +105,5 @@ class Command(TrackedCommand):
print e.message
user = User.objects.get(email=options['email'])
if options['course']:
- CourseEnrollment.enroll(user, options['course'], mode=options['mode'])
+ CourseEnrollment.enroll(user, course, mode=options['mode'])
translation.deactivate()
diff --git a/common/djangoapps/student/migrations/0034_auto__add_courseaccessrole.py b/common/djangoapps/student/migrations/0034_auto__add_courseaccessrole.py
new file mode 100644
index 0000000000000000000000000000000000000000..d6267ecb017ae003750eeb8307127598b01b0db8
--- /dev/null
+++ b/common/djangoapps/student/migrations/0034_auto__add_courseaccessrole.py
@@ -0,0 +1,189 @@
+# -*- coding: utf-8 -*-
+import datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+ def forwards(self, orm):
+ # Adding model 'CourseAccessRole'
+ db.create_table('student_courseaccessrole', (
+ ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)),
+ ('user', self.gf('django.db.models.fields.related.ForeignKey')(to=orm['auth.User'])),
+ ('org', self.gf('django.db.models.fields.CharField')(db_index=True, max_length=64, blank=True)),
+ ('course_id', self.gf('xmodule_django.models.CourseKeyField')(db_index=True, max_length=255, blank=True)),
+ ('role', self.gf('django.db.models.fields.CharField')(max_length=64, db_index=True)),
+ ))
+ db.send_create_signal('student', ['CourseAccessRole'])
+
+ # Adding unique constraint on 'CourseAccessRole', fields ['user', 'org', 'course_id', 'role']
+ db.create_unique('student_courseaccessrole', ['user_id', 'org', 'course_id', 'role'])
+
+
+ # Changing field 'AnonymousUserId.course_id'
+ db.alter_column('student_anonymoususerid', 'course_id', self.gf('xmodule_django.models.CourseKeyField')(max_length=255))
+
+ # Changing field 'CourseEnrollment.course_id'
+ db.alter_column('student_courseenrollment', 'course_id', self.gf('xmodule_django.models.CourseKeyField')(max_length=255))
+
+ # Changing field 'CourseEnrollmentAllowed.course_id'
+ db.alter_column('student_courseenrollmentallowed', 'course_id', self.gf('xmodule_django.models.CourseKeyField')(max_length=255))
+
+ def backwards(self, orm):
+ # Removing unique constraint on 'CourseAccessRole', fields ['user', 'org', 'course_id', 'role']
+ db.delete_unique('student_courseaccessrole', ['user_id', 'org', 'course_id', 'role'])
+
+ # Deleting model 'CourseAccessRole'
+ db.delete_table('student_courseaccessrole')
+
+
+ # Changing field 'AnonymousUserId.course_id'
+ db.alter_column('student_anonymoususerid', 'course_id', self.gf('django.db.models.fields.CharField')(max_length=255))
+
+ # Changing field 'CourseEnrollment.course_id'
+ db.alter_column('student_courseenrollment', 'course_id', self.gf('django.db.models.fields.CharField')(max_length=255))
+
+ # Changing field 'CourseEnrollmentAllowed.course_id'
+ db.alter_column('student_courseenrollmentallowed', 'course_id', self.gf('django.db.models.fields.CharField')(max_length=255))
+
+ models = {
+ 'auth.group': {
+ 'Meta': {'object_name': 'Group'},
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
+ 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
+ },
+ 'auth.permission': {
+ 'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
+ 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+ 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+ },
+ 'auth.user': {
+ 'Meta': {'object_name': 'User'},
+ 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+ 'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}),
+ 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+ 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+ 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+ 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+ 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+ 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+ 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+ 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}),
+ 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'})
+ },
+ 'contenttypes.contenttype': {
+ 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
+ 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+ 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
+ },
+ 'student.anonymoususerid': {
+ 'Meta': {'object_name': 'AnonymousUserId'},
+ 'anonymous_user_id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '32'}),
+ 'course_id': ('xmodule_django.models.CourseKeyField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"})
+ },
+ 'student.courseaccessrole': {
+ 'Meta': {'unique_together': "(('user', 'org', 'course_id', 'role'),)", 'object_name': 'CourseAccessRole'},
+ 'course_id': ('xmodule_django.models.CourseKeyField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'org': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '64', 'blank': 'True'}),
+ 'role': ('django.db.models.fields.CharField', [], {'max_length': '64', 'db_index': 'True'}),
+ 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"})
+ },
+ 'student.courseenrollment': {
+ 'Meta': {'ordering': "('user', 'course_id')", 'unique_together': "(('user', 'course_id'),)", 'object_name': 'CourseEnrollment'},
+ 'course_id': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}),
+ 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'null': 'True', 'db_index': 'True', 'blank': 'True'}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+ 'mode': ('django.db.models.fields.CharField', [], {'default': "'honor'", 'max_length': '100'}),
+ 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"})
+ },
+ 'student.courseenrollmentallowed': {
+ 'Meta': {'unique_together': "(('email', 'course_id'),)", 'object_name': 'CourseEnrollmentAllowed'},
+ 'auto_enroll': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+ 'course_id': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}),
+ 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'null': 'True', 'db_index': 'True', 'blank': 'True'}),
+ 'email': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'})
+ },
+ 'student.loginfailures': {
+ 'Meta': {'object_name': 'LoginFailures'},
+ 'failure_count': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'lockout_until': ('django.db.models.fields.DateTimeField', [], {'null': 'True'}),
+ 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"})
+ },
+ 'student.passwordhistory': {
+ 'Meta': {'object_name': 'PasswordHistory'},
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+ 'time_set': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+ 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"})
+ },
+ 'student.pendingemailchange': {
+ 'Meta': {'object_name': 'PendingEmailChange'},
+ 'activation_key': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '32', 'db_index': 'True'}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'new_email': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}),
+ 'user': ('django.db.models.fields.related.OneToOneField', [], {'to': "orm['auth.User']", 'unique': 'True'})
+ },
+ 'student.pendingnamechange': {
+ 'Meta': {'object_name': 'PendingNameChange'},
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'new_name': ('django.db.models.fields.CharField', [], {'max_length': '255', 'blank': 'True'}),
+ 'rationale': ('django.db.models.fields.CharField', [], {'max_length': '1024', 'blank': 'True'}),
+ 'user': ('django.db.models.fields.related.OneToOneField', [], {'to': "orm['auth.User']", 'unique': 'True'})
+ },
+ 'student.registration': {
+ 'Meta': {'object_name': 'Registration', 'db_table': "'auth_registration'"},
+ 'activation_key': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '32', 'db_index': 'True'}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'unique': 'True'})
+ },
+ 'student.userprofile': {
+ 'Meta': {'object_name': 'UserProfile', 'db_table': "'auth_userprofile'"},
+ 'allow_certificate': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+ 'city': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}),
+ 'country': ('django_countries.fields.CountryField', [], {'max_length': '2', 'null': 'True', 'blank': 'True'}),
+ 'courseware': ('django.db.models.fields.CharField', [], {'default': "'course.xml'", 'max_length': '255', 'blank': 'True'}),
+ 'gender': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '6', 'null': 'True', 'blank': 'True'}),
+ 'goals': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'language': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}),
+ 'level_of_education': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '6', 'null': 'True', 'blank': 'True'}),
+ 'location': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}),
+ 'mailing_address': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}),
+ 'meta': ('django.db.models.fields.TextField', [], {'blank': 'True'}),
+ 'name': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}),
+ 'user': ('django.db.models.fields.related.OneToOneField', [], {'related_name': "'profile'", 'unique': 'True', 'to': "orm['auth.User']"}),
+ 'year_of_birth': ('django.db.models.fields.IntegerField', [], {'db_index': 'True', 'null': 'True', 'blank': 'True'})
+ },
+ 'student.userstanding': {
+ 'Meta': {'object_name': 'UserStanding'},
+ 'account_status': ('django.db.models.fields.CharField', [], {'max_length': '31', 'blank': 'True'}),
+ 'changed_by': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'blank': 'True'}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'standing_last_changed_at': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'blank': 'True'}),
+ 'user': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'standing'", 'unique': 'True', 'to': "orm['auth.User']"})
+ },
+ 'student.usertestgroup': {
+ 'Meta': {'object_name': 'UserTestGroup'},
+ 'description': ('django.db.models.fields.TextField', [], {'blank': 'True'}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'name': ('django.db.models.fields.CharField', [], {'max_length': '32', 'db_index': 'True'}),
+ 'users': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.User']", 'db_index': 'True', 'symmetrical': 'False'})
+ }
+ }
+
+ complete_apps = ['student']
\ No newline at end of file
diff --git a/common/djangoapps/student/tests/test_authz.py b/common/djangoapps/student/tests/test_authz.py
index dee2eb84f4bbeddcd37d8ad2f821e91540f5d996..0d6929c19031fedc6a9c7c3f286fddcda30de49c 100644
--- a/common/djangoapps/student/tests/test_authz.py
+++ b/common/djangoapps/student/tests/test_authz.py
@@ -5,12 +5,12 @@ import mock
from django.test import TestCase
from django.contrib.auth.models import User, AnonymousUser
-from xmodule.modulestore import Location
from django.core.exceptions import PermissionDenied
from student.roles import CourseInstructorRole, CourseStaffRole, CourseCreatorRole
from student.tests.factories import AdminFactory
from student.auth import has_access, add_users, remove_users
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
class CreatorGroupTest(TestCase):
@@ -137,54 +137,54 @@ class CourseGroupTest(TestCase):
self.global_admin = AdminFactory()
self.creator = User.objects.create_user('testcreator', 'testcreator+courses@edx.org', 'foo')
self.staff = User.objects.create_user('teststaff', 'teststaff+courses@edx.org', 'foo')
- self.location = Location('i4x', 'mitX', '101', 'course', 'test')
+ self.course_key = SlashSeparatedCourseKey('mitX', '101', 'test')
def test_add_user_to_course_group(self):
"""
Tests adding user to course group (happy path).
"""
# Create groups for a new course (and assign instructor role to the creator).
- self.assertFalse(has_access(self.creator, CourseInstructorRole(self.location)))
- add_users(self.global_admin, CourseInstructorRole(self.location), self.creator)
- add_users(self.global_admin, CourseStaffRole(self.location), self.creator)
- self.assertTrue(has_access(self.creator, CourseInstructorRole(self.location)))
+ self.assertFalse(has_access(self.creator, CourseInstructorRole(self.course_key)))
+ add_users(self.global_admin, CourseInstructorRole(self.course_key), self.creator)
+ add_users(self.global_admin, CourseStaffRole(self.course_key), self.creator)
+ self.assertTrue(has_access(self.creator, CourseInstructorRole(self.course_key)))
# Add another user to the staff role.
- self.assertFalse(has_access(self.staff, CourseStaffRole(self.location)))
- add_users(self.creator, CourseStaffRole(self.location), self.staff)
- self.assertTrue(has_access(self.staff, CourseStaffRole(self.location)))
+ self.assertFalse(has_access(self.staff, CourseStaffRole(self.course_key)))
+ add_users(self.creator, CourseStaffRole(self.course_key), self.staff)
+ self.assertTrue(has_access(self.staff, CourseStaffRole(self.course_key)))
def test_add_user_to_course_group_permission_denied(self):
"""
Verifies PermissionDenied if caller of add_user_to_course_group is not instructor role.
"""
- add_users(self.global_admin, CourseInstructorRole(self.location), self.creator)
- add_users(self.global_admin, CourseStaffRole(self.location), self.creator)
+ add_users(self.global_admin, CourseInstructorRole(self.course_key), self.creator)
+ add_users(self.global_admin, CourseStaffRole(self.course_key), self.creator)
with self.assertRaises(PermissionDenied):
- add_users(self.staff, CourseStaffRole(self.location), self.staff)
+ add_users(self.staff, CourseStaffRole(self.course_key), self.staff)
def test_remove_user_from_course_group(self):
"""
Tests removing user from course group (happy path).
"""
- add_users(self.global_admin, CourseInstructorRole(self.location), self.creator)
- add_users(self.global_admin, CourseStaffRole(self.location), self.creator)
+ add_users(self.global_admin, CourseInstructorRole(self.course_key), self.creator)
+ add_users(self.global_admin, CourseStaffRole(self.course_key), self.creator)
- add_users(self.creator, CourseStaffRole(self.location), self.staff)
- self.assertTrue(has_access(self.staff, CourseStaffRole(self.location)))
+ add_users(self.creator, CourseStaffRole(self.course_key), self.staff)
+ self.assertTrue(has_access(self.staff, CourseStaffRole(self.course_key)))
- remove_users(self.creator, CourseStaffRole(self.location), self.staff)
- self.assertFalse(has_access(self.staff, CourseStaffRole(self.location)))
+ remove_users(self.creator, CourseStaffRole(self.course_key), self.staff)
+ self.assertFalse(has_access(self.staff, CourseStaffRole(self.course_key)))
- remove_users(self.creator, CourseInstructorRole(self.location), self.creator)
- self.assertFalse(has_access(self.creator, CourseInstructorRole(self.location)))
+ remove_users(self.creator, CourseInstructorRole(self.course_key), self.creator)
+ self.assertFalse(has_access(self.creator, CourseInstructorRole(self.course_key)))
def test_remove_user_from_course_group_permission_denied(self):
"""
Verifies PermissionDenied if caller of remove_user_from_course_group is not instructor role.
"""
- add_users(self.global_admin, CourseInstructorRole(self.location), self.creator)
+ add_users(self.global_admin, CourseInstructorRole(self.course_key), self.creator)
another_staff = User.objects.create_user('another', 'teststaff+anothercourses@edx.org', 'foo')
- add_users(self.global_admin, CourseStaffRole(self.location), self.creator, self.staff, another_staff)
+ add_users(self.global_admin, CourseStaffRole(self.course_key), self.creator, self.staff, another_staff)
with self.assertRaises(PermissionDenied):
- remove_users(self.staff, CourseStaffRole(self.location), another_staff)
+ remove_users(self.staff, CourseStaffRole(self.course_key), another_staff)
diff --git a/common/djangoapps/student/tests/test_auto_auth.py b/common/djangoapps/student/tests/test_auto_auth.py
index 67834127431a93efbf035891cdd9990f3d1d325e..6228962d0b858b99a2ba51f919ec4000bf1b0647 100644
--- a/common/djangoapps/student/tests/test_auto_auth.py
+++ b/common/djangoapps/student/tests/test_auto_auth.py
@@ -6,6 +6,7 @@ from django_comment_common.models import (
from django_comment_common.utils import seed_permissions_roles
from student.models import CourseEnrollment, UserProfile
from util.testing import UrlResetMixin
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
from mock import patch
@@ -23,6 +24,8 @@ class AutoAuthEnabledTestCase(UrlResetMixin, TestCase):
super(AutoAuthEnabledTestCase, self).setUp()
self.url = '/auto_auth'
self.client = Client()
+ self.course_id = 'edX/Test101/2014_Spring'
+ self.course_key = SlashSeparatedCourseKey.from_deprecated_string(self.course_id)
def test_create_user(self):
"""
@@ -83,43 +86,39 @@ class AutoAuthEnabledTestCase(UrlResetMixin, TestCase):
def test_course_enrollment(self):
# Create a user and enroll in a course
- course_id = "edX/Test101/2014_Spring"
- self._auto_auth(username='test', course_id=course_id)
+ self._auto_auth(username='test', course_id=self.course_id)
# Check that a course enrollment was created for the user
self.assertEqual(CourseEnrollment.objects.count(), 1)
- enrollment = CourseEnrollment.objects.get(course_id=course_id)
+ enrollment = CourseEnrollment.objects.get(course_id=self.course_key)
self.assertEqual(enrollment.user.username, "test")
def test_double_enrollment(self):
# Create a user and enroll in a course
- course_id = "edX/Test101/2014_Spring"
- self._auto_auth(username='test', course_id=course_id)
+ self._auto_auth(username='test', course_id=self.course_id)
# Make the same call again, re-enrolling the student in the same course
- self._auto_auth(username='test', course_id=course_id)
+ self._auto_auth(username='test', course_id=self.course_id)
# Check that only one course enrollment was created for the user
self.assertEqual(CourseEnrollment.objects.count(), 1)
- enrollment = CourseEnrollment.objects.get(course_id=course_id)
+ enrollment = CourseEnrollment.objects.get(course_id=self.course_key)
self.assertEqual(enrollment.user.username, "test")
def test_set_roles(self):
-
- course_id = "edX/Test101/2014_Spring"
- seed_permissions_roles(course_id)
- course_roles = dict((r.name, r) for r in Role.objects.filter(course_id=course_id))
+ seed_permissions_roles(self.course_key)
+ course_roles = dict((r.name, r) for r in Role.objects.filter(course_id=self.course_key))
self.assertEqual(len(course_roles), 4) # sanity check
# Student role is assigned by default on course enrollment.
- self._auto_auth(username='a_student', course_id=course_id)
+ self._auto_auth(username='a_student', course_id=self.course_id)
user = User.objects.get(username='a_student')
user_roles = user.roles.all()
self.assertEqual(len(user_roles), 1)
self.assertEqual(user_roles[0], course_roles[FORUM_ROLE_STUDENT])
- self._auto_auth(username='a_moderator', course_id=course_id, roles='Moderator')
+ self._auto_auth(username='a_moderator', course_id=self.course_id, roles='Moderator')
user = User.objects.get(username='a_moderator')
user_roles = user.roles.all()
self.assertEqual(
@@ -128,7 +127,7 @@ class AutoAuthEnabledTestCase(UrlResetMixin, TestCase):
course_roles[FORUM_ROLE_MODERATOR]]))
# check multiple roles work.
- self._auto_auth(username='an_admin', course_id=course_id,
+ self._auto_auth(username='an_admin', course_id=self.course_id,
roles='{},{}'.format(FORUM_ROLE_MODERATOR, FORUM_ROLE_ADMINISTRATOR))
user = User.objects.get(username='an_admin')
user_roles = user.roles.all()
diff --git a/common/djangoapps/student/tests/test_bulk_email_settings.py b/common/djangoapps/student/tests/test_bulk_email_settings.py
index 6b903d9979058b7d47a97f0fa47e06f1e14e1af6..f164aea76186e40d57bbb943b1b6e630fd8720d5 100644
--- a/common/djangoapps/student/tests/test_bulk_email_settings.py
+++ b/common/djangoapps/student/tests/test_bulk_email_settings.py
@@ -15,6 +15,7 @@ from student.tests.factories import UserFactory, CourseEnrollmentFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
from bulk_email.models import CourseAuthorization
@@ -100,7 +101,10 @@ class TestStudentDashboardEmailViewXMLBacked(ModuleStoreTestCase):
# Create student account
student = UserFactory.create()
- CourseEnrollmentFactory.create(user=student, course_id=self.course_name)
+ CourseEnrollmentFactory.create(
+ user=student,
+ course_id=SlashSeparatedCourseKey.from_deprecated_string(self.course_name)
+ )
self.client.login(username=student.username, password="test")
try:
diff --git a/common/djangoapps/student/tests/test_login.py b/common/djangoapps/student/tests/test_login.py
index 34083c90b47dd7cb82fc45db69ae130a940ff22c..00fc67282ae476ccfa9f8d974ebad28267490f06 100644
--- a/common/djangoapps/student/tests/test_login.py
+++ b/common/djangoapps/student/tests/test_login.py
@@ -20,6 +20,7 @@ from xmodule.modulestore.inheritance import own_metadata
from xmodule.modulestore.django import editable_modulestore
from external_auth.models import ExternalAuthMap
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
TEST_DATA_MIXED_MODULESTORE = mixed_store_config(settings.COMMON_TEST_DATA_ROOT, {})
@@ -275,7 +276,10 @@ class UtilFnTest(TestCase):
COURSE_ID = u'org/num/run' # pylint: disable=C0103
COURSE_URL = u'/courses/{}/otherstuff'.format(COURSE_ID) # pylint: disable=C0103
NON_COURSE_URL = u'/blahblah' # pylint: disable=C0103
- self.assertEqual(_parse_course_id_from_string(COURSE_URL), COURSE_ID)
+ self.assertEqual(
+ _parse_course_id_from_string(COURSE_URL),
+ SlashSeparatedCourseKey.from_deprecated_string(COURSE_ID)
+ )
self.assertIsNone(_parse_course_id_from_string(NON_COURSE_URL))
@@ -320,7 +324,7 @@ class ExternalAuthShibTest(ModuleStoreTestCase):
"""
Tests the _get_course_enrollment_domain utility function
"""
- self.assertIsNone(_get_course_enrollment_domain("I/DONT/EXIST"))
+ self.assertIsNone(_get_course_enrollment_domain(SlashSeparatedCourseKey("I", "DONT", "EXIST")))
self.assertIsNone(_get_course_enrollment_domain(self.course.id))
self.assertEqual(self.shib_course.enrollment_domain, _get_course_enrollment_domain(self.shib_course.id))
@@ -340,7 +344,7 @@ class ExternalAuthShibTest(ModuleStoreTestCase):
Tests the redirects when visiting course-specific URL with @login_required.
Should vary by course depending on its enrollment_domain
"""
- TARGET_URL = reverse('courseware', args=[self.course.id]) # pylint: disable=C0103
+ TARGET_URL = reverse('courseware', args=[self.course.id.to_deprecated_string()]) # pylint: disable=C0103
noshib_response = self.client.get(TARGET_URL, follow=True)
self.assertEqual(noshib_response.redirect_chain[-1],
('http://testserver/accounts/login?next={url}'.format(url=TARGET_URL), 302))
@@ -348,7 +352,7 @@ class ExternalAuthShibTest(ModuleStoreTestCase):
.format(platform_name=settings.PLATFORM_NAME)))
self.assertEqual(noshib_response.status_code, 200)
- TARGET_URL_SHIB = reverse('courseware', args=[self.shib_course.id]) # pylint: disable=C0103
+ TARGET_URL_SHIB = reverse('courseware', args=[self.shib_course.id.to_deprecated_string()]) # pylint: disable=C0103
shib_response = self.client.get(**{'path': TARGET_URL_SHIB,
'follow': True,
'REMOTE_USER': self.extauth.external_id,
diff --git a/common/djangoapps/student/tests/test_userstanding.py b/common/djangoapps/student/tests/test_userstanding.py
index 19fe957ab76694e88666d4ed5942a7a68e5babd5..c6f9cdcbc7e48395c83d6a62c6b0d5168db2e2d9 100644
--- a/common/djangoapps/student/tests/test_userstanding.py
+++ b/common/djangoapps/student/tests/test_userstanding.py
@@ -53,7 +53,7 @@ class UserStandingTest(TestCase):
try:
self.some_url = reverse('dashboard')
except NoReverseMatch:
- self.some_url = '/course'
+ self.some_url = '/course/'
# since it's only possible to disable accounts from lms, we're going
# to skip tests for cms
diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py
index 34c30c45035afba565fff00a4994892a067fd7a8..61affc3d1f0b366f5c7a12e209b0deaebbaa3419 100644
--- a/common/djangoapps/student/views.py
+++ b/common/djangoapps/student/views.py
@@ -55,6 +55,7 @@ from dark_lang.models import DarkLangConfig
from xmodule.course_module import CourseDescriptor
from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
from xmodule.modulestore import XML_MODULESTORE_TYPE, Location
from collections import namedtuple
@@ -132,8 +133,7 @@ def index(request, extra_context={}, user=AnonymousUser()):
def course_from_id(course_id):
"""Return the CourseDescriptor corresponding to this course_id"""
- course_loc = CourseDescriptor.id_to_location(course_id)
- return modulestore().get_instance(course_id, course_loc)
+ return modulestore().get_course(course_id)
day_pattern = re.compile(r'\s\d+,\s')
multimonth_pattern = re.compile(r'\s?\-\s?\S+\s')
@@ -269,8 +269,8 @@ def get_course_enrollment_pairs(user, course_org_filter, org_filter_out_set):
a student's dashboard.
"""
for enrollment in CourseEnrollment.enrollments_for_user(user):
- try:
- course = course_from_id(enrollment.course_id)
+ course = course_from_id(enrollment.course_id)
+ if course:
# if we are in a Microsite, then filter out anything that is not
# attributed (by ORG) to that Microsite
@@ -282,7 +282,7 @@ def get_course_enrollment_pairs(user, course_org_filter, org_filter_out_set):
continue
yield (course, enrollment)
- except ItemNotFoundError:
+ else:
log.error("User {0} enrolled in non-existent course {1}"
.format(user.username, enrollment.course_id))
@@ -478,13 +478,13 @@ def dashboard(request):
# Global staff can see what courses errored on their dashboard
staff_access = False
errored_courses = {}
- if has_access(user, 'global', 'staff'):
+ if has_access(user, 'staff', 'global'):
# Show any courses that errored on load
staff_access = True
errored_courses = modulestore().get_errored_courses()
show_courseware_links_for = frozenset(course.id for course, _enrollment in course_enrollment_pairs
- if has_access(request.user, course, 'load'))
+ if has_access(request.user, 'load', course))
course_modes = {course.id: complete_course_mode_info(course.id, enrollment) for course, enrollment in course_enrollment_pairs}
cert_statuses = {course.id: cert_info(request.user, course) for course, _enrollment in course_enrollment_pairs}
@@ -617,10 +617,11 @@ def change_enrollment(request):
user = request.user
action = request.POST.get("enrollment_action")
- course_id = request.POST.get("course_id")
- if course_id is None:
+ if 'course_id' not in request.POST:
return HttpResponseBadRequest(_("Course id not specified"))
+ course_id = SlashSeparatedCourseKey.from_deprecated_string(request.POST.get("course_id"))
+
if not user.is_authenticated():
return HttpResponseForbidden()
@@ -634,7 +635,7 @@ def change_enrollment(request):
.format(user.username, course_id))
return HttpResponseBadRequest(_("Course id is invalid"))
- if not has_access(user, course, 'enroll'):
+ if not has_access(user, 'enroll', course):
return HttpResponseBadRequest(_("Enrollment is closed"))
# see if we have already filled up all allowed enrollments
@@ -648,7 +649,7 @@ def change_enrollment(request):
available_modes = CourseMode.modes_for_course(course_id)
if len(available_modes) > 1:
return HttpResponse(
- reverse("course_modes_choose", kwargs={'course_id': course_id})
+ reverse("course_modes_choose", kwargs={'course_id': course_id.to_deprecated_string()})
)
current_mode = available_modes[0]
@@ -664,7 +665,7 @@ def change_enrollment(request):
# the user to the shopping cart page always, where they can reasonably discern the status of their cart,
# whether things got added, etc
- shoppingcart.views.add_course_to_cart(request, course_id)
+ shoppingcart.views.add_course_to_cart(request, course_id.to_deprecated_string())
return HttpResponse(
reverse("shoppingcart.views.show_cart")
)
@@ -686,7 +687,7 @@ def _parse_course_id_from_string(input_str):
"""
m_obj = re.match(r'^/courses/(?P<course_id>[^/]+/[^/]+/[^/]+)', input_str)
if m_obj:
- return m_obj.group('course_id')
+ return SlashSeparatedCourseKey.from_deprecated_string(m_obj.group('course_id'))
return None
@@ -696,12 +697,12 @@ def _get_course_enrollment_domain(course_id):
@param course_id:
@return:
"""
- try:
- course = course_from_id(course_id)
- return course.enrollment_domain
- except ItemNotFoundError:
+ course = course_from_id(course_id)
+ if course is None:
return None
+ return course.enrollment_domain
+
@ensure_csrf_cookie
def accounts_login(request):
@@ -1378,6 +1379,9 @@ def auto_auth(request):
full_name = request.GET.get('full_name', username)
is_staff = request.GET.get('staff', None)
course_id = request.GET.get('course_id', None)
+ course_key = None
+ if course_id:
+ course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
role_names = [v.strip() for v in request.GET.get('roles', '').split(',') if v.strip()]
# Get or create the user object
@@ -1413,12 +1417,12 @@ def auto_auth(request):
reg.save()
# Enroll the user in a course
- if course_id is not None:
- CourseEnrollment.enroll(user, course_id)
+ if course_key is not None:
+ CourseEnrollment.enroll(user, course_key)
# Apply the roles
for role_name in role_names:
- role = Role.objects.get(name=role_name, course_id=course_id)
+ role = Role.objects.get(name=role_name, course_id=course_key)
user.roles.add(role)
# Log in as the user
@@ -1865,15 +1869,16 @@ def change_email_settings(request):
user = request.user
course_id = request.POST.get("course_id")
+ course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
receive_emails = request.POST.get("receive_emails")
if receive_emails:
- optout_object = Optout.objects.filter(user=user, course_id=course_id)
+ optout_object = Optout.objects.filter(user=user, course_id=course_key)
if optout_object:
optout_object.delete()
log.info(u"User {0} ({1}) opted in to receive emails from course {2}".format(user.username, user.email, course_id))
track.views.server_track(request, "change-email-settings", {"receive_emails": "yes", "course": course_id}, page='dashboard')
else:
- Optout.objects.get_or_create(user=user, course_id=course_id)
+ Optout.objects.get_or_create(user=user, course_id=course_key)
log.info(u"User {0} ({1}) opted out of receiving emails from course {2}".format(user.username, user.email, course_id))
track.views.server_track(request, "change-email-settings", {"receive_emails": "no", "course": course_id}, page='dashboard')
@@ -1889,7 +1894,7 @@ def token(request):
the token was issued. This will be stored with the user along with
the id for identification purposes in the backend.
'''
- course_id = request.GET.get("course_id")
+ course_id = SlashSeparatedCourseKey.from_deprecated_string(request.GET.get("course_id"))
course = course_from_id(course_id)
dtnow = datetime.datetime.now()
dtutcnow = datetime.datetime.utcnow()
diff --git a/common/djangoapps/track/contexts.py b/common/djangoapps/track/contexts.py
index 070ac10ebdea35c8d7325c472d99668455718520..bb8673480e3c2ea26445338c63c21ee2287081b6 100644
--- a/common/djangoapps/track/contexts.py
+++ b/common/djangoapps/track/contexts.py
@@ -1,7 +1,8 @@
"""Generates common contexts"""
import logging
-from xmodule.course_module import CourseDescriptor
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
+from opaque_keys import InvalidKeyError
from util.request import COURSE_REGEX
log = logging.getLogger(__name__)
@@ -9,15 +10,24 @@ log = logging.getLogger(__name__)
def course_context_from_url(url):
"""
- Extracts the course_id from the given `url` and passes it on to
+ Extracts the course_context from the given `url` and passes it on to
`course_context_from_course_id()`.
"""
url = url or ''
match = COURSE_REGEX.match(url)
- course_id = ''
+ course_id = None
if match:
- course_id = match.group('course_id') or ''
+ course_id_string = match.group('course_id')
+ try:
+ course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id_string)
+ except InvalidKeyError:
+ log.warning(
+ 'unable to parse course_id "{course_id}"'.format(
+ course_id=course_id_string
+ ),
+ exc_info=True
+ )
return course_context_from_course_id(course_id)
@@ -34,23 +44,12 @@ def course_context_from_course_id(course_id):
}
"""
-
- course_id = course_id or ''
- context = {
- 'course_id': course_id,
- 'org_id': ''
+ if course_id is None:
+ return {'course_id': '', 'org_id': ''}
+
+ # TODO: Make this accept any CourseKey, and serialize it using .to_string
+ assert(isinstance(course_id, SlashSeparatedCourseKey))
+ return {
+ 'course_id': course_id.to_deprecated_string(),
+ 'org_id': course_id.org,
}
-
- if course_id:
- try:
- location = CourseDescriptor.id_to_location(course_id)
- context['org_id'] = location.org
- except ValueError:
- log.warning(
- 'Unable to parse course_id "{course_id}"'.format(
- course_id=course_id
- ),
- exc_info=True
- )
-
- return context
diff --git a/common/djangoapps/user_api/middleware.py b/common/djangoapps/user_api/middleware.py
index 3a33db7143598aba42c038c97dc631c6a6e632ef..97387e50072a01ee11660eec0e20ecfbe00ad789 100644
--- a/common/djangoapps/user_api/middleware.py
+++ b/common/djangoapps/user_api/middleware.py
@@ -5,6 +5,7 @@ Adds user's tags to tracking event context.
from track.contexts import COURSE_REGEX
from eventtracking import tracker
from user_api.models import UserCourseTag
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
class UserTagsEventContextMiddleware(object):
@@ -19,6 +20,7 @@ class UserTagsEventContextMiddleware(object):
course_id = None
if match:
course_id = match.group('course_id')
+ course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
context = {}
@@ -29,7 +31,7 @@ class UserTagsEventContextMiddleware(object):
context['course_user_tags'] = dict(
UserCourseTag.objects.filter(
user=request.user.pk,
- course_id=course_id
+ course_id=course_key,
).values_list('key', 'value')
)
else:
diff --git a/common/djangoapps/user_api/models.py b/common/djangoapps/user_api/models.py
index 76b8cd5053f4f0ce349962ba66ab7047b80d18fa..e5a31db820c1a35662aa88fa058ae91e832c9e1e 100644
--- a/common/djangoapps/user_api/models.py
+++ b/common/djangoapps/user_api/models.py
@@ -2,6 +2,8 @@ from django.contrib.auth.models import User
from django.core.validators import RegexValidator
from django.db import models
+from xmodule_django.models import CourseKeyField
+
class UserPreference(models.Model):
"""A user's preference, stored as generic text to be processed by client"""
@@ -44,7 +46,7 @@ class UserCourseTag(models.Model):
"""
user = models.ForeignKey(User, db_index=True, related_name="+")
key = models.CharField(max_length=255, db_index=True)
- course_id = models.CharField(max_length=255, db_index=True)
+ course_id = CourseKeyField(max_length=255, db_index=True)
value = models.TextField()
class Meta: # pylint: disable=missing-docstring
diff --git a/common/djangoapps/user_api/tests/factories.py b/common/djangoapps/user_api/tests/factories.py
index 535e888a59b99a033a13367bb007d5ecc889e2bc..e5bd4debac6b663b39659c81235f69004bafb786 100644
--- a/common/djangoapps/user_api/tests/factories.py
+++ b/common/djangoapps/user_api/tests/factories.py
@@ -3,6 +3,7 @@ from factory.django import DjangoModelFactory
from factory import SubFactory
from student.tests.factories import UserFactory
from user_api.models import UserPreference, UserCourseTag
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
# Factories don't have __init__ methods, and are self documenting
# pylint: disable=W0232, C0111
@@ -18,6 +19,6 @@ class UserCourseTagFactory(DjangoModelFactory):
FACTORY_FOR = UserCourseTag
user = SubFactory(UserFactory)
- course_id = 'org/course/run'
+ course_id = SlashSeparatedCourseKey('org', 'course', 'run')
key = None
value = None
diff --git a/common/djangoapps/user_api/tests/test_user_service.py b/common/djangoapps/user_api/tests/test_user_service.py
index f63f702bcbb2c01305032edc1a775a757cf58322..63be81b049d60351cee64e626c38c4b918b00486 100644
--- a/common/djangoapps/user_api/tests/test_user_service.py
+++ b/common/djangoapps/user_api/tests/test_user_service.py
@@ -5,6 +5,7 @@ from django.test import TestCase
from student.tests.factories import UserFactory
from user_api import user_service
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
class TestUserService(TestCase):
@@ -13,7 +14,7 @@ class TestUserService(TestCase):
"""
def setUp(self):
self.user = UserFactory.create()
- self.course_id = 'test_org/test_course_number/test_run'
+ self.course_id = SlashSeparatedCourseKey('test_org', 'test_course_number', 'test_run')
self.test_key = 'test_key'
def test_get_set_course_tag(self):
diff --git a/common/djangoapps/util/request.py b/common/djangoapps/util/request.py
index 813a3347d37f3493586121907a62b4209b0ca9d4..b3b369cff4ec47e6d29abd9b7385d943901b6f06 100644
--- a/common/djangoapps/util/request.py
+++ b/common/djangoapps/util/request.py
@@ -3,6 +3,7 @@ import re
from django.conf import settings
from microsite_configuration import microsite
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
COURSE_REGEX = re.compile(r'^.*?/courses/(?P<course_id>[^/]+/[^/]+/[^/]+)')
@@ -26,11 +27,17 @@ def course_id_from_url(url):
"""
Extracts the course_id from the given `url`.
"""
- url = url or ''
+ if not url:
+ return None
match = COURSE_REGEX.match(url)
- course_id = ''
- if match:
- course_id = match.group('course_id') or ''
- return course_id
+ if match is None:
+ return None
+
+ course_id = match.group('course_id')
+
+ if course_id is None:
+ return None
+
+ return SlashSeparatedCourseKey.from_deprecated_string(course_id)
diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py
index 77d590777f7306100110208c457ad0538924e48f..be9b98e4ba265680b6c8ad1162e3325514e4b855 100644
--- a/common/djangoapps/xmodule_modifiers.py
+++ b/common/djangoapps/xmodule_modifiers.py
@@ -18,7 +18,7 @@ from xmodule.vertical_module import VerticalModule
from xmodule.x_module import shim_xmodule_js, XModuleDescriptor, XModule
from lms.lib.xblock.runtime import quote_slashes
from xmodule.modulestore import MONGO_MODULESTORE_TYPE
-from xmodule.modulestore.django import modulestore, loc_mapper
+from xmodule.modulestore.django import modulestore
log = logging.getLogger(__name__)
@@ -33,7 +33,7 @@ def wrap_fragment(fragment, new_content):
return wrapper_frag
-def wrap_xblock(runtime_class, block, view, frag, context, display_name_only=False, extra_data=None): # pylint: disable=unused-argument
+def wrap_xblock(runtime_class, block, view, frag, context, usage_id_serializer, display_name_only=False, extra_data=None): # pylint: disable=unused-argument
"""
Wraps the results of rendering an XBlock view in a standard <section> with identifying
data so that the appropriate javascript module can be loaded onto it.
@@ -43,6 +43,8 @@ def wrap_xblock(runtime_class, block, view, frag, context, display_name_only=Fal
:param view: The name of the view that rendered the fragment being wrapped
:param frag: The :class:`Fragment` to be wrapped
:param context: The context passed to the view being rendered
+ :param usage_id_serializer: A function to serialize the block's usage_id for use by the
+ front-end Javascript Runtime.
:param display_name_only: If true, don't render the fragment content at all.
Instead, just render the `display_name` of `block`
:param extra_data: A dictionary with extra data values to be set on the wrapper
@@ -74,13 +76,14 @@ def wrap_xblock(runtime_class, block, view, frag, context, display_name_only=Fal
data['runtime-class'] = runtime_class
data['runtime-version'] = frag.js_init_version
data['block-type'] = block.scope_ids.block_type
- data['usage-id'] = quote_slashes(unicode(block.scope_ids.usage_id))
+ data['usage-id'] = usage_id_serializer(block.scope_ids.usage_id)
template_context = {
'content': block.display_name if display_name_only else frag.content,
'classes': css_classes,
'display_name': block.display_name_with_default,
- 'data_attributes': u' '.join(u'data-{}="{}"'.format(key, value) for key, value in data.items()),
+ 'data_attributes': u' '.join(u'data-{}="{}"'.format(key, value)
+ for key, value in data.iteritems()),
}
return wrap_fragment(frag, render_to_string('xblock_wrapper.html', template_context))
@@ -145,7 +148,7 @@ def grade_histogram(module_id):
WHERE courseware_studentmodule.module_id=%s
GROUP BY courseware_studentmodule.grade"""
# Passing module_id this way prevents sql-injection.
- cursor.execute(q, [module_id])
+ cursor.execute(q, [module_id.to_deprecated_string()])
grades = list(cursor.fetchall())
grades.sort(key=lambda x: x[0]) # Add ORDER BY to sql query?
@@ -167,14 +170,14 @@ def add_staff_markup(user, block, view, frag, context): # pylint: disable=unuse
# TODO: make this more general, eg use an XModule attribute instead
if isinstance(block, VerticalModule):
# check that the course is a mongo backed Studio course before doing work
- is_mongo_course = modulestore().get_modulestore_type(block.course_id) == MONGO_MODULESTORE_TYPE
+ is_mongo_course = modulestore().get_modulestore_type(block.location.course_key) == MONGO_MODULESTORE_TYPE
is_studio_course = block.course_edit_method == "Studio"
if is_studio_course and is_mongo_course:
- # get relative url/location of unit in Studio
- locator = loc_mapper().translate_location(block.course_id, block.location, False, True)
- # build edit link to unit in CMS
- edit_link = "//" + settings.CMS_BASE + locator.url_reverse('unit', '')
+ # build edit link to unit in CMS. Can't use reverse here as lms doesn't load cms's urls.py
+ # reverse for contentstore.views.unit_handler
+ edit_link = "//" + settings.CMS_BASE + '/unit/' + unicode(block.location)
+
# return edit link in rendered HTML for display
return wrap_fragment(frag, render_to_string("edit_unit_link.html", {'frag_content': frag.content, 'edit_link': edit_link}))
else:
@@ -183,7 +186,7 @@ def add_staff_markup(user, block, view, frag, context): # pylint: disable=unuse
if isinstance(block, SequenceModule):
return frag
- block_id = block.id
+ block_id = block.location
if block.has_score and settings.FEATURES.get('DISPLAY_HISTOGRAMS_TO_STAFF'):
histogram = grade_histogram(block_id)
render_histogram = len(histogram) > 0