Merge pull request #685 from edx/jkarni/fix/cms_pylint

Contentstore views pylint fixes

cms/djangoapps/contentstore/views/access.py

@@ -26,12 +26,16 @@ def has_access(user, location, role=STAFF_ROLE_NAME):
     There is a super-admin permissions if user.is_staff is set
     Also, since we're unifying the user database between LMS and CAS,
     I'm presuming that the course instructor (formally known as admin)
-    will not be in both INSTRUCTOR and STAFF groups, so we have to cascade our queries here as INSTRUCTOR
-    has all the rights that STAFF do
+    will not be in both INSTRUCTOR and STAFF groups, so we have to cascade our
+    queries here as INSTRUCTOR has all the rights that STAFF do
     '''
     course_location = get_course_location_for_item(location)
     _has_access = is_user_in_course_group_role(user, course_location, role)
     # if we're not in STAFF, perhaps we're in INSTRUCTOR groups
     if not _has_access and role == STAFF_ROLE_NAME:
-        _has_access = is_user_in_course_group_role(user, course_location, INSTRUCTOR_ROLE_NAME)
+        _has_access = is_user_in_course_group_role(
+                user,
+                course_location,
+                INSTRUCTOR_ROLE_NAME
+        )
     return _has_access

cms/djangoapps/contentstore/views/assets.py

@@ -4,6 +4,7 @@ import os
 import tarfile
 import shutil
 import cgi
+from functools import partial
 from tempfile import mkdtemp
 from path import path
 
@@ -34,7 +35,8 @@ from .access import get_location_and_verify_access
 from util.json_request import JsonResponse
 
 
-__all__ = ['asset_index', 'upload_asset', 'import_course', 'generate_export_course', 'export_course']
+__all__ = ['asset_index', 'upload_asset', 'import_course',
+        'generate_export_course', 'export_course']
 
 
 def assets_to_json_dict(assets):
@@ -58,13 +60,14 @@ def assets_to_json_dict(assets):
             obj["thumbnail"] = thumbnail
         id_info = asset.get("_id")
         if id_info:
-            obj["id"] = "/{tag}/{org}/{course}/{revision}/{category}/{name}".format(
-                org=id_info.get("org", ""),
-                course=id_info.get("course", ""),
-                revision=id_info.get("revision", ""),
-                tag=id_info.get("tag", ""),
-                category=id_info.get("category", ""),
-                name=id_info.get("name", ""),
+            obj["id"] = "/{tag}/{org}/{course}/{revision}/{category}/{name}" \
+                .format(
+                    org=id_info.get("org", ""),
+                    course=id_info.get("course", ""),
+                    revision=id_info.get("revision", ""),
+                    tag=id_info.get("tag", ""),
+                    category=id_info.get("category", ""),
+                    name=id_info.get("name", ""),
             )
         ret.append(obj)
     return ret
@@ -132,14 +135,14 @@ def asset_index(request, org, course, name):
 @login_required
 def upload_asset(request, org, course, coursename):
     '''
-    This method allows for POST uploading of files into the course asset library, which will
-    be supported by GridFS in MongoDB.
+    This method allows for POST uploading of files into the course asset
+    library, which will be supported by GridFS in MongoDB.
     '''
     # construct a location from the passed in path
     location = get_location_and_verify_access(request, org, course, coursename)
 
-    # Does the course actually exist?!? Get anything from it to prove its existance
-
+    # Does the course actually exist?!? Get anything from it to prove its
+    # existence
     try:
         modulestore().get_item(location)
     except:
@@ -150,9 +153,10 @@ def upload_asset(request, org, course, coursename):
     if 'file' not in request.FILES:
         return HttpResponseBadRequest()
 
-    # compute a 'filename' which is similar to the location formatting, we're using the 'filename'
-    # nomenclature since we're using a FileSystem paradigm here. We're just imposing
-    # the Location string formatting expectations to keep things a bit more consistent
+    # compute a 'filename' which is similar to the location formatting, we're
+    # using the 'filename' nomenclature since we're using a FileSystem paradigm
+    # here. We're just imposing the Location string formatting expectations to
+    # keep things a bit more consistent
     upload_file = request.FILES['file']
     filename = upload_file.name
     mime_type = upload_file.content_type
@@ -160,20 +164,25 @@ def upload_asset(request, org, course, coursename):
     content_loc = StaticContent.compute_location(org, course, filename)
 
     chunked = upload_file.multiple_chunks()
+    sc_partial = partial(StaticContent, content_loc, filename, mime_type)
     if chunked:
-        content = StaticContent(content_loc, filename, mime_type, upload_file.chunks())
+        content = sc_partial(upload_file.chunks())
+        temp_filepath = upload_file.temporary_file_path()
     else:
-        content = StaticContent(content_loc, filename, mime_type, upload_file.read())
+        content = sc_partial(upload_file.read())
+        tempfile_path = None
 
     thumbnail_content = None
     thumbnail_location = None
 
     # first let's see if a thumbnail can be created
-    (thumbnail_content, thumbnail_location) = contentstore().generate_thumbnail(content,
-                                                                                tempfile_path=None if not chunked else
-                                                                                upload_file.temporary_file_path())
+    (thumbnail_content, thumbnail_location) = contentstore().generate_thumbnail(
+            content,
+            tempfile_path=tempfile_path
+    )
 
-    # delete cached thumbnail even if one couldn't be created this time (else the old thumbnail will continue to show)
+    # delete cached thumbnail even if one couldn't be created this time (else
+    # the old thumbnail will continue to show)
     del_cached_content(thumbnail_location)
     # now store thumbnail location only if we could create it
     if thumbnail_content is not None:
@@ -186,13 +195,15 @@ def upload_asset(request, org, course, coursename):
     # readback the saved content - we need the database timestamp
     readback = contentstore().find(content.location)
 
-    response_payload = {'displayname': content.name,
-                        'uploadDate': get_default_time_display(readback.last_modified_at),
-                        'url': StaticContent.get_url_path_from_location(content.location),
-                        'portable_url': StaticContent.get_static_path_from_location(content.location),
-                        'thumb_url': StaticContent.get_url_path_from_location(thumbnail_location) if thumbnail_content is not None else None,
-                        'msg': 'Upload completed'
-                        }
+    response_payload = {
+            'displayname': content.name,
+            'uploadDate': get_default_time_display(readback.last_modified_at),
+            'url': StaticContent.get_url_path_from_location(content.location),
+            'portable_url': StaticContent.get_static_path_from_location(content.location),
+            'thumb_url': StaticContent.get_url_path_from_location(thumbnail_location)
+                if thumbnail_content is not None else None,
+            'msg': 'Upload completed'
+    }
 
     response = JsonResponse(response_payload)
     return response
@@ -202,8 +213,8 @@ def upload_asset(request, org, course, coursename):
 @login_required
 def remove_asset(request, org, course, name):
     '''
-    This method will perform a 'soft-delete' of an asset, which is basically to copy the asset from
-    the main GridFS collection and into a Trashcan
+    This method will perform a 'soft-delete' of an asset, which is basically to
+    copy the asset from the main GridFS collection and into a Trashcan
     '''
     get_location_and_verify_access(request, org, course, name)
 

cms/djangoapps/contentstore/views/checklist.py

@@ -30,7 +30,8 @@ def get_checklists(request, org, course, name):
     modulestore = get_modulestore(location)
     course_module = modulestore.get_item(location)
 
-    # If course was created before checklists were introduced, copy them over from the template.
+    # If course was created before checklists were introduced, copy them over
+    # from the template.
     copied = False
     if not course_module.checklists:
         course_module.checklists = CourseDescriptor.checklists.default
@@ -68,7 +69,8 @@ def update_checklist(request, org, course, name, checklist_index=None):
         if checklist_index is not None and 0 <= int(checklist_index) < len(course_module.checklists):
             index = int(checklist_index)
             course_module.checklists[index] = json.loads(request.body)
-            # seeming noop which triggers kvs to record that the metadata is not default
+            # seeming noop which triggers kvs to record that the metadata is
+            # not default
             course_module.checklists = course_module.checklists
             checklists, _ = expand_checklist_action_urls(course_module)
             course_module.save()
@@ -76,10 +78,13 @@ def update_checklist(request, org, course, name, checklist_index=None):
             return JsonResponse(checklists[index])
         else:
             return HttpResponseBadRequest(
-                "Could not save checklist state because the checklist index was out of range or unspecified.",
-                content_type="text/plain")
+                ( "Could not save checklist state because the checklist index "
+                "was out of range or unspecified."),
+                content_type="text/plain"
+            )
     elif request.method == 'GET':
-        # In the JavaScript view initialize method, we do a fetch to get all the checklists.
+        # In the JavaScript view initialize method, we do a fetch to get all
+        # the checklists.
         checklists, modified = expand_checklist_action_urls(course_module)
         if modified:
             course_module.save()