Merge pull request #3861 from edx/sarina/fix-quality

Fix quality violations from opaque-keys

cms/djangoapps/contentstore/views/course.py

@@ -21,7 +21,7 @@ from xmodule.modulestore.django import modulestore
 from xmodule.contentstore.content import StaticContent
 from xmodule.tabs import PDFTextbookTabs
 
-from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError, InsufficientSpecificationError
+from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError
 from opaque_keys import InvalidKeyError
 from xmodule.modulestore.locations import Location, SlashSeparatedCourseKey
 

cms/djangoapps/contentstore/views/import_export.py

@@ -30,7 +30,6 @@ from xmodule.modulestore.xml_exporter import export_to_xml
 
 from .access import has_course_access
 
-from .access import has_course_access
 from extract_tar import safetar_extractall
 from student import auth
 from student.roles import CourseInstructorRole, CourseStaffRole, GlobalStaff

common/djangoapps/course_groups/cohorts.py

@@ -32,6 +32,7 @@ def local_random():
 
     return _local_random
 
+
 def is_course_cohorted(course_key):
     """
     Given a course key, return a boolean for whether or not the course is

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?")
 

common/djangoapps/student/management/commands/anonymized_id_mapping.py

@@ -13,7 +13,6 @@ import csv
 from django.contrib.auth.models import User
 from django.core.management.base import BaseCommand, CommandError
 
-from opaque_keys import InvalidKeyError
 from student.models import anonymous_id_for_user
 from xmodule.modulestore.locations import SlashSeparatedCourseKey
 

common/djangoapps/student/management/commands/create_random_users.py

-##
-## A script to create some dummy users
-
+"""
+A script to create some dummy users
+"""
 from django.core.management.base import BaseCommand
 from student.models import CourseEnrollment
-
+from opaque_keys import InvalidKeyError
+from xmodule.modulestore.keys import CourseKey
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 from student.views import _do_create_account, get_random_post_override
 
 
-def create(n, course_key):
-    """Create n users, enrolling them in course_key if it's not None"""
-    for i in range(n):
-        (user, user_profile, _) = _do_create_account(get_random_post_override())
+def create(num, course_key):
+    """Create num users, enrolling them in course_key if it's not None"""
+    for idx in range(num):
+        (user, user_profile, __) = _do_create_account(get_random_post_override())
         if course_key is not None:
             CourseEnrollment.enroll(user, course_key)
 
@@ -31,7 +33,7 @@ Examples:
             print Command.help
             return
 
-        n = int(args[0])
+        num = int(args[0])
 
         if len(args) == 2:
             try:
@@ -41,4 +43,4 @@ Examples:
         else:
             course_key = None
 
-        create(n, course_key)
+        create(num, course_key)

common/djangoapps/student/management/commands/get_grades.py

+"""
+Management command to generate a list of grades for
+all students that are enrolled in a course.
+"""
 from courseware import grades, courses
 from certificates.models import GeneratedCertificate
 from django.test.client import RequestFactory

common/djangoapps/student/roles.py

@@ -7,7 +7,6 @@ from abc import ABCMeta, abstractmethod
 
 from django.contrib.auth.models import User
 from student.models import CourseAccessRole
-from xmodule_django.models import CourseKeyField
 
 
 class AccessRole(object):

common/djangoapps/student/tests/test_roles.py

@@ -18,13 +18,13 @@ class RolesTestCase(TestCase):
     """
 
     def setUp(self):
-        self.course_id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')
-        self.course_loc = self.course_id.make_usage_key('course', '2012_Fall')
+        self.course_key = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')
+        self.course_loc = self.course_key.make_usage_key('course', '2012_Fall')
         self.anonymous_user = AnonymousUserFactory()
         self.student = UserFactory()
         self.global_staff = UserFactory(is_staff=True)
-        self.course_staff = StaffFactory(course=self.course_id)
-        self.course_instructor = InstructorFactory(course=self.course_id)
+        self.course_staff = StaffFactory(course_key=self.course_key)
+        self.course_instructor = InstructorFactory(course_key=self.course_key)
 
     def test_global_staff(self):
         self.assertFalse(GlobalStaff().has_user(self.student))
@@ -55,20 +55,20 @@ class RolesTestCase(TestCase):
         Test that giving a user a course role enables access appropriately
         """
         self.assertFalse(
-            CourseStaffRole(self.course_id).has_user(self.student),
-            "Student has premature access to {}".format(self.course_id)
+            CourseStaffRole(self.course_key).has_user(self.student),
+            "Student has premature access to {}".format(self.course_key)
         )
-        CourseStaffRole(self.course_id).add_users(self.student)
+        CourseStaffRole(self.course_key).add_users(self.student)
         self.assertTrue(
-            CourseStaffRole(self.course_id).has_user(self.student),
-            "Student doesn't have access to {}".format(unicode(self.course_id))
+            CourseStaffRole(self.course_key).has_user(self.student),
+            "Student doesn't have access to {}".format(unicode(self.course_key))
         )
 
         # remove access and confirm
-        CourseStaffRole(self.course_id).remove_users(self.student)
+        CourseStaffRole(self.course_key).remove_users(self.student)
         self.assertFalse(
-            CourseStaffRole(self.course_id).has_user(self.student),
-            "Student still has access to {}".format(self.course_id)
+            CourseStaffRole(self.course_key).has_user(self.student),
+            "Student still has access to {}".format(self.course_key)
         )
 
     def test_org_role(self):
@@ -76,68 +76,67 @@ class RolesTestCase(TestCase):
         Test that giving a user an org role enables access appropriately
         """
         self.assertFalse(
-            OrgStaffRole(self.course_id.org).has_user(self.student),
-            "Student has premature access to {}".format(self.course_id.org)
+            OrgStaffRole(self.course_key.org).has_user(self.student),
+            "Student has premature access to {}".format(self.course_key.org)
         )
-        OrgStaffRole(self.course_id.org).add_users(self.student)
+        OrgStaffRole(self.course_key.org).add_users(self.student)
         self.assertTrue(
-            OrgStaffRole(self.course_id.org).has_user(self.student),
-            "Student doesn't have access to {}".format(unicode(self.course_id.org))
+            OrgStaffRole(self.course_key.org).has_user(self.student),
+            "Student doesn't have access to {}".format(unicode(self.course_key.org))
         )
 
         # remove access and confirm
-        OrgStaffRole(self.course_id.org).remove_users(self.student)
+        OrgStaffRole(self.course_key.org).remove_users(self.student)
         if hasattr(self.student, '_roles'):
             del self.student._roles
         self.assertFalse(
-            OrgStaffRole(self.course_id.org).has_user(self.student),
-            "Student still has access to {}".format(self.course_id.org)
+            OrgStaffRole(self.course_key.org).has_user(self.student),
+            "Student still has access to {}".format(self.course_key.org)
         )
 
     def test_org_and_course_roles(self):
         """
         Test that Org and course roles don't interfere with course roles or vice versa
         """
-        OrgInstructorRole(self.course_id.org).add_users(self.student)
-        CourseInstructorRole(self.course_id).add_users(self.student)
+        OrgInstructorRole(self.course_key.org).add_users(self.student)
+        CourseInstructorRole(self.course_key).add_users(self.student)
         self.assertTrue(
-            OrgInstructorRole(self.course_id.org).has_user(self.student),
-            "Student doesn't have access to {}".format(unicode(self.course_id.org))
+            OrgInstructorRole(self.course_key.org).has_user(self.student),
+            "Student doesn't have access to {}".format(unicode(self.course_key.org))
         )
         self.assertTrue(
-            CourseInstructorRole(self.course_id).has_user(self.student),
-            "Student doesn't have access to {}".format(unicode(self.course_id))
+            CourseInstructorRole(self.course_key).has_user(self.student),
+            "Student doesn't have access to {}".format(unicode(self.course_key))
         )
 
         # remove access and confirm
-        OrgInstructorRole(self.course_id.org).remove_users(self.student)
+        OrgInstructorRole(self.course_key.org).remove_users(self.student)
         self.assertFalse(
-            OrgInstructorRole(self.course_id.org).has_user(self.student),
-            "Student still has access to {}".format(self.course_id.org)
+            OrgInstructorRole(self.course_key.org).has_user(self.student),
+            "Student still has access to {}".format(self.course_key.org)
         )
         self.assertTrue(
-            CourseInstructorRole(self.course_id).has_user(self.student),
-            "Student doesn't have access to {}".format(unicode(self.course_id))
+            CourseInstructorRole(self.course_key).has_user(self.student),
+            "Student doesn't have access to {}".format(unicode(self.course_key))
         )
 
         # ok now keep org role and get rid of course one
-        OrgInstructorRole(self.course_id.org).add_users(self.student)
-        CourseInstructorRole(self.course_id).remove_users(self.student)
+        OrgInstructorRole(self.course_key.org).add_users(self.student)
+        CourseInstructorRole(self.course_key).remove_users(self.student)
         self.assertTrue(
-            OrgInstructorRole(self.course_id.org).has_user(self.student),
-            "Student lost has access to {}".format(self.course_id.org)
+            OrgInstructorRole(self.course_key.org).has_user(self.student),
+            "Student lost has access to {}".format(self.course_key.org)
         )
         self.assertFalse(
-            CourseInstructorRole(self.course_id).has_user(self.student),
-            "Student doesn't have access to {}".format(unicode(self.course_id))
+            CourseInstructorRole(self.course_key).has_user(self.student),
+            "Student doesn't have access to {}".format(unicode(self.course_key))
         )
 
-
     def test_get_user_for_role(self):
         """
         test users_for_role
         """
-        role = CourseStaffRole(self.course_id)
+        role = CourseStaffRole(self.course_key)
         role.add_users(self.student)
         self.assertGreater(len(role.users_with_role()), 0)
 
@@ -146,7 +145,7 @@ class RolesTestCase(TestCase):
         Tests that calling add_users multiple times before a single call
         to remove_users does not result in the user remaining in the group.
         """
-        role = CourseStaffRole(self.course_id)
+        role = CourseStaffRole(self.course_key)
         role.add_users(self.student)
         self.assertTrue(role.has_user(self.student))
         # Call add_users a second time, then remove just once.

common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py

@@ -10,7 +10,6 @@ import urllib
 from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError
 from xmodule.modulestore.locator import BlockUsageLocator, CourseLocator
 from xmodule.modulestore.locations import SlashSeparatedCourseKey
-from xmodule.modulestore.keys import UsageKey
 
 
 class LocMapperStore(object):

common/lib/xmodule/xmodule/modulestore/locator.py

@@ -88,13 +88,12 @@ class BlockLocatorBase(Locator):
         ({BLOCK_TYPE_PREFIX}\+(?P<block_type>{ALLOWED_ID_CHARS}+)\+?)?
         ({BLOCK_PREFIX}\+(?P<block_id>{ALLOWED_ID_CHARS}+))?
         """.format(
-            ALLOWED_ID_CHARS=Locator.ALLOWED_ID_CHARS, BRANCH_PREFIX=BRANCH_PREFIX,
-            VERSION_PREFIX=Locator.VERSION_PREFIX, BLOCK_TYPE_PREFIX=Locator.BLOCK_TYPE_PREFIX, BLOCK_PREFIX=BLOCK_PREFIX
-        )
+        ALLOWED_ID_CHARS=Locator.ALLOWED_ID_CHARS, BRANCH_PREFIX=BRANCH_PREFIX,
+        VERSION_PREFIX=Locator.VERSION_PREFIX, BLOCK_TYPE_PREFIX=Locator.BLOCK_TYPE_PREFIX, BLOCK_PREFIX=BLOCK_PREFIX
+    )
 
     URL_RE = re.compile('^' + URL_RE_SOURCE + '$', re.IGNORECASE | re.VERBOSE | re.UNICODE)
 
-
     @classmethod
     def parse_url(cls, string):
         """

common/lib/xmodule/xmodule/modulestore/tests/test_locators.py

@@ -158,12 +158,12 @@ class LocatorTest(TestCase):
         )
         testobj = UsageKey.from_string(testurn)
         self.check_block_locn_fields(
-             testobj,
-             org=expected_org,
-             offering=expected_offering,
-             branch=expected_branch,
-             block_type='problem',
-             block=expected_block_ref
+            testobj,
+            org=expected_org,
+            offering=expected_offering,
+            branch=expected_branch,
+            block_type='problem',
+            block=expected_block_ref
         )
         self.assertEqual(unicode(testobj), testurn)
         testobj = testobj.for_version(ObjectId())

lms/djangoapps/bulk_email/forms.py

@@ -11,7 +11,6 @@ from bulk_email.models import CourseEmailTemplate, COURSE_EMAIL_MESSAGE_BODY_TAG
 from opaque_keys import InvalidKeyError
 from xmodule.modulestore import XML_MODULESTORE_TYPE
 from xmodule.modulestore.django import modulestore
-from opaque_keys import InvalidKeyError
 from xmodule.modulestore.keys import CourseKey
 from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
@@ -67,7 +66,7 @@ class CourseAuthorizationAdminForm(forms.ModelForm):  # pylint: disable=R0924
             try:
                 course_key = SlashSeparatedCourseKey.from_deprecated_string(cleaned_id)
             except InvalidKeyError:
-                msg = u'Course id invalid.' 
+                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)

lms/djangoapps/bulk_email/tasks.py

@@ -192,7 +192,6 @@ def perform_delegate_email_batches(entry_id, course_id, task_input, action_name)
         log.warning(u"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.
     course = get_course(course_id)
 

lms/djangoapps/bulk_email/tests/test_email.py

@@ -54,10 +54,10 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase):
         course_title = u"ẗëṡẗ title ｲ乇丂ｲ ﾶ乇丂丂ﾑg乇 ｷo尺 ﾑﾚﾚ тэѕт мэѕѕаБэ"
         self.course = CourseFactory.create(display_name=course_title)
 
-        self.instructor = InstructorFactory(course=self.course.id)
+        self.instructor = InstructorFactory(course_key=self.course.id)
 
         # Create staff
-        self.staff = [StaffFactory(course=self.course.id)
+        self.staff = [StaffFactory(course_key=self.course.id)
                       for _ in xrange(STAFF_COUNT)]
 
         # Create students

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

+"""
+Management command which sets or gets the certificate whitelist for a given
+user/course
+"""
 from django.core.management.base import BaseCommand, CommandError
 from optparse import make_option
 from opaque_keys import InvalidKeyError
@@ -56,7 +60,7 @@ class Command(BaseCommand):
         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)
+            print("Course id {} could not be parsed as a CourseKey; falling back to SSCK.from_dep_str".format(course_id))
             course = SlashSeparatedCourseKey.from_deprecated_string(course_id)
 
         if options['add'] and options['del']:

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

+"""
+Management command to find all students that need certificates for
+courses that have finished, and put their cert requests on the queue.
+"""
 from django.core.management.base import BaseCommand, CommandError
 from certificates.models import certificate_status_for_student
 from certificates.queue import XQueueCertInterface
@@ -73,7 +77,7 @@ class Command(BaseCommand):
             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)
+                print("Course id {} could not be parsed as a CourseKey; falling back to SSCK.from_dep_str".format(options['course']))
                 course = SlashSeparatedCourseKey.from_deprecated_string(options['course'])
             ended_courses = [course]
         else:

lms/djangoapps/course_wiki/tests/test_access.py

@@ -48,8 +48,8 @@ class TestWikiAccessBase(ModuleStoreTestCase):
         """Creates and returns users with instructor and staff access to course."""
 
         return [
-            InstructorFactory(course=course.id),  # Creates instructor_org/number/run role name
-            StaffFactory(course=course.id),  # Creates staff_org/number/run role name
+            InstructorFactory(course_key=course.id),  # Creates instructor_org/number/run role name
+            StaffFactory(course_key=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.id, username='instructor', password='secret')
+        self.course_math101_instructor = InstructorFactory(course_key=self.course_math101.id, username='instructor', password='secret')
         self.wiki_math101 = URLPath.create_article(self.wiki, 'math101', title='math101')
 
         self.client = Client()

lms/djangoapps/courseware/features/lti.py

@@ -196,9 +196,9 @@ def i_am_registered_for_the_course(coursenum, metadata, user='Instructor'):
         course_descriptor = world.scenario_dict['COURSE']
 
         # create beta tester
-        user = BetaTesterFactory(course=course_descriptor.id)
+        user = BetaTesterFactory(course_key=course_descriptor.id)
         normal_student = UserFactory()
-        instructor = InstructorFactory(course=course_descriptor.id)
+        instructor = InstructorFactory(course_key=course_descriptor.id)
 
         assert not has_access(normal_student, 'load', course_descriptor)
         assert has_access(user, 'load', course_descriptor)
@@ -207,7 +207,7 @@ def i_am_registered_for_the_course(coursenum, metadata, user='Instructor'):
         metadata.update({'start': datetime.datetime(1970, 1, 1, tzinfo=UTC)})
         create_course_for_lti(coursenum, metadata)
         course_descriptor = world.scenario_dict['COURSE']
-        user = InstructorFactory(course=course_descriptor.id)
+        user = InstructorFactory(course_key=course_descriptor.id)
 
     # Enroll the user in the course and log them in
     if has_access(user, 'load', course_descriptor):

lms/djangoapps/courseware/models.py

@@ -113,7 +113,12 @@ class StudentModuleHistory(models.Model):
     max_grade = models.FloatField(null=True, blank=True)
 
     @receiver(post_save, sender=StudentModule)
-    def save_history(sender, instance, **kwargs):  # pylint: disable=no-self-argument
+    def save_history(sender, instance, **kwargs):  # pylint: disable=no-self-argument, unused-argument
+        """
+        Checks the instance's module_type, and creates & saves a
+        StudentModuleHistory entry if the module_type is one that
+        we save.
+        """
         if instance.module_type in StudentModuleHistory.HISTORY_SAVING_TYPES:
             history_entry = StudentModuleHistory(student_module=instance,
                                                  version=None,

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):

lms/djangoapps/courseware/tests/factories.py

+# Factories don't have __init__ methods, and are self documenting
+# pylint: disable=W0232, C0111
 import json
 from functools import partial
 import factory
@@ -23,6 +25,8 @@ from student.roles import (
 from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 
+# TODO fix this (course_id and location are invalid names as constants, and course_id should really be COURSE_KEY)
+# pylint: disable=invalid-name
 course_id = SlashSeparatedCourseKey(u'edX', u'test_course', u'test')
 location = partial(course_id.make_usage_key, u'problem')
 
@@ -31,6 +35,11 @@ class UserProfileFactory(StudentUserProfileFactory):
     courseware = 'course.xml'
 
 
+# For the following factories, these are disabled because we're ok ignoring the
+# unused arguments create and **kwargs in the line:
+# course_key(self, create, extracted, **kwargs)
+# pylint: disable=unused-argument
+
 class InstructorFactory(UserFactory):
     """
     Given a course Location, returns a User object with instructor
@@ -39,8 +48,7 @@ class InstructorFactory(UserFactory):
     last_name = "Instructor"
 
     @factory.post_generation
-    # TODO Change this from course to course_key at next opportunity
-    def course(self, create, extracted, **kwargs):
+    def course_key(self, create, extracted, **kwargs):
         if extracted is None:
             raise ValueError("Must specify a CourseKey for a course instructor user")
         CourseInstructorRole(extracted).add_users(self)
@@ -54,8 +62,7 @@ class StaffFactory(UserFactory):
     last_name = "Staff"
 
     @factory.post_generation
-    # TODO Change this from course to course_key at next opportunity
-    def course(self, create, extracted, **kwargs):
+    def course_key(self, create, extracted, **kwargs):
         if extracted is None:
             raise ValueError("Must specify a CourseKey for a course staff user")
         CourseStaffRole(extracted).add_users(self)
@@ -69,8 +76,7 @@ class BetaTesterFactory(UserFactory):
     last_name = "Beta-Tester"
 
     @factory.post_generation
-    # TODO Change this from course to course_key at next opportunity
-    def course(self, create, extracted, **kwargs):
+    def course_key(self, create, extracted, **kwargs):
         if extracted is None:
             raise ValueError("Must specify a CourseKey for a beta-tester user")
         CourseBetaTesterRole(extracted).add_users(self)
@@ -84,8 +90,7 @@ class OrgStaffFactory(UserFactory):
     last_name = "Org-Staff"
 
     @factory.post_generation
-    # TODO Change this from course to course_key at next opportunity
-    def course(self, create, extracted, **kwargs):
+    def course_key(self, create, extracted, **kwargs):
         if extracted is None:
             raise ValueError("Must specify a CourseKey for an org-staff user")
         OrgStaffRole(extracted.org).add_users(self)
@@ -99,8 +104,7 @@ class OrgInstructorFactory(UserFactory):
     last_name = "Org-Instructor"
 
     @factory.post_generation
-    # TODO Change this from course to course_key at next opportunity
-    def course(self, create, extracted, **kwargs):
+    def course_key(self, create, extracted, **kwargs):
         if extracted is None:
             raise ValueError("Must specify a CourseKey for an org-instructor user")
         OrgInstructorRole(extracted.org).add_users(self)
@@ -115,6 +119,7 @@ class GlobalStaffFactory(UserFactory):
     @factory.post_generation
     def set_staff(self, create, extracted, **kwargs):
         GlobalStaff().add_users(self)
+# pylint: enable=unused-argument
 
 
 class StudentModuleFactory(DjangoModelFactory):

lms/djangoapps/courseware/tests/test_access.py

@@ -26,9 +26,8 @@ class AccessTestCase(TestCase):
         self.anonymous_user = AnonymousUserFactory()
         self.student = UserFactory()
         self.global_staff = UserFactory(is_staff=True)
-        # TODO please change the StaffFactory and InstructorFactory parameters ASAP!
-        self.course_staff = StaffFactory(course=self.course.course_key)
-        self.course_instructor = InstructorFactory(course=self.course.course_key)
+        self.course_staff = StaffFactory(course_key=self.course.course_key)
+        self.course_instructor = InstructorFactory(course_key=self.course.course_key)
 
     def test_has_access_to_course(self):
         self.assertFalse(access._has_access_to_course(
@@ -144,8 +143,8 @@ class UserRoleTestCase(TestCase):
         self.anonymous_user = AnonymousUserFactory()
         self.student = UserFactory()
         self.global_staff = UserFactory(is_staff=True)
-        self.course_staff = StaffFactory(course=self.course_key)
-        self.course_instructor = InstructorFactory(course=self.course_key)
+        self.course_staff = StaffFactory(course_key=self.course_key)
+        self.course_instructor = InstructorFactory(course_key=self.course_key)
 
     def test_user_role_staff(self):
         """Ensure that user role is student for staff masqueraded as student."""

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/test_masquerade.py

@@ -36,7 +36,7 @@ class TestStaffMasqueradeAsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
         self.graded_course = modulestore().get_course(SlashSeparatedCourseKey("edX", "graded", "2012_Fall"))
 
         # Create staff account
-        self.staff = StaffFactory(course=self.graded_course.id)
+        self.staff = StaffFactory(course_key=self.graded_course.id)
 
         self.logout()
         # self.staff.password is the sha hash but login takes the plain text

lms/djangoapps/courseware/tests/test_video_handlers.py

@@ -22,7 +22,6 @@ from xmodule.video_module.transcripts_utils import (
     TranscriptException,
     TranscriptsGenerationException,
 )
-from xmodule.modulestore.mongo.base import MongoModuleStore
 from xmodule.modulestore.locations import AssetLocation
 
 SRT_content = textwrap.dedent("""

lms/djangoapps/courseware/tests/test_view_authentication.py

@@ -138,11 +138,10 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
         CourseEnrollmentFactory(user=self.enrolled_user, course_id=self.course.id)
         CourseEnrollmentFactory(user=self.enrolled_user, course_id=self.test_course.id)
 
-        self.staff_user = StaffFactory(course=self.course.id)
-        self.instructor_user = InstructorFactory(
-            course=self.course.id)
-        self.org_staff_user = OrgStaffFactory(course=self.course.id)
-        self.org_instructor_user = OrgInstructorFactory(course=self.course.id)
+        self.staff_user = StaffFactory(course_key=self.course.id)
+        self.instructor_user = InstructorFactory(course_key=self.course.id)
+        self.org_staff_user = OrgStaffFactory(course_key=self.course.id)
+        self.org_instructor_user = OrgInstructorFactory(course_key=self.course.id)
 
     def test_redirection_unenrolled(self):
         """
@@ -374,7 +373,7 @@ class TestBetatesterAccess(ModuleStoreTestCase):
         self.content = ItemFactory(parent=self.course)
 
         self.normal_student = UserFactory()
-        self.beta_tester = BetaTesterFactory(course=self.course.id)
+        self.beta_tester = BetaTesterFactory(course_key=self.course.id)
 
     @patch.dict('courseware.access.settings.FEATURES', {'DISABLE_START_DATES': False})
     def test_course_beta_period(self):

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,

lms/djangoapps/courseware/views.py

@@ -47,7 +47,6 @@ from opaque_keys import InvalidKeyError
 
 from microsite_configuration import microsite
 from xmodule.modulestore.locations import SlashSeparatedCourseKey
-from xmodule.modulestore.keys import UsageKey
 
 log = logging.getLogger("edx.courseware")
 

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

+"""
+Management command to seed default permissions and roles.
+"""
 from django.core.management.base import BaseCommand, CommandError
 from django_comment_common.utils import seed_permissions_roles
 from xmodule.modulestore.locations import SlashSeparatedCourseKey

lms/djangoapps/instructor/features/bulk_email.py

@@ -29,19 +29,19 @@ def make_populated_course(step):  # pylint: disable=unused-argument
         number='888',
         display_name='Bulk Email Test Course'
     )
-    world.bulk_email_course_id = course.id
+    world.bulk_email_course_key = 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=world.bulk_email_course_id)
-        world.enroll_user(world.bulk_email_instructor, world.bulk_email_course_id)
+        world.bulk_email_instructor = InstructorFactory(course_key=world.bulk_email_course_key)
+        world.enroll_user(world.bulk_email_instructor, world.bulk_email_course_key)
 
         # Make & register a staff member
-        world.bulk_email_staff = StaffFactory(course=course.id)
-        world.enroll_user(world.bulk_email_staff, world.bulk_email_course_id)
+        world.bulk_email_staff = StaffFactory(course_key=course.id)
+        world.enroll_user(world.bulk_email_staff, world.bulk_email_course_key)
 
     # Make & register a student
     world.register_by_course_key(

lms/djangoapps/instructor/features/common.py

@@ -43,13 +43,13 @@ def i_am_staff_or_instructor(step, role):  # pylint: disable=unused-argument
         display_name='Test Course'
     )
 
-    world.course_id = course.id
+    world.course_key = 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=world.course_id)
-        world.enroll_user(world.instructor, world.course_id)
+        world.instructor = InstructorFactory(course_key=world.course_key)
+        world.enroll_user(world.instructor, world.course_key)
 
         world.log_in(
             username=world.instructor.username,
@@ -61,8 +61,8 @@ 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=world.course_id)
-        world.enroll_user(world.staff, world.course_id)
+        world.staff = StaffFactory(course_key=world.course_key)
+        world.enroll_user(world.staff, world.course_key)
 
         world.log_in(
             username=world.staff.username,

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

 #!/usr/bin/python
-#
-# django management command: dump grades to csv files
-# for use by batch processes
-
+"""
+django management command: dump grades to csv files
+for use by batch processes
+"""
 from instructor.offline_gradecalc import offline_grade_calculation
 from courseware.courses import get_course_by_id
 from xmodule.modulestore.django import modulestore
@@ -37,7 +37,7 @@ class Command(BaseCommand):
         except InvalidKeyError:
             course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
         try:
-            course = get_course_by_id(course_key)
+            _course = get_course_by_id(course_key)
         except Exception as err:
             print "-----------------------------------------------------------------------------"
             print "Sorry, cannot find course with id {}".format(course_id)

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

 #!/usr/bin/python
-#
-# django management command: dump grades to csv files
-# for use by batch processes
-
+"""
+django management command: dump grades to csv files
+for use by batch processes
+"""
 import csv
 
 from instructor.views.legacy import get_student_grade_summary_data
@@ -52,7 +52,9 @@ class Command(BaseCommand):
 
         try:
             course = get_course_by_id(course_key)
-        except Exception as err:
+        # Ok with catching general exception here because this is run as a management command
+        # and the exception is exposed right away to the user.
+        except Exception as err:  # pylint: disable=broad-except
             print "-----------------------------------------------------------------------------"
             print "Sorry, cannot find course with id {}".format(course_id)
             print "Got exception {}".format(err)

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

@@ -6,7 +6,6 @@ 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

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

@@ -9,7 +9,6 @@ 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
 

lms/djangoapps/instructor/offline_gradecalc.py

-# ======== Offline calculation of grades =============================================================================
-#
-# Computing grades of a large number of students can take a long time.  These routines allow grades to
-# be computed offline, by a batch process (eg cronjob).
-#
-# The grades are stored in the OfflineComputedGrade table of the courseware model.
+"""
+======== Offline calculation of grades =============================================================
 
+Computing grades of a large number of students can take a long time.  These routines allow grades to
+be computed offline, by a batch process (eg cronjob).
+
+The grades are stored in the OfflineComputedGrade table of the courseware model.
+"""
 import json
 import time
 
@@ -49,7 +50,7 @@ def offline_grade_calculation(course_key):
 
         gradeset = grades.grade(student, request, course, keep_raw_scores=True)
         gs = enc.encode(gradeset)
-        ocg, created = models.OfflineComputedGrade.objects.get_or_create(user=student, course_id=course_key)
+        ocg, _created = models.OfflineComputedGrade.objects.get_or_create(user=student, course_id=course_key)
         ocg.gradeset = gs
         ocg.save()
         print "%s done" % student  	# print statement used because this is run by a management command

lms/djangoapps/instructor/tests/test_api.py

@@ -13,7 +13,6 @@ from nose.tools import raises
 from mock import Mock, patch
 from django.conf import settings
 from django.test.utils import override_settings
-from django.conf import settings
 from django.core.urlresolvers import reverse
 from django.http import HttpRequest, HttpResponse
 from django_comment_common.models import FORUM_ROLE_COMMUNITY_TA, Role
@@ -197,7 +196,7 @@ class TestInstructorAPIDenyLevels(ModuleStoreTestCase, LoginEnrollmentTestCase):
         """
         Ensure that a staff member can't access instructor endpoints.
         """
-        staff_member = StaffFactory(course=self.course.id)
+        staff_member = StaffFactory(course_key=self.course.id)
         CourseEnrollment.enroll(staff_member, self.course.id)
         self.client.login(username=staff_member.username, password='test')
         # Try to promote to forums admin - not working
@@ -226,7 +225,7 @@ class TestInstructorAPIDenyLevels(ModuleStoreTestCase, LoginEnrollmentTestCase):
         """
         Ensure that an instructor member can access all endpoints.
         """
-        inst = InstructorFactory(course=self.course.id)
+        inst = InstructorFactory(course_key=self.course.id)
         CourseEnrollment.enroll(inst, self.course.id)
         self.client.login(username=inst.username, password='test')
 
@@ -264,7 +263,7 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase):
     def setUp(self):
         self.request = RequestFactory().request()
         self.course = CourseFactory.create()
-        self.instructor = InstructorFactory(course=self.course.id)
+        self.instructor = InstructorFactory(course_key=self.course.id)
         self.client.login(username=self.instructor.username, password='test')
 
         self.enrolled_student = UserFactory(username='EnrolledStudent', first_name='Enrolled', last_name='Student')
@@ -752,10 +751,10 @@ class TestInstructorAPIBulkBetaEnrollment(ModuleStoreTestCase, LoginEnrollmentTe
     """
     def setUp(self):
         self.course = CourseFactory.create()
-        self.instructor = InstructorFactory(course=self.course.id)
+        self.instructor = InstructorFactory(course_key=self.course.id)
         self.client.login(username=self.instructor.username, password='test')
 
-        self.beta_tester = BetaTesterFactory(course=self.course.id)
+        self.beta_tester = BetaTesterFactory(course_key=self.course.id)
         CourseEnrollment.enroll(
             self.beta_tester,
             self.course.id
@@ -1057,11 +1056,11 @@ class TestInstructorAPILevelsAccess(ModuleStoreTestCase, LoginEnrollmentTestCase
     """
     def setUp(self):
         self.course = CourseFactory.create()
-        self.instructor = InstructorFactory(course=self.course.id)
+        self.instructor = InstructorFactory(course_key=self.course.id)
         self.client.login(username=self.instructor.username, password='test')
 
-        self.other_instructor = InstructorFactory(course=self.course.id)
-        self.other_staff = StaffFactory(course=self.course.id)
+        self.other_instructor = InstructorFactory(course_key=self.course.id)
+        self.other_staff = StaffFactory(course_key=self.course.id)
         self.other_user = UserFactory()
 
     def test_modify_access_noparams(self):
@@ -1300,7 +1299,7 @@ class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCa
     """
     def setUp(self):
         self.course = CourseFactory.create()
-        self.instructor = InstructorFactory(course=self.course.id)
+        self.instructor = InstructorFactory(course_key=self.course.id)
         self.client.login(username=self.instructor.username, password='test')
 
         self.students = [UserFactory() for _ in xrange(6)]
@@ -1479,7 +1478,7 @@ class TestInstructorAPIRegradeTask(ModuleStoreTestCase, LoginEnrollmentTestCase)
     """
     def setUp(self):
         self.course = CourseFactory.create()
-        self.instructor = InstructorFactory(course=self.course.id)
+        self.instructor = InstructorFactory(course_key=self.course.id)
         self.client.login(username=self.instructor.username, password='test')
 
         self.student = UserFactory()
@@ -1617,7 +1616,7 @@ class TestInstructorSendEmail(ModuleStoreTestCase, LoginEnrollmentTestCase):
     """
     def setUp(self):
         self.course = CourseFactory.create()
-        self.instructor = InstructorFactory(course=self.course.id)
+        self.instructor = InstructorFactory(course_key=self.course.id)
         self.client.login(username=self.instructor.username, password='test')
         test_subject = u'\u1234 test subject'
         test_message = u'\u6824 test message'
@@ -1739,7 +1738,7 @@ class TestInstructorAPITaskLists(ModuleStoreTestCase, LoginEnrollmentTestCase):
 
     def setUp(self):
         self.course = CourseFactory.create()
-        self.instructor = InstructorFactory(course=self.course.id)
+        self.instructor = InstructorFactory(course_key=self.course.id)
         self.client.login(username=self.instructor.username, password='test')
 
         self.student = UserFactory()
@@ -1872,7 +1871,7 @@ class TestInstructorAPIAnalyticsProxy(ModuleStoreTestCase, LoginEnrollmentTestCa
 
     def setUp(self):
         self.course = CourseFactory.create()
-        self.instructor = InstructorFactory(course=self.course.id)
+        self.instructor = InstructorFactory(course_key=self.course.id)
         self.client.login(username=self.instructor.username, password='test')
 
     @patch.object(instructor.views.api.requests, 'get')
@@ -2042,7 +2041,7 @@ class TestDueDateExtensions(ModuleStoreTestCase, LoginEnrollmentTestCase):
         self.user1 = user1
         self.user2 = user2
 
-        self.instructor = InstructorFactory(course=course.id)
+        self.instructor = InstructorFactory(course_key=course.id)
         self.client.login(username=self.instructor.username, password='test')
 
     def test_change_due_date(self):

lms/djangoapps/instructor/tests/test_enrollment.py

@@ -9,7 +9,6 @@ from courseware.models import StudentModule
 from django.conf import settings
 from django.test import TestCase
 from django.test.utils import override_settings
-from django.test.client import RequestFactory
 from student.tests.factories import UserFactory
 from xmodule.modulestore.tests.factories import CourseFactory
 from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
@@ -27,7 +26,6 @@ from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 from submissions import api as sub_api
 from student.models import anonymous_id_for_user
-from .test_tools import msk_from_problem_urlname
 
 
 class TestSettableEnrollmentState(TestCase):

lms/djangoapps/instructor/tests/test_legacy_gradebook.py

@@ -11,7 +11,6 @@ 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
 
 

lms/djangoapps/instructor/tests/test_legacy_reset.py

@@ -16,7 +16,6 @@ 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)

lms/djangoapps/notes/api.py

@@ -148,7 +148,7 @@ def create(request, course_key):
     return ApiResponse(http_response=response, data=None)
 
 
-def read(request, course_key, note_id):
+def read(request, course_key, note_id):  # pylint: disable=unused-argument (course_key)
     '''
     Returns a single annotation object.
     '''
@@ -163,7 +163,7 @@ def read(request, course_key, note_id):
     return ApiResponse(http_response=HttpResponse(), data=note.as_dict())
 
 
-def update(request, course_key, note_id):
+def update(request, course_key, note_id):  # pylint: disable=unused-argument (course_key)
     '''
     Updates an annotation object and returns a 303 with the read location.
     '''
@@ -247,7 +247,7 @@ def search(request, course_key):
     return ApiResponse(http_response=HttpResponse(), data=result)
 
 
-def root(request, course_key):
+def root(request, course_key):  # pylint: disable=unused-argument (course_key, request)
     '''
     Returns version information about the API.
     '''

lms/djangoapps/psychometrics/management/commands/init_psychometrics.py

@@ -7,7 +7,6 @@ import json
 from courseware.models import StudentModule
 from track.models import TrackingLog
 from psychometrics.models import PsychometricData
-from xmodule.modulestore.keys import UsageKey
 
 from django.conf import settings
 from django.core.management.base import BaseCommand