From 6f7bc7e1742cf9cb1acd04c119c22a4148605fce Mon Sep 17 00:00:00 2001 From: Julia Hansbrough Date: Fri, 23 May 2014 18:48:34 +0000 Subject: [PATCH] Fixed some TODOs, removed irrelevant ones --- common/djangoapps/course_groups/models.py | 1 - lms/djangoapps/courseware/module_render.py | 6 +++--- .../courseware/tests/test_lti_integration.py | 2 +- lms/djangoapps/courseware/tests/tests.py | 15 +++++++-------- 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/common/djangoapps/course_groups/models.py b/common/djangoapps/course_groups/models.py index 52c22b6e9a..7c17898899 100644 --- a/common/djangoapps/course_groups/models.py +++ b/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?") diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 6127607b56..0a074b2313 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/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 -# there's a couple tricky ones I'm too afraid to change before we merge the jellyfish branches. -# This should be fixed after the jellyfish merge, before merge into master. +# TODO: course_id and course_key are used interchangeably in this file, which is wrong. +# Some brave person should make the variable names consistently someday, but the code's +# coupled enough that it's kind of tricky--you've been warned! class LmsModuleRenderError(Exception): diff --git a/lms/djangoapps/courseware/tests/test_lti_integration.py b/lms/djangoapps/courseware/tests/test_lti_integration.py index a17d5776f5..558633d7a8 100644 --- a/lms/djangoapps/courseware/tests/test_lti_integration.py +++ b/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 diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index 08ec754129..2132a01c8e 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/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_id): + def check_all_pages_load(self, course_key): """ 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,