Make course ids and usage ids opaque to LMS and Studio [partial commit]

This commit adds the non-courseware lms/djangoapps and lms/lib.

These keys are now objects with a limited interface, and the particular
internal representation is managed by the data storage layer (the
modulestore).

For the LMS, there should be no outward-facing changes to the system.
The keys are, for now, a change to internal representation only. For
Studio, the new serialized form of the keys is used in urls, to allow
for further migration in the future.

Co-Author: Andy Armstrong <andya@edx.org>
Co-Author: Christina Roberts <christina@edx.org>
Co-Author: David Baumgold <db@edx.org>
Co-Author: Diana Huang <dkh@edx.org>
Co-Author: Don Mitchell <dmitchell@edx.org>
Co-Author: Julia Hansbrough <julia@edx.org>
Co-Author: Nimisha Asthagiri <nasthagiri@edx.org>
Co-Author: Sarina Canelake <sarina@edx.org>

[LMS-2370]

lms/djangoapps/analytics/basic.py

@@ -70,7 +70,7 @@ def dump_grading_context(course):
             subgrader.index = 1
             graders[subgrader.type] = subgrader
     msg += hbar
-    msg += "Listing grading context for course %s\n" % course.id
+    msg += "Listing grading context for course %s\n" % course.id.to_deprecated_string()
 
     gcontext = course.grading_context
     msg += "graded sections:\n"

lms/djangoapps/analytics/tests/test_basic.py

@@ -5,6 +5,7 @@ Tests for instructor.basic
 from django.test import TestCase
 from student.models import CourseEnrollment
 from student.tests.factories import UserFactory
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 from analytics.basic import enrolled_students_features, AVAILABLE_FEATURES, STUDENT_FEATURES, PROFILE_FEATURES
 
@@ -13,14 +14,14 @@ class TestAnalyticsBasic(TestCase):
     """ Test basic analytics functions. """
 
     def setUp(self):
-        self.course_id = 'some/robot/course/id'
+        self.course_key = SlashSeparatedCourseKey('robot', 'course', 'id')
         self.users = tuple(UserFactory() for _ in xrange(30))
-        self.ces = tuple(CourseEnrollment.enroll(user, self.course_id)
+        self.ces = tuple(CourseEnrollment.enroll(user, self.course_key)
                          for user in self.users)
 
     def test_enrolled_students_features_username(self):
         self.assertIn('username', AVAILABLE_FEATURES)
-        userreports = enrolled_students_features(self.course_id, ['username'])
+        userreports = enrolled_students_features(self.course_key, ['username'])
         self.assertEqual(len(userreports), len(self.users))
         for userreport in userreports:
             self.assertEqual(userreport.keys(), ['username'])
@@ -30,7 +31,7 @@ class TestAnalyticsBasic(TestCase):
         query_features = ('username', 'name', 'email')
         for feature in query_features:
             self.assertIn(feature, AVAILABLE_FEATURES)
-        userreports = enrolled_students_features(self.course_id, query_features)
+        userreports = enrolled_students_features(self.course_key, query_features)
         self.assertEqual(len(userreports), len(self.users))
         for userreport in userreports:
             self.assertEqual(set(userreport.keys()), set(query_features))

lms/djangoapps/analytics/tests/test_distributions.py

@@ -4,6 +4,7 @@ from django.test import TestCase
 from nose.tools import raises
 from student.models import CourseEnrollment
 from student.tests.factories import UserFactory
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 from analytics.distributions import profile_distribution, AVAILABLE_PROFILE_FEATURES
 
@@ -12,7 +13,7 @@ class TestAnalyticsDistributions(TestCase):
     '''Test analytics distribution gathering.'''
 
     def setUp(self):
-        self.course_id = 'some/robot/course/id'
+        self.course_id = SlashSeparatedCourseKey('robot', 'course', 'id')
 
         self.users = [UserFactory(
             profile__gender=['m', 'f', 'o'][i % 3],
@@ -53,7 +54,7 @@ class TestAnalyticsDistributionsNoData(TestCase):
     '''Test analytics distribution gathering.'''
 
     def setUp(self):
-        self.course_id = 'some/robot/course/id'
+        self.course_id = SlashSeparatedCourseKey('robot', 'course', 'id')
 
         self.users = [UserFactory(
             profile__year_of_birth=i + 1930,

lms/djangoapps/bulk_email/forms.py

@@ -8,9 +8,10 @@ from django.core.exceptions import ValidationError
 
 from bulk_email.models import CourseEmailTemplate, COURSE_EMAIL_MESSAGE_BODY_TAG, CourseAuthorization
 
-from courseware.courses import get_course_by_id
+from opaque_keys import InvalidKeyError
 from xmodule.modulestore import XML_MODULESTORE_TYPE
 from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 log = logging.getLogger(__name__)
 
@@ -57,22 +58,26 @@ class CourseAuthorizationAdminForm(forms.ModelForm):  # pylint: disable=R0924
 
     def clean_course_id(self):
         """Validate the course id"""
-        course_id = self.cleaned_data["course_id"]
+        cleaned_id = self.cleaned_data["course_id"]
         try:
-            # Just try to get the course descriptor.
-            # If we can do that, it's a real course.
-            get_course_by_id(course_id, depth=1)
-        except Exception as exc:
-            msg = 'Error encountered ({0})'.format(str(exc).capitalize())
-            msg += u' --- Entered course id was: "{0}". '.format(course_id)
-            msg += 'Please recheck that you have supplied a course id in the format: ORG/COURSE/RUN'
+            course_id = SlashSeparatedCourseKey.from_deprecated_string(cleaned_id)
+        except InvalidKeyError:
+            msg = u'Course id invalid.' 
+            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)
+
+        if not modulestore().has_course(course_id):
+            msg = u'COURSE NOT FOUND'
+            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)
 
         # Now, try and discern if it is a Studio course - HTML editor doesn't work with XML courses
         is_studio_course = modulestore().get_modulestore_type(course_id) != XML_MODULESTORE_TYPE
         if not is_studio_course:
             msg = "Course Email feature is only available for courses authored in Studio. "
-            msg += '"{0}" appears to be an XML backed course.'.format(course_id)
+            msg += '"{0}" appears to be an XML backed course.'.format(course_id.to_deprecated_string())
             raise forms.ValidationError(msg)
 
-        return course_id
+        return course_id.to_deprecated_string()

lms/djangoapps/bulk_email/models.py

@@ -19,6 +19,8 @@ from django.db import models, transaction
 from html_to_text import html_to_text
 from mail_utils import wrap_message
 
+from xmodule_django.models import CourseKeyField
+
 log = logging.getLogger(__name__)
 
 # Bulk email to_options - the send to options that users can
@@ -63,7 +65,7 @@ class CourseEmail(Email):
         (SEND_TO_STAFF, 'Staff and instructors'),
         (SEND_TO_ALL, 'All')
     )
-    course_id = models.CharField(max_length=255, db_index=True)
+    course_id = CourseKeyField(max_length=255, db_index=True)
     to_option = models.CharField(max_length=64, choices=TO_OPTION_CHOICES, default=SEND_TO_MYSELF)
 
     def __unicode__(self):
@@ -127,7 +129,7 @@ class Optout(models.Model):
     # We need to first create the 'user' column with some sort of default in order to run the data migration,
     # and given the unique index, 'null' is the best default value.
     user = models.ForeignKey(User, db_index=True, null=True)
-    course_id = models.CharField(max_length=255, db_index=True)
+    course_id = CourseKeyField(max_length=255, db_index=True)
 
     class Meta:  # pylint: disable=C0111
         unique_together = ('user', 'course_id')
@@ -220,7 +222,7 @@ class CourseAuthorization(models.Model):
     Enable the course email feature on a course-by-course basis.
     """
     # The course that these features are attached to.
-    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 enable instructor email
     email_enabled = models.BooleanField(default=False)
@@ -247,4 +249,5 @@ class CourseAuthorization(models.Model):
         not_en = "Not "
         if self.email_enabled:
             not_en = ""
-        return u"Course '{}': Instructor Email {}Enabled".format(self.course_id, not_en)
+        # pylint: disable=no-member
+        return u"Course '{}': Instructor Email {}Enabled".format(self.course_id.to_deprecated_string(), not_en)

lms/djangoapps/bulk_email/tasks.py

@@ -107,8 +107,8 @@ def _get_recipient_queryset(user_id, to_option, course_id, course_location):
     if to_option == SEND_TO_MYSELF:
         recipient_qset = User.objects.filter(id=user_id)
     else:
-        staff_qset = CourseStaffRole(course_location).users_with_role()
-        instructor_qset = CourseInstructorRole(course_location).users_with_role()
+        staff_qset = CourseStaffRole(course_id).users_with_role()
+        instructor_qset = CourseInstructorRole(course_id).users_with_role()
         recipient_qset = staff_qset | instructor_qset
         if to_option == SEND_TO_ALL:
             # We also require students to have activated their accounts to
@@ -129,7 +129,7 @@ def _get_course_email_context(course):
     """
     Returns context arguments to apply to all emails, independent of recipient.
     """
-    course_id = course.id
+    course_id = course.id.to_deprecated_string()
     course_title = course.display_name
     course_url = 'https://{}{}'.format(
         settings.SITE_NAME,
@@ -160,9 +160,9 @@ def perform_delegate_email_batches(entry_id, course_id, task_input, action_name)
     # Perfunctory check, since expansion is made for convenience of other task
     # code that doesn't need the entry_id.
     if course_id != entry.course_id:
-        format_msg = u"Course id conflict: explicit value {} does not match task value {}"
-        log.warning("Task %s: %s", task_id, format_msg.format(course_id, entry.course_id))
-        raise ValueError("Course id conflict: explicit value does not match task value")
+        format_msg = u"Course id conflict: explicit value %r does not match task value %r"
+        log.warning("Task %s: " + format_msg, task_id, course_id, entry.course_id)
+        raise ValueError(format_msg % (course_id, entry.course_id))
 
     # Fetch the CourseEmail.
     email_id = task_input['email_id']
@@ -188,16 +188,17 @@ def perform_delegate_email_batches(entry_id, course_id, task_input, action_name)
 
     # Sanity check that course for email_obj matches that of the task referencing it.
     if course_id != email_obj.course_id:
-        format_msg = u"Course id conflict: explicit value {} does not match email value {}"
-        log.warning("Task %s: %s", task_id, format_msg.format(course_id, entry.course_id))
-        raise ValueError("Course id conflict: explicit value does not match email value")
+        format_msg = u"Course id conflict: explicit value %r does not match email value %r"
+        log.warning("Task %s: " + format_msg, task_id, course_id, email_obj.course_id)
+        raise ValueError(format_msg % (course_id, email_obj.course_id))
 
     # Fetch the course object.
-    try:
-        course = get_course(course_id)
-    except ValueError:
-        log.exception("Task %s: course not found: %s", task_id, course_id)
-        raise
+    course = get_course(course_id)
+
+    if course is None:
+        msg = "Task %s: course not found: %s"
+        log.error(msg, task_id, course_id)
+        raise ValueError(msg % (task_id, course_id))
 
     # Get arguments that will be passed to every subtask.
     to_option = email_obj.to_option
@@ -369,15 +370,14 @@ def _get_source_address(course_id, course_title):
     """
     course_title_no_quotes = re.sub(r'"', '', course_title)
 
-    # The course_id is assumed to be in the form 'org/course_num/run',
-    # so pull out the course_num.  Then make sure that it can be used
+    # For the email address, get the course.  Then make sure that it can be used
     # in an email address, by substituting a '_' anywhere a non-(ascii, period, or dash)
     # character appears.
-    course_num = Location.parse_course_id(course_id)['course']
-    invalid_chars = re.compile(r"[^\w.-]")
-    course_num = invalid_chars.sub('_', course_num)
-
-    from_addr = u'"{0}" Course Staff <{1}-{2}>'.format(course_title_no_quotes, course_num, settings.BULK_EMAIL_DEFAULT_FROM_EMAIL)
+    from_addr = u'"{0}" Course Staff <{1}-{2}>'.format(
+        course_title_no_quotes,
+        re.sub(r"[^\w.-]", '_', course_id.course),
+        settings.BULK_EMAIL_DEFAULT_FROM_EMAIL
+    )
     return from_addr
 
 

lms/djangoapps/bulk_email/tests/test_course_optout.py

@@ -47,7 +47,7 @@ class TestOptoutCourseEmails(ModuleStoreTestCase):
     def navigate_to_email_view(self):
         """Navigate to the instructor dash's email view"""
         # Pull up email view on instructor dashboard
-        url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id.to_deprecated_string()})
         response = self.client.get(url)
         email_link = '<a href="#" onclick="goto(\'Email\')" class="None">Email</a>'
         # If this fails, it is likely because ENABLE_INSTRUCTOR_EMAIL is set to False
@@ -69,7 +69,7 @@ class TestOptoutCourseEmails(ModuleStoreTestCase):
         url = reverse('change_email_settings')
         # This is a checkbox, so on the post of opting out (that is, an Un-check of the box),
         # the Post that is sent will not contain 'receive_emails'
-        response = self.client.post(url, {'course_id': self.course.id})
+        response = self.client.post(url, {'course_id': self.course.id.to_deprecated_string()})
         self.assertEquals(json.loads(response.content), {'success': True})
 
         self.client.logout()
@@ -77,7 +77,7 @@ class TestOptoutCourseEmails(ModuleStoreTestCase):
         self.client.login(username=self.instructor.username, password="test")
         self.navigate_to_email_view()
 
-        url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id.to_deprecated_string()})
         test_email = {
             'action': 'Send email',
             'to_option': 'all',
@@ -96,7 +96,7 @@ class TestOptoutCourseEmails(ModuleStoreTestCase):
         Make sure student receives course email after opting in.
         """
         url = reverse('change_email_settings')
-        response = self.client.post(url, {'course_id': self.course.id, 'receive_emails': 'on'})
+        response = self.client.post(url, {'course_id': self.course.id.to_deprecated_string(), 'receive_emails': 'on'})
         self.assertEquals(json.loads(response.content), {'success': True})
 
         self.client.logout()
@@ -106,7 +106,7 @@ class TestOptoutCourseEmails(ModuleStoreTestCase):
         self.client.login(username=self.instructor.username, password="test")
         self.navigate_to_email_view()
 
-        url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id.to_deprecated_string()})
         test_email = {
             'action': 'Send email',
             'to_option': 'all',

lms/djangoapps/bulk_email/tests/test_email.py

@@ -51,10 +51,10 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase):
         course_title = u"ẗëṡẗ title ｲ乇丂ｲ ﾶ乇丂丂ﾑg乇 ｷo尺 ﾑﾚﾚ тэѕт мэѕѕаБэ"
         self.course = CourseFactory.create(display_name=course_title)
 
-        self.instructor = InstructorFactory(course=self.course.location)
+        self.instructor = InstructorFactory(course=self.course.id)
 
         # Create staff
-        self.staff = [StaffFactory(course=self.course.location)
+        self.staff = [StaffFactory(course=self.course.id)
                       for _ in xrange(STAFF_COUNT)]
 
         # Create students
@@ -68,7 +68,7 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase):
         self.client.login(username=self.instructor.username, password="test")
 
         # Pull up email view on instructor dashboard
-        self.url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id})
+        self.url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id.to_deprecated_string()})
         response = self.client.get(self.url)
         email_link = '<a href="#" onclick="goto(\'Email\')" class="None">Email</a>'
         # If this fails, it is likely because ENABLE_INSTRUCTOR_EMAIL is set to False

lms/djangoapps/bulk_email/tests/test_err_handling.py

@@ -17,6 +17,7 @@ from django.db import DatabaseError
 from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 from student.tests.factories import UserFactory, AdminFactory, CourseEnrollmentFactory
 
 from bulk_email.models import CourseEmail, SEND_TO_ALL
@@ -51,7 +52,7 @@ class TestEmailErrors(ModuleStoreTestCase):
 
         # load initial content (since we don't run migrations as part of tests):
         call_command("loaddata", "course_email_template.json")
-        self.url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id})
+        self.url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id.to_deprecated_string()})
 
     def tearDown(self):
         patch.stopall()
@@ -171,12 +172,13 @@ class TestEmailErrors(ModuleStoreTestCase):
         """
         Tests exception when the course in the email doesn't exist
         """
-        course_id = "I/DONT/EXIST"
+        course_id = SlashSeparatedCourseKey("I", "DONT", "EXIST")
         email = CourseEmail(course_id=course_id)
         email.save()
         entry = InstructorTask.create(course_id, "task_type", "task_key", "task_input", self.instructor)
         task_input = {"email_id": email.id}  # pylint: disable=E1101
-        with self.assertRaisesRegexp(ValueError, "Course not found"):
+        # (?i) is a regex for ignore case
+        with self.assertRaisesRegexp(ValueError, r"(?i)course not found"):
             perform_delegate_email_batches(entry.id, course_id, task_input, "action_name")  # pylint: disable=E1101
 
     def test_nonexistent_to_option(self):
@@ -205,7 +207,7 @@ class TestEmailErrors(ModuleStoreTestCase):
         """
         Tests exception when the course_id in CourseEmail is not the same as one explicitly passed in.
         """
-        email = CourseEmail(course_id="bogus_course_id", to_option=SEND_TO_ALL)
+        email = CourseEmail(course_id=SlashSeparatedCourseKey("bogus", "course", "id"), to_option=SEND_TO_ALL)
         email.save()
         entry = InstructorTask.create(self.course.id, "task_type", "task_key", "task_input", self.instructor)
         task_input = {"email_id": email.id}  # pylint: disable=E1101

lms/djangoapps/bulk_email/tests/test_forms.py

@@ -11,12 +11,13 @@ from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
 from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
 
 from xmodule.modulestore.django import modulestore
-from xmodule.modulestore import XML_MODULESTORE_TYPE, Location
+from xmodule.modulestore import XML_MODULESTORE_TYPE
 
 from mock import patch
 
 from bulk_email.models import CourseAuthorization
 from bulk_email.forms import CourseAuthorizationAdminForm
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 
 @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
@@ -38,7 +39,7 @@ class CourseAuthorizationFormTest(ModuleStoreTestCase):
         # Initially course shouldn't be authorized
         self.assertFalse(CourseAuthorization.instructor_email_enabled(self.course.id))
         # Test authorizing the course, which should totally work
-        form_data = {'course_id': self.course.id, 'email_enabled': True}
+        form_data = {'course_id': self.course.id.to_deprecated_string(), 'email_enabled': True}
         form = CourseAuthorizationAdminForm(data=form_data)
         # Validation should work
         self.assertTrue(form.is_valid())
@@ -51,7 +52,7 @@ class CourseAuthorizationFormTest(ModuleStoreTestCase):
         # Initially course shouldn't be authorized
         self.assertFalse(CourseAuthorization.instructor_email_enabled(self.course.id))
         # Test authorizing the course, which should totally work
-        form_data = {'course_id': self.course.id, 'email_enabled': True}
+        form_data = {'course_id': self.course.id.to_deprecated_string(), 'email_enabled': True}
         form = CourseAuthorizationAdminForm(data=form_data)
         # Validation should work
         self.assertTrue(form.is_valid())
@@ -60,7 +61,7 @@ class CourseAuthorizationFormTest(ModuleStoreTestCase):
         self.assertTrue(CourseAuthorization.instructor_email_enabled(self.course.id))
 
         # Now make a new course authorization with the same course id that tries to turn email off
-        form_data = {'course_id': self.course.id, 'email_enabled': False}
+        form_data = {'course_id': self.course.id.to_deprecated_string(), 'email_enabled': False}
         form = CourseAuthorizationAdminForm(data=form_data)
         # Validation should not work because course_id field is unique
         self.assertFalse(form.is_valid())
@@ -77,16 +78,31 @@ class CourseAuthorizationFormTest(ModuleStoreTestCase):
     @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True})
     def test_form_typo(self):
         # Munge course id
-        bad_id = self.course.id + '_typo'
+        bad_id = SlashSeparatedCourseKey(u'Broken{}'.format(self.course.id.org), '', self.course.id.run + '_typo')
 
-        form_data = {'course_id': bad_id, 'email_enabled': True}
+        form_data = {'course_id': bad_id.to_deprecated_string(), 'email_enabled': True}
         form = CourseAuthorizationAdminForm(data=form_data)
         # Validation shouldn't work
         self.assertFalse(form.is_valid())
 
-        msg = u'Error encountered (Course not found.)'
-        msg += u' --- Entered course id was: "{0}". '.format(bad_id)
-        msg += 'Please recheck that you have supplied a course id in the format: ORG/COURSE/RUN'
+        msg = u'COURSE NOT FOUND'
+        msg += u' --- Entered course id was: "{0}". '.format(bad_id.to_deprecated_string())
+        msg += 'Please recheck that you have supplied a valid course id.'
+        self.assertEquals(msg, form._errors['course_id'][0])  # pylint: disable=protected-access
+
+        with self.assertRaisesRegexp(ValueError, "The CourseAuthorization could not be created because the data didn't validate."):
+            form.save()
+
+    @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True})
+    def test_form_invalid_key(self):
+        form_data = {'course_id': "asd::**!@#$%^&*())//foobar!!", 'email_enabled': True}
+        form = CourseAuthorizationAdminForm(data=form_data)
+        # Validation shouldn't work
+        self.assertFalse(form.is_valid())
+
+        msg = u'Course id invalid.'
+        msg += u' --- Entered course id was: "asd::**!@#$%^&*())//foobar!!". '
+        msg += 'Please recheck that you have supplied a valid course id.'
         self.assertEquals(msg, form._errors['course_id'][0])  # pylint: disable=protected-access
 
         with self.assertRaisesRegexp(ValueError, "The CourseAuthorization could not be created because the data didn't validate."):
@@ -95,16 +111,14 @@ class CourseAuthorizationFormTest(ModuleStoreTestCase):
     @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True})
     def test_course_name_only(self):
         # Munge course id - common
-        bad_id = Location.parse_course_id(self.course.id)['name']
-
-        form_data = {'course_id': bad_id, 'email_enabled': True}
+        form_data = {'course_id': self.course.id.run, 'email_enabled': True}
         form = CourseAuthorizationAdminForm(data=form_data)
         # Validation shouldn't work
         self.assertFalse(form.is_valid())
 
         error_msg = form._errors['course_id'][0]
-        self.assertIn(u'--- Entered course id was: "{0}". '.format(bad_id), error_msg)
-        self.assertIn(u'Please recheck that you have supplied a course id in the format: ORG/COURSE/RUN', error_msg)
+        self.assertIn(u'--- Entered course id was: "{0}". '.format(self.course.id.run), error_msg)
+        self.assertIn(u'Please recheck that you have supplied a valid course id.', error_msg)
 
         with self.assertRaisesRegexp(ValueError, "The CourseAuthorization could not be created because the data didn't validate."):
             form.save()
@@ -116,17 +130,17 @@ class CourseAuthorizationXMLFormTest(ModuleStoreTestCase):
 
     @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True})
     def test_xml_course_authorization(self):
-        course_id = 'edX/toy/2012_Fall'
+        course_id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')
         # Assert this is an XML course
         self.assertEqual(modulestore().get_modulestore_type(course_id), XML_MODULESTORE_TYPE)
 
-        form_data = {'course_id': course_id, 'email_enabled': True}
+        form_data = {'course_id': course_id.to_deprecated_string(), 'email_enabled': True}
         form = CourseAuthorizationAdminForm(data=form_data)
         # Validation shouldn't work
         self.assertFalse(form.is_valid())
 
         msg = u"Course Email feature is only available for courses authored in Studio. "
-        msg += u'"{0}" appears to be an XML backed course.'.format(course_id)
+        msg += u'"{0}" appears to be an XML backed course.'.format(course_id.to_deprecated_string())
         self.assertEquals(msg, form._errors['course_id'][0])  # pylint: disable=protected-access
 
         with self.assertRaisesRegexp(ValueError, "The CourseAuthorization could not be created because the data didn't validate."):

lms/djangoapps/bulk_email/tests/test_models.py

@@ -10,13 +10,14 @@ from student.tests.factories import UserFactory
 from mock import patch
 
 from bulk_email.models import CourseEmail, SEND_TO_STAFF, CourseEmailTemplate, CourseAuthorization
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 
 class CourseEmailTest(TestCase):
     """Test the CourseEmail model."""
 
     def test_creation(self):
-        course_id = 'abc/123/doremi'
+        course_id = SlashSeparatedCourseKey('abc', '123', 'doremi')
         sender = UserFactory.create()
         to_option = SEND_TO_STAFF
         subject = "dummy subject"
@@ -29,7 +30,7 @@ class CourseEmailTest(TestCase):
         self.assertEquals(email.sender, sender)
 
     def test_bad_to_option(self):
-        course_id = 'abc/123/doremi'
+        course_id = SlashSeparatedCourseKey('abc', '123', 'doremi')
         sender = UserFactory.create()
         to_option = "fake"
         subject = "dummy subject"
@@ -109,7 +110,7 @@ class CourseAuthorizationTest(TestCase):
 
     @patch.dict(settings.FEATURES, {'REQUIRE_COURSE_EMAIL_AUTH': True})
     def test_creation_auth_on(self):
-        course_id = 'abc/123/doremi'
+        course_id = SlashSeparatedCourseKey('abc', '123', 'doremi')
         # Test that course is not authorized by default
         self.assertFalse(CourseAuthorization.instructor_email_enabled(course_id))
 
@@ -135,7 +136,7 @@ class CourseAuthorizationTest(TestCase):
 
     @patch.dict(settings.FEATURES, {'REQUIRE_COURSE_EMAIL_AUTH': False})
     def test_creation_auth_off(self):
-        course_id = 'blahx/blah101/ehhhhhhh'
+        course_id = SlashSeparatedCourseKey('blahx', 'blah101', 'ehhhhhhh')
         # Test that course is authorized by default, since auth is turned off
         self.assertTrue(CourseAuthorization.instructor_email_enabled(course_id))
 

lms/djangoapps/bulk_email/tests/test_tasks.py

@@ -35,6 +35,7 @@ from instructor_task.subtasks import update_subtask_status, SubtaskStatus
 from instructor_task.models import InstructorTask
 from instructor_task.tests.test_base import InstructorTaskCourseTestCase
 from instructor_task.tests.factories import InstructorTaskFactory
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 
 class TestTaskFailure(Exception):
@@ -119,7 +120,7 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
 
     def test_email_undefined_course(self):
         # Check that we fail when passing in a course that doesn't exist.
-        task_entry = self._create_input_entry(course_id="bogus/course/id")
+        task_entry = self._create_input_entry(course_id=SlashSeparatedCourseKey("bogus", "course", "id"))
         with self.assertRaises(ValueError):
             self._run_task_with_mock_celery(send_bulk_course_email, task_entry.id, task_entry.task_id)
 

lms/djangoapps/certificates/management/commands/cert_whitelist.py

 from django.core.management.base import BaseCommand, CommandError
 from optparse import make_option
+from opaque_keys import InvalidKeyError
+from xmodule.modulestore.keys import CourseKey
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 from certificates.models import CertificateWhitelist
 from django.contrib.auth.models import User
 
@@ -48,6 +51,14 @@ class Command(BaseCommand):
         course_id = options['course_id']
         if not course_id:
             raise CommandError("You must specify a course-id")
+
+        # try to parse the serialized course key into a CourseKey
+        try:
+            course = CourseKey.from_string(course_id)
+        except InvalidKeyError:
+            log.warning("Course id %s could not be parsed as a CourseKey; falling back to SSCK.from_dep_str", course_id)
+            course = SlashSeparatedCourseKey.from_deprecated_string(course_id)
+
         if options['add'] and options['del']:
             raise CommandError("Either remove or add a user, not both")
 
@@ -60,14 +71,14 @@ class Command(BaseCommand):
 
             cert_whitelist, created = \
                 CertificateWhitelist.objects.get_or_create(
-                    user=user, course_id=course_id)
+                    user=user, course_id=course)
             if options['add']:
                 cert_whitelist.whitelist = True
             elif options['del']:
                 cert_whitelist.whitelist = False
             cert_whitelist.save()
 
-        whitelist = CertificateWhitelist.objects.filter(course_id=course_id)
+        whitelist = CertificateWhitelist.objects.filter(course_id=course)
         print "User whitelist for course {0}:\n{1}".format(course_id,
               '\n'.join(["{0} {1} {2}".format(
                   u.user.username, u.user.email, u.whitelist)

lms/djangoapps/certificates/management/commands/gen_cert_report.py

@@ -40,8 +40,7 @@ class Command(BaseCommand):
         for course_id in [course  # all courses in COURSE_LISTINGS
                 for sub in settings.COURSE_LISTINGS
                     for course in settings.COURSE_LISTINGS[sub]]:
-            course_loc = CourseDescriptor.id_to_location(course_id)
-            course = modulestore().get_instance(course_id, course_loc)
+            course = modulestore().get_course(course_id)
             if course.has_ended():
                 yield course_id
 

lms/djangoapps/certificates/management/commands/regenerate_user.py

@@ -62,7 +62,7 @@ class Command(BaseCommand):
             student = User.objects.get(username=user, courseenrollment__course_id=course_id)
 
         print "Fetching course data for {0}".format(course_id)
-        course = modulestore().get_instance(course_id, CourseDescriptor.id_to_location(course_id), depth=2)
+        course = modulestore().get_course(course_id, depth=2)
 
         if not options['noop']:
             # Add the certificate request to the queue

lms/djangoapps/certificates/management/commands/ungenerated_certs.py

-from django.core.management.base import BaseCommand
+from django.core.management.base import BaseCommand, CommandError
 from certificates.models import certificate_status_for_student
 from certificates.queue import XQueueCertInterface
 from django.contrib.auth.models import User
 from optparse import make_option
 from django.conf import settings
+from opaque_keys import InvalidKeyError
+from xmodule.modulestore.keys import CourseKey
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 from xmodule.course_module import CourseDescriptor
 from xmodule.modulestore.django import modulestore
 from certificates.models import CertificateStatuses
@@ -66,25 +69,23 @@ class Command(BaseCommand):
         STATUS_INTERVAL = 500
 
         if options['course']:
-            ended_courses = [options['course']]
+            # try to parse out the course from the serialized form
+            try:
+                course = CourseKey.from_string(options['course'])
+            except InvalidKeyError:
+                log.warning("Course id %s could not be parsed as a CourseKey; falling back to SSCK.from_dep_str", course_id)
+                course = SlashSeparatedCourseKey.from_deprecated_string(options['course'])
+            ended_courses = [course]
         else:
-            # Find all courses that have ended
-            ended_courses = []
-            for course_id in [course  # all courses in COURSE_LISTINGS
-                              for sub in settings.COURSE_LISTINGS
-                              for course in settings.COURSE_LISTINGS[sub]]:
-                course_loc = CourseDescriptor.id_to_location(course_id)
-                course = modulestore().get_instance(course_id, course_loc)
-                if course.has_ended():
-                    ended_courses.append(course_id)
-
-        for course_id in ended_courses:
+            raise CommandError("You must specify a course")
+
+        for course_key in ended_courses:
             # prefetch all chapters/sequentials by saying depth=2
-            course = modulestore().get_instance(course_id, CourseDescriptor.id_to_location(course_id), depth=2)
+            course = modulestore().get_course(course_key, depth=2)
 
-            print "Fetching enrolled students for {0}".format(course_id)
+            print "Fetching enrolled students for {0}".format(course_key.to_deprecated_string())
             enrolled_students = User.objects.filter(
-                courseenrollment__course_id=course_id)
+                courseenrollment__course_id=course_key)
 
             xq = XQueueCertInterface()
             if options['insecure']:
@@ -108,9 +109,9 @@ class Command(BaseCommand):
                     start = datetime.datetime.now(UTC)
 
                 if certificate_status_for_student(
-                        student, course_id)['status'] in valid_statuses:
+                        student, course_key)['status'] in valid_statuses:
                     if not options['noop']:
                         # Add the certificate request to the queue
-                        ret = xq.add_cert(student, course_id, course=course)
+                        ret = xq.add_cert(student, course_key, course=course)
                         if ret == 'generating':
                             print '{0} - {1}'.format(student, ret)

lms/djangoapps/certificates/models.py

@@ -2,6 +2,7 @@ from django.contrib.auth.models import User
 from django.db import models
 from datetime import datetime
 from model_utils import Choices
+from xmodule_django.models import CourseKeyField, NoneToEmptyManager
 
 """
 Certificates are created for a student and an offering of a course.
@@ -71,14 +72,17 @@ class CertificateWhitelist(models.Model):
     embargoed country restriction list
     (allow_certificate set to False in userprofile).
     """
+
+    objects = NoneToEmptyManager()
+
     user = models.ForeignKey(User)
-    course_id = models.CharField(max_length=255, blank=True, default='')
+    course_id = CourseKeyField(max_length=255, blank=True, default=None)
     whitelist = models.BooleanField(default=0)
 
 
 class GeneratedCertificate(models.Model):
     user = models.ForeignKey(User)
-    course_id = models.CharField(max_length=255, blank=True, default='')
+    course_id = CourseKeyField(max_length=255, blank=True, default=None)
     verify_uuid = models.CharField(max_length=32, blank=True, default='')
     download_uuid = models.CharField(max_length=32, blank=True, default='')
     download_url = models.CharField(max_length=128, blank=True,  default='')

lms/djangoapps/certificates/queue.py

@@ -130,7 +130,7 @@ class XQueueCertInterface(object):
 
         Arguments:
           student   - User.object
-          course_id - courseenrollment.course_id (string)
+          course_id - courseenrollment.course_id (CourseKey)
           forced_grade - a string indicating a grade parameter to pass with
                          the certificate request. If this is given, grading
                          will be skipped.
@@ -181,16 +181,15 @@ class XQueueCertInterface(object):
             mode_is_verified = (enrollment_mode == GeneratedCertificate.MODES.verified)
             user_is_verified = SoftwareSecurePhotoVerification.user_is_verified(student)
             user_is_reverified = SoftwareSecurePhotoVerification.user_is_reverified_for_all(course_id, student)
-            course_id_dict = Location.parse_course_id(course_id)
             cert_mode = enrollment_mode
             if (mode_is_verified and user_is_verified and user_is_reverified):
-                template_pdf = "certificate-template-{org}-{course}-verified.pdf".format(**course_id_dict)
+                template_pdf = "certificate-template-{id.org}-{id.course}-verified.pdf".format(id=course_id)
             elif (mode_is_verified and not (user_is_verified and user_is_reverified)):
-                template_pdf = "certificate-template-{org}-{course}.pdf".format(**course_id_dict)
+                template_pdf = "certificate-template-{id.org}-{id.course}.pdf".format(id=course_id)
                 cert_mode = GeneratedCertificate.MODES.honor
             else:
                 # honor code and audit students
-                template_pdf = "certificate-template-{org}-{course}.pdf".format(**course_id_dict)
+                template_pdf = "certificate-template-{id.org}-{id.course}.pdf".format(id=course_id)
             if forced_grade:
                 grade['grade'] = forced_grade
 
@@ -219,7 +218,7 @@ class XQueueCertInterface(object):
                     contents = {
                         'action': 'create',
                         'username': student.username,
-                        'course_id': course_id,
+                        'course_id': course_id.to_deprecated_string(),
                         'name': profile.name,
                         'grade': grade['grade'],
                         'template_pdf': template_pdf,

lms/djangoapps/certificates/views.py

@@ -5,6 +5,7 @@ from django.views.decorators.csrf import csrf_exempt
 from django.http import HttpResponse
 import json
 from dogapi import dog_stats_api
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 from capa.xqueue_interface import XQUEUE_METRIC_NAME
 
 logger = logging.getLogger(__name__)
@@ -27,21 +28,23 @@ def update_certificate(request):
         xqueue_header = json.loads(request.POST.get('xqueue_header'))
 
         try:
+            course_key = SlashSeparatedCourseKey.from_deprecated_string(xqueue_body['course_id'])
+
             cert = GeneratedCertificate.objects.get(
-                   user__username=xqueue_body['username'],
-                   course_id=xqueue_body['course_id'],
-                   key=xqueue_header['lms_key'])
+                user__username=xqueue_body['username'],
+                course_id=course_key,
+                key=xqueue_header['lms_key'])
 
         except GeneratedCertificate.DoesNotExist:
             logger.critical('Unable to lookup certificate\n'
-                         'xqueue_body: {0}\n'
-                         'xqueue_header: {1}'.format(
-                                      xqueue_body, xqueue_header))
+                            'xqueue_body: {0}\n'
+                            'xqueue_header: {1}'.format(
+                                xqueue_body, xqueue_header))
 
             return HttpResponse(json.dumps({
-                            'return_code': 1,
-                            'content': 'unable to lookup key'}),
-                             mimetype='application/json')
+                'return_code': 1,
+                'content': 'unable to lookup key'}),
+                mimetype='application/json')
 
         if 'error' in xqueue_body:
             cert.status = status.error

lms/djangoapps/class_dashboard/test/test_dashboard_data.py → lms/djangoapps/class_dashboard/tests/test_dashboard_data.py

@@ -14,7 +14,6 @@ from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
 from courseware.tests.factories import StudentModuleFactory
 from student.tests.factories import UserFactory, CourseEnrollmentFactory, AdminFactory
 from capa.tests.response_xml_factory import StringResponseXMLFactory
-from xmodule.modulestore import Location
 
 from class_dashboard.dashboard_data import (get_problem_grade_distribution, get_sequential_open_distrib,
                                             get_problem_set_grade_distrib, get_d3_problem_grade_distrib,
@@ -82,7 +81,7 @@ class TestGetProblemGradeDistribution(ModuleStoreTestCase):
                     max_grade=1 if i < j else 0.5,
                     student=user,
                     course_id=self.course.id,
-                    module_state_key=Location(self.item.location).url(),
+                    module_state_key=self.item.location,
                     state=json.dumps({'attempts': self.attempts}),
                 )
 
@@ -90,7 +89,7 @@ class TestGetProblemGradeDistribution(ModuleStoreTestCase):
                 StudentModuleFactory.create(
                     course_id=self.course.id,
                     module_type='sequential',
-                    module_state_key=Location(self.item.location).url(),
+                    module_state_key=self.item.location,
                 )
 
     def test_get_problem_grade_distribution(self):
@@ -156,7 +155,7 @@ class TestGetProblemGradeDistribution(ModuleStoreTestCase):
 
     def test_get_students_problem_grades(self):
 
-        attributes = '?module_id=' + self.item.location.url()
+        attributes = '?module_id=' + self.item.location.to_deprecated_string()
         request = self.request_factory.get(reverse('get_students_problem_grades') + attributes)
 
         response = get_students_problem_grades(request)
@@ -174,7 +173,7 @@ class TestGetProblemGradeDistribution(ModuleStoreTestCase):
     def test_get_students_problem_grades_max(self):
 
         with patch('class_dashboard.dashboard_data.MAX_SCREEN_LIST_LENGTH', 2):
-            attributes = '?module_id=' + self.item.location.url()
+            attributes = '?module_id=' + self.item.location.to_deprecated_string()
             request = self.request_factory.get(reverse('get_students_problem_grades') + attributes)
 
             response = get_students_problem_grades(request)
@@ -188,7 +187,7 @@ class TestGetProblemGradeDistribution(ModuleStoreTestCase):
     def test_get_students_problem_grades_csv(self):
 
         tooltip = 'P1.2.1 Q1 - 3382 Students (100%: 1/1 questions)'
-        attributes = '?module_id=' + self.item.location.url() + '&tooltip=' + tooltip + '&csv=true'
+        attributes = '?module_id=' + self.item.location.to_deprecated_string() + '&tooltip=' + tooltip + '&csv=true'
         request = self.request_factory.get(reverse('get_students_problem_grades') + attributes)
 
         response = get_students_problem_grades(request)
@@ -208,7 +207,7 @@ class TestGetProblemGradeDistribution(ModuleStoreTestCase):
 
     def test_get_students_opened_subsection(self):
 
-        attributes = '?module_id=' + self.item.location.url()
+        attributes = '?module_id=' + self.item.location.to_deprecated_string()
         request = self.request_factory.get(reverse('get_students_opened_subsection') + attributes)
 
         response = get_students_opened_subsection(request)
@@ -221,7 +220,7 @@ class TestGetProblemGradeDistribution(ModuleStoreTestCase):
 
         with patch('class_dashboard.dashboard_data.MAX_SCREEN_LIST_LENGTH', 2):
 
-            attributes = '?module_id=' + self.item.location.url()
+            attributes = '?module_id=' + self.item.location.to_deprecated_string()
             request = self.request_factory.get(reverse('get_students_opened_subsection') + attributes)
 
             response = get_students_opened_subsection(request)
@@ -235,7 +234,7 @@ class TestGetProblemGradeDistribution(ModuleStoreTestCase):
     def test_get_students_opened_subsection_csv(self):
 
         tooltip = '4162 student(s) opened Subsection 5: Relational Algebra Exercises'
-        attributes = '?module_id=' + self.item.location.url() + '&tooltip=' + tooltip + '&csv=true'
+        attributes = '?module_id=' + self.item.location.to_deprecated_string() + '&tooltip=' + tooltip + '&csv=true'
         request = self.request_factory.get(reverse('get_students_opened_subsection') + attributes)
 
         response = get_students_opened_subsection(request)
@@ -255,7 +254,7 @@ class TestGetProblemGradeDistribution(ModuleStoreTestCase):
 
     def test_dashboard(self):
 
-        url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id.to_deprecated_string()})
         response = self.client.post(
             url,
             {

lms/djangoapps/class_dashboard/views.py

@@ -19,8 +19,8 @@ def has_instructor_access_for_class(user, course_id):
     Returns true if the `user` is an instructor for the course.
     """
 
-    course = get_course_with_access(user, course_id, 'staff', depth=None)
-    return has_access(user, course, 'staff')
+    course = get_course_with_access(user, 'staff', course_id, depth=None)
+    return has_access(user, 'staff', course)
 
 
 def all_sequential_open_distrib(request, course_id):

lms/djangoapps/course_wiki/middleware.py

@@ -29,8 +29,8 @@ class WikiAccessMiddleware(object):
         if course_id:
             # See if we are able to view the course. If we are, redirect to it
             try:
-                course = get_course_with_access(request.user, course_id, 'load')
-                return redirect("/courses/{course_id}/wiki/{path}".format(course_id=course.id, path=wiki_path))
+                _course = get_course_with_access(request.user, 'load', course_id)
+                return redirect("/courses/{course_id}/wiki/{path}".format(course_id=course_id.to_deprecated_string(), path=wiki_path))
             except Http404:
                 # Even though we came from the course, we can't see it. So don't worry about it.
                 pass
@@ -44,22 +44,23 @@ class WikiAccessMiddleware(object):
         if not view_func.__module__.startswith('wiki.'):
             return
 
-        course_id = course_id_from_url(request.path)
-        wiki_path = request.path.split('/wiki/', 1)[1]
-
         # wiki pages are login required
         if not request.user.is_authenticated():
             return redirect(reverse('accounts_login'), next=request.path)
 
+        course_id = course_id_from_url(request.path)
+        wiki_path = request.path.partition('/wiki/')[2]
+
         if course_id:
             # This is a /courses/org/name/run/wiki request
-            # HACK: django-wiki monkeypatches the django reverse function to enable urls to be rewritten
-            url_prefix = "/courses/{0}".format(course_id)
-            reverse._transform_url = lambda url: url_prefix + url  # pylint: disable=protected-access
+            course_path = "/courses/{}".format(course_id.to_deprecated_string())
+            # HACK: django-wiki monkeypatches the reverse function to enable
+            # urls to be rewritten
+            reverse._transform_url = lambda url: course_path + url  # pylint: disable=protected-access
             # Authorization Check
             # Let's see if user is enrolled or the course allows for public access
             try:
-                course = get_course_with_access(request.user, course_id, 'load')
+                course = get_course_with_access(request.user, 'load', course_id)
             except Http404:
                 # course does not exist. redirect to root wiki.
                 # clearing the referrer will cause process_response not to redirect
@@ -69,11 +70,11 @@ class WikiAccessMiddleware(object):
 
             if not course.allow_public_wiki_access:
                 is_enrolled = CourseEnrollment.is_enrolled(request.user, course.id)
-                is_staff = has_access(request.user, course, 'staff')
+                is_staff = has_access(request.user, 'staff', course)
                 if not (is_enrolled or is_staff):
                     # if a user is logged in, but not authorized to see a page,
                     # we'll redirect them to the course about page
-                    return redirect('about_course', course_id)
+                    return redirect('about_course', course_id.to_deprecated_string())
             # set the course onto here so that the wiki template can show the course navigation
             request.course = course
         else:

lms/djangoapps/course_wiki/tests/test_access.py

@@ -4,7 +4,6 @@ Tests for wiki permissions
 
 from django.contrib.auth.models import Group
 from student.tests.factories import UserFactory
-from xmodule.modulestore.django import loc_mapper
 from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 
@@ -48,12 +47,9 @@ class TestWikiAccessBase(ModuleStoreTestCase):
     def create_staff_for_course(self, course):
         """Creates and returns users with instructor and staff access to course."""
 
-        course_locator = loc_mapper().translate_location(course.id, course.location)
         return [
-            InstructorFactory(course=course.location),  # Creates instructor_org/number/run role name
-            StaffFactory(course=course.location),  # Creates staff_org/number/run role name
-            InstructorFactory(course=course_locator),  # Creates instructor_org.number.run role name
-            StaffFactory(course=course_locator),  # Creates staff_org.number.run role name
+            InstructorFactory(course=course.id),  # Creates instructor_org/number/run role name
+            StaffFactory(course=course.id),  # Creates staff_org/number/run role name
         ]
 
 

lms/djangoapps/course_wiki/tests/test_middleware.py

@@ -23,7 +23,7 @@ class TestWikiAccessMiddleware(ModuleStoreTestCase):
         self.wiki = get_or_create_root()
 
         self.course_math101 = CourseFactory.create(org='edx', number='math101', display_name='2014', metadata={'use_unique_wiki_id': 'false'})
-        self.course_math101_instructor = InstructorFactory(course=self.course_math101.location, username='instructor', password='secret')
+        self.course_math101_instructor = InstructorFactory(course=self.course_math101.id, username='instructor', password='secret')
         self.wiki_math101 = URLPath.create_article(self.wiki, 'math101', title='math101')
 
         self.client = Client()

lms/djangoapps/course_wiki/tests/tests.py

@@ -4,7 +4,7 @@ from django.test.utils import override_settings
 from courseware.tests.tests import LoginEnrollmentTestCase
 from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
 from xmodule.modulestore.django import modulestore
-from xmodule.modulestore.tests.factories import CourseFactory
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 from mock import patch
 
@@ -15,7 +15,7 @@ class WikiRedirectTestCase(LoginEnrollmentTestCase):
     def setUp(self):
 
         # Load the toy course
-        self.toy = modulestore().get_course('edX/toy/2012_Fall')
+        self.toy = modulestore().get_course(SlashSeparatedCourseKey('edX', 'toy', '2012_Fall'))
 
         # Create two accounts
         self.student = 'view@test.com'
@@ -43,7 +43,7 @@ class WikiRedirectTestCase(LoginEnrollmentTestCase):
 
         self.enroll(self.toy)
 
-        referer = reverse("progress", kwargs={'course_id': self.toy.id})
+        referer = reverse("progress", kwargs={'course_id': self.toy.id.to_deprecated_string()})
         destination = reverse("wiki:get", kwargs={'path': 'some/fake/wiki/page/'})
 
         redirected_to = referer.replace("progress", "wiki/some/fake/wiki/page/")
@@ -72,7 +72,7 @@ class WikiRedirectTestCase(LoginEnrollmentTestCase):
 
         self.enroll(self.toy)
 
-        referer = reverse("progress", kwargs={'course_id': self.toy.id})
+        referer = reverse("progress", kwargs={'course_id': self.toy.id.to_deprecated_string()})
         destination = reverse("wiki:get", kwargs={'path': 'some/fake/wiki/page/'})
 
         resp = self.client.get(destination, HTTP_REFERER=referer)
@@ -84,8 +84,8 @@ class WikiRedirectTestCase(LoginEnrollmentTestCase):
         The user must be enrolled in the course to see the page.
         """
 
-        course_wiki_home = reverse('course_wiki', kwargs={'course_id': course.id})
-        referer = reverse("progress", kwargs={'course_id': self.toy.id})
+        course_wiki_home = reverse('course_wiki', kwargs={'course_id': course.id.to_deprecated_string()})
+        referer = reverse("progress", kwargs={'course_id': self.toy.id.to_deprecated_string()})
 
         resp = self.client.get(course_wiki_home, follow=True, HTTP_REFERER=referer)
 
@@ -117,8 +117,7 @@ class WikiRedirectTestCase(LoginEnrollmentTestCase):
         self.create_course_page(self.toy)
 
         course_wiki_page = reverse('wiki:get', kwargs={'path': self.toy.wiki_slug + '/'})
-
-        referer = reverse("courseware", kwargs={'course_id': self.toy.id})
+        referer = reverse("courseware", kwargs={'course_id': self.toy.id.to_deprecated_string()})
 
         resp = self.client.get(course_wiki_page, follow=True, HTTP_REFERER=referer)
 
@@ -137,7 +136,7 @@ class WikiRedirectTestCase(LoginEnrollmentTestCase):
 
         self.login(self.student, self.password)
         course_wiki_page = reverse('wiki:get', kwargs={'path': self.toy.wiki_slug + '/'})
-        referer = reverse("courseware", kwargs={'course_id': self.toy.id})
+        referer = reverse("courseware", kwargs={'course_id': self.toy.id.to_deprecated_string()})
 
         # When not enrolled, we should get a 302
         resp = self.client.get(course_wiki_page, follow=False, HTTP_REFERER=referer)
@@ -147,7 +146,7 @@ class WikiRedirectTestCase(LoginEnrollmentTestCase):
         resp = self.client.get(course_wiki_page, follow=True, HTTP_REFERER=referer)
         target_url, __ = resp.redirect_chain[-1]
         self.assertTrue(
-            target_url.endswith(reverse('about_course', args=[self.toy.id]))
+            target_url.endswith(reverse('about_course', args=[self.toy.id.to_deprecated_string()]))
         )
 
     @patch.dict("django.conf.settings.FEATURES", {'ALLOW_WIKI_ROOT_ACCESS': True})

lms/djangoapps/course_wiki/utils.py

@@ -33,12 +33,12 @@ def user_is_article_course_staff(user, article):
     # course numbered '202_' or '202' and so we need to consider both.
 
     courses = modulestore.django.modulestore().get_courses_for_wiki(wiki_slug)
-    if any(courseware.access.has_access(user, course, 'staff', course.course_id) for course in courses):
+    if any(courseware.access.has_access(user, 'staff', course, course.course_key) for course in courses):
         return True
 
     if (wiki_slug.endswith('_') and slug_is_numerical(wiki_slug[:-1])):
         courses = modulestore.django.modulestore().get_courses_for_wiki(wiki_slug[:-1])
-        if any(courseware.access.has_access(user, course, 'staff', course.course_id) for course in courses):
+        if any(courseware.access.has_access(user, 'staff', course, course.course_key) for course in courses):
             return True
 
     return False

lms/djangoapps/course_wiki/views.py

@@ -16,6 +16,7 @@ from wiki.models import URLPath, Article
 
 from courseware.courses import get_course_by_id
 from course_wiki.utils import course_wiki_slug
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 log = logging.getLogger(__name__)
 
@@ -35,7 +36,7 @@ def course_wiki_redirect(request, course_id):  # pylint: disable=W0613
     as it's home page. A course's wiki must be an article on the root (for
     example, "/6.002x") to keep things simple.
     """
-    course = get_course_by_id(course_id)
+    course = get_course_by_id(SlashSeparatedCourseKey.from_deprecated_string(course_id))
     course_slug = course_wiki_slug(course)
 
     valid_slug = True

lms/djangoapps/dashboard/git_import.py

@@ -17,7 +17,7 @@ from django.utils.translation import ugettext as _
 import mongoengine
 
 from dashboard.models import CourseImportLog
-from xmodule.modulestore import Location
+from xmodule.modulestore.keys import CourseKey
 
 log = logging.getLogger(__name__)
 
@@ -222,19 +222,16 @@ def add_repo(repo, rdir_in, branch=None):
         logger.setLevel(logging.NOTSET)
         logger.removeHandler(import_log_handler)
 
-    course_id = 'unknown'
+    course_key = None
     location = 'unknown'
 
     # extract course ID from output of import-command-run and make symlink
     # this is needed in order for custom course scripts to work
-    match = re.search('(?ms)===> IMPORTING course to location (\S+)',
-                      ret_import)
+    match = re.search(r'(?ms)===> IMPORTING course (\S+)', ret_import)
     if match:
-        location = Location(match.group(1))
-        log.debug('location = {0}'.format(location))
-        course_id = location.course_id
-
-        cdir = '{0}/{1}'.format(GIT_REPO_DIR, location.course)
+        course_id = match.group(1)
+        course_key = CourseKey.from_string(course_id)
+        cdir = '{0}/{1}'.format(GIT_REPO_DIR, course_key.course)
         log.debug('Studio course dir = {0}'.format(cdir))
 
         if os.path.exists(cdir) and not os.path.islink(cdir):
@@ -267,8 +264,8 @@ def add_repo(repo, rdir_in, branch=None):
         log.exception('Unable to connect to mongodb to save log, please '
                       'check MONGODB_LOG settings')
     cil = CourseImportLog(
-        course_id=course_id,
-        location=unicode(location),
+        course_id=course_key,
+        location=location,
         repo_dir=rdir,
         created=timezone.now(),
         import_log=ret_import,

lms/djangoapps/dashboard/management/commands/git_add_course.py

@@ -2,19 +2,13 @@
 Script for importing courseware from git/xml into a mongo modulestore
 """
 
-import os
-import re
-import StringIO
-import subprocess
 import logging
 
-from django.core import management
 from django.core.management.base import BaseCommand, CommandError
 from django.utils.translation import ugettext as _
 
 import dashboard.git_import
 from dashboard.git_import import GitImportError
-from dashboard.models import CourseImportLog
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.xml import XMLModuleStore
 

lms/djangoapps/dashboard/management/commands/tests/test_git_add_course.py

@@ -17,10 +17,12 @@ from django.test.utils import override_settings
 from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
 from xmodule.contentstore.django import contentstore
 from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 from xmodule.modulestore.store_utilities import delete_course
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 import dashboard.git_import as git_import
 from dashboard.git_import import GitImportError
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 TEST_MONGODB_LOG = {
     'host': 'localhost',
@@ -45,7 +47,7 @@ class TestGitAddCourse(ModuleStoreTestCase):
     TEST_REPO = 'https://github.com/mitocw/edx4edx_lite.git'
     TEST_COURSE = 'MITx/edx4edx/edx4edx'
     TEST_BRANCH = 'testing_do_not_delete'
-    TEST_BRANCH_COURSE = 'MITx/edx4edx_branch/edx4edx'
+    TEST_BRANCH_COURSE = SlashSeparatedCourseKey('MITx', 'edx4edx_branch', 'edx4edx')
     GIT_REPO_DIR = getattr(settings, 'GIT_REPO_DIR')
 
     def assertCommandFailureRegexp(self, regex, *args):
@@ -162,14 +164,14 @@ class TestGitAddCourse(ModuleStoreTestCase):
 
         # Delete to test branching back to master
         delete_course(def_ms, contentstore(),
-                      def_ms.get_course(self.TEST_BRANCH_COURSE).location,
+                      self.TEST_BRANCH_COURSE,
                       True)
         self.assertIsNone(def_ms.get_course(self.TEST_BRANCH_COURSE))
         git_import.add_repo(self.TEST_REPO,
                             repo_dir / 'edx4edx_lite',
                             'master')
         self.assertIsNone(def_ms.get_course(self.TEST_BRANCH_COURSE))
-        self.assertIsNotNone(def_ms.get_course(self.TEST_COURSE))
+        self.assertIsNotNone(def_ms.get_course(SlashSeparatedCourseKey.from_deprecated_string(self.TEST_COURSE)))
 
     def test_branch_exceptions(self):
         """

lms/djangoapps/dashboard/models.py

 """Models for dashboard application"""
 
 import mongoengine
+from xmodule.modulestore.mongoengine_fields import CourseKeyField
 
 
 class CourseImportLog(mongoengine.Document):
     """Mongoengine model for git log"""
     # pylint: disable=R0924
 
-    course_id = mongoengine.StringField(max_length=128)
+    course_id = CourseKeyField(max_length=128)
+    # NOTE: this location is not a Location object but a pathname
     location = mongoengine.StringField(max_length=168)
     import_log = mongoengine.StringField(max_length=20 * 65535)
     git_log = mongoengine.StringField(max_length=65535)

lms/djangoapps/dashboard/sysadmin.py

@@ -42,6 +42,7 @@ from xmodule.modulestore import XML_MODULESTORE_TYPE
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.store_utilities import delete_course
 from xmodule.modulestore.xml import XMLModuleStore
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 
 log = logging.getLogger(__name__)
@@ -78,7 +79,7 @@ class SysadminDashboardView(TemplateView):
         """ Get an iterable list of courses."""
 
         courses = self.def_ms.get_courses()
-        courses = dict([c.id, c] for c in courses)  # no course directory
+        courses = dict([c.id.to_deprecated_string(), c] for c in courses)  # no course directory
 
         return courses
 
@@ -258,7 +259,7 @@ class Users(SysadminDashboardView):
         self.msg += u'<ol>'
         for (cdir, course) in courses.items():
             self.msg += u'<li>{0} ({1})</li>'.format(
-                escape(cdir), course.location.url())
+                escape(cdir), course.location.to_deprecated_string())
         self.msg += u'</ol>'
 
     def get(self, request):
@@ -469,7 +470,7 @@ class Courses(SysadminDashboardView):
             course = self.def_ms.courses[os.path.abspath(gdir)]
             msg += _('Loaded course {0} {1}<br/>Errors:').format(
                 cdir, course.display_name)
-            errors = self.def_ms.get_item_errors(course.location)
+            errors = self.def_ms.get_course_errors(course.id)
             if not errors:
                 msg += u'None'
             else:
@@ -489,9 +490,7 @@ class Courses(SysadminDashboardView):
         courses = self.get_courses()
 
         for (cdir, course) in courses.items():
-            gdir = cdir
-            if '/' in cdir:
-                gdir = cdir.rsplit('/', 1)[1]
+            gdir = cdir.run
             data.append([course.display_name, cdir]
                         + self.git_info_for_course(gdir))
 
@@ -535,13 +534,14 @@ class Courses(SysadminDashboardView):
 
         elif action == 'del_course':
             course_id = request.POST.get('course_id', '').strip()
+            course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
             course_found = False
-            if course_id in courses:
+            if course_key in courses:
                 course_found = True
-                course = courses[course_id]
+                course = courses[course_key]
             else:
                 try:
-                    course = get_course_by_id(course_id)
+                    course = get_course_by_id(course_key)
                     course_found = True
                 except Exception, err:   # pylint: disable=broad-except
                     self.msg += _('Error - cannot get course with ID '
@@ -549,7 +549,7 @@ class Courses(SysadminDashboardView):
                                       course_id, escape(str(err))
                                   )
 
-            is_xml_course = (modulestore().get_modulestore_type(course_id) == XML_MODULESTORE_TYPE)
+            is_xml_course = (modulestore().get_modulestore_type(course_key) == XML_MODULESTORE_TYPE)
             if course_found and is_xml_course:
                 cdir = course.data_dir
                 self.def_ms.courses.pop(cdir)
@@ -567,14 +567,13 @@ class Courses(SysadminDashboardView):
 
             elif course_found and not is_xml_course:
                 # delete course that is stored with mongodb backend
-                loc = course.location
                 content_store = contentstore()
                 commit = True
-                delete_course(self.def_ms, content_store, loc, commit)
+                delete_course(self.def_ms, content_store, course.id, commit)
                 # don't delete user permission groups, though
                 self.msg += \
-                    u"<font color='red'>{0} {1} = {2} ({3})</font>".format(
-                        _('Deleted'), loc, course.id, course.display_name)
+                    u"<font color='red'>{0} {1} ({2})</font>".format(
+                        _('Deleted'), course.id.to_deprecated_string(), course.display_name)
             datatable = self.make_datatable()
 
         context = {
@@ -606,9 +605,9 @@ class Staffing(SysadminDashboardView):
             datum = [course.display_name, course.id]
             datum += [CourseEnrollment.objects.filter(
                 course_id=course.id).count()]
-            datum += [CourseStaffRole(course.location).users_with_role().count()]
+            datum += [CourseStaffRole(course.id).users_with_role().count()]
             datum += [','.join([x.username for x in CourseInstructorRole(
-                course.location).users_with_role()])]
+                course.id).users_with_role()])]
             data.append(datum)
 
         datatable = dict(header=[_('Course Name'), _('course_id'),
@@ -640,7 +639,7 @@ class Staffing(SysadminDashboardView):
 
             for (cdir, course) in courses.items():  # pylint: disable=unused-variable
                 for role in roles:
-                    for user in role(course.location).users_with_role():
+                    for user in role(course.id).users_with_role():
                         datum = [course.id, role, user.username, user.email,
                                  user.profile.name]
                         data.append(datum)
@@ -667,6 +666,8 @@ class GitLogs(TemplateView):
         """Shows logs of imports that happened as a result of a git import"""
 
         course_id = kwargs.get('course_id')
+        if course_id:
+            course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id)
 
         # Set mongodb defaults even if it isn't defined in settings
         mongo_db = {
@@ -709,16 +710,15 @@ class GitLogs(TemplateView):
 
             # Allow only course team, instructors, and staff
             if not (request.user.is_staff or
-                    CourseInstructorRole(course.location).has_user(request.user) or
-                    CourseStaffRole(course.location).has_user(request.user)):
+                    CourseInstructorRole(course.id).has_user(request.user) or
+                    CourseStaffRole(course.id).has_user(request.user)):
                 raise Http404
             log.debug('course_id={0}'.format(course_id))
-            cilset = CourseImportLog.objects.filter(
-                course_id=course_id).order_by('-created')
+            cilset = CourseImportLog.objects.filter(course_id=course_id).order_by('-created')
             log.debug('cilset length={0}'.format(len(cilset)))
         mdb.disconnect()
         context = {'cilset': cilset,
-                   'course_id': course_id,
+                   'course_id': course_id.to_deprecated_string() if course_id else None,
                    'error_msg': error_msg}
 
         return render_to_response(self.template_name, context)

lms/djangoapps/dashboard/tests/test_sysadmin.py

@@ -13,7 +13,6 @@ from django.contrib.auth.models import User
 from django.core.urlresolvers import reverse
 from django.test.client import Client
 from django.test.utils import override_settings
-from django.utils.html import escape
 from django.utils.translation import ugettext as _
 import mongoengine
 
@@ -27,6 +26,7 @@ from student.tests.factories import UserFactory
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.xml import XMLModuleStore
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 
 TEST_MONGODB_LOG = {
@@ -47,7 +47,7 @@ class SysadminBaseTestCase(ModuleStoreTestCase):
 
     TEST_REPO = 'https://github.com/mitocw/edx4edx_lite.git'
     TEST_BRANCH = 'testing_do_not_delete'
-    TEST_BRANCH_COURSE = 'MITx/edx4edx_branch/edx4edx'
+    TEST_BRANCH_COURSE = SlashSeparatedCourseKey('MITx', 'edx4edx_branch', 'edx4edx')
 
     def setUp(self):
         """Setup test case by adding primary user."""
@@ -79,12 +79,16 @@ class SysadminBaseTestCase(ModuleStoreTestCase):
             course = def_ms.courses.get(course_path, None)
         except AttributeError:
             # Using mongo store
-            course = def_ms.get_course('MITx/edx4edx/edx4edx')
+            course = def_ms.get_course(SlashSeparatedCourseKey('MITx', 'edx4edx', 'edx4edx'))
 
         # Delete git loaded course
-        response = self.client.post(reverse('sysadmin_courses'),
-                                {'course_id': course.id,
-                                 'action': 'del_course', })
+        response = self.client.post(
+            reverse('sysadmin_courses'),
+            {
+                'course_id': course.id.to_deprecated_string(),
+                'action': 'del_course',
+            }
+        )
         self.addCleanup(self._rm_glob, '{0}_deleted_*'.format(course_path))
 
         return response
@@ -321,7 +325,7 @@ class TestSysadmin(SysadminBaseTestCase):
         course = def_ms.courses.get('{0}/edx4edx_lite'.format(
             os.path.abspath(settings.DATA_DIR)), None)
         self.assertIsNotNone(course)
-        self.assertIn(self.TEST_BRANCH_COURSE, course.location.course_id)
+        self.assertEqual(self.TEST_BRANCH_COURSE, course.id)
         self._rm_edx4edx()
 
         # Try and delete a non-existent course
@@ -363,8 +367,8 @@ class TestSysadmin(SysadminBaseTestCase):
         self._add_edx4edx()
 
         def_ms = modulestore()
-        course = def_ms.get_course('MITx/edx4edx/edx4edx')
-        CourseStaffRole(course.location).add_users(self.user)
+        course = def_ms.get_course(SlashSeparatedCourseKey('MITx', 'edx4edx', 'edx4edx'))
+        CourseStaffRole(course.id).add_users(self.user)
 
         response = self.client.post(reverse('sysadmin_staffing'),
                                     {'action': 'get_staff_csv', })
@@ -447,11 +451,11 @@ class TestSysAdminMongoCourseImport(SysadminBaseTestCase):
         self.assertFalse(isinstance(def_ms, XMLModuleStore))
 
         self._add_edx4edx()
-        course = def_ms.get_course('MITx/edx4edx/edx4edx')
+        course = def_ms.get_course(SlashSeparatedCourseKey('MITx', 'edx4edx', 'edx4edx'))
         self.assertIsNotNone(course)
 
         self._rm_edx4edx()
-        course = def_ms.get_course('MITx/edx4edx/edx4edx')
+        course = def_ms.get_course(SlashSeparatedCourseKey('MITx', 'edx4edx', 'edx4edx'))
         self.assertIsNone(course)
 
     def test_gitlogs(self):
@@ -472,7 +476,7 @@ class TestSysAdminMongoCourseImport(SysadminBaseTestCase):
             reverse('gitlogs_detail', kwargs={
                 'course_id': 'MITx/edx4edx/edx4edx'}))
 
-        self.assertIn('======> IMPORTING course to location',
+        self.assertIn('======> IMPORTING course',
                       response.content)
 
         self._rm_edx4edx()
@@ -505,23 +509,25 @@ class TestSysAdminMongoCourseImport(SysadminBaseTestCase):
         self.assertEqual(response.status_code, 404)
         # Or specific logs
         response = self.client.get(reverse('gitlogs_detail', kwargs={
-            'course_id': 'MITx/edx4edx/edx4edx'}))
+            'course_id': 'MITx/edx4edx/edx4edx'
+        }))
         self.assertEqual(response.status_code, 404)
 
         # Add user as staff in course team
         def_ms = modulestore()
-        course = def_ms.get_course('MITx/edx4edx/edx4edx')
-        CourseStaffRole(course.location).add_users(self.user)
+        course = def_ms.get_course(SlashSeparatedCourseKey('MITx', 'edx4edx', 'edx4edx'))
+        CourseStaffRole(course.id).add_users(self.user)
 
-        self.assertTrue(CourseStaffRole(course.location).has_user(self.user))
+        self.assertTrue(CourseStaffRole(course.id).has_user(self.user))
         logged_in = self.client.login(username=self.user.username,
                                       password='foo')
         self.assertTrue(logged_in)
 
         response = self.client.get(
             reverse('gitlogs_detail', kwargs={
-                'course_id': 'MITx/edx4edx/edx4edx'}))
-        self.assertIn('======> IMPORTING course to location',
+                'course_id': 'MITx/edx4edx/edx4edx'
+            }))
+        self.assertIn('======> IMPORTING course',
                       response.content)
 
         self._rm_edx4edx()

lms/djangoapps/debug/management/commands/dump_xml_courses.py

@@ -42,7 +42,7 @@ class Command(BaseCommand):
         for course_id, course_modules in xml_module_store.modules.iteritems():
             course_path = course_id.replace('/', '_')
             for location, descriptor in course_modules.iteritems():
-                location_path = location.url().replace('/', '_')
+                location_path = location.to_deprecated_string().replace('/', '_')
                 data = {}
                 for field_name, field in descriptor.fields.iteritems():
                     try:

lms/djangoapps/django_comment_client/base/views.py

 import time
 import random
-import os
 import os.path
 import logging
 import urlparse
@@ -13,12 +12,11 @@ import django_comment_client.settings as cc_settings
 
 from django.core import exceptions
 from django.contrib.auth.decorators import login_required
-from django.views.decorators.http import require_POST, require_GET
+from django.views.decorators.http import require_POST
 from django.views.decorators import csrf
 from django.core.files.storage import get_storage_class
 from django.utils.translation import ugettext as _
 
-from edxmako.shortcuts import render_to_string
 from courseware.courses import get_course_with_access, get_course_by_id
 from course_groups.cohorts import get_cohort_id, is_commentable_cohorted
 
@@ -26,6 +24,8 @@ from django_comment_client.utils import JsonResponse, JsonError, extract, add_co
 
 from django_comment_client.permissions import check_permissions_by_view, cached_has_permission
 from courseware.access import has_access
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
+from xmodule.modulestore.keys import CourseKey
 
 log = logging.getLogger(__name__)
 
@@ -41,7 +41,8 @@ def permitted(fn):
             else:
                 content = None
             return content
-        if check_permissions_by_view(request.user, kwargs['course_id'], fetch_content(), request.view_name):
+        course_key = SlashSeparatedCourseKey.from_deprecated_string(kwargs['course_id'])
+        if check_permissions_by_view(request.user, course_key, fetch_content(), request.view_name):
             return fn(request, *args, **kwargs)
         else:
             return JsonError("unauthorized", status=401)
@@ -49,10 +50,6 @@ def permitted(fn):
 
 
 def ajax_content_response(request, course_id, content):
-    context = {
-        'course_id': course_id,
-        'content': content,
-    }
     user_info = cc.User.from_django_user(request.user).to_dict()
     annotated_content_info = utils.get_annotated_content_info(course_id, content, request.user, user_info)
     return JsonResponse({
@@ -70,7 +67,8 @@ def create_thread(request, course_id, commentable_id):
     """
 
     log.debug("Creating new thread in %r, id %r", course_id, commentable_id)
-    course = get_course_with_access(request.user, course_id, 'load')
+    course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id)
+    course = get_course_with_access(request.user, 'load', course_id)
     post = request.POST
 
     if course.allow_anonymous:
@@ -93,7 +91,7 @@ def create_thread(request, course_id, commentable_id):
         'anonymous': anonymous,
         'anonymous_to_peers': anonymous_to_peers,
         'commentable_id': commentable_id,
-        'course_id': course_id,
+        'course_id': course_id.to_deprecated_string(),
         'user_id': request.user.id,
     })
 
@@ -152,23 +150,24 @@ def update_thread(request, course_id, thread_id):
     thread.update_attributes(**extract(request.POST, ['body', 'title']))
     thread.save()
     if request.is_ajax():
-        return ajax_content_response(request, course_id, thread.to_dict())
+        return ajax_content_response(request, SlashSeparatedCourseKey.from_deprecated_string(course_id), thread.to_dict())
     else:
         return JsonResponse(utils.safe_content(thread.to_dict()))
 
 
-def _create_comment(request, course_id, thread_id=None, parent_id=None):
+def _create_comment(request, course_key, thread_id=None, parent_id=None):
     """
     given a course_id, thread_id, and parent_id, create a comment,
     called from create_comment to do the actual creation
     """
+    assert isinstance(course_key, CourseKey)
     post = request.POST
 
     if 'body' not in post or not post['body'].strip():
         return JsonError(_("Body can't be empty"))
     comment = cc.Comment(**extract(post, ['body']))
 
-    course = get_course_with_access(request.user, course_id, 'load')
+    course = get_course_with_access(request.user, 'load', course_key)
     if course.allow_anonymous:
         anonymous = post.get('anonymous', 'false').lower() == 'true'
     else:
@@ -183,7 +182,7 @@ def _create_comment(request, course_id, thread_id=None, parent_id=None):
         'anonymous': anonymous,
         'anonymous_to_peers': anonymous_to_peers,
         'user_id': request.user.id,
-        'course_id': course_id,
+        'course_id': course_key,
         'thread_id': thread_id,
         'parent_id': parent_id,
     })
@@ -192,7 +191,7 @@ def _create_comment(request, course_id, thread_id=None, parent_id=None):
         user = cc.User.from_django_user(request.user)
         user.follow(comment.thread)
     if request.is_ajax():
-        return ajax_content_response(request, course_id, comment.to_dict())
+        return ajax_content_response(request, course_key, comment.to_dict())
     else:
         return JsonResponse(utils.safe_content(comment.to_dict()))
 
@@ -208,7 +207,7 @@ def create_comment(request, course_id, thread_id):
     if cc_settings.MAX_COMMENT_DEPTH is not None:
         if cc_settings.MAX_COMMENT_DEPTH < 0:
             return JsonError(_("Comment level too deep"))
-    return _create_comment(request, course_id, thread_id=thread_id)
+    return _create_comment(request, SlashSeparatedCourseKey.from_deprecated_string(course_id), thread_id=thread_id)
 
 
 @require_POST
@@ -238,7 +237,7 @@ def update_comment(request, course_id, comment_id):
     comment.update_attributes(**extract(request.POST, ['body']))
     comment.save()
     if request.is_ajax():
-        return ajax_content_response(request, course_id, comment.to_dict())
+        return ajax_content_response(request, SlashSeparatedCourseKey.from_deprecated_string(course_id), comment.to_dict())
     else:
         return JsonResponse(utils.safe_content(comment.to_dict()))
 
@@ -271,7 +270,7 @@ def openclose_thread(request, course_id, thread_id):
     thread = thread.to_dict()
     return JsonResponse({
         'content': utils.safe_content(thread),
-        'ability': utils.get_ability(course_id, thread, request.user),
+        'ability': utils.get_ability(SlashSeparatedCourseKey.from_deprecated_string(course_id), thread, request.user),
     })
 
 
@@ -286,7 +285,7 @@ def create_sub_comment(request, course_id, comment_id):
     if cc_settings.MAX_COMMENT_DEPTH is not None:
         if cc_settings.MAX_COMMENT_DEPTH <= cc.Comment.find(comment_id).depth:
             return JsonError(_("Comment level too deep"))
-    return _create_comment(request, course_id, parent_id=comment_id)
+    return _create_comment(request, SlashSeparatedCourseKey.from_deprecated_string(course_id), parent_id=comment_id)
 
 
 @require_POST
@@ -366,10 +365,11 @@ def un_flag_abuse_for_thread(request, course_id, thread_id):
     ajax only
     """
     user = cc.User.from_django_user(request.user)
+    course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id)
     course = get_course_by_id(course_id)
     thread = cc.Thread.find(thread_id)
-    removeAll = cached_has_permission(request.user, 'openclose_thread', course_id) or has_access(request.user, course, 'staff')
-    thread.unFlagAbuse(user, thread, removeAll)
+    remove_all = cached_has_permission(request.user, 'openclose_thread', course_id) or has_access(request.user, 'staff', course)
+    thread.unFlagAbuse(user, thread, remove_all)
     return JsonResponse(utils.safe_content(thread.to_dict()))
 
 
@@ -396,10 +396,11 @@ def un_flag_abuse_for_comment(request, course_id, comment_id):
     ajax only
     """
     user = cc.User.from_django_user(request.user)
-    course = get_course_by_id(course_id)
-    removeAll = cached_has_permission(request.user, 'openclose_thread', course_id) or has_access(request.user, course, 'staff')
+    course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
+    course = get_course_by_id(course_key)
+    remove_all = cached_has_permission(request.user, 'openclose_thread', course_key) or has_access(request.user, 'staff', course)
     comment = cc.Comment.find(comment_id)
-    comment.unFlagAbuse(user, comment, removeAll)
+    comment.unFlagAbuse(user, comment, remove_all)
     return JsonResponse(utils.safe_content(comment.to_dict()))
 
 

lms/djangoapps/django_comment_client/forum/tests.py

@@ -64,7 +64,7 @@ class ViewsExceptionTestCase(UrlResetMixin, ModuleStoreTestCase):
         mock_from_django_user.return_value = Mock()
 
         url = reverse('django_comment_client.forum.views.user_profile',
-                      kwargs={'course_id': self.course.id, 'user_id': '12345'})  # There is no user 12345
+                      kwargs={'course_id': self.course.id.to_deprecated_string(), 'user_id': '12345'})  # There is no user 12345
         self.response = self.client.get(url)
         self.assertEqual(self.response.status_code, 404)
 
@@ -81,7 +81,7 @@ class ViewsExceptionTestCase(UrlResetMixin, ModuleStoreTestCase):
         mock_from_django_user.return_value = Mock()
 
         url = reverse('django_comment_client.forum.views.followed_threads',
-                      kwargs={'course_id': self.course.id, 'user_id': '12345'})  # There is no user 12345
+                      kwargs={'course_id': self.course.id.to_deprecated_string(), 'user_id': '12345'})  # There is no user 12345
         self.response = self.client.get(url)
         self.assertEqual(self.response.status_code, 404)
 
@@ -173,7 +173,7 @@ class SingleThreadTestCase(ModuleStoreTestCase):
         request.user = self.student
         response = views.single_thread(
             request,
-            self.course.id,
+            self.course.id.to_deprecated_string(),
             "dummy_discussion_id",
             "test_thread_id"
         )
@@ -208,7 +208,7 @@ class SingleThreadTestCase(ModuleStoreTestCase):
         request.user = self.student
         response = views.single_thread(
             request,
-            self.course.id,
+            self.course.id.to_deprecated_string(),
             "dummy_discussion_id",
             "test_thread_id"
         )
@@ -237,7 +237,7 @@ class SingleThreadTestCase(ModuleStoreTestCase):
         request = RequestFactory().post("dummy_url")
         response = views.single_thread(
             request,
-            self.course.id,
+            self.course.id.to_deprecated_string(),
             "dummy_discussion_id",
             "dummy_thread_id"
         )
@@ -252,7 +252,7 @@ class SingleThreadTestCase(ModuleStoreTestCase):
             Http404,
             views.single_thread,
             request,
-            self.course.id,
+            self.course.id.to_deprecated_string(),
             "test_discussion_id",
             "test_thread_id"
         )
@@ -277,7 +277,7 @@ class UserProfileTestCase(ModuleStoreTestCase):
         request.user = self.student
         response = views.user_profile(
             request,
-            self.course.id,
+            self.course.id.to_deprecated_string(),
             self.profiled_user.id
         )
         mock_request.assert_any_call(
@@ -342,7 +342,7 @@ class UserProfileTestCase(ModuleStoreTestCase):
         with self.assertRaises(Http404):
             response = views.user_profile(
                 request,
-                self.course.id,
+                self.course.id.to_deprecated_string(),
                 -999
             )
 
@@ -362,7 +362,7 @@ class UserProfileTestCase(ModuleStoreTestCase):
         request.user = self.student
         response = views.user_profile(
             request,
-            self.course.id,
+            self.course.id.to_deprecated_string(),
             self.profiled_user.id
         )
         self.assertEqual(response.status_code, 405)
@@ -406,7 +406,7 @@ class CommentsServiceRequestHeadersTestCase(UrlResetMixin, ModuleStoreTestCase):
             reverse(
                 "django_comment_client.forum.views.single_thread",
                 kwargs={
-                    "course_id": self.course.id,
+                    "course_id": self.course.id.to_deprecated_string(),
                     "discussion_id": "dummy",
                     "thread_id": thread_id,
                 }
@@ -422,7 +422,7 @@ class CommentsServiceRequestHeadersTestCase(UrlResetMixin, ModuleStoreTestCase):
         self.client.get(
             reverse(
                 "django_comment_client.forum.views.forum_form_discussion",
-                kwargs={"course_id": self.course.id}
+                kwargs={"course_id": self.course.id.to_deprecated_string()}
             ),
         )
         self.assert_all_calls_have_header(mock_request, "X-Edx-Api-Key", "test_api_key")
@@ -441,7 +441,7 @@ class InlineDiscussionUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin):
         request = RequestFactory().get("dummy_url")
         request.user = self.student
 
-        response = views.inline_discussion(request, self.course.id, "dummy_discussion_id")
+        response = views.inline_discussion(request, self.course.id.to_deprecated_string(), "dummy_discussion_id")
         self.assertEqual(response.status_code, 200)
         response_data = json.loads(response.content)
         self.assertEqual(response_data["discussion_data"][0]["title"], text)
@@ -462,7 +462,7 @@ class ForumFormDiscussionUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin):
         request.user = self.student
         request.META["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" # so request.is_ajax() == True
 
-        response = views.forum_form_discussion(request, self.course.id)
+        response = views.forum_form_discussion(request, self.course.id.to_deprecated_string())
         self.assertEqual(response.status_code, 200)
         response_data = json.loads(response.content)
         self.assertEqual(response_data["discussion_data"][0]["title"], text)
@@ -484,7 +484,7 @@ class SingleThreadUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin):
         request.user = self.student
         request.META["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" # so request.is_ajax() == True
 
-        response = views.single_thread(request, self.course.id, "dummy_discussion_id", thread_id)
+        response = views.single_thread(request, self.course.id.to_deprecated_string(), "dummy_discussion_id", thread_id)
         self.assertEqual(response.status_code, 200)
         response_data = json.loads(response.content)
         self.assertEqual(response_data["content"]["title"], text)
@@ -505,7 +505,7 @@ class UserProfileUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin):
         request.user = self.student
         request.META["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" # so request.is_ajax() == True
 
-        response = views.user_profile(request, self.course.id, str(self.student.id))
+        response = views.user_profile(request, self.course.id.to_deprecated_string(), str(self.student.id))
         self.assertEqual(response.status_code, 200)
         response_data = json.loads(response.content)
         self.assertEqual(response_data["discussion_data"][0]["title"], text)
@@ -526,7 +526,7 @@ class FollowedThreadsUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin):
         request.user = self.student
         request.META["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" # so request.is_ajax() == True
 
-        response = views.followed_threads(request, self.course.id, str(self.student.id))
+        response = views.followed_threads(request, self.course.id.to_deprecated_string(), str(self.student.id))
         self.assertEqual(response.status_code, 200)
         response_data = json.loads(response.content)
         self.assertEqual(response_data["discussion_data"][0]["title"], text)

lms/djangoapps/django_comment_client/forum/views.py

@@ -21,6 +21,8 @@ from django_comment_client.utils import (merge_dict, extract, strip_none, add_co
 import django_comment_client.utils as utils
 import lms.lib.comment_client as cc
 
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
+
 THREADS_PER_PAGE = 20
 INLINE_THREADS_PER_PAGE = 20
 PAGES_NEARBY_DELTA = 2
@@ -41,7 +43,7 @@ def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAG
         'sort_order': 'desc',
         'text': '',
         'commentable_id': discussion_id,
-        'course_id': course_id,
+        'course_id': course_id.to_deprecated_string(),
         'user_id': request.user.id,
     }
 
@@ -111,8 +113,9 @@ def inline_discussion(request, course_id, discussion_id):
     Renders JSON for DiscussionModules
     """
     nr_transaction = newrelic.agent.current_transaction()
+    course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id)
 
-    course = get_course_with_access(request.user, course_id, 'load_forum')
+    course = get_course_with_access(request.user, 'load_forum', course_id)
 
     threads, query_params = get_threads(request, course_id, discussion_id, per_page=INLINE_THREADS_PER_PAGE)
     cc_user = cc.User.from_django_user(request.user)
@@ -166,9 +169,10 @@ def forum_form_discussion(request, course_id):
     """
     Renders the main Discussion page, potentially filtered by a search query
     """
+    course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id)
     nr_transaction = newrelic.agent.current_transaction()
 
-    course = get_course_with_access(request.user, course_id, 'load_forum')
+    course = get_course_with_access(request.user, 'load_forum', course_id)
     with newrelic.agent.FunctionTrace(nr_transaction, "get_discussion_category_map"):
         category_map = utils.get_discussion_category_map(course)
 
@@ -206,13 +210,13 @@ def forum_form_discussion(request, course_id):
             'csrf': csrf(request)['csrf_token'],
             'course': course,
             #'recent_active_threads': recent_active_threads,
-            'staff_access': has_access(request.user, course, 'staff'),
+            'staff_access': has_access(request.user, 'staff', course),
             'threads': saxutils.escape(json.dumps(threads), escapedict),
             'thread_pages': query_params['num_pages'],
             'user_info': saxutils.escape(json.dumps(user_info), escapedict),
-            'flag_moderator': cached_has_permission(request.user, 'openclose_thread', course.id) or has_access(request.user, course, 'staff'),
+            'flag_moderator': cached_has_permission(request.user, 'openclose_thread', course.id) or has_access(request.user, 'staff', course),
             'annotated_content_info': saxutils.escape(json.dumps(annotated_content_info), escapedict),
-            'course_id': course.id,
+            'course_id': course.id.to_deprecated_string(),
             'category_map': category_map,
             'roles': saxutils.escape(json.dumps(utils.get_role_ids(course_id)), escapedict),
             'is_moderator': cached_has_permission(request.user, "see_all_cohorts", course_id),
@@ -228,9 +232,10 @@ def forum_form_discussion(request, course_id):
 @require_GET
 @login_required
 def single_thread(request, course_id, discussion_id, thread_id):
+    course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id)
     nr_transaction = newrelic.agent.current_transaction()
 
-    course = get_course_with_access(request.user, course_id, 'load_forum')
+    course = get_course_with_access(request.user, 'load_forum', course_id)
     cc_user = cc.User.from_django_user(request.user)
     user_info = cc_user.to_dict()
 
@@ -267,7 +272,7 @@ def single_thread(request, course_id, discussion_id, thread_id):
         threads, query_params = get_threads(request, course_id)
         threads.append(thread.to_dict())
 
-        course = get_course_with_access(request.user, course_id, 'load_forum')
+        course = get_course_with_access(request.user, 'load_forum', course_id)
 
         with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"):
             add_courseware_context(threads, course)
@@ -298,7 +303,7 @@ def single_thread(request, course_id, discussion_id, thread_id):
             'annotated_content_info': saxutils.escape(json.dumps(annotated_content_info), escapedict),
             'course': course,
             #'recent_active_threads': recent_active_threads,
-            'course_id': course.id,   # TODO: Why pass both course and course.id to template?
+            'course_id': course.id.to_deprecated_string(),   # TODO: Why pass both course and course.id to template?
             'thread_id': thread_id,
             'threads': saxutils.escape(json.dumps(threads), escapedict),
             'category_map': category_map,
@@ -306,7 +311,7 @@ def single_thread(request, course_id, discussion_id, thread_id):
             'thread_pages': query_params['num_pages'],
             'is_course_cohorted': is_course_cohorted(course_id),
             'is_moderator': cached_has_permission(request.user, "see_all_cohorts", course_id),
-            'flag_moderator': cached_has_permission(request.user, 'openclose_thread', course.id) or has_access(request.user, course, 'staff'),
+            'flag_moderator': cached_has_permission(request.user, 'openclose_thread', course.id) or has_access(request.user, 'staff', course),
             'cohorts': cohorts,
             'user_cohort': get_cohort_id(request.user, course_id),
             'cohorted_commentables': cohorted_commentables
@@ -317,10 +322,11 @@ def single_thread(request, course_id, discussion_id, thread_id):
 @require_GET
 @login_required
 def user_profile(request, course_id, user_id):
+    course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id)
     nr_transaction = newrelic.agent.current_transaction()
 
     #TODO: Allow sorting?
-    course = get_course_with_access(request.user, course_id, 'load_forum')
+    course = get_course_with_access(request.user, 'load_forum', course_id)
     try:
         profiled_user = cc.User(id=user_id, course_id=course_id)
 
@@ -365,9 +371,10 @@ def user_profile(request, course_id, user_id):
 
 @login_required
 def followed_threads(request, course_id, user_id):
+    course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id)
     nr_transaction = newrelic.agent.current_transaction()
 
-    course = get_course_with_access(request.user, course_id, 'load_forum')
+    course = get_course_with_access(request.user, 'load_forum', course_id)
     try:
         profiled_user = cc.User(id=user_id, course_id=course_id)
 

lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py

 from django.core.management.base import BaseCommand, CommandError
 from django_comment_common.utils import seed_permissions_roles
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 
 class Command(BaseCommand):
@@ -11,6 +12,6 @@ class Command(BaseCommand):
             raise CommandError("Please provide a course id")
         if len(args) > 1:
             raise CommandError("Too many arguments")
-        course_id = args[0]
+        course_id = SlashSeparatedCourseKey.from_deprecated_string(args[0])
 
         seed_permissions_roles(course_id)

lms/djangoapps/django_comment_client/tests/test_models.py

 import django_comment_common.models as models
 from django.test import TestCase
 
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
+
 
 class RoleClassTestCase(TestCase):
     def setUp(self):
         # For course ID, syntax edx/classname/classdate is important
         # because xmodel.course_module.id_to_location looks for a string to split
 
-        self.course_id = "edX/toy/2012_Fall"
+        self.course_id = SlashSeparatedCourseKey("edX", "toy", "2012_Fall")
         self.student_role = models.Role.objects.get_or_create(name="Student",
                                                               course_id=self.course_id)[0]
         self.student_role.add_permission("delete_thread")
@@ -15,7 +17,7 @@ class RoleClassTestCase(TestCase):
                                                                 course_id=self.course_id)[0]
         self.TA_role = models.Role.objects.get_or_create(name="Community TA",
                                                          course_id=self.course_id)[0]
-        self.course_id_2 = "edx/6.002x/2012_Fall"
+        self.course_id_2 = SlashSeparatedCourseKey("edx", "6.002x", "2012_Fall")
         self.TA_role_2 = models.Role.objects.get_or_create(name="Community TA",
                                                            course_id=self.course_id_2)[0]
 

lms/djangoapps/django_comment_client/tests/test_utils.py

@@ -41,9 +41,11 @@ class DictionaryTestCase(TestCase):
         self.assertEqual(utils.merge_dict(d1, d2), expected)
 
 
+@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
 class AccessUtilsTestCase(TestCase):
     def setUp(self):
-        self.course_id = 'edX/toy/2012_Fall'
+        self.course = CourseFactory.create()
+        self.course_id = self.course.id
         self.student_role = RoleFactory(name='Student', course_id=self.course_id)
         self.moderator_role = RoleFactory(name='Moderator', course_id=self.course_id)
         self.community_ta_role = RoleFactory(name='Community TA', course_id=self.course_id)
@@ -121,8 +123,8 @@ class CoursewareContextTestCase(ModuleStoreTestCase):
                 reverse(
                     "jump_to",
                     kwargs={
-                        "course_id": self.course.location.course_id,
-                        "location": discussion.location
+                        "course_id": self.course.id.to_deprecated_string(),
+                        "location": discussion.location.to_deprecated_string()
                     }
                 )
             )

lms/djangoapps/django_comment_client/utils.py

@@ -15,8 +15,9 @@ from edxmako import lookup_template
 import pystache_custom as pystache
 
 from xmodule.modulestore.django import modulestore
-from xmodule.modulestore import Location
 from django.utils.timezone import UTC
+from xmodule.modulestore.locations import i4xEncoder, SlashSeparatedCourseKey
+import json
 
 log = logging.getLogger(__name__)
 
@@ -55,10 +56,7 @@ def has_forum_access(uname, course_id, rolename):
 
 
 def _get_discussion_modules(course):
-    all_modules = modulestore().get_items(
-        Location('i4x', course.location.org, course.location.course, 'discussion', None),
-        course_id=course.id
-    )
+    all_modules = modulestore().get_items(course.id, category='discussion')
 
     def has_required_keys(module):
         for key in ('discussion_id', 'discussion_category', 'discussion_target'):
@@ -198,7 +196,7 @@ def get_discussion_category_map(course):
 
 class JsonResponse(HttpResponse):
     def __init__(self, data=None):
-        content = simplejson.dumps(data)
+        content = json.dumps(data, cls=i4xEncoder)
         super(JsonResponse, self).__init__(content,
                                            mimetype='application/json; charset=utf-8')
 
@@ -311,12 +309,16 @@ def render_mustache(template_name, dictionary, *args, **kwargs):
 
 
 def permalink(content):
+    if isinstance(content['course_id'], SlashSeparatedCourseKey):
+        course_id = content['course_id'].to_deprecated_string()
+    else:
+        course_id = content['course_id']
     if content['type'] == 'thread':
         return reverse('django_comment_client.forum.views.single_thread',
-                       args=[content['course_id'], content['commentable_id'], content['id']])
+                       args=[course_id, content['commentable_id'], content['id']])
     else:
         return reverse('django_comment_client.forum.views.single_thread',
-                       args=[content['course_id'], content['commentable_id'], content['thread_id']]) + '#' + content['id']
+                       args=[course_id, content['commentable_id'], content['thread_id']]) + '#' + content['id']
 
 
 def extend_content(content):
@@ -344,10 +346,10 @@ def add_courseware_context(content_list, course):
     for content in content_list:
         commentable_id = content['commentable_id']
         if commentable_id in id_map:
-            location = id_map[commentable_id]["location"].url()
+            location = id_map[commentable_id]["location"].to_deprecated_string()
             title = id_map[commentable_id]["title"]
 
-            url = reverse('jump_to', kwargs={"course_id": course.location.course_id,
+            url = reverse('jump_to', kwargs={"course_id": course.id.to_deprecated_string(),
                           "location": location})
 
             content.update({"courseware_url": url, "courseware_title": title})

lms/djangoapps/foldit/tests.py

@@ -8,11 +8,12 @@ from django.core.urlresolvers import reverse
 
 from foldit.views import foldit_ops, verify_code
 from foldit.models import PuzzleComplete, Score
-from student.models import unique_id_for_user
-from student.tests.factories import CourseEnrollmentFactory, UserFactory
+from student.models import unique_id_for_user, CourseEnrollment
+from student.tests.factories import UserFactory
 
 from datetime import datetime, timedelta
 from pytz import UTC
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 log = logging.getLogger(__name__)
 
@@ -23,18 +24,14 @@ class FolditTestCase(TestCase):
         self.factory = RequestFactory()
         self.url = reverse('foldit_ops')
 
-        self.course_id = 'course/id/1'
-        self.course_id2 = 'course/id/2'
+        self.course_id = SlashSeparatedCourseKey('course', 'id', '1')
+        self.course_id2 = SlashSeparatedCourseKey('course', 'id', '2')
 
         self.user = UserFactory.create()
         self.user2 = UserFactory.create()
 
-        self.course_enrollment = CourseEnrollmentFactory.create(
-            user=self.user, course_id=self.course_id
-        )
-        self.course_enrollment2 = CourseEnrollmentFactory.create(
-            user=self.user2, course_id=self.course_id2
-        )
+        CourseEnrollment.enroll(self.user, self.course_id)
+        CourseEnrollment.enroll(self.user2, self.course_id2)
 
         now = datetime.now(UTC)
         self.tomorrow = now + timedelta(days=1)

lms/djangoapps/instructor/access.py

@@ -31,7 +31,7 @@ def list_with_level(course, level):
     There could be other levels specific to the course.
     If there is no Group for that course-level, returns an empty list
     """
-    return ROLES[level](course.location).users_with_role()
+    return ROLES[level](course.id).users_with_role()
 
 
 def allow_access(course, user, level):
@@ -63,7 +63,7 @@ def _change_access(course, user, level, action):
     """
 
     try:
-        role = ROLES[level](course.location)
+        role = ROLES[level](course.id)
     except KeyError:
         raise ValueError("unrecognized level '{}'".format(level))
 

lms/djangoapps/instructor/enrollment.py

@@ -86,7 +86,6 @@ def enroll_email(course_id, student_email, auto_enroll=False, email_students=Fal
     returns two EmailEnrollmentState's
         representing state before and after the action.
     """
-
     previous_state = EmailEnrollmentState(course_id, student_email)
 
     if previous_state.user:
@@ -121,7 +120,6 @@ def unenroll_email(course_id, student_email, email_students=False, email_params=
     returns two EmailEnrollmentState's
         representing state before and after the action.
     """
-
     previous_state = EmailEnrollmentState(course_id, student_email)
 
     if previous_state.enrollment:
@@ -200,7 +198,7 @@ def reset_student_attempts(course_id, student, module_state_key, delete_module=F
     module_to_reset = StudentModule.objects.get(
         student_id=student.id,
         course_id=course_id,
-        module_state_key=module_state_key
+        module_id=module_state_key
     )
 
     if delete_module:
@@ -237,13 +235,15 @@ def get_email_params(course, auto_enroll):
         'SITE_NAME',
         settings.SITE_NAME
     )
+    # TODO: Use request.build_absolute_uri rather than 'https://{}{}'.format
+    # and check with the Services team that this works well with microsites
     registration_url = u'https://{}{}'.format(
         stripped_site_name,
         reverse('student.views.register_user')
     )
     course_url = u'https://{}{}'.format(
         stripped_site_name,
-        reverse('course_root', kwargs={'course_id': course.id})
+        reverse('course_root', kwargs={'course_id': course.id.to_deprecated_string()})
     )
 
     # We can't get the url to the course's About page if the marketing site is enabled.
@@ -251,7 +251,7 @@ def get_email_params(course, auto_enroll):
     if not settings.FEATURES.get('ENABLE_MKTG_SITE', False):
         course_about_url = u'https://{}{}'.format(
             stripped_site_name,
-            reverse('about_course', kwargs={'course_id': course.id})
+            reverse('about_course', kwargs={'course_id': course.id.to_deprecated_string()})
         )
 
     is_shib_course = uses_shib(course)

lms/djangoapps/instructor/features/bulk_email.py

@@ -29,23 +29,23 @@ def make_populated_course(step):  # pylint: disable=unused-argument
         number='888',
         display_name='Bulk Email Test Course'
     )
-    world.bulk_email_course_id = 'edx/888/Bulk_Email_Test_Course'
+    world.bulk_email_course_id = course.id
 
     try:
         # See if we've defined the instructor & staff user yet
         world.bulk_email_instructor
     except AttributeError:
         # Make & register an instructor for the course
-        world.bulk_email_instructor = InstructorFactory(course=course.location)
+        world.bulk_email_instructor = InstructorFactory(course=world.bulk_email_course_id)
         world.enroll_user(world.bulk_email_instructor, world.bulk_email_course_id)
 
         # Make & register a staff member
-        world.bulk_email_staff = StaffFactory(course=course.location)
+        world.bulk_email_staff = StaffFactory(course=course.id)
         world.enroll_user(world.bulk_email_staff, world.bulk_email_course_id)
 
     # Make & register a student
-    world.register_by_course_id(
-        'edx/888/Bulk_Email_Test_Course',
+    world.register_by_course_key(
+        course.id,
         username='student',
         password='test',
         is_staff=False

lms/djangoapps/instructor/features/common.py

@@ -43,12 +43,12 @@ def i_am_staff_or_instructor(step, role):  # pylint: disable=unused-argument
         display_name='Test Course'
     )
 
-    world.course_id = 'edx/999/Test_Course'
+    world.course_id = course.id
     world.role = 'instructor'
     # Log in as the an instructor or staff for the course
     if role == 'instructor':
         # Make & register an instructor for the course
-        world.instructor = InstructorFactory(course=course.location)
+        world.instructor = InstructorFactory(course=world.course_id)
         world.enroll_user(world.instructor, world.course_id)
 
         world.log_in(
@@ -61,7 +61,7 @@ def i_am_staff_or_instructor(step, role):  # pylint: disable=unused-argument
     else:
         world.role = 'staff'
         # Make & register a staff member
-        world.staff = StaffFactory(course=course.location)
+        world.staff = StaffFactory(course=world.course_id)
         world.enroll_user(world.staff, world.course_id)
 
         world.log_in(

lms/djangoapps/instructor/hint_manager.py

@@ -19,8 +19,9 @@ from courseware.courses import get_course_with_access
 from courseware.models import XModuleUserStateSummaryField
 import courseware.module_render as module_render
 import courseware.model_data as model_data
-from xmodule.modulestore import Location
 from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
+from xmodule.modulestore.exceptions import ItemNotFoundError
 
 
 @ensure_csrf_cookie
@@ -28,13 +29,14 @@ def hint_manager(request, course_id):
     """
     The URL landing function for all calls to the hint manager, both POST and GET.
     """
+    course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
     try:
-        get_course_with_access(request.user, course_id, 'staff', depth=None)
+        get_course_with_access(request.user, 'staff', course_key, depth=None)
     except Http404:
         out = 'Sorry, but students are not allowed to access the hint manager!'
         return HttpResponse(out)
     if request.method == 'GET':
-        out = get_hints(request, course_id, 'mod_queue')
+        out = get_hints(request, course_key, 'mod_queue')
         out.update({'error': ''})
         return render_to_response('instructor/hint_manager.html', out)
     field = request.POST['field']
@@ -52,10 +54,10 @@ def hint_manager(request, course_id):
     }
 
     # Do the operation requested, and collect any error messages.
-    error_text = switch_dict[request.POST['op']](request, course_id, field)
+    error_text = switch_dict[request.POST['op']](request, course_key, field)
     if error_text is None:
         error_text = ''
-    render_dict = get_hints(request, course_id, field)
+    render_dict = get_hints(request, course_key, field)
     render_dict.update({'error': error_text})
     rendered_html = render_to_string('instructor/hint_manager_inner.html', render_dict)
     return HttpResponse(json.dumps({'success': True, 'contents': rendered_html}))
@@ -86,13 +88,13 @@ def get_hints(request, course_id, field):
         other_field = 'mod_queue'
         field_label = 'Approved Hints'
         other_field_label = 'Hints Awaiting Moderation'
-    # The course_id is of the form school/number/classname.
     # We want to use the course_id to find all matching usage_id's.
     # To do this, just take the school/number part - leave off the classname.
-    course_id_dict = Location.parse_course_id(course_id)
-    chopped_id = u'{org}/{course}'.format(**course_id_dict)
-    chopped_id = re.escape(chopped_id)
-    all_hints = XModuleUserStateSummaryField.objects.filter(field_name=field, usage_id__regex=chopped_id)
+    # FIXME: we need to figure out how to do this with opaque keys
+    all_hints = XModuleUserStateSummaryField.objects.filter(
+        field_name=field,
+        usage_id__regex=re.escape(u'{0.org}/{0.course}'.format(course_id)),
+    )
     # big_out_dict[problem id] = [[answer, {pk: [hint, votes]}], sorted by answer]
     # big_out_dict maps a problem id to a list of [answer, hints] pairs, sorted in order of answer.
     big_out_dict = {}
@@ -101,8 +103,8 @@ def get_hints(request, course_id, field):
     id_to_name = {}
 
     for hints_by_problem in all_hints:
-        loc = Location(hints_by_problem.usage_id)
-        name = location_to_problem_name(course_id, loc)
+        hints_by_problem.usage_id = hints_by_problem.usage_id.map_into_course(course_id)
+        name = location_to_problem_name(course_id, hints_by_problem.usage_id)
         if name is None:
             continue
         id_to_name[hints_by_problem.usage_id] = name
@@ -138,9 +140,9 @@ def location_to_problem_name(course_id, loc):
     problem it wraps around.  Return None if the hinter no longer exists.
     """
     try:
-        descriptor = modulestore().get_items(loc, course_id=course_id)[0]
+        descriptor = modulestore().get_item(loc)
         return descriptor.get_children()[0].display_name
-    except IndexError:
+    except ItemNotFoundError:
         # Sometimes, the problem is no longer in the course.  Just
         # don't include said problem.
         return None
@@ -164,9 +166,10 @@ def delete_hints(request, course_id, field):
         if key == 'op' or key == 'field':
             continue
         problem_id, answer, pk = request.POST.getlist(key)
+        problem_key = course_id.make_usage_key_from_deprecated_string(problem_id)
         # Can be optimized - sort the delete list by problem_id, and load each problem
         # from the database only once.
-        this_problem = XModuleUserStateSummaryField.objects.get(field_name=field, usage_id=problem_id)
+        this_problem = XModuleUserStateSummaryField.objects.get(field_name=field, usage_id=problem_key)
         problem_dict = json.loads(this_problem.value)
         del problem_dict[answer][pk]
         this_problem.value = json.dumps(problem_dict)
@@ -191,7 +194,8 @@ def change_votes(request, course_id, field):
         if key == 'op' or key == 'field':
             continue
         problem_id, answer, pk, new_votes = request.POST.getlist(key)
-        this_problem = XModuleUserStateSummaryField.objects.get(field_name=field, usage_id=problem_id)
+        problem_key = course_id.make_usage_key_from_deprecated_string(problem_id)
+        this_problem = XModuleUserStateSummaryField.objects.get(field_name=field, usage_id=problem_key)
         problem_dict = json.loads(this_problem.value)
         # problem_dict[answer][pk] points to a [hint_text, #votes] pair.
         problem_dict[answer][pk][1] = int(new_votes)
@@ -210,23 +214,27 @@ def add_hint(request, course_id, field):
     """
 
     problem_id = request.POST['problem']
+    problem_key = course_id.make_usage_key_from_deprecated_string(problem_id)
     answer = request.POST['answer']
     hint_text = request.POST['hint']
 
     # Validate the answer.  This requires initializing the xmodules, which
     # is annoying.
-    loc = Location(problem_id)
-    descriptors = modulestore().get_items(loc, course_id=course_id)
+    try:
+        descriptor = modulestore().get_item(problem_key)
+        descriptors = [descriptor]
+    except ItemNotFoundError:
+        descriptors = []
     field_data_cache = model_data.FieldDataCache(descriptors, course_id, request.user)
-    hinter_module = module_render.get_module(request.user, request, loc, field_data_cache, course_id)
+    hinter_module = module_render.get_module(request.user, request, problem_key, field_data_cache, course_id)
     if not hinter_module.validate_answer(answer):
         # Invalid answer.  Don't add it to the database, or else the
         # hinter will crash when we encounter it.
         return 'Error - the answer you specified is not properly formatted: ' + str(answer)
 
-    this_problem = XModuleUserStateSummaryField.objects.get(field_name=field, usage_id=problem_id)
+    this_problem = XModuleUserStateSummaryField.objects.get(field_name=field, usage_id=problem_key)
 
-    hint_pk_entry = XModuleUserStateSummaryField.objects.get(field_name='hint_pk', usage_id=problem_id)
+    hint_pk_entry = XModuleUserStateSummaryField.objects.get(field_name='hint_pk', usage_id=problem_key)
     this_pk = int(hint_pk_entry.value)
     hint_pk_entry.value = this_pk + 1
     hint_pk_entry.save()
@@ -253,16 +261,17 @@ def approve(request, course_id, field):
         if key == 'op' or key == 'field':
             continue
         problem_id, answer, pk = request.POST.getlist(key)
+        problem_key = course_id.make_usage_key_from_deprecated_string(problem_id)
         # Can be optimized - sort the delete list by problem_id, and load each problem
         # from the database only once.
-        problem_in_mod = XModuleUserStateSummaryField.objects.get(field_name=field, usage_id=problem_id)
+        problem_in_mod = XModuleUserStateSummaryField.objects.get(field_name=field, usage_id=problem_key)
         problem_dict = json.loads(problem_in_mod.value)
         hint_to_move = problem_dict[answer][pk]
         del problem_dict[answer][pk]
         problem_in_mod.value = json.dumps(problem_dict)
         problem_in_mod.save()
 
-        problem_in_hints = XModuleUserStateSummaryField.objects.get(field_name='hints', usage_id=problem_id)
+        problem_in_hints = XModuleUserStateSummaryField.objects.get(field_name='hints', usage_id=problem_key)
         problem_dict = json.loads(problem_in_hints.value)
         if answer not in problem_dict:
             problem_dict[answer] = {}

lms/djangoapps/instructor/management/commands/openended_post.py

@@ -6,6 +6,8 @@ from django.core.management.base import BaseCommand
 from optparse import make_option
 
 from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.keys import UsageKey
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 from xmodule.open_ended_grading_classes.openendedchild import OpenEndedChild
 from xmodule.open_ended_grading_classes.open_ended_module import OpenEndedModule
 
@@ -37,8 +39,8 @@ class Command(BaseCommand):
         task_number = options['task_number']
 
         if len(args) == 4:
-            course_id = args[0]
-            location = args[1]
+            course_id = SlashSeparatedCourseKey.from_deprecated_string(args[0])
+            location = course_id.make_usage_key_from_deprecated_string(args[1])
             students_ids = [line.strip() for line in open(args[2])]
             hostname = args[3]
         else:
@@ -51,7 +53,7 @@ class Command(BaseCommand):
             print err
             return
 
-        descriptor = modulestore().get_instance(course.id, location, depth=0)
+        descriptor = modulestore().get_item(location, depth=0)
         if descriptor is None:
             print "Location not found in course"
             return
@@ -76,7 +78,7 @@ def post_submission_for_student(student, course, location, task_number, dry_run=
     request.host = hostname
 
     try:
-        module = get_module_for_student(student, course, location, request=request)
+        module = get_module_for_student(student, location, request=request)
         if module is None:
             print "  WARNING: No state found."
             return False

lms/djangoapps/instructor/management/commands/openended_stats.py

@@ -9,6 +9,8 @@ from optparse import make_option
 
 from xmodule.modulestore import Location
 from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.keys import UsageKey
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 from xmodule.open_ended_grading_classes.openendedchild import OpenEndedChild
 
 from courseware.courses import get_course
@@ -37,8 +39,8 @@ class Command(BaseCommand):
         task_number = options['task_number']
 
         if len(args) == 2:
-            course_id = args[0]
-            location = args[1]
+            course_id = SlashSeparatedCourseKey.from_deprecated_string(args[0])
+            usage_key = UsageKey.from_string(args[1])
         else:
             print self.help
             return
@@ -49,16 +51,16 @@ class Command(BaseCommand):
             print err
             return
 
-        descriptor = modulestore().get_instance(course.id, location, depth=0)
+        descriptor = modulestore().get_item(usage_key, depth=0)
         if descriptor is None:
-            print "Location {0} not found in course".format(location)
+            print "Location {0} not found in course".format(usage_key)
             return
 
         try:
             enrolled_students = CourseEnrollment.users_enrolled_in(course_id)
             print "Total students enrolled in {0}: {1}".format(course_id, enrolled_students.count())
 
-            calculate_task_statistics(enrolled_students, course, location, task_number)
+            calculate_task_statistics(enrolled_students, course, usage_key, task_number)
 
         except KeyboardInterrupt:
             print "\nOperation Cancelled"
@@ -79,7 +81,7 @@ def calculate_task_statistics(students, course, location, task_number, write_to_
     students_with_graded_submissions = []  # pylint: disable=invalid-name
     students_with_no_state = []
 
-    student_modules = StudentModule.objects.filter(module_state_key=location, student__in=students).order_by('student')
+    student_modules = StudentModule.objects.filter(module_id=location, student__in=students).order_by('student')
     print "Total student modules: {0}".format(student_modules.count())
 
     for index, student_module in enumerate(student_modules):
@@ -89,7 +91,7 @@ def calculate_task_statistics(students, course, location, task_number, write_to_
         student = student_module.student
         print "{0}:{1}".format(student.id, student.username)
 
-        module = get_module_for_student(student, course, location)
+        module = get_module_for_student(student, location)
         if module is None:
             print "  WARNING: No state found"
             students_with_no_state.append(student)
@@ -113,8 +115,6 @@ def calculate_task_statistics(students, course, location, task_number, write_to_
         elif task_state == OpenEndedChild.POST_ASSESSMENT or task_state == OpenEndedChild.DONE:
             students_with_graded_submissions.append(student)
 
-    location = Location(location)
-
     print "----------------------------------"
     print "Time: {0}".format(time.strftime("%Y %b %d %H:%M:%S +0000", time.gmtime()))
     print "Course: {0}".format(course.id)
@@ -132,7 +132,7 @@ def calculate_task_statistics(students, course, location, task_number, write_to_
         with open('{0}.{1}.csv'.format(filename, time_stamp), 'wb') as csv_file:
             writer = csv.writer(csv_file, delimiter=' ', quoting=csv.QUOTE_MINIMAL)
             for student in students_with_ungraded_submissions:
-                writer.writerow(("ungraded", student.id, anonymous_id_for_user(student, ''), student.username))
+                writer.writerow(("ungraded", student.id, anonymous_id_for_user(student, None), student.username))
             for student in students_with_graded_submissions:
-                writer.writerow(("graded", student.id, anonymous_id_for_user(student, ''), student.username))
+                writer.writerow(("graded", student.id, anonymous_id_for_user(student, None), student.username))
     return stats

lms/djangoapps/instructor/management/tests/test_openended_commands.py

@@ -24,14 +24,16 @@ from instructor.management.commands.openended_post import post_submission_for_st
 from instructor.management.commands.openended_stats import calculate_task_statistics
 from instructor.utils import get_module_for_student
 
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
+
 
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
 class OpenEndedPostTest(ModuleStoreTestCase):
     """Test the openended_post management command."""
 
     def setUp(self):
-        self.course_id = "edX/open_ended/2012_Fall"
-        self.problem_location = Location(["i4x", "edX", "open_ended", "combinedopenended", "SampleQuestion"])
+        self.course_id = SlashSeparatedCourseKey("edX", "open_ended", "2012_Fall")
+        self.problem_location = Location("edX", "open_ended", "2012_Fall", "combinedopenended", "SampleQuestion")
         self.self_assessment_task_number = 0
         self.open_ended_task_number = 1
 
@@ -41,7 +43,7 @@ class OpenEndedPostTest(ModuleStoreTestCase):
 
         StudentModuleFactory.create(
             course_id=self.course_id,
-            module_state_key=self.problem_location,
+            module_id=self.problem_location,
             student=self.student_on_initial,
             grade=0,
             max_grade=1,
@@ -50,7 +52,7 @@ class OpenEndedPostTest(ModuleStoreTestCase):
 
         StudentModuleFactory.create(
             course_id=self.course_id,
-            module_state_key=self.problem_location,
+            module_id=self.problem_location,
             student=self.student_on_accessing,
             grade=0,
             max_grade=1,
@@ -59,7 +61,7 @@ class OpenEndedPostTest(ModuleStoreTestCase):
 
         StudentModuleFactory.create(
             course_id=self.course_id,
-            module_state_key=self.problem_location,
+            module_id=self.problem_location,
             student=self.student_on_post_assessment,
             grade=0,
             max_grade=1,
@@ -67,7 +69,7 @@ class OpenEndedPostTest(ModuleStoreTestCase):
         )
 
     def test_post_submission_for_student_on_initial(self):
-        course = get_course_with_access(self.student_on_initial, self.course_id, 'load')
+        course = get_course_with_access(self.student_on_initial, 'load', self.course_id)
 
         dry_run_result = post_submission_for_student(self.student_on_initial, course, self.problem_location, self.open_ended_task_number, dry_run=True)
         self.assertFalse(dry_run_result)
@@ -76,7 +78,7 @@ class OpenEndedPostTest(ModuleStoreTestCase):
         self.assertFalse(result)
 
     def test_post_submission_for_student_on_accessing(self):
-        course = get_course_with_access(self.student_on_accessing, self.course_id, 'load')
+        course = get_course_with_access(self.student_on_accessing, 'load', self.course_id)
 
         dry_run_result = post_submission_for_student(self.student_on_accessing, course, self.problem_location, self.open_ended_task_number, dry_run=True)
         self.assertFalse(dry_run_result)
@@ -84,11 +86,11 @@ class OpenEndedPostTest(ModuleStoreTestCase):
         with patch('capa.xqueue_interface.XQueueInterface.send_to_queue') as mock_send_to_queue:
             mock_send_to_queue.return_value = (0, "Successfully queued")
 
-            module = get_module_for_student(self.student_on_accessing, course, self.problem_location)
+            module = get_module_for_student(self.student_on_accessing, self.problem_location)
             task = module.child_module.get_task_number(self.open_ended_task_number)
 
             student_response = "Here is an answer."
-            student_anonymous_id = anonymous_id_for_user(self.student_on_accessing, '')
+            student_anonymous_id = anonymous_id_for_user(self.student_on_accessing, None)
             submission_time = datetime.strftime(datetime.now(UTC), xqueue_interface.dateformat)
 
             result = post_submission_for_student(self.student_on_accessing, course, self.problem_location, self.open_ended_task_number, dry_run=False)
@@ -102,7 +104,7 @@ class OpenEndedPostTest(ModuleStoreTestCase):
             self.assertGreaterEqual(body_arg_student_info['submission_time'], submission_time)
 
     def test_post_submission_for_student_on_post_assessment(self):
-        course = get_course_with_access(self.student_on_post_assessment, self.course_id, 'load')
+        course = get_course_with_access(self.student_on_post_assessment, 'load', self.course_id)
 
         dry_run_result = post_submission_for_student(self.student_on_post_assessment, course, self.problem_location, self.open_ended_task_number, dry_run=True)
         self.assertFalse(dry_run_result)
@@ -111,7 +113,7 @@ class OpenEndedPostTest(ModuleStoreTestCase):
         self.assertFalse(result)
 
     def test_post_submission_for_student_invalid_task(self):
-        course = get_course_with_access(self.student_on_accessing, self.course_id, 'load')
+        course = get_course_with_access(self.student_on_accessing, 'load', self.course_id)
 
         result = post_submission_for_student(self.student_on_accessing, course, self.problem_location, self.self_assessment_task_number, dry_run=False)
         self.assertFalse(result)
@@ -126,8 +128,8 @@ class OpenEndedStatsTest(ModuleStoreTestCase):
     """Test the openended_stats management command."""
 
     def setUp(self):
-        self.course_id = "edX/open_ended/2012_Fall"
-        self.problem_location = Location(["i4x", "edX", "open_ended", "combinedopenended", "SampleQuestion"])
+        self.course_id = SlashSeparatedCourseKey("edX", "open_ended", "2012_Fall")
+        self.problem_location = Location("edX", "open_ended", "2012_Fall", "combinedopenended", "SampleQuestion")
         self.task_number = 1
         self.invalid_task_number = 3
 
@@ -137,7 +139,7 @@ class OpenEndedStatsTest(ModuleStoreTestCase):
 
         StudentModuleFactory.create(
             course_id=self.course_id,
-            module_state_key=self.problem_location,
+            module_id=self.problem_location,
             student=self.student_on_initial,
             grade=0,
             max_grade=1,
@@ -146,7 +148,7 @@ class OpenEndedStatsTest(ModuleStoreTestCase):
 
         StudentModuleFactory.create(
             course_id=self.course_id,
-            module_state_key=self.problem_location,
+            module_id=self.problem_location,
             student=self.student_on_accessing,
             grade=0,
             max_grade=1,
@@ -155,7 +157,7 @@ class OpenEndedStatsTest(ModuleStoreTestCase):
 
         StudentModuleFactory.create(
             course_id=self.course_id,
-            module_state_key=self.problem_location,
+            module_id=self.problem_location,
             student=self.student_on_post_assessment,
             grade=0,
             max_grade=1,
@@ -165,7 +167,7 @@ class OpenEndedStatsTest(ModuleStoreTestCase):
         self.students = [self.student_on_initial, self.student_on_accessing, self.student_on_post_assessment]
 
     def test_calculate_task_statistics(self):
-        course = get_course_with_access(self.student_on_accessing, self.course_id, 'load')
+        course = get_course_with_access(self.student_on_accessing, 'load', self.course_id)
         stats = calculate_task_statistics(self.students, course, self.problem_location, self.task_number, write_to_file=False)
         self.assertEqual(stats[OpenEndedChild.INITIAL], 1)
         self.assertEqual(stats[OpenEndedChild.ASSESSING], 1)

lms/djangoapps/instructor/tests/test_access.py

@@ -50,19 +50,19 @@ class TestInstructorAccessAllow(ModuleStoreTestCase):
     def test_allow(self):
         user = UserFactory()
         allow_access(self.course, user, 'staff')
-        self.assertTrue(CourseStaffRole(self.course.location).has_user(user))
+        self.assertTrue(CourseStaffRole(self.course.id).has_user(user))
 
     def test_allow_twice(self):
         user = UserFactory()
         allow_access(self.course, user, 'staff')
         allow_access(self.course, user, 'staff')
-        self.assertTrue(CourseStaffRole(self.course.location).has_user(user))
+        self.assertTrue(CourseStaffRole(self.course.id).has_user(user))
 
     def test_allow_beta(self):
         """ Test allow beta against list beta. """
         user = UserFactory()
         allow_access(self.course, user, 'beta')
-        self.assertTrue(CourseBetaTesterRole(self.course.location).has_user(user))
+        self.assertTrue(CourseBetaTesterRole(self.course.id).has_user(user))
 
     @raises(ValueError)
     def test_allow_badlevel(self):
@@ -91,17 +91,17 @@ class TestInstructorAccessRevoke(ModuleStoreTestCase):
     def test_revoke(self):
         user = self.staff[0]
         revoke_access(self.course, user, 'staff')
-        self.assertFalse(CourseStaffRole(self.course.location).has_user(user))
+        self.assertFalse(CourseStaffRole(self.course.id).has_user(user))
 
     def test_revoke_twice(self):
         user = self.staff[0]
         revoke_access(self.course, user, 'staff')
-        self.assertFalse(CourseStaffRole(self.course.location).has_user(user))
+        self.assertFalse(CourseStaffRole(self.course.id).has_user(user))
 
     def test_revoke_beta(self):
         user = self.beta_testers[0]
         revoke_access(self.course, user, 'beta')
-        self.assertFalse(CourseBetaTesterRole(self.course.location).has_user(user))
+        self.assertFalse(CourseBetaTesterRole(self.course.id).has_user(user))
 
     @raises(ValueError)
     def test_revoke_badrolename(self):

lms/djangoapps/instructor/tests/test_email.py

@@ -18,6 +18,7 @@ from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
 from mock import patch
 
 from bulk_email.models import CourseAuthorization
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 
 @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
@@ -34,7 +35,7 @@ class TestNewInstructorDashboardEmailViewMongoBacked(ModuleStoreTestCase):
         self.client.login(username=instructor.username, password="test")
 
         # URL for instructor dash
-        self.url = reverse('instructor_dashboard_2', kwargs={'course_id': self.course.id})
+        self.url = reverse('instructor_dashboard_2', kwargs={'course_id': self.course.id.to_deprecated_string()})
         # URL for email view
         self.email_link = '<a href="" data-section="send_email">Email</a>'
 
@@ -115,14 +116,14 @@ class TestNewInstructorDashboardEmailViewXMLBacked(ModuleStoreTestCase):
     Check for email view on the new instructor dashboard
     """
     def setUp(self):
-        self.course_name = 'edX/toy/2012_Fall'
+        self.course_key = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')
 
         # Create instructor account
         instructor = AdminFactory.create()
         self.client.login(username=instructor.username, password="test")
 
         # URL for instructor dash
-        self.url = reverse('instructor_dashboard_2', kwargs={'course_id': self.course_name})
+        self.url = reverse('instructor_dashboard_2', kwargs={'course_id': self.course_key.to_deprecated_string()})
         # URL for email view
         self.email_link = '<a href="" data-section="send_email">Email</a>'
 

lms/djangoapps/instructor/tests/test_hint_manager.py

@@ -26,8 +26,8 @@ class HintManagerTest(ModuleStoreTestCase):
         self.user = UserFactory.create(username='robot', email='robot@edx.org', password='test', is_staff=True)
         self.c = Client()
         self.c.login(username='robot', password='test')
-        self.problem_id = 'i4x://Me/19.002/crowdsource_hinter/crowdsource_hinter_001'
-        self.course_id = 'Me/19.002/test_course'
+        self.course_id = self.course.id
+        self.problem_id = self.course_id.make_usage_key('crowdsource_hinter', 'crowdsource_hinter_001')
         UserStateSummaryFactory.create(field_name='hints',
                               usage_id=self.problem_id,
                               value=json.dumps({'1.0': {'1': ['Hint 1', 2],
@@ -60,7 +60,7 @@ class HintManagerTest(ModuleStoreTestCase):
         """
         Makes sure that staff can access the hint management view.
         """
-        out = self.c.get('/courses/Me/19.002/test_course/hint_manager')
+        out = self.c.get(self.url)
         print out
         self.assertTrue('Hints Awaiting Moderation' in out.content)
 
@@ -115,7 +115,7 @@ class HintManagerTest(ModuleStoreTestCase):
         request = RequestFactory()
         post = request.post(self.url, {'field': 'hints',
                                        'op': 'delete hints',
-                                       1: [self.problem_id, '1.0', '1']})
+                                       1: [self.problem_id.to_deprecated_string(), '1.0', '1']})
         view.delete_hints(post, self.course_id, 'hints')
         problem_hints = XModuleUserStateSummaryField.objects.get(field_name='hints', usage_id=self.problem_id).value
         self.assertTrue('1' not in json.loads(problem_hints)['1.0'])
@@ -127,7 +127,7 @@ class HintManagerTest(ModuleStoreTestCase):
         request = RequestFactory()
         post = request.post(self.url, {'field': 'hints',
                                        'op': 'change votes',
-                                       1: [self.problem_id, '1.0', '1', 5]})
+                                       1: [self.problem_id.to_deprecated_string(), '1.0', '1', 5]})
         view.change_votes(post, self.course_id, 'hints')
         problem_hints = XModuleUserStateSummaryField.objects.get(field_name='hints', usage_id=self.problem_id).value
         # hints[answer][hint_pk (string)] = [hint text, vote count]
@@ -146,7 +146,7 @@ class HintManagerTest(ModuleStoreTestCase):
         request = RequestFactory()
         post = request.post(self.url, {'field': 'mod_queue',
                                        'op': 'add hint',
-                                       'problem': self.problem_id,
+                                       'problem': self.problem_id.to_deprecated_string(),
                                        'answer': '3.14',
                                        'hint': 'This is a new hint.'})
         post.user = 'fake user'
@@ -167,7 +167,7 @@ class HintManagerTest(ModuleStoreTestCase):
         request = RequestFactory()
         post = request.post(self.url, {'field': 'mod_queue',
                                        'op': 'add hint',
-                                       'problem': self.problem_id,
+                                       'problem': self.problem_id.to_deprecated_string(),
                                        'answer': 'fish',
                                        'hint': 'This is a new hint.'})
         post.user = 'fake user'
@@ -185,7 +185,7 @@ class HintManagerTest(ModuleStoreTestCase):
         request = RequestFactory()
         post = request.post(self.url, {'field': 'mod_queue',
                                        'op': 'approve',
-                                       1: [self.problem_id, '2.0', '2']})
+                                       1: [self.problem_id.to_deprecated_string(), '2.0', '2']})
         view.approve(post, self.course_id, 'mod_queue')
         problem_hints = XModuleUserStateSummaryField.objects.get(field_name='mod_queue', usage_id=self.problem_id).value
         self.assertTrue('2.0' not in json.loads(problem_hints) or len(json.loads(problem_hints)['2.0']) == 0)

lms/djangoapps/instructor/tests/test_legacy_anon_csv.py

@@ -20,6 +20,7 @@ from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
 from student.roles import CourseStaffRole
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.django import modulestore, clear_existing_modulestores
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 from mock import patch
 
@@ -33,7 +34,7 @@ class TestInstructorDashboardAnonCSV(ModuleStoreTestCase, LoginEnrollmentTestCas
     # Note -- I copied this setUp from a similar test
     def setUp(self):
         clear_existing_modulestores()
-        self.toy = modulestore().get_course("edX/toy/2012_Fall")
+        self.toy = modulestore().get_course(SlashSeparatedCourseKey("edX", "toy", "2012_Fall"))
 
         # Create two accounts
         self.student = 'view@test.com'
@@ -44,7 +45,7 @@ class TestInstructorDashboardAnonCSV(ModuleStoreTestCase, LoginEnrollmentTestCas
         self.activate_user(self.student)
         self.activate_user(self.instructor)
 
-        CourseStaffRole(self.toy.location).add_users(User.objects.get(email=self.instructor))
+        CourseStaffRole(self.toy.id).add_users(User.objects.get(email=self.instructor))
 
         self.logout()
         self.login(self.instructor, self.password)
@@ -52,7 +53,7 @@ class TestInstructorDashboardAnonCSV(ModuleStoreTestCase, LoginEnrollmentTestCas
 
     def test_download_anon_csv(self):
         course = self.toy
-        url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': course.id.to_deprecated_string()})
 
         with patch('instructor.views.legacy.unique_id_for_user') as mock_unique:
             mock_unique.return_value = 42

lms/djangoapps/instructor/tests/test_legacy_download_csv.py

@@ -20,6 +20,7 @@ from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
 from student.roles import CourseStaffRole
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.django import modulestore, clear_existing_modulestores
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
@@ -30,7 +31,7 @@ class TestInstructorDashboardGradeDownloadCSV(ModuleStoreTestCase, LoginEnrollme
 
     def setUp(self):
         clear_existing_modulestores()
-        self.toy = modulestore().get_course("edX/toy/2012_Fall")
+        self.toy = modulestore().get_course(SlashSeparatedCourseKey("edX", "toy", "2012_Fall"))
 
         # Create two accounts
         self.student = 'view@test.com'
@@ -41,7 +42,7 @@ class TestInstructorDashboardGradeDownloadCSV(ModuleStoreTestCase, LoginEnrollme
         self.activate_user(self.student)
         self.activate_user(self.instructor)
 
-        CourseStaffRole(self.toy.location).add_users(User.objects.get(email=self.instructor))
+        CourseStaffRole(self.toy.id).add_users(User.objects.get(email=self.instructor))
 
         self.logout()
         self.login(self.instructor, self.password)
@@ -49,7 +50,7 @@ class TestInstructorDashboardGradeDownloadCSV(ModuleStoreTestCase, LoginEnrollme
 
     def test_download_grades_csv(self):
         course = self.toy
-        url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': course.id.to_deprecated_string()})
         msg = "url = {0}\n".format(url)
         response = self.client.post(url, {'action': 'Download CSV of all student grades for this course'})
         msg += "instructor dashboard download csv grades: response = '{0}'\n".format(response)
@@ -58,7 +59,7 @@ class TestInstructorDashboardGradeDownloadCSV(ModuleStoreTestCase, LoginEnrollme
 
         cdisp = response['Content-Disposition']
         msg += "Content-Disposition = '%s'\n" % cdisp
-        self.assertEqual(cdisp, 'attachment; filename=grades_{0}.csv'.format(course.id), msg)
+        self.assertEqual(cdisp, 'attachment; filename=grades_{0}.csv'.format(course.id.to_deprecated_string()), msg)
 
         body = response.content.replace('\r', '')
         msg += "body = '{0}'\n".format(body)

lms/djangoapps/instructor/tests/test_legacy_email.py

@@ -32,7 +32,7 @@ class TestInstructorDashboardEmailView(ModuleStoreTestCase):
         self.client.login(username=instructor.username, password="test")
 
         # URL for instructor dash
-        self.url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id})
+        self.url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id.to_deprecated_string()})
         # URL for email view
         self.email_link = '<a href="#" onclick="goto(\'Email\')" class="None">Email</a>'
 

lms/djangoapps/instructor/tests/test_legacy_enrollment.py

@@ -52,7 +52,7 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
         course = self.course
 
         # Run the Un-enroll students command
-        url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': course.id.to_deprecated_string()})
         response = self.client.post(
             url,
             {
@@ -84,7 +84,7 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
         course = self.course
 
         # Run the Enroll students command
-        url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': course.id.to_deprecated_string()})
         response = self.client.post(url, {'action': 'Enroll multiple students', 'multiple_students': 'student1_1@test.com, student1_2@test.com', 'auto_enroll': 'on'})
 
         # Check the page output
@@ -129,7 +129,7 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
 
         course = self.course
 
-        url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': course.id.to_deprecated_string()})
         response = self.client.post(url, {'action': 'Enroll multiple students', 'multiple_students': 'student0@test.com', 'auto_enroll': 'on'})
         self.assertContains(response, '<td>student0@test.com</td>')
         self.assertContains(response, '<td>already enrolled</td>')
@@ -142,7 +142,7 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
         course = self.course
 
         # Run the Enroll students command
-        url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': course.id.to_deprecated_string()})
         response = self.client.post(url, {'action': 'Enroll multiple students', 'multiple_students': 'student2_1@test.com, student2_2@test.com'})
 
         # Check the page output
@@ -199,7 +199,7 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
         # Create activated, but not enrolled, user
         UserFactory.create(username="student3_0", email="student3_0@test.com", first_name='Autoenrolled')
 
-        url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': course.id.to_deprecated_string()})
         response = self.client.post(url, {'action': 'Enroll multiple students', 'multiple_students': 'student3_0@test.com, student3_1@test.com, student3_2@test.com', 'auto_enroll': 'on', 'email_students': 'on'})
 
         # Check the page output
@@ -254,7 +254,7 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
         cea = CourseEnrollmentAllowed(email='student4_0@test.com', course_id=course.id)
         cea.save()
 
-        url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': course.id.to_deprecated_string()})
         response = self.client.post(url, {'action': 'Unenroll multiple students', 'multiple_students': 'student4_0@test.com, student2@test.com, student3@test.com', 'email_students': 'on'})
 
         # Check the page output
@@ -301,7 +301,7 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
         # Create activated, but not enrolled, user
         UserFactory.create(username="student5_0", email="student5_0@test.com", first_name="ShibTest", last_name="Enrolled")
 
-        url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': course.id.to_deprecated_string()})
         response = self.client.post(url, {'action': 'Enroll multiple students', 'multiple_students': 'student5_0@test.com, student5_1@test.com', 'auto_enroll': 'on', 'email_students': 'on'})
 
         # Check the page output

lms/djangoapps/instructor/tests/test_legacy_forum_admin.py

@@ -17,6 +17,7 @@ from courseware.tests.helpers import LoginEnrollmentTestCase
 from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
 from student.roles import CourseStaffRole
 from xmodule.modulestore.django import modulestore, clear_existing_modulestores
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 
 
@@ -42,7 +43,7 @@ class TestInstructorDashboardForumAdmin(ModuleStoreTestCase, LoginEnrollmentTest
         clear_existing_modulestores()
         courses = modulestore().get_courses()
 
-        self.course_id = "edX/toy/2012_Fall"
+        self.course_id = SlashSeparatedCourseKey("edX", "toy", "2012_Fall")
         self.toy = modulestore().get_course(self.course_id)
 
         # Create two accounts
@@ -54,7 +55,7 @@ class TestInstructorDashboardForumAdmin(ModuleStoreTestCase, LoginEnrollmentTest
         self.activate_user(self.student)
         self.activate_user(self.instructor)
 
-        CourseStaffRole(self.toy.location).add_users(User.objects.get(email=self.instructor))
+        CourseStaffRole(self.toy.id).add_users(User.objects.get(email=self.instructor))
 
         self.logout()
         self.login(self.instructor, self.password)
@@ -67,7 +68,7 @@ class TestInstructorDashboardForumAdmin(ModuleStoreTestCase, LoginEnrollmentTest
 
     def test_add_forum_admin_users_for_unknown_user(self):
         course = self.toy
-        url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': course.id.to_deprecated_string()})
         username = 'unknown'
         for action in ['Add', 'Remove']:
             for rolename in FORUM_ROLES:
@@ -76,7 +77,7 @@ class TestInstructorDashboardForumAdmin(ModuleStoreTestCase, LoginEnrollmentTest
 
     def test_add_forum_admin_users_for_missing_roles(self):
         course = self.toy
-        url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': course.id.to_deprecated_string()})
         username = 'u1'
         for action in ['Add', 'Remove']:
             for rolename in FORUM_ROLES:
@@ -86,7 +87,7 @@ class TestInstructorDashboardForumAdmin(ModuleStoreTestCase, LoginEnrollmentTest
     def test_remove_forum_admin_users_for_missing_users(self):
         course = self.toy
         self.initialize_roles(course.id)
-        url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': course.id.to_deprecated_string()})
         username = 'u1'
         action = 'Remove'
         for rolename in FORUM_ROLES:
@@ -96,20 +97,20 @@ class TestInstructorDashboardForumAdmin(ModuleStoreTestCase, LoginEnrollmentTest
     def test_add_and_remove_forum_admin_users(self):
         course = self.toy
         self.initialize_roles(course.id)
-        url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': course.id.to_deprecated_string()})
         username = 'u2'
         for rolename in FORUM_ROLES:
             response = self.client.post(url, {'action': action_name('Add', rolename), FORUM_ADMIN_USER[rolename]: username})
-            self.assertTrue(response.content.find('Added "{0}" to "{1}" forum role = "{2}"'.format(username, course.id, rolename)) >= 0)
+            self.assertContains(response, 'Added "{0}" to "{1}" forum role = "{2}"'.format(username, course.id.to_deprecated_string(), rolename))
             self.assertTrue(has_forum_access(username, course.id, rolename))
             response = self.client.post(url, {'action': action_name('Remove', rolename), FORUM_ADMIN_USER[rolename]: username})
-            self.assertTrue(response.content.find('Removed "{0}" from "{1}" forum role = "{2}"'.format(username, course.id, rolename)) >= 0)
+            self.assertContains(response, 'Removed "{0}" from "{1}" forum role = "{2}"'.format(username, course.id.to_deprecated_string(), rolename))
             self.assertFalse(has_forum_access(username, course.id, rolename))
 
     def test_add_and_read_forum_admin_users(self):
         course = self.toy
         self.initialize_roles(course.id)
-        url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': course.id.to_deprecated_string()})
         username = 'u2'
         for rolename in FORUM_ROLES:
             # perform an add, and follow with a second identical add:
@@ -121,7 +122,7 @@ class TestInstructorDashboardForumAdmin(ModuleStoreTestCase, LoginEnrollmentTest
     def test_add_nonstaff_forum_admin_users(self):
         course = self.toy
         self.initialize_roles(course.id)
-        url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': course.id.to_deprecated_string()})
         username = 'u1'
         rolename = FORUM_ROLE_ADMINISTRATOR
         response = self.client.post(url, {'action': action_name('Add', rolename), FORUM_ADMIN_USER[rolename]: username})
@@ -130,7 +131,7 @@ class TestInstructorDashboardForumAdmin(ModuleStoreTestCase, LoginEnrollmentTest
     def test_list_forum_admin_users(self):
         course = self.toy
         self.initialize_roles(course.id)
-        url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': course.id.to_deprecated_string()})
         username = 'u2'
         added_roles = [FORUM_ROLE_STUDENT]  # u2 is already added as a student to the discussion forums
         self.assertTrue(has_forum_access(username, course.id, 'Student'))

lms/djangoapps/instructor/tests/test_legacy_gradebook.py

@@ -11,6 +11,7 @@ from courseware.tests.tests import TEST_DATA_MIXED_MODULESTORE
 from capa.tests.response_xml_factory import StringResponseXMLFactory
 from courseware.tests.factories import StudentModuleFactory
 from xmodule.modulestore import Location
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 from xmodule.modulestore.django import modulestore
 
 
@@ -64,10 +65,13 @@ class TestGradebook(ModuleStoreTestCase):
                     max_grade=1,
                     student=user,
                     course_id=self.course.id,
-                    module_state_key=Location(item.location).url()
+                    module_id=item.location
                 )
 
-        self.response = self.client.get(reverse('gradebook', args=(self.course.id,)))
+        self.response = self.client.get(reverse(
+            'gradebook',
+            args=(self.course.id.to_deprecated_string(),)
+        ))
 
     def test_response_code(self):
         self.assertEquals(self.response.status_code, 200)

lms/djangoapps/instructor/tests/test_legacy_raw_download_csv.py

@@ -24,7 +24,7 @@ class TestRawGradeCSV(TestSubmittingProblems):
         self.instructor = 'view2@test.com'
         self.create_account('u2', self.instructor, self.password)
         self.activate_user(self.instructor)
-        CourseStaffRole(self.course.location).add_users(User.objects.get(email=self.instructor))
+        CourseStaffRole(self.course.id).add_users(User.objects.get(email=self.instructor))
         self.logout()
         self.login(self.instructor, self.password)
         self.enroll(self.course)
@@ -45,7 +45,7 @@ class TestRawGradeCSV(TestSubmittingProblems):
         resp = self.submit_question_answer('p2', {'2_1': 'Correct'})
         self.assertEqual(resp.status_code, 200)
 
-        url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id.to_deprecated_string()})
         msg = "url = {0}\n".format(url)
         response = self.client.post(url, {'action': 'Download CSV of all RAW grades'})
         msg += "instructor dashboard download raw csv grades: response = '{0}'\n".format(response)

lms/djangoapps/instructor/tests/test_legacy_reset.py

@@ -16,6 +16,7 @@ from courseware.models import StudentModule
 
 from submissions import api as sub_api
 from student.models import anonymous_id_for_user
+from .test_tools import msk_from_problem_urlname
 
 
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
@@ -35,29 +36,36 @@ class InstructorResetStudentStateTest(ModuleStoreTestCase, LoginEnrollmentTestCa
         CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id)
 
     def test_delete_student_state_resets_scores(self):
-        item_id = 'i4x://MITx/999/openassessment/b3dce2586c9c4876b73e7f390e42ef8f'
+        problem_location = msk_from_problem_urlname(
+            self.course.id,
+            'b3dce2586c9c4876b73e7f390e42ef8f',
+            block_type='openassessment'
+        )
 
         # Create a student module for the user
         StudentModule.objects.create(
-            student=self.student, course_id=self.course.id, module_state_key=item_id, state=json.dumps({})
+            student=self.student,
+            course_id=self.course.id,
+            module_state_key=problem_location,
+            state=json.dumps({})
         )
 
         # Create a submission and score for the student using the submissions API
         student_item = {
             'student_id': anonymous_id_for_user(self.student, self.course.id),
             'course_id': self.course.id,
-            'item_id': item_id,
+            'item_id': problem_location,
             'item_type': 'openassessment'
         }
         submission = sub_api.create_submission(student_item, 'test answer')
         sub_api.set_score(submission['uuid'], 1, 2)
 
         # Delete student state using the instructor dash
-        url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id})
+        url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id.to_deprecated_string()})
         response = self.client.post(url, {
             'action': 'Delete student state for module',
             'unique_student_identifier': self.student.email,
-            'problem_for_student': 'openassessment/b3dce2586c9c4876b73e7f390e42ef8f',
+            'problem_for_student': str(problem_location),
         })
 
         self.assertEqual(response.status_code, 200)

lms/djangoapps/instructor/tests/test_legacy_xss.py

@@ -48,7 +48,7 @@ class TestXss(ModuleStoreTestCase):
         )
         req.user = self._instructor
         req.session = {}
-        resp = legacy.instructor_dashboard(req, self._course.id)
+        resp = legacy.instructor_dashboard(req, self._course.id.to_deprecated_string())
         respUnicode = resp.content.decode(settings.DEFAULT_CHARSET)
         self.assertNotIn(self._evil_student.profile.name, respUnicode)
         self.assertIn(escape(self._evil_student.profile.name), respUnicode)

lms/djangoapps/instructor/tests/test_tools.py

@@ -17,6 +17,7 @@ from student.tests.factories import UserFactory
 from xmodule.fields import Date
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
+from xmodule.modulestore.keys import CourseKey
 
 from ..views import tools
 
@@ -86,10 +87,8 @@ class TestFindUnit(ModuleStoreTestCase):
         Fixtures.
         """
         course = CourseFactory.create()
-        week1 = ItemFactory.create()
-        homework = ItemFactory.create(parent_location=week1.location)
-        week1.children.append(homework.location)
-        course.children.append(week1.location)
+        week1 = ItemFactory.create(parent=course)
+        homework = ItemFactory.create(parent=week1)
 
         self.course = course
         self.homework = homework
@@ -98,7 +97,7 @@ class TestFindUnit(ModuleStoreTestCase):
         """
         Test finding a nested unit.
         """
-        url = self.homework.location.url()
+        url = self.homework.location.to_deprecated_string()
         self.assertEqual(tools.find_unit(self.course, url), self.homework)
 
     def test_find_unit_notfound(self):
@@ -121,15 +120,13 @@ class TestGetUnitsWithDueDate(ModuleStoreTestCase):
         """
         due = datetime.datetime(2010, 5, 12, 2, 42, tzinfo=utc)
         course = CourseFactory.create()
-        week1 = ItemFactory.create(due=due)
-        week2 = ItemFactory.create(due=due)
-        course.children = [week1.location.url(), week2.location.url()]
+        week1 = ItemFactory.create(due=due, parent=course)
+        week2 = ItemFactory.create(due=due, parent=course)
 
         homework = ItemFactory.create(
-            parent_location=week1.location,
+            parent=week1,
             due=due
         )
-        week1.children = [homework.location.url()]
 
         self.course = course
         self.week1 = week1
@@ -139,7 +136,7 @@ class TestGetUnitsWithDueDate(ModuleStoreTestCase):
 
         def urls(seq):
             "URLs for sequence of nodes."
-            return sorted(i.location.url() for i in seq)
+            return sorted(i.location.to_deprecated_string() for i in seq)
 
         self.assertEquals(
             urls(tools.get_units_with_due_date(self.course)),
@@ -156,7 +153,7 @@ class TestTitleOrUrl(unittest.TestCase):
 
     def test_url(self):
         unit = mock.Mock(display_name=None)
-        unit.location.url.return_value = 'test:hello'
+        unit.location.to_deprecated_string.return_value = 'test:hello'
         self.assertEquals(tools.title_or_url(unit), 'test:hello')
 
 
@@ -171,27 +168,25 @@ class TestSetDueDateExtension(ModuleStoreTestCase):
         """
         due = datetime.datetime(2010, 5, 12, 2, 42, tzinfo=utc)
         course = CourseFactory.create()
-        week1 = ItemFactory.create(due=due)
-        week2 = ItemFactory.create(due=due)
-        course.children = [week1.location.url(), week2.location.url()]
+        week1 = ItemFactory.create(due=due, parent=course)
+        week2 = ItemFactory.create(due=due, parent=course)
 
         homework = ItemFactory.create(
-            parent_location=week1.location,
+            parent=week1,
             due=due
         )
-        week1.children = [homework.location.url()]
 
         user = UserFactory.create()
         StudentModule(
             state='{}',
             student_id=user.id,
             course_id=course.id,
-            module_state_key=week1.location.url()).save()
+            module_state_key=week1.location).save()
         StudentModule(
             state='{}',
             student_id=user.id,
             course_id=course.id,
-            module_state_key=homework.location.url()).save()
+            module_state_key=homework.location).save()
 
         self.course = course
         self.week1 = week1
@@ -226,63 +221,60 @@ class TestDataDumps(ModuleStoreTestCase):
         """
         due = datetime.datetime(2010, 5, 12, 2, 42, tzinfo=utc)
         course = CourseFactory.create()
-        week1 = ItemFactory.create(due=due)
-        week2 = ItemFactory.create(due=due)
-        week3 = ItemFactory.create(due=due)
-        course.children = [week1.location.url(), week2.location.url(),
-                           week3.location.url()]
+        week1 = ItemFactory.create(due=due, parent=course)
+        week2 = ItemFactory.create(due=due, parent=course)
+        week3 = ItemFactory.create(due=due, parent=course)
 
         homework = ItemFactory.create(
-            parent_location=week1.location,
+            parent=week1,
             due=due
         )
-        week1.children = [homework.location.url()]
 
         user1 = UserFactory.create()
         StudentModule(
             state='{}',
             student_id=user1.id,
             course_id=course.id,
-            module_state_key=week1.location.url()).save()
+            module_state_key=week1.location).save()
         StudentModule(
             state='{}',
             student_id=user1.id,
             course_id=course.id,
-            module_state_key=week2.location.url()).save()
+            module_state_key=week2.location).save()
         StudentModule(
             state='{}',
             student_id=user1.id,
             course_id=course.id,
-            module_state_key=week3.location.url()).save()
+            module_state_key=week3.location).save()
         StudentModule(
             state='{}',
             student_id=user1.id,
             course_id=course.id,
-            module_state_key=homework.location.url()).save()
+            module_state_key=homework.location).save()
 
         user2 = UserFactory.create()
         StudentModule(
             state='{}',
             student_id=user2.id,
             course_id=course.id,
-            module_state_key=week1.location.url()).save()
+            module_state_key=week1.location).save()
         StudentModule(
             state='{}',
             student_id=user2.id,
             course_id=course.id,
-            module_state_key=homework.location.url()).save()
+            module_state_key=homework.location).save()
 
         user3 = UserFactory.create()
         StudentModule(
             state='{}',
             student_id=user3.id,
             course_id=course.id,
-            module_state_key=week1.location.url()).save()
+            module_state_key=week1.location).save()
         StudentModule(
             state='{}',
             student_id=user3.id,
             course_id=course.id,
-            module_state_key=homework.location.url()).save()
+            module_state_key=homework.location).save()
 
         self.course = course
         self.week1 = week1
@@ -337,10 +329,22 @@ def get_extended_due(course, unit, student):
     student_module = StudentModule.objects.get(
         student_id=student.id,
         course_id=course.id,
-        module_state_key=unit.location.url()
+        module_id=unit.location
     )
 
     state = json.loads(student_module.state)
     extended = state.get('extended_due', None)
     if extended:
         return DATE_FIELD.from_json(extended)
+
+
+def msk_from_problem_urlname(course_id, urlname, block_type='problem'):
+    """
+    Convert a 'problem urlname' to a module state key (db field)
+    """
+    if not isinstance(course_id, CourseKey):
+        raise ValueError
+    if urlname.endswith(".xml"):
+        urlname = urlname[:-4]
+
+    return course_id.make_usage_key(block_type, urlname)

lms/djangoapps/instructor/utils.py

@@ -27,12 +27,12 @@ class DummyRequest(object):
         return False
 
 
-def get_module_for_student(student, course, location, request=None):
+def get_module_for_student(student, usage_key, request=None):
     """Return the module for the (student, location) using a DummyRequest."""
     if request is None:
         request = DummyRequest()
         request.user = student
 
-    descriptor = modulestore().get_instance(course.id, location, depth=0)
-    field_data_cache = FieldDataCache([descriptor], course.id, student)
-    return get_module(student, request, location, field_data_cache, course.id)
+    descriptor = modulestore().get_item(usage_key, depth=0)
+    field_data_cache = FieldDataCache([descriptor], usage_key.course_key, student)
+    return get_module(student, request, usage_key, field_data_cache)

lms/djangoapps/instructor/views/tools.py

@@ -88,7 +88,7 @@ def find_unit(course, url):
         """
         Find node in course tree for url.
         """
-        if node.location.url() == url:
+        if node.location.to_deprecated_string() == url:
             return node
         for child in node.get_children():
             found = find(child, url)
@@ -132,7 +132,7 @@ def title_or_url(node):
     """
     title = getattr(node, 'display_name', None)
     if not title:
-        title = node.location.url()
+        title = node.location.to_deprecated_string()
     return title
 
 
@@ -148,7 +148,7 @@ def set_due_date_extension(course, unit, student, due_date):
             student_module = StudentModule.objects.get(
                 student_id=student.id,
                 course_id=course.id,
-                module_state_key=node.location.url()
+                module_id=node.location
             )
 
             state = json.loads(student_module.state)
@@ -173,7 +173,7 @@ def dump_module_extensions(course, unit):
     header = [_("Username"), _("Full Name"), _("Extended Due Date")]
     query = StudentModule.objects.filter(
         course_id=course.id,
-        module_state_key=unit.location.url())
+        module_id=unit.location)
     for module in query:
         state = json.loads(module.state)
         extended_due = state.get("extended_due")
@@ -202,7 +202,7 @@ def dump_student_extensions(course, student):
     data = []
     header = [_("Unit"), _("Extended Due Date")]
     units = get_units_with_due_date(course)
-    units = dict([(u.location.url(), u) for u in units])
+    units = dict([(u.location, u) for u in units])
     query = StudentModule.objects.filter(
         course_id=course.id,
         student_id=student.id)

lms/djangoapps/instructor_task/api.py

@@ -38,14 +38,14 @@ def get_running_instructor_tasks(course_id):
     return instructor_tasks.order_by('-id')
 
 
-def get_instructor_task_history(course_id, problem_url=None, student=None, task_type=None):
+def get_instructor_task_history(course_id, usage_key=None, student=None, task_type=None):
     """
     Returns a query of InstructorTask objects of historical tasks for a given course,
     that optionally match a particular problem, a student, and/or a task type.
     """
     instructor_tasks = InstructorTask.objects.filter(course_id=course_id)
-    if problem_url is not None or student is not None:
-        _, task_key = encode_problem_and_student_input(problem_url, student)
+    if usage_key is not None or student is not None:
+        _, task_key = encode_problem_and_student_input(usage_key, student)
         instructor_tasks = instructor_tasks.filter(task_key=task_key)
     if task_type is not None:
         instructor_tasks = instructor_tasks.filter(task_type=task_type)
@@ -53,7 +53,8 @@ def get_instructor_task_history(course_id, problem_url=None, student=None, task_
     return instructor_tasks.order_by('-id')
 
 
-def submit_rescore_problem_for_student(request, course_id, problem_url, student):
+# Disabling invalid-name because this fn name is longer than 30 chars.
+def submit_rescore_problem_for_student(request, usage_key, student):  # pylint: disable=invalid-name
     """
     Request a problem to be rescored as a background task.
 
@@ -74,15 +75,15 @@ def submit_rescore_problem_for_student(request, course_id, problem_url, student)
 
     """
     # check arguments:  let exceptions return up to the caller.
-    check_arguments_for_rescoring(course_id, problem_url)
+    check_arguments_for_rescoring(usage_key)
 
     task_type = 'rescore_problem'
     task_class = rescore_problem
-    task_input, task_key = encode_problem_and_student_input(problem_url, student)
-    return submit_task(request, task_type, task_class, course_id, task_input, task_key)
+    task_input, task_key = encode_problem_and_student_input(usage_key, student)
+    return submit_task(request, task_type, task_class, usage_key.course_key, task_input, task_key)
 
 
-def submit_rescore_problem_for_all_students(request, course_id, problem_url):
+def submit_rescore_problem_for_all_students(request, usage_key):  # pylint: disable=invalid-name
     """
     Request a problem to be rescored as a background task.
 
@@ -103,23 +104,22 @@ def submit_rescore_problem_for_all_students(request, course_id, problem_url):
     separate transaction.
     """
     # check arguments:  let exceptions return up to the caller.
-    check_arguments_for_rescoring(course_id, problem_url)
+    check_arguments_for_rescoring(usage_key)
 
     # check to see if task is already running, and reserve it otherwise
     task_type = 'rescore_problem'
     task_class = rescore_problem
-    task_input, task_key = encode_problem_and_student_input(problem_url)
-    return submit_task(request, task_type, task_class, course_id, task_input, task_key)
+    task_input, task_key = encode_problem_and_student_input(usage_key)
+    return submit_task(request, task_type, task_class, usage_key.course_key, task_input, task_key)
 
 
-def submit_reset_problem_attempts_for_all_students(request, course_id, problem_url):
+def submit_reset_problem_attempts_for_all_students(request, usage_key):  # pylint: disable=invalid-name
     """
     Request to have attempts reset for a problem as a background task.
 
     The problem's attempts will be reset for all students who have accessed the
     particular problem in a course.  Parameters are the `course_id` and
-    the `problem_url`.  The url must specify the location of the problem,
-    using i4x-type notation.
+    the `usage_key`, which must be a :class:`Location`.
 
     ItemNotFoundException is raised if the problem doesn't exist, or AlreadyRunningError
     if the problem is already being reset.
@@ -131,25 +131,24 @@ def submit_reset_problem_attempts_for_all_students(request, course_id, problem_u
     save here.  Any future database operations will take place in a
     separate transaction.
     """
-    # check arguments:  make sure that the problem_url is defined
+    # check arguments:  make sure that the usage_key is defined
     # (since that's currently typed in).  If the corresponding module descriptor doesn't exist,
     # an exception will be raised.  Let it pass up to the caller.
-    modulestore().get_instance(course_id, problem_url)
+    modulestore().get_item(usage_key)
 
     task_type = 'reset_problem_attempts'
     task_class = reset_problem_attempts
-    task_input, task_key = encode_problem_and_student_input(problem_url)
-    return submit_task(request, task_type, task_class, course_id, task_input, task_key)
+    task_input, task_key = encode_problem_and_student_input(usage_key)
+    return submit_task(request, task_type, task_class, usage_key.course_key, task_input, task_key)
 
 
-def submit_delete_problem_state_for_all_students(request, course_id, problem_url):
+def submit_delete_problem_state_for_all_students(request, usage_key):  # pylint: disable=invalid-name
     """
     Request to have state deleted for a problem as a background task.
 
     The problem's state will be deleted for all students who have accessed the
     particular problem in a course.  Parameters are the `course_id` and
-    the `problem_url`.  The url must specify the location of the problem,
-    using i4x-type notation.
+    the `usage_key`, which must be a :class:`Location`.
 
     ItemNotFoundException is raised if the problem doesn't exist, or AlreadyRunningError
     if the particular problem's state is already being deleted.
@@ -161,23 +160,23 @@ def submit_delete_problem_state_for_all_students(request, course_id, problem_url
     save here.  Any future database operations will take place in a
     separate transaction.
     """
-    # check arguments:  make sure that the problem_url is defined
+    # check arguments:  make sure that the usage_key is defined
     # (since that's currently typed in).  If the corresponding module descriptor doesn't exist,
     # an exception will be raised.  Let it pass up to the caller.
-    modulestore().get_instance(course_id, problem_url)
+    modulestore().get_item(usage_key)
 
     task_type = 'delete_problem_state'
     task_class = delete_problem_state
-    task_input, task_key = encode_problem_and_student_input(problem_url)
-    return submit_task(request, task_type, task_class, course_id, task_input, task_key)
+    task_input, task_key = encode_problem_and_student_input(usage_key)
+    return submit_task(request, task_type, task_class, usage_key.course_key, task_input, task_key)
 
 
-def submit_bulk_course_email(request, course_id, email_id):
+def submit_bulk_course_email(request, course_key, email_id):
     """
     Request to have bulk email sent as a background task.
 
     The specified CourseEmail object will be sent be updated for all students who have enrolled
-    in a course.  Parameters are the `course_id` and the `email_id`, the id of the CourseEmail object.
+    in a course.  Parameters are the `course_key` and the `email_id`, the id of the CourseEmail object.
 
     AlreadyRunningError is raised if the same recipients are already being emailed with the same
     CourseEmail object.
@@ -206,10 +205,10 @@ def submit_bulk_course_email(request, course_id, email_id):
     task_key_stub = "{email_id}_{to_option}".format(email_id=email_id, to_option=to_option)
     # create the key value by using MD5 hash:
     task_key = hashlib.md5(task_key_stub).hexdigest()
-    return submit_task(request, task_type, task_class, course_id, task_input, task_key)
+    return submit_task(request, task_type, task_class, course_key, task_input, task_key)
 
 
-def submit_calculate_grades_csv(request, course_id):
+def submit_calculate_grades_csv(request, course_key):
     """
     AlreadyRunningError is raised if the course's grades are already being updated.
     """
@@ -218,4 +217,4 @@ def submit_calculate_grades_csv(request, course_id):
     task_input = {}
     task_key = ""
 
-    return submit_task(request, task_type, task_class, course_id, task_input, task_key)
+    return submit_task(request, task_type, task_class, course_key, task_input, task_key)

lms/djangoapps/instructor_task/api_helper.py

+"""
+Helper lib for instructor_tasks API.
+
+Includes methods to check args for rescoring task, encoding student input,
+and task submission logic, including handling the Celery backend.
+"""
 import hashlib
 import json
 import logging
@@ -8,6 +14,7 @@ from celery.states import READY_STATES, SUCCESS, FAILURE, REVOKED
 from courseware.module_render import get_xqueue_callback_url_prefix
 
 from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.locations import Location
 from instructor_task.models import InstructorTask, PROGRESS
 
 
@@ -21,11 +28,13 @@ class AlreadyRunningError(Exception):
 
 def _task_is_running(course_id, task_type, task_key):
     """Checks if a particular task is already running"""
-    runningTasks = InstructorTask.objects.filter(course_id=course_id, task_type=task_type, task_key=task_key)
+    running_tasks = InstructorTask.objects.filter(
+        course_id=course_id, task_type=task_type, task_key=task_key
+    )
     # exclude states that are "ready" (i.e. not "running", e.g. failure, success, revoked):
     for state in READY_STATES:
-        runningTasks = runningTasks.exclude(task_state=state)
-    return len(runningTasks) > 0
+        running_tasks = running_tasks.exclude(task_state=state)
+    return len(running_tasks) > 0
 
 
 def _reserve_task(course_id, task_type, task_key, task_input, requester):
@@ -229,34 +238,37 @@ def get_status_from_instructor_task(instructor_task):
     return status
 
 
-def check_arguments_for_rescoring(course_id, problem_url):
+def check_arguments_for_rescoring(usage_key):
     """
     Do simple checks on the descriptor to confirm that it supports rescoring.
 
-    Confirms first that the problem_url is defined (since that's currently typed
+    Confirms first that the usage_key is defined (since that's currently typed
     in).  An ItemNotFoundException is raised if the corresponding module
     descriptor doesn't exist.  NotImplementedError is raised if the
     corresponding module doesn't support rescoring calls.
     """
-    descriptor = modulestore().get_instance(course_id, problem_url)
+    descriptor = modulestore().get_item(usage_key)
     if not hasattr(descriptor, 'module_class') or not hasattr(descriptor.module_class, 'rescore_problem'):
         msg = "Specified module does not support rescoring."
         raise NotImplementedError(msg)
 
 
-def encode_problem_and_student_input(problem_url, student=None):
+def encode_problem_and_student_input(usage_key, student=None):  # pylint: disable=invalid-name
     """
-    Encode optional problem_url and optional student into task_key and task_input values.
+    Encode optional usage_key and optional student into task_key and task_input values.
 
-    `problem_url` is full URL of the problem.
-    `student` is the user object of the student
+    Args:
+        usage_key (Location): The usage_key identifying the problem.
+        student (User): the student affected
     """
+
+    assert isinstance(usage_key, Location)
     if student is not None:
-        task_input = {'problem_url': problem_url, 'student': student.username}
-        task_key_stub = "{student}_{problem}".format(student=student.id, problem=problem_url)
+        task_input = {'problem_url': usage_key.to_deprecated_string(), 'student': student.username}
+        task_key_stub = "{student}_{problem}".format(student=student.id, problem=usage_key.to_deprecated_string())
     else:
-        task_input = {'problem_url': problem_url}
-        task_key_stub = "_{problem}".format(problem=problem_url)
+        task_input = {'problem_url': usage_key.to_deprecated_string()}
+        task_key_stub = "_{problem}".format(problem=usage_key.to_deprecated_string())
 
     # create the key value by using MD5 hash:
     task_key = hashlib.md5(task_key_stub).hexdigest()
@@ -264,11 +276,11 @@ def encode_problem_and_student_input(problem_url, student=None):
     return task_input, task_key
 
 
-def submit_task(request, task_type, task_class, course_id, task_input, task_key):
+def submit_task(request, task_type, task_class, course_key, task_input, task_key):
     """
     Helper method to submit a task.
 
-    Reserves the requested task, based on the `course_id`, `task_type`, and `task_key`,
+    Reserves the requested task, based on the `course_key`, `task_type`, and `task_key`,
     checking to see if the task is already running.  The `task_input` is also passed so that
     it can be stored in the resulting InstructorTask entry.  Arguments are extracted from
     the `request` provided by the originating server request.  Then the task is submitted to run
@@ -285,7 +297,7 @@ def submit_task(request, task_type, task_class, course_id, task_input, task_key)
 
     """
     # check to see if task is already running, and reserve it otherwise:
-    instructor_task = _reserve_task(course_id, task_type, task_key, task_input, request.user)
+    instructor_task = _reserve_task(course_key, task_type, task_key, task_input, request.user)
 
     # submit task:
     task_id = instructor_task.task_id

lms/djangoapps/instructor_task/models.py

@@ -18,7 +18,6 @@ from uuid import uuid4
 import csv
 import json
 import hashlib
-import os
 import os.path
 import urllib
 
@@ -29,6 +28,8 @@ from django.conf import settings
 from django.contrib.auth.models import User
 from django.db import models, transaction
 
+from xmodule_django.models import CourseKeyField
+
 
 # define custom states used by InstructorTask
 QUEUING = 'QUEUING'
@@ -58,7 +59,7 @@ class InstructorTask(models.Model):
     `updated` stores date that entry was last modified
     """
     task_type = models.CharField(max_length=50, db_index=True)
-    course_id = models.CharField(max_length=255, db_index=True)
+    course_id = CourseKeyField(max_length=255, db_index=True)
     task_key = models.CharField(max_length=255, db_index=True)
     task_input = models.CharField(max_length=255)
     task_id = models.CharField(max_length=255, db_index=True)  # max_length from celery_taskmeta
@@ -251,9 +252,9 @@ class S3ReportStore(ReportStore):
         )
 
     def key_for(self, course_id, filename):
-        """Return the S3 key we would use to store and retrive the data for the
+        """Return the S3 key we would use to store and retrieve the data for the
         given filename."""
-        hashed_course_id = hashlib.sha1(course_id)
+        hashed_course_id = hashlib.sha1(course_id.to_deprecated_string())
 
         key = Key(self.bucket)
         key.key = "{}/{}/{}".format(
@@ -360,7 +361,7 @@ class LocalFSReportStore(ReportStore):
 
     def path_to(self, course_id, filename):
         """Return the full path to a given file for a given course."""
-        return os.path.join(self.root_path, urllib.quote(course_id, safe=''), filename)
+        return os.path.join(self.root_path, urllib.quote(course_id.to_deprecated_string(), safe=''), filename)
 
     def store(self, course_id, filename, buff):
         """

lms/djangoapps/instructor_task/subtasks.py

@@ -375,7 +375,7 @@ def check_subtask_is_valid(entry_id, current_task_id, new_subtask_status):
         format_str = "Unexpected task_id '{}': unable to find subtasks of instructor task '{}': rejecting task {}"
         msg = format_str.format(current_task_id, entry, new_subtask_status)
         TASK_LOG.warning(msg)
-        dog_stats_api.increment('instructor_task.subtask.duplicate.nosubtasks', tags=[entry.course_id])
+        dog_stats_api.increment('instructor_task.subtask.duplicate.nosubtasks', tags=[_statsd_tag(entry.course_id)])
         raise DuplicateTaskException(msg)
 
     # Confirm that the InstructorTask knows about this particular subtask.
@@ -385,7 +385,7 @@ def check_subtask_is_valid(entry_id, current_task_id, new_subtask_status):
         format_str = "Unexpected task_id '{}': unable to find status for subtask of instructor task '{}': rejecting task {}"
         msg = format_str.format(current_task_id, entry, new_subtask_status)
         TASK_LOG.warning(msg)
-        dog_stats_api.increment('instructor_task.subtask.duplicate.unknown', tags=[entry.course_id])
+        dog_stats_api.increment('instructor_task.subtask.duplicate.unknown', tags=[_statsd_tag(entry.course_id)])
         raise DuplicateTaskException(msg)
 
     # Confirm that the InstructorTask doesn't think that this subtask has already been
@@ -396,7 +396,7 @@ def check_subtask_is_valid(entry_id, current_task_id, new_subtask_status):
         format_str = "Unexpected task_id '{}': already completed - status {} for subtask of instructor task '{}': rejecting task {}"
         msg = format_str.format(current_task_id, subtask_status, entry, new_subtask_status)
         TASK_LOG.warning(msg)
-        dog_stats_api.increment('instructor_task.subtask.duplicate.completed', tags=[entry.course_id])
+        dog_stats_api.increment('instructor_task.subtask.duplicate.completed', tags=[_statsd_tag(entry.course_id)])
         raise DuplicateTaskException(msg)
 
     # Confirm that the InstructorTask doesn't think that this subtask is already being
@@ -410,7 +410,7 @@ def check_subtask_is_valid(entry_id, current_task_id, new_subtask_status):
             format_str = "Unexpected task_id '{}': already retried - status {} for subtask of instructor task '{}': rejecting task {}"
             msg = format_str.format(current_task_id, subtask_status, entry, new_subtask_status)
             TASK_LOG.warning(msg)
-            dog_stats_api.increment('instructor_task.subtask.duplicate.retried', tags=[entry.course_id])
+            dog_stats_api.increment('instructor_task.subtask.duplicate.retried', tags=[_statsd_tag(entry.course_id)])
             raise DuplicateTaskException(msg)
 
     # Now we are ready to start working on this.  Try to lock it.
@@ -420,7 +420,7 @@ def check_subtask_is_valid(entry_id, current_task_id, new_subtask_status):
         format_str = "Unexpected task_id '{}': already being executed - for subtask of instructor task '{}'"
         msg = format_str.format(current_task_id, entry)
         TASK_LOG.warning(msg)
-        dog_stats_api.increment('instructor_task.subtask.duplicate.locked', tags=[entry.course_id])
+        dog_stats_api.increment('instructor_task.subtask.duplicate.locked', tags=[_statsd_tag(entry.course_id)])
         raise DuplicateTaskException(msg)
 
 
@@ -552,3 +552,11 @@ def _update_subtask_status(entry_id, current_task_id, new_subtask_status):
     else:
         TASK_LOG.debug("about to commit....")
         transaction.commit()
+
+
+def _statsd_tag(course_id):
+    """
+    Calculate the tag we will use for DataDog.
+    """
+    tag = unicode(course_id).encode('utf-8')
+    return tag[:200]

lms/djangoapps/instructor_task/tasks_helper.py

@@ -244,15 +244,14 @@ def perform_module_state_update(update_fcn, filter_fcn, _entry_id, course_id, ta
     # get start time for task:
     start_time = time()
 
-    module_state_key = task_input.get('problem_url')
+    usage_key = course_id.make_usage_key_from_deprecated_string(task_input.get('problem_url'))
     student_identifier = task_input.get('student')
 
     # find the problem descriptor:
-    module_descriptor = modulestore().get_instance(course_id, module_state_key)
+    module_descriptor = modulestore().get_item(usage_key)
 
     # find the module in question
-    modules_to_update = StudentModule.objects.filter(course_id=course_id,
-                                                     module_state_key=module_state_key)
+    modules_to_update = StudentModule.objects.filter(course_id=course_id, module_id=usage_key)
 
     # give the option of updating an individual student. If not specified,
     # then updates all students who have responded to a problem so far
@@ -394,13 +393,13 @@ def rescore_problem_module_state(xmodule_instance_args, module_descriptor, stude
     # unpack the StudentModule:
     course_id = student_module.course_id
     student = student_module.student
-    module_state_key = student_module.module_state_key
+    usage_key = student_module.module_state_key
     instance = _get_module_instance_for_task(course_id, student, module_descriptor, xmodule_instance_args, grade_bucket_type='rescore')
 
     if instance is None:
         # Either permissions just changed, or someone is trying to be clever
         # and load something they shouldn't have access to.
-        msg = "No module {loc} for student {student}--access denied?".format(loc=module_state_key,
+        msg = "No module {loc} for student {student}--access denied?".format(loc=usage_key,
                                                                              student=student)
         TASK_LOG.debug(msg)
         raise UpdateProblemModuleStateError(msg)
@@ -416,15 +415,15 @@ def rescore_problem_module_state(xmodule_instance_args, module_descriptor, stude
     if 'success' not in result:
         # don't consider these fatal, but false means that the individual call didn't complete:
         TASK_LOG.warning(u"error processing rescore call for course {course}, problem {loc} and student {student}: "
-                         u"unexpected response {msg}".format(msg=result, course=course_id, loc=module_state_key, student=student))
+                         u"unexpected response {msg}".format(msg=result, course=course_id, loc=usage_key, student=student))
         return UPDATE_STATUS_FAILED
     elif result['success'] not in ['correct', 'incorrect']:
         TASK_LOG.warning(u"error processing rescore call for course {course}, problem {loc} and student {student}: "
-                         u"{msg}".format(msg=result['success'], course=course_id, loc=module_state_key, student=student))
+                         u"{msg}".format(msg=result['success'], course=course_id, loc=usage_key, student=student))
         return UPDATE_STATUS_FAILED
     else:
         TASK_LOG.debug(u"successfully processed rescore call for course {course}, problem {loc} and student {student}: "
-                       u"{msg}".format(msg=result['success'], course=course_id, loc=module_state_key, student=student))
+                       u"{msg}".format(msg=result['success'], course=course_id, loc=usage_key, student=student))
         return UPDATE_STATUS_SUCCEEDED
 
 
@@ -552,7 +551,7 @@ def push_grades_to_s3(_xmodule_instance_args, _entry_id, course_id, _task_input,
 
     # Generate parts of the file name
     timestamp_str = start_time.strftime("%Y-%m-%d-%H%M")
-    course_id_prefix = urllib.quote(course_id.replace("/", "_"))
+    course_id_prefix = urllib.quote(course_id.to_deprecated_string().replace("/", "_"))
 
     # Perform the actual upload
     report_store = ReportStore.from_config()

lms/djangoapps/instructor_task/tests/factories.py

@@ -5,13 +5,14 @@ from factory.django import DjangoModelFactory
 from student.tests.factories import UserFactory as StudentUserFactory
 from instructor_task.models import InstructorTask
 from celery.states import PENDING
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 
 class InstructorTaskFactory(DjangoModelFactory):
     FACTORY_FOR = InstructorTask
 
     task_type = 'rescore_problem'
-    course_id = "MITx/999/Robot_Super_Course"
+    course_id = SlashSeparatedCourseKey("MITx", "999", "Robot_Super_Course")
     task_input = json.dumps({})
     task_key = None
     task_id = None

lms/djangoapps/instructor_task/tests/test_api.py

@@ -22,7 +22,7 @@ from instructor_task.models import InstructorTask, PROGRESS
 from instructor_task.tests.test_base import (InstructorTaskTestCase,
                                              InstructorTaskCourseTestCase,
                                              InstructorTaskModuleTestCase,
-                                             TEST_COURSE_ID)
+                                             TEST_COURSE_KEY)
 
 
 class InstructorTaskReportTest(InstructorTaskTestCase):
@@ -36,7 +36,7 @@ class InstructorTaskReportTest(InstructorTaskTestCase):
             self._create_failure_entry()
             self._create_success_entry()
         progress_task_ids = [self._create_progress_entry().task_id for _ in range(1, 5)]
-        task_ids = [instructor_task.task_id for instructor_task in get_running_instructor_tasks(TEST_COURSE_ID)]
+        task_ids = [instructor_task.task_id for instructor_task in get_running_instructor_tasks(TEST_COURSE_KEY)]
         self.assertEquals(set(task_ids), set(progress_task_ids))
 
     def test_get_instructor_task_history(self):
@@ -47,21 +47,21 @@ class InstructorTaskReportTest(InstructorTaskTestCase):
             expected_ids.append(self._create_success_entry().task_id)
             expected_ids.append(self._create_progress_entry().task_id)
         task_ids = [instructor_task.task_id for instructor_task
-                    in get_instructor_task_history(TEST_COURSE_ID, problem_url=self.problem_url)]
+                    in get_instructor_task_history(TEST_COURSE_KEY, usage_key=self.problem_url)]
         self.assertEquals(set(task_ids), set(expected_ids))
         # make the same call using explicit task_type:
         task_ids = [instructor_task.task_id for instructor_task
                     in get_instructor_task_history(
-                        TEST_COURSE_ID,
-                        problem_url=self.problem_url,
+                        TEST_COURSE_KEY,
+                        usage_key=self.problem_url,
                         task_type='rescore_problem'
                     )]
         self.assertEquals(set(task_ids), set(expected_ids))
         # make the same call using a non-existent task_type:
         task_ids = [instructor_task.task_id for instructor_task
                     in get_instructor_task_history(
-                        TEST_COURSE_ID,
-                        problem_url=self.problem_url,
+                        TEST_COURSE_KEY,
+                        usage_key=self.problem_url,
                         task_type='dummy_type'
                     )]
         self.assertEquals(set(task_ids), set())
@@ -81,25 +81,25 @@ class InstructorTaskModuleSubmitTest(InstructorTaskModuleTestCase):
         course_id = self.course.id
         request = None
         with self.assertRaises(ItemNotFoundError):
-            submit_rescore_problem_for_student(request, course_id, problem_url, self.student)
+            submit_rescore_problem_for_student(request, problem_url, self.student)
         with self.assertRaises(ItemNotFoundError):
-            submit_rescore_problem_for_all_students(request, course_id, problem_url)
+            submit_rescore_problem_for_all_students(request, problem_url)
         with self.assertRaises(ItemNotFoundError):
-            submit_reset_problem_attempts_for_all_students(request, course_id, problem_url)
+            submit_reset_problem_attempts_for_all_students(request, problem_url)
         with self.assertRaises(ItemNotFoundError):
-            submit_delete_problem_state_for_all_students(request, course_id, problem_url)
+            submit_delete_problem_state_for_all_students(request, problem_url)
 
     def test_submit_nonrescorable_modules(self):
         # confirm that a rescore of an existent but unscorable module returns an exception
         # (Note that it is easier to test a scoreable but non-rescorable module in test_tasks,
         # where we are creating real modules.)
-        problem_url = self.problem_section.location.url()
+        problem_url = self.problem_section.location
         course_id = self.course.id
         request = None
         with self.assertRaises(NotImplementedError):
-            submit_rescore_problem_for_student(request, course_id, problem_url, self.student)
+            submit_rescore_problem_for_student(request, problem_url, self.student)
         with self.assertRaises(NotImplementedError):
-            submit_rescore_problem_for_all_students(request, course_id, problem_url)
+            submit_rescore_problem_for_all_students(request, problem_url)
 
     def _test_submit_with_long_url(self, task_function, student=None):
         problem_url_name = 'x' * 255
@@ -107,9 +107,9 @@ class InstructorTaskModuleSubmitTest(InstructorTaskModuleTestCase):
         location = InstructorTaskModuleTestCase.problem_location(problem_url_name)
         with self.assertRaises(ValueError):
             if student is not None:
-                task_function(self.create_task_request(self.instructor), self.course.id, location, student)
+                task_function(self.create_task_request(self.instructor), location, student)
             else:
-                task_function(self.create_task_request(self.instructor), self.course.id, location)
+                task_function(self.create_task_request(self.instructor), location)
 
     def test_submit_rescore_all_with_long_url(self):
         self._test_submit_with_long_url(submit_rescore_problem_for_all_students)
@@ -129,11 +129,9 @@ class InstructorTaskModuleSubmitTest(InstructorTaskModuleTestCase):
         self.define_option_problem(problem_url_name)
         location = InstructorTaskModuleTestCase.problem_location(problem_url_name)
         if student is not None:
-            instructor_task = task_function(self.create_task_request(self.instructor),
-                                            self.course.id, location, student)
+            instructor_task = task_function(self.create_task_request(self.instructor), location, student)
         else:
-            instructor_task = task_function(self.create_task_request(self.instructor),
-                                            self.course.id, location)
+            instructor_task = task_function(self.create_task_request(self.instructor), location)
 
         # test resubmitting, by updating the existing record:
         instructor_task = InstructorTask.objects.get(id=instructor_task.id)
@@ -142,9 +140,9 @@ class InstructorTaskModuleSubmitTest(InstructorTaskModuleTestCase):
 
         with self.assertRaises(AlreadyRunningError):
             if student is not None:
-                task_function(self.create_task_request(self.instructor), self.course.id, location, student)
+                task_function(self.create_task_request(self.instructor), location, student)
             else:
-                task_function(self.create_task_request(self.instructor), self.course.id, location)
+                task_function(self.create_task_request(self.instructor), location)
 
     def test_submit_rescore_all(self):
         self._test_submit_task(submit_rescore_problem_for_all_students)

lms/djangoapps/instructor_task/tests/test_base.py

@@ -16,6 +16,7 @@ from capa.tests.response_xml_factory import OptionResponseXMLFactory
 from xmodule.modulestore.django import editable_modulestore
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.locations import Location, SlashSeparatedCourseKey
 
 from student.tests.factories import CourseEnrollmentFactory, UserFactory
 from courseware.model_data import StudentModule
@@ -28,10 +29,10 @@ from instructor_task.views import instructor_task_status
 
 
 TEST_COURSE_ORG = 'edx'
-TEST_COURSE_NAME = 'test course'
+TEST_COURSE_NAME = 'test_course'
 TEST_COURSE_NUMBER = '1.23x'
+TEST_COURSE_KEY = SlashSeparatedCourseKey(TEST_COURSE_ORG, TEST_COURSE_NUMBER, TEST_COURSE_NAME)
 TEST_SECTION_NAME = "Problem"
-TEST_COURSE_ID = 'edx/1.23x/test_course'
 
 TEST_FAILURE_MESSAGE = 'task failed horribly'
 TEST_FAILURE_EXCEPTION = 'RandomCauseError'
@@ -54,9 +55,7 @@ class InstructorTaskTestCase(TestCase):
         """
         Create an internal location for a test problem.
         """
-        return "i4x://{org}/{number}/problem/{problem_url_name}".format(org='edx',
-                                                                        number='1.23x',
-                                                                        problem_url_name=problem_url_name)
+        return TEST_COURSE_KEY.make_usage_key('problem', problem_url_name)
 
     def _create_entry(self, task_state=QUEUING, task_output=None, student=None):
         """Creates a InstructorTask entry for testing."""
@@ -64,7 +63,7 @@ class InstructorTaskTestCase(TestCase):
         progress_json = json.dumps(task_output) if task_output is not None else None
         task_input, task_key = encode_problem_and_student_input(self.problem_url, student)
 
-        instructor_task = InstructorTaskFactory.create(course_id=TEST_COURSE_ID,
+        instructor_task = InstructorTaskFactory.create(course_id=TEST_COURSE_KEY,
                                                        requester=self.instructor,
                                                        task_input=json.dumps(task_input),
                                                        task_key=task_key,
@@ -180,11 +179,9 @@ class InstructorTaskModuleTestCase(InstructorTaskCourseTestCase):
         Create an internal location for a test problem.
         """
         if "i4x:" in problem_url_name:
-            return problem_url_name
+            return Location.from_deprecated_string(problem_url_name)
         else:
-            return "i4x://{org}/{number}/problem/{problem_url_name}".format(org=TEST_COURSE_ORG,
-                                                                            number=TEST_COURSE_NUMBER,
-                                                                            problem_url_name=problem_url_name)
+            return TEST_COURSE_KEY.make_usage_key('problem', problem_url_name)
 
     def define_option_problem(self, problem_url_name):
         """Create the problem definition so the answer is Option 1"""
@@ -195,6 +192,7 @@ class InstructorTaskModuleTestCase(InstructorTaskCourseTestCase):
                         'num_responses': 2}
         problem_xml = factory.build_xml(**factory_args)
         ItemFactory.create(parent_location=self.problem_section.location,
+                           parent=self.problem_section,
                            category="problem",
                            display_name=str(problem_url_name),
                            data=problem_xml)
@@ -208,7 +206,7 @@ class InstructorTaskModuleTestCase(InstructorTaskCourseTestCase):
                         'num_responses': 2}
         problem_xml = factory.build_xml(**factory_args)
         location = InstructorTaskTestCase.problem_location(problem_url_name)
-        item = self.module_store.get_instance(self.course.id, location)
+        item = self.module_store.get_item(location)
         item.data = problem_xml
         self.module_store.update_item(item, '**replace_user**')
 
@@ -217,5 +215,5 @@ class InstructorTaskModuleTestCase(InstructorTaskCourseTestCase):
         return StudentModule.objects.get(course_id=self.course.id,
                                          student=User.objects.get(username=username),
                                          module_type=descriptor.location.category,
-                                         module_state_key=descriptor.location.url(),
+                                         module_id=descriptor.location,
                                          )

lms/djangoapps/instructor_task/tests/test_tasks.py

@@ -13,6 +13,7 @@ from mock import Mock, MagicMock, patch
 from celery.states import SUCCESS, FAILURE
 
 from xmodule.modulestore.exceptions import ItemNotFoundError
+from xmodule.modulestore.locations import i4xEncoder
 
 from courseware.models import StudentModule
 from courseware.tests.factories import StudentModuleFactory
@@ -37,21 +38,21 @@ class TestInstructorTasks(InstructorTaskModuleTestCase):
         super(InstructorTaskModuleTestCase, self).setUp()
         self.initialize_course()
         self.instructor = self.create_instructor('instructor')
-        self.problem_url = InstructorTaskModuleTestCase.problem_location(PROBLEM_URL_NAME)
+        self.location = InstructorTaskModuleTestCase.problem_location(PROBLEM_URL_NAME)
 
     def _create_input_entry(self, student_ident=None, use_problem_url=True, course_id=None):
         """Creates a InstructorTask entry for testing."""
         task_id = str(uuid4())
         task_input = {}
         if use_problem_url:
-            task_input['problem_url'] = self.problem_url
+            task_input['problem_url'] = self.location
         if student_ident is not None:
             task_input['student'] = student_ident
 
         course_id = course_id or self.course.id
         instructor_task = InstructorTaskFactory.create(course_id=course_id,
                                                        requester=self.instructor,
-                                                       task_input=json.dumps(task_input),
+                                                       task_input=json.dumps(task_input, cls=i4xEncoder),
                                                        task_key='dummy value',
                                                        task_id=task_id)
         return instructor_task
@@ -127,7 +128,7 @@ class TestInstructorTasks(InstructorTaskModuleTestCase):
         for student in students:
             CourseEnrollmentFactory.create(course_id=self.course.id, user=student)
             StudentModuleFactory.create(course_id=self.course.id,
-                                        module_state_key=self.problem_url,
+                                        module_id=self.location,
                                         student=student,
                                         grade=grade,
                                         max_grade=max_grade,
@@ -139,7 +140,7 @@ class TestInstructorTasks(InstructorTaskModuleTestCase):
         for student in students:
             module = StudentModule.objects.get(course_id=self.course.id,
                                                student=student,
-                                               module_state_key=self.problem_url)
+                                               module_id=self.location)
             state = json.loads(module.state)
             self.assertEquals(state['attempts'], num_attempts)
 
@@ -356,7 +357,7 @@ class TestResetAttemptsInstructorTask(TestInstructorTasks):
         for student in students:
             module = StudentModule.objects.get(course_id=self.course.id,
                                                student=student,
-                                               module_state_key=self.problem_url)
+                                               module_id=self.location)
             state = json.loads(module.state)
             self.assertEquals(state['attempts'], initial_attempts)
 
@@ -382,7 +383,7 @@ class TestResetAttemptsInstructorTask(TestInstructorTasks):
         for index, student in enumerate(students):
             module = StudentModule.objects.get(course_id=self.course.id,
                                                student=student,
-                                               module_state_key=self.problem_url)
+                                               module_id=self.location)
             state = json.loads(module.state)
             if index == 3:
                 self.assertEquals(state['attempts'], 0)
@@ -429,11 +430,11 @@ class TestDeleteStateInstructorTask(TestInstructorTasks):
         for student in students:
             StudentModule.objects.get(course_id=self.course.id,
                                       student=student,
-                                      module_state_key=self.problem_url)
+                                      module_id=self.location)
         self._test_run_with_task(delete_problem_state, 'deleted', num_students)
         # confirm that no state can be found anymore:
         for student in students:
             with self.assertRaises(StudentModule.DoesNotExist):
                 StudentModule.objects.get(course_id=self.course.id,
                                           student=student,
-                                          module_state_key=self.problem_url)
+                                          module_id=self.location)

lms/djangoapps/licenses/models.py

@@ -4,6 +4,8 @@ from django.db import models, transaction
 
 from student.models import User
 
+from xmodule_django.models import CourseKeyField
+
 log = logging.getLogger("edx.licenses")
 
 
@@ -11,7 +13,7 @@ class CourseSoftware(models.Model):
     name = models.CharField(max_length=255)
     full_name = models.CharField(max_length=255)
     url = models.CharField(max_length=255)
-    course_id = models.CharField(max_length=255)
+    course_id = CourseKeyField(max_length=255)
 
     def __unicode__(self):
         return u'{0} for {1}'.format(self.name, self.course_id)

lms/djangoapps/licenses/views.py

@@ -6,6 +6,7 @@ from collections import namedtuple, defaultdict
 
 
 from edxmako.shortcuts import render_to_string
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 from django.contrib.auth.decorators import login_required
 from django.contrib.auth.models import User
@@ -59,6 +60,7 @@ def user_software_license(request):
     if not match:
         raise Http404
     course_id = match.groupdict().get('id', '')
+    course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
 
     user_id = request.session.get('_auth_user_id')
     software_name = request.POST.get('software')
@@ -66,7 +68,7 @@ def user_software_license(request):
 
     try:
         software = CourseSoftware.objects.get(name=software_name,
-                                              course_id=course_id)
+                                              course_id=course_key)
     except CourseSoftware.DoesNotExist:
         raise Http404
 

lms/djangoapps/linkedin/management/commands/tests/test_mailusers.py

-# -*- coding: utf-8 -*-
-"""
-Test email scripts.
-"""
-from smtplib import SMTPDataError, SMTPServerDisconnected
-import datetime
-import json
-import mock
-
-from boto.ses.exceptions import SESIllegalAddressError, SESIdentityNotVerifiedError
-from certificates.models import GeneratedCertificate
-from django.contrib.auth.models import User
-from django.conf import settings
-from django.test.utils import override_settings
-from django.core import mail
-from django.utils.timezone import utc
-from django.test import TestCase
-
-from xmodule.modulestore.tests.factories import CourseFactory
-from student.models import UserProfile
-from xmodule.modulestore.tests.django_utils import mixed_store_config
-from linkedin.models import LinkedIn
-from linkedin.management.commands import linkedin_mailusers as mailusers
-from linkedin.management.commands.linkedin_mailusers import MAX_ATTEMPTS
-
-MODULE = 'linkedin.management.commands.linkedin_mailusers.'
-
-TEST_DATA_MIXED_MODULESTORE = mixed_store_config(settings.COMMON_TEST_DATA_ROOT, {})
-
-
-@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
-class MailusersTests(TestCase):
-    """
-    Test mail users command.
-    """
-
-    def setUp(self):
-        CourseFactory.create(org='TESTX', number='1', display_name='TEST1',
-                             start=datetime.datetime(2010, 5, 12, 2, 42, tzinfo=utc),
-                             end=datetime.datetime(2011, 5, 12, 2, 42, tzinfo=utc))
-        CourseFactory.create(org='TESTX', number='2', display_name='TEST2',
-                             start=datetime.datetime(2010, 5, 12, 2, 42, tzinfo=utc),
-                             end=datetime.datetime(2011, 5, 12, 2, 42, tzinfo=utc))
-        CourseFactory.create(org='TESTX', number='3', display_name='TEST3',
-                             start=datetime.datetime(2010, 5, 12, 2, 42, tzinfo=utc),
-                             end=datetime.datetime(2011, 5, 12, 2, 42, tzinfo=utc))
-
-        self.fred = fred = User(username='fred', email='fred@bedrock.gov')
-        fred.save()
-        UserProfile(user=fred, name='Fred Flintstone').save()
-        LinkedIn(user=fred, has_linkedin_account=True).save()
-        self.barney = barney = User(
-            username='barney', email='barney@bedrock.gov')
-        barney.save()
-
-        LinkedIn(user=barney, has_linkedin_account=True).save()
-        UserProfile(user=barney, name='Barney Rubble').save()
-
-        self.adam = adam = User(
-            username='adam', email='adam@adam.gov')
-        adam.save()
-
-        LinkedIn(user=adam, has_linkedin_account=True).save()
-        UserProfile(user=adam, name='Adam (חיים פּלי)').save()
-        self.cert1 = cert1 = GeneratedCertificate(
-            status='downloadable',
-            user=fred,
-            course_id='TESTX/1/TEST1',
-            name='TestX/Intro101',
-            download_url='http://test.foo/test')
-        cert1.save()
-        cert2 = GeneratedCertificate(
-            status='downloadable',
-            user=fred,
-            course_id='TESTX/2/TEST2')
-        cert2.save()
-        cert3 = GeneratedCertificate(
-            status='downloadable',
-            user=barney,
-            course_id='TESTX/3/TEST3')
-        cert3.save()
-        cert5 = GeneratedCertificate(
-            status='downloadable',
-            user=adam,
-            course_id='TESTX/3/TEST3')
-        cert5.save()
-
-    @mock.patch.dict('django.conf.settings.LINKEDIN_API',
-                     {'EMAIL_WHITELIST': ['barney@bedrock.gov']})
-    def test_mail_users_with_whitelist(self):
-        """
-        Test emailing users.
-        """
-        fut = mailusers.Command().handle
-        fut()
-        self.assertEqual(
-            json.loads(self.barney.linkedin.emailed_courses), ['TESTX/3/TEST3'])
-        self.assertEqual(len(mail.outbox), 1)
-        self.assertEqual(
-            mail.outbox[0].to, ['Barney Rubble <barney@bedrock.gov>'])
-
-    def test_mail_users_grandfather(self):
-        """
-        Test sending grandfather emails.
-        """
-        fut = mailusers.Command().handle
-        fut()
-        self.assertEqual(
-            json.loads(self.fred.linkedin.emailed_courses), ['TESTX/1/TEST1', 'TESTX/2/TEST2'])
-        self.assertEqual(
-            json.loads(self.barney.linkedin.emailed_courses), ['TESTX/3/TEST3'])
-        self.assertEqual(
-            json.loads(self.adam.linkedin.emailed_courses), ['TESTX/3/TEST3'])
-        self.assertEqual(len(mail.outbox), 3)
-        self.assertEqual(
-            mail.outbox[0].to, ['Fred Flintstone <fred@bedrock.gov>'])
-        self.assertEqual(
-            mail.outbox[0].subject, 'Fred Flintstone, Add your Achievements to your LinkedIn Profile')
-        self.assertEqual(
-            mail.outbox[1].to, ['Barney Rubble <barney@bedrock.gov>'])
-        self.assertEqual(
-            mail.outbox[1].subject, 'Barney Rubble, Add your Achievements to your LinkedIn Profile')
-        self.assertEqual(
-            mail.outbox[2].subject, u'Adam (חיים פּלי), Add your Achievements to your LinkedIn Profile')
-
-    def test_mail_users_grandfather_mock(self):
-        """
-        test that we aren't sending anything when in mock_run mode
-        """
-        fut = mailusers.Command().handle
-        fut(mock_run=True)
-        self.assertEqual(
-            json.loads(self.fred.linkedin.emailed_courses), [])
-        self.assertEqual(
-            json.loads(self.barney.linkedin.emailed_courses), [])
-        self.assertEqual(
-            json.loads(self.adam.linkedin.emailed_courses), [])
-        self.assertEqual(len(mail.outbox), 0)
-
-    def test_transaction_semantics(self):
-        fut = mailusers.Command().handle
-        with mock.patch('linkedin.management.commands.linkedin_mailusers.Command.send_grandfather_email',
-                        return_value=True, side_effect=[True, KeyboardInterrupt]):
-            try:
-                fut()
-            except KeyboardInterrupt:
-                # expect that this will be uncaught
-
-                # check that fred's emailed_courses were updated
-                self.assertEqual(
-                    json.loads(self.fred.linkedin.emailed_courses), ['TESTX/1/TEST1', 'TESTX/2/TEST2']
-                )
-
-                #check that we did not update barney
-                self.assertEqual(
-                    json.loads(self.barney.linkedin.emailed_courses), []
-                )
-
-
-    def test_certificate_url(self):
-        self.cert1.created_date = datetime.datetime(
-            2010, 8, 15, 0, 0, tzinfo=utc)
-        self.cert1.save()
-        fut = mailusers.Command().certificate_url
-        self.assertEqual(
-            fut(self.cert1),
-            'http://www.linkedin.com/profile/guided?'
-            'pfCertificationName=TEST1&'
-            'pfAuthorityId=0000000&'
-            'pfCertificationUrl=http%3A%2F%2Ftest.foo%2Ftest&pfLicenseNo=TESTX%2F1%2FTEST1&'
-            'pfCertStartDate=201005&_mSplash=1&'
-            'trk=eml-prof-edX-1-gf&startTask=CERTIFICATION_NAME&force=true')
-
-    def assert_fred_worked(self):
-        self.assertEqual(json.loads(self.fred.linkedin.emailed_courses), ['TESTX/1/TEST1', 'TESTX/2/TEST2'])
-
-    def assert_fred_failed(self):
-        self.assertEqual(json.loads(self.fred.linkedin.emailed_courses), [])
-
-    def assert_barney_worked(self):
-        self.assertEqual(json.loads(self.barney.linkedin.emailed_courses), ['TESTX/3/TEST3'])
-
-    def assert_barney_failed(self):
-        self.assertEqual(json.loads(self.barney.linkedin.emailed_courses),[])
-
-    def test_single_email_failure(self):
-        # Test error that will immediately fail a single user, but not the run
-        with mock.patch('django.core.mail.EmailMessage.send', side_effect=[SESIllegalAddressError, None]):
-            mailusers.Command().handle()
-            # Fred should fail with a send error, but we should still run Barney
-            self.assert_fred_failed()
-            self.assert_barney_worked()
-
-    def test_limited_retry_errors_both_succeed(self):
-        errors = [
-            SMTPServerDisconnected, SMTPServerDisconnected, SMTPServerDisconnected, None,
-            SMTPServerDisconnected, None
-        ]
-        with mock.patch('django.core.mail.EmailMessage.send', side_effect=errors):
-            mailusers.Command().handle()
-            self.assert_fred_worked()
-            self.assert_barney_worked()
-
-    def test_limited_retry_errors_first_fails(self):
-        errors = (MAX_ATTEMPTS + 1) * [SMTPServerDisconnected] + [None]
-        with mock.patch('django.core.mail.EmailMessage.send', side_effect=errors):
-            mailusers.Command().handle()
-            self.assert_fred_failed()
-            self.assert_barney_worked()
-
-    def test_limited_retry_errors_both_fail(self):
-        errors = (MAX_ATTEMPTS * 2) * [SMTPServerDisconnected]
-        with mock.patch('django.core.mail.EmailMessage.send', side_effect=errors):
-            mailusers.Command().handle()
-            self.assert_fred_failed()
-            self.assert_barney_failed()
-
-    @mock.patch('time.sleep')
-    def test_infinite_retry_errors(self, sleep):
-
-        def _raise_err():
-            """Need this because SMTPDataError takes args"""
-            raise SMTPDataError("", "")
-
-        errors = (MAX_ATTEMPTS * 2) * [_raise_err] + [None, None]
-        with mock.patch('django.core.mail.EmailMessage.send', side_effect=errors):
-            mailusers.Command().handle()
-            self.assert_fred_worked()
-            self.assert_barney_worked()
-
-    def test_total_failure(self):
-        # If we get this error, we just stop, so neither user gets email.
-        errors = [SESIdentityNotVerifiedError]
-        with mock.patch('django.core.mail.EmailMessage.send', side_effect=errors):
-            mailusers.Command().handle()
-            self.assert_fred_failed()
-            self.assert_barney_failed()

lms/djangoapps/lms_migration/migrate.py

@@ -121,7 +121,7 @@ def manage_modulestores(request, reload_dir=None, commit_id=None):
             settings.EDX_ROOT_URL,
             escape(cdir),
             escape(cdir),
-            course.location.url()
+            course.location.to_deprecated_string()
         )
     html += '</ol>'
 

lms/djangoapps/notes/api.py

+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 from django.contrib.auth.decorators import login_required
 from django.http import HttpResponse, Http404
 from django.core.exceptions import ValidationError
@@ -34,11 +35,11 @@ ApiResponse = collections.namedtuple('ApiResponse', ['http_response', 'data'])
 # API requests are routed through api_request() using the resource map.
 
 
-def api_enabled(request, course_id):
+def api_enabled(request, course_key):
     '''
     Returns True if the api is enabled for the course, otherwise False.
     '''
-    course = _get_course(request, course_id)
+    course = _get_course(request, course_key)
     return notes_enabled_for_course(course)
 
 
@@ -49,9 +50,11 @@ def api_request(request, course_id, **kwargs):
     Raises a 404 if the requested resource does not exist or notes are
         disabled for the course.
     '''
+    assert isinstance(course_id, basestring)
+    course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
 
     # Verify that the api should be accessible to this course
-    if not api_enabled(request, course_id):
+    if not api_enabled(request, course_key):
         log.debug('Notes are disabled for course: {0}'.format(course_id))
         raise Http404
 
@@ -78,7 +81,7 @@ def api_request(request, course_id, **kwargs):
 
     log.debug('API request: {0} {1}'.format(resource_method, resource_name))
 
-    api_response = module[func](request, course_id, **kwargs)
+    api_response = module[func](request, course_key, **kwargs)
     http_response = api_format(api_response)
 
     return http_response
@@ -104,33 +107,33 @@ def api_format(api_response):
     return http_response
 
 
-def _get_course(request, course_id):
+def _get_course(request, course_key):
     '''
     Helper function to load and return a user's course.
     '''
-    return get_course_with_access(request.user, course_id, 'load')
+    return get_course_with_access(request.user, 'load', course_key)
 
 #----------------------------------------------------------------------#
 # API actions exposed via the resource map.
 
 
-def index(request, course_id):
+def index(request, course_key):
     '''
     Returns a list of annotation objects.
     '''
     MAX_LIMIT = API_SETTINGS.get('MAX_NOTE_LIMIT')
 
-    notes = Note.objects.order_by('id').filter(course_id=course_id,
+    notes = Note.objects.order_by('id').filter(course_id=course_key,
                                                user=request.user)[:MAX_LIMIT]
 
     return ApiResponse(http_response=HttpResponse(), data=[note.as_dict() for note in notes])
 
 
-def create(request, course_id):
+def create(request, course_key):
     '''
     Receives an annotation object to create and returns a 303 with the read location.
     '''
-    note = Note(course_id=course_id, user=request.user)
+    note = Note(course_id=course_key, user=request.user)
 
     try:
         note.clean(request.body)
@@ -145,7 +148,7 @@ def create(request, course_id):
     return ApiResponse(http_response=response, data=None)
 
 
-def read(request, course_id, note_id):
+def read(request, course_key, note_id):
     '''
     Returns a single annotation object.
     '''
@@ -160,7 +163,7 @@ def read(request, course_id, note_id):
     return ApiResponse(http_response=HttpResponse(), data=note.as_dict())
 
 
-def update(request, course_id, note_id):
+def update(request, course_key, note_id):
     '''
     Updates an annotation object and returns a 303 with the read location.
     '''
@@ -203,7 +206,7 @@ def delete(request, course_id, note_id):
     return ApiResponse(http_response=HttpResponse('', status=204), data=None)
 
 
-def search(request, course_id):
+def search(request, course_key):
     '''
     Returns a subset of  annotation objects based on a search query.
     '''
@@ -228,7 +231,7 @@ def search(request, course_id):
         limit = MAX_LIMIT
 
     # set filters
-    filters = {'course_id': course_id, 'user': request.user}
+    filters = {'course_id': course_key, 'user': request.user}
     if uri != '':
         filters['uri'] = uri
 
@@ -244,7 +247,7 @@ def search(request, course_id):
     return ApiResponse(http_response=HttpResponse(), data=result)
 
 
-def root(request, course_id):
+def root(request, course_key):
     '''
     Returns version information about the API.
     '''