Merge pull request #1857 from MITx/fix/cdodge/violation-fixes

more cleanups in CMS project

cms/djangoapps/auth/authz.py

-import logging
-import sys
-
 from django.contrib.auth.models import User, Group
 from django.core.exceptions import PermissionDenied
 
@@ -131,7 +128,7 @@ def remove_user_from_course_group(caller, user, location, role):
         raise PermissionDenied
 
     # see if the user is actually in that role, if not then we don't have to do anything
-    if is_user_in_course_group_role(user, location, role) == True:
+    if is_user_in_course_group_role(user, location, role):
         groupname = get_course_groupname_for_role(location, role)
 
         group = Group.objects.get(name=groupname)

cms/djangoapps/contentstore/features/checklists.py

@@ -6,6 +6,7 @@ from nose.tools import assert_true, assert_equal
 from terrain.steps import reload_the_page
 from selenium.common.exceptions import StaleElementReferenceException
 
+
 ############### ACTIONS ####################
 @step('I select Checklists from the Tools menu$')
 def i_select_checklists(step):

cms/djangoapps/contentstore/management/commands/clone.py

@@ -5,7 +5,6 @@ from django.core.management.base import BaseCommand, CommandError
 from xmodule.modulestore.store_utilities import clone_course
 from xmodule.modulestore.django import modulestore
 from xmodule.contentstore.django import contentstore
-from xmodule.modulestore import Location
 from xmodule.course_module import CourseDescriptor
 
 from auth.authz import _copy_course_group
@@ -16,8 +15,7 @@ from auth.authz import _copy_course_group
 
 
 class Command(BaseCommand):
-    help = \
-'''Clone a MongoDB backed course to another location'''
+    help = 'Clone a MongoDB backed course to another location'
 
     def handle(self, *args, **options):
         if len(args) != 2:

cms/djangoapps/contentstore/management/commands/delete_course.py

@@ -5,7 +5,6 @@ from django.core.management.base import BaseCommand, CommandError
 from xmodule.modulestore.store_utilities import delete_course
 from xmodule.modulestore.django import modulestore
 from xmodule.contentstore.django import contentstore
-from xmodule.modulestore import Location
 from xmodule.course_module import CourseDescriptor
 from .prompt import query_yes_no
 
@@ -38,7 +37,7 @@ class Command(BaseCommand):
         if query_yes_no("Deleting course {0}. Confirm?".format(loc_str), default="no"):
             if query_yes_no("Are you sure. This action cannot be undone!", default="no"):
                 loc = CourseDescriptor.id_to_location(loc_str)
-                if delete_course(ms, cs, loc, commit) == True:
+                if delete_course(ms, cs, loc, commit):
                     print 'removing User permissions from course....'
                     # in the django layer, we need to remove all the user permissions groups associated with this course
                     if commit:

cms/djangoapps/contentstore/management/commands/export.py

@@ -7,7 +7,6 @@ from django.core.management.base import BaseCommand, CommandError
 from xmodule.modulestore.xml_exporter import export_to_xml
 from xmodule.modulestore.django import modulestore
 from xmodule.contentstore.django import contentstore
-from xmodule.modulestore import Location
 from xmodule.course_module import CourseDescriptor
 
 
@@ -15,8 +14,7 @@ unnamed_modules = 0
 
 
 class Command(BaseCommand):
-    help = \
-'''Import the specified data directory into the default ModuleStore'''
+    help = 'Import the specified data directory into the default ModuleStore'
 
     def handle(self, *args, **options):
         if len(args) != 2:

cms/djangoapps/contentstore/management/commands/import.py

@@ -12,8 +12,7 @@ unnamed_modules = 0
 
 
 class Command(BaseCommand):
-    help = \
-'''Import the specified data directory into the default ModuleStore'''
+    help = 'Import the specified data directory into the default ModuleStore'
 
     def handle(self, *args, **options):
         if len(args) == 0:

cms/djangoapps/contentstore/management/commands/prompt.py

@@ -11,8 +11,8 @@ def query_yes_no(question, default="yes"):
 
     The "answer" return value is one of "yes" or "no".
     """
-    valid = {"yes":True,   "y":True,  "ye":True,
-             "no":False,     "n":False}
+    valid = {"yes": True,   "y": True,  "ye": True,
+             "no": False,     "n": False}
     if default is None:
         prompt = " [y/n] "
     elif default == "yes":
@@ -30,5 +30,4 @@ def query_yes_no(question, default="yes"):
         elif choice in valid:
             return valid[choice]
         else:
-            sys.stdout.write("Please respond with 'yes' or 'no' "\
-                             "(or 'y' or 'n').\n")
+            sys.stdout.write("Please respond with 'yes' or 'no' (or 'y' or 'n').\n")

cms/djangoapps/contentstore/management/commands/update_templates.py

 from xmodule.templates import update_templates
 from django.core.management.base import BaseCommand
 
+
 class Command(BaseCommand):
-    help = \
-'''Imports and updates the Studio component templates from the code pack and put in the DB'''
+    help = 'Imports and updates the Studio component templates from the code pack and put in the DB'
 
     def handle(self, *args, **options):
         update_templates()

cms/djangoapps/contentstore/management/commands/xlint.py

 from django.core.management.base import BaseCommand, CommandError
 from xmodule.modulestore.xml_importer import perform_xlint
-from xmodule.modulestore.django import modulestore
-from xmodule.contentstore.django import contentstore
 
 
 unnamed_modules = 0
@@ -13,6 +11,7 @@ class Command(BaseCommand):
         Verify the structure of courseware as to it's suitability for import
         To run test: rake cms:xlint DATA_DIR=../data [COURSE_DIR=content-edx-101 (optional parameter)]
         '''
+
     def handle(self, *args, **options):
         if len(args) == 0:
             raise CommandError("import requires at least one argument: <data directory> [<course dir>...]")

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -6,12 +6,11 @@ from django.conf import settings
 from django.core.urlresolvers import reverse
 from path import path
 from tempdir import mkdtemp_clean
-from datetime import timedelta
-import json
 from fs.osfs import OSFS
 import copy
 from json import loads
 import traceback
+from datetime import timedelta
 
 from django.contrib.auth.models import User
 from django.dispatch import Signal
@@ -39,6 +38,7 @@ TEST_DATA_MODULESTORE = copy.deepcopy(settings.MODULESTORE)
 TEST_DATA_MODULESTORE['default']['OPTIONS']['fs_root'] = path('common/test/data')
 TEST_DATA_MODULESTORE['direct']['OPTIONS']['fs_root'] = path('common/test/data')
 
+
 class MongoCollectionFindWrapper(object):
     def __init__(self, original):
         self.original = original
@@ -203,7 +203,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         # make sure we can query that item and verify that it is a draft
         draft_problem = modulestore('draft').get_item(Location(['i4x', 'edX', 'simple',
                                                                 'problem', 'ps01-simple', None]))
-        self.assertTrue(getattr(draft_problem,'is_draft', False))
+        self.assertTrue(getattr(draft_problem, 'is_draft', False))
 
         #now requery with depth
         course = modulestore('draft').get_item(Location(['i4x', 'edX', 'simple',
@@ -213,7 +213,6 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         num_drafts = self._get_draft_counts(course)
         self.assertEqual(num_drafts, 1)
 
-
     def test_static_tab_reordering(self):
         import_from_xml(modulestore(), 'common/test/data/', ['full'])
 
@@ -268,7 +267,8 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         self.assertTrue(sequential.location.url() in chapter.children)
 
         self.client.post(reverse('delete_item'),
-            json.dumps({'id': sequential.location.url(), 'delete_children': 'true', 'delete_all_versions': 'true'}),
+                         json.dumps({'id': sequential.location.url(), 'delete_children': 'true',
+                                     'delete_all_versions': 'true'}),
                          "application/json")
 
         found = False
@@ -676,8 +676,7 @@ class ContentStoreTest(ModuleStoreTestCase):
 
         self.assertEqual(resp.status_code, 200)
         data = parse_json(resp)
-        self.assertRegexpMatches(data['id'],
-            '^i4x:\/\/MITx\/999\/chapter\/([0-9]|[a-f]){32}$')
+        self.assertRegexpMatches(data['id'], '^i4x:\/\/MITx\/999\/chapter\/([0-9]|[a-f]){32}$')
 
     def test_capa_module(self):
         """Test that a problem treats markdown specially."""

cms/djangoapps/contentstore/tests/test_course_settings.py

@@ -8,8 +8,7 @@ from django.core.urlresolvers import reverse
 from django.utils.timezone import UTC
 
 from xmodule.modulestore import Location
-from models.settings.course_details import (CourseDetails,
-                                                    CourseSettingsEncoder)
+from models.settings.course_details import (CourseDetails, CourseSettingsEncoder)
 from models.settings.course_grading import CourseGradingModel
 from contentstore.utils import get_modulestore
 
@@ -21,6 +20,7 @@ from xmodule.modulestore.xml_importer import import_from_xml
 from xmodule.modulestore.django import modulestore
 from xmodule.fields import Date
 
+
 class CourseTestCase(ModuleStoreTestCase):
     def setUp(self):
         """
@@ -249,6 +249,7 @@ class CourseGradingTest(CourseTestCase):
         altered_grader = CourseGradingModel.update_grader_from_json(test_grader.course_location, test_grader.graders[1])
         self.assertDictEqual(test_grader.graders[1], altered_grader, "drop_count[1] + 2")
 
+
 class CourseMetadataEditingTest(CourseTestCase):
     def setUp(self):
         CourseTestCase.setUp(self)
@@ -256,7 +257,6 @@ class CourseMetadataEditingTest(CourseTestCase):
         import_from_xml(modulestore(), 'common/test/data/', ['full'])
         self.fullcourse_location = Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])
 
-
     def test_fetch_initial_fields(self):
         test_model = CourseMetadata.fetch(self.course_location)
         self.assertIn('display_name', test_model, 'Missing editable metadata field')
@@ -272,17 +272,17 @@ class CourseMetadataEditingTest(CourseTestCase):
 
     def test_update_from_json(self):
         test_model = CourseMetadata.update_from_json(self.course_location,
-            { "advertised_start" : "start A",
-              "testcenter_info" : { "c" : "test" },
-              "days_early_for_beta" : 2})
+                                                     {"advertised_start": "start A",
+                                                      "testcenter_info": {"c": "test"},
+                                                      "days_early_for_beta": 2})
         self.update_check(test_model)
         # try fresh fetch to ensure persistence
         test_model = CourseMetadata.fetch(self.course_location)
         self.update_check(test_model)
         # now change some of the existing metadata
         test_model = CourseMetadata.update_from_json(self.course_location,
-            { "advertised_start" : "start B",
-              "display_name" : "jolly roger"})
+                                                     {"advertised_start": "start B",
+                                                      "display_name": "jolly roger"})
         self.assertIn('display_name', test_model, 'Missing editable metadata field')
         self.assertEqual(test_model['display_name'], 'jolly roger', "not expected value")
         self.assertIn('advertised_start', test_model, 'Missing revised advertised_start metadata field')
@@ -294,13 +294,12 @@ class CourseMetadataEditingTest(CourseTestCase):
         self.assertIn('advertised_start', test_model, 'Missing new advertised_start metadata field')
         self.assertEqual(test_model['advertised_start'], 'start A', "advertised_start not expected value")
         self.assertIn('testcenter_info', test_model, 'Missing testcenter_info metadata field')
-        self.assertDictEqual(test_model['testcenter_info'], { "c" : "test" }, "testcenter_info not expected value")
+        self.assertDictEqual(test_model['testcenter_info'], {"c": "test"}, "testcenter_info not expected value")
         self.assertIn('days_early_for_beta', test_model, 'Missing days_early_for_beta metadata field')
         self.assertEqual(test_model['days_early_for_beta'], 2, "days_early_for_beta not expected value")
 
-
     def test_delete_key(self):
-        test_model = CourseMetadata.delete_key(self.fullcourse_location, { 'deleteKeys' : ['doesnt_exist', 'showanswer', 'xqa_key']})
+        test_model = CourseMetadata.delete_key(self.fullcourse_location, {'deleteKeys': ['doesnt_exist', 'showanswer', 'xqa_key']})
         # ensure no harm
         self.assertNotIn('graceperiod', test_model, 'blacklisted field leaked in')
         self.assertIn('display_name', test_model, 'full missing editable metadata field')

cms/djangoapps/contentstore/tests/test_utils.py

@@ -70,3 +70,4 @@ class UrlReverseTestCase(ModuleStoreTestCase):
             'https://edge.edx.org/courses/edX/edX101/How_to_Create_an_edX_Course/about',
             utils.get_url_reverse('https://edge.edx.org/courses/edX/edX101/How_to_Create_an_edX_Course/about', course)
         )
+        
\ No newline at end of file

cms/djangoapps/contentstore/tests/tests.py

-import json
-import shutil
 from django.test.client import Client
-from django.conf import settings
 from django.core.urlresolvers import reverse
-from path import path
-import json
-from fs.osfs import OSFS
-import copy
-
-from contentstore.utils import get_modulestore
-
-from xmodule.modulestore import Location
-from xmodule.modulestore.store_utilities import clone_course
-from xmodule.modulestore.store_utilities import delete_course
-from xmodule.modulestore.django import modulestore, _MODULESTORES
-from xmodule.contentstore.django import contentstore
-from xmodule.templates import update_templates
-from xmodule.modulestore.xml_exporter import export_to_xml
-from xmodule.modulestore.xml_importer import import_from_xml
-
-from xmodule.capa_module import CapaDescriptor
-from xmodule.course_module import CourseDescriptor
-from xmodule.seq_module import SequenceDescriptor
-
-from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
+
 from .utils import ModuleStoreTestCase, parse_json, user, registration
 
 
@@ -84,6 +61,7 @@ class ContentStoreTestCase(ModuleStoreTestCase):
         # Now make sure that the user is now actually activated
         self.assertTrue(user(email).is_active)
 
+
 class AuthTestCase(ContentStoreTestCase):
     """Check that various permissions-related things work"""
 

cms/djangoapps/contentstore/views.py

@@ -839,6 +839,7 @@ def upload_asset(request, org, course, coursename):
     response['asset_url'] = StaticContent.get_url_path_from_location(content.location)
     return response
 
+
 @login_required
 @ensure_csrf_cookie
 def manage_users(request, location):