Fixed some TODOs, removed irrelevant ones

common/djangoapps/course_groups/models.py

@@ -24,7 +24,6 @@ class CourseUserGroup(models.Model):
 
     # Note: groups associated with particular runs of a course.  E.g. Fall 2012 and Spring
     # 2013 versions of 6.00x will have separate groups.
-    # TODO change field name to course_key
     course_id = CourseKeyField(max_length=255, db_index=True,
                                  help_text="Which course is this group associated with?")
 

lms/djangoapps/courseware/module_render.py

@@ -60,9 +60,9 @@ XQUEUE_INTERFACE = XQueueInterface(
     REQUESTS_AUTH,
 )
 
-# TODO basically all instances of course_id in this file *should* be changed to course_key, but
+# TODO: course_id and course_key are used interchangeably in this file, which is wrong.
-# there's a couple tricky ones I'm too afraid to change before we merge the jellyfish branches.
+# Some brave person should make the variable names consistently someday, but the code's
-# This should be fixed after the jellyfish merge, before merge into master.
+# coupled enough that it's kind of tricky--you've been warned!
 
 
 class LmsModuleRenderError(Exception):

lms/djangoapps/courseware/tests/test_lti_integration.py

@@ -39,7 +39,7 @@ class TestLTI(BaseTestXmodule):
         mocked_signature_after_sign = u'my_signature%3D'
         mocked_decoded_signature = u'my_signature='
 
-        # TODO this course_id is actually a course_key; please change this ASAP!
+        # Note: this course_id is actually a course_key
         context_id = self.item_descriptor.course_id.to_deprecated_string()
         user_id = unicode(self.item_descriptor.xmodule_runtime.anonymous_student_id)
         hostname = self.item_descriptor.xmodule_runtime.hostname

lms/djangoapps/courseware/tests/tests.py

@@ -48,8 +48,7 @@ class PageLoaderTestCase(LoginEnrollmentTestCase):
     Base class that adds a function to load all pages in a modulestore.
     """
 
-    # TODO once everything is merged can someone please check whether this function takes a course_id or course_key
+    def check_all_pages_load(self, course_key):
-    def check_all_pages_load(self, course_id):
         """
         Assert that all pages in the course load correctly.
         `course_id` is the ID of the course to check.
@@ -58,11 +57,11 @@ class PageLoaderTestCase(LoginEnrollmentTestCase):
         store = modulestore()
 
         # Enroll in the course before trying to access pages
-        course = store.get_course(course_id)
+        course = store.get_course(course_key)
         self.enroll(course, True)
 
         # Search for items in the course
-        items = store.get_items(course_id)
+        items = store.get_items(course_key)
 
         if len(items) < 1:
             self.fail('Could not retrieve any items from course')
@@ -72,21 +71,21 @@ class PageLoaderTestCase(LoginEnrollmentTestCase):
 
             if descriptor.location.category == 'about':
                 self._assert_loads('about_course',
-                                   {'course_id': course_id.to_deprecated_string()},
+                                   {'course_id': course_key.to_deprecated_string()},
                                    descriptor)
 
             elif descriptor.location.category == 'static_tab':
-                kwargs = {'course_id': course_id.to_deprecated_string(),
+                kwargs = {'course_id': course_key.to_deprecated_string(),
                           'tab_slug': descriptor.location.name}
                 self._assert_loads('static_tab', kwargs, descriptor)
 
             elif descriptor.location.category == 'course_info':
-                self._assert_loads('info', {'course_id': course_id.to_deprecated_string()},
+                self._assert_loads('info', {'course_id': course_key.to_deprecated_string()},
                                    descriptor)
 
             else:
 
-                kwargs = {'course_id': course_id.to_deprecated_string(),
+                kwargs = {'course_id': course_key.to_deprecated_string(),
                           'location': descriptor.location.to_deprecated_string()}
 
                 self._assert_loads('jump_to', kwargs, descriptor,