add docstrings, cleanup (minor)

lms/djangoapps/analytics/basic.py

@@ -28,7 +28,7 @@ def enrolled_students_profiles(course_id, features):
     students = User.objects.filter(courseenrollment__course_id=course_id)\
         .order_by('username').select_related('profile')
 
-    def extract_student(student):
+    def extract_student(student, features):
         """ convert student to dictionary """
         student_features = [x for x in STUDENT_FEATURES if x in features]
         profile_features = [x for x in PROFILE_FEATURES if x in features]
@@ -41,7 +41,7 @@ def enrolled_students_profiles(course_id, features):
         student_dict.update(profile_dict)
         return student_dict
 
-    return [extract_student(student) for student in students]
+    return [extract_student(student, features) for student in students]
 
 
 def dump_grading_context(course):

lms/djangoapps/instructor/access.py

@@ -13,16 +13,15 @@ from django.contrib.auth.models import Group
 from courseware.access import (get_access_group_name,
                                course_beta_test_group_name)
 from django_comment_common.models import Role
-                                          # FORUM_ROLE_ADMINISTRATOR,
-                                          # FORUM_ROLE_MODERATOR,
-                                          # FORUM_ROLE_COMMUNITY_TA)
 
 
 def list_with_level(course, level):
     """
     List users who have 'level' access.
 
-    level is in ['instructor', 'staff', 'beta']
+    level is in ['instructor', 'staff', 'beta'] for standard courses.
+    There could be other levels specific to the course.
+    If there is no Group for that course-level, returns an empty list
     """
     if level in ['beta']:
         grpname = course_beta_test_group_name(course.location)

lms/djangoapps/instructor/enrollment.py

@@ -25,7 +25,7 @@ def enroll_emails(course_id, student_emails, auto_enroll=False):
     return a mapping from status to emails.
     """
 
-    auto_string = {False: 'allowed', True: 'willautoenroll'}[auto_enroll]
+    auto_string = 'willautoenroll' if auto_enroll else 'allowed'
 
     status_map = {
         'user/ce/alreadyenrolled': [],
@@ -75,7 +75,7 @@ def unenroll_emails(course_id, student_emails):
     """
     Unenroll multiple students by email.
 
-    students is a list of student emails e.g. ["foo@bar.com", "bar@foo.com]
+    `students` is a list of student emails e.g. ["foo@bar.com", "bar@foo.com]
     each of whom possibly does not exist in db.
 
     Fail quietly on student emails that do not match any users or allowed enrollments.
@@ -126,15 +126,12 @@ def split_input_list(str_list):
     in: "Lorem@ipsum.dolor, sit@amet.consectetur\nadipiscing@elit.Aenean\r convallis@at.lacus\r, ut@lacinia.Sed"
     out: ['Lorem@ipsum.dolor', 'sit@amet.consectetur', 'adipiscing@elit.Aenean', 'convallis@at.lacus', 'ut@lacinia.Sed']
 
-    In:
-    students: string coming from the input text area
-    Return:
-    students: list of cleaned student emails
-    students_lc: list of lower case cleaned student emails
+    `str_list` is a string coming from an input text area
+    returns a list of separated values
     """
 
     new_list = re.split(r'[\n\r\s,]', str_list)
-    new_list = [str(s.strip()) for s in new_list]
+    new_list = [s.strip() for s in new_list]
     new_list = [s for s in new_list if s != '']
 
     return new_list
@@ -147,9 +144,11 @@ def reset_student_attempts(course_id, student, module_state_key, delete_module=F
     In the previous instructor dashboard it was possible to modify/delete
     modules that were not problems. That has been disabled for safety.
 
-    student is a User
-    problem_to_reset is the name of a problem e.g. 'L2Node1'.
-    To build the module_state_key 'problem/' and course information will be appended to problem_to_reset.
+    `student` is a User
+    `problem_to_reset` is the name of a problem e.g. 'L2Node1'.
+    To build the module_state_key 'problem/' and course information will be appended to `problem_to_reset`.
+
+    Throws ValueError if `problem_state` is invalid JSON.
     """
     module_to_reset = StudentModule.objects.get(student_id=student.id,
                                                 course_id=course_id,
@@ -162,7 +161,11 @@ def reset_student_attempts(course_id, student, module_state_key, delete_module=F
 
 
 def _reset_module_attempts(studentmodule):
-    """ Reset the number of attempts on a studentmodule. """
+    """
+    Reset the number of attempts on a studentmodule.
+
+    Throws ValueError if `problem_state` is invalid JSON.
+    """
     # load the state json
     problem_state = json.loads(studentmodule.state)
     # old_number_of_attempts = problem_state["attempts"]

lms/djangoapps/instructor/tests/test_access.py

@@ -9,8 +9,7 @@ from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 
 from django.test.utils import override_settings
-from django.conf import settings
-from uuid import uuid4
+from courseware.tests.modulestore_config import TEST_DATA_MONGO_MODULESTORE
 
 from courseware.access import get_access_group_name
 from django_comment_common.models import (Role,
@@ -19,46 +18,12 @@ from django_comment_common.models import (Role,
                                           FORUM_ROLE_COMMUNITY_TA)
 from instructor.access import allow_access, revoke_access, list_with_level, update_forum_role_membership
 
-# mock dependency
-# get_access_group_name = lambda course, role: '{0}_{1}'.format(course.course_id, role)
-
-
-# moved here from old courseware/tests/tests.py
-# when it disappeared this test broke.
-def mongo_store_config(data_dir):
-    """
-    Defines default module store using MongoModuleStore
-
-    Use of this config requires mongo to be running
-    """
-    store = {
-        'default': {
-            'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore',
-            'OPTIONS': {
-                'default_class': 'xmodule.raw_module.RawDescriptor',
-                'host': 'localhost',
-                'db': 'test_xmodule',
-                'collection': 'modulestore_%s' % uuid4().hex,
-                'fs_root': data_dir,
-                'render_template': 'mitxmako.shortcuts.render_to_string',
-            }
-        }
-    }
-    store['direct'] = store['default']
-    return store
-
-TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT
-TEST_DATA_MONGO_MODULESTORE = mongo_store_config(TEST_DATA_DIR)
-# TEST_DATA_XML_MODULESTORE = xml_store_config(TEST_DATA_DIR)
-
 
 @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
 class TestInstructorAccessControlDB(ModuleStoreTestCase):
     """ Test instructor access administration against database effects """
 
     def setUp(self):
-        # self.course_id = 'jus:/a/fake/c::rse/id'
-        # self.course = MockCourse('jus:/a/fake/c::rse/id')
         self.course = CourseFactory.create()
 
     def test_allow(self):

lms/djangoapps/instructor/tests/test_enrollment.py

@@ -4,7 +4,6 @@ Unit tests for instructor.enrollment methods.
 
 import json
 from django.contrib.auth.models import User
-# from courseware.access import _course_staff_group_name
 from courseware.models import StudentModule
 from django.test import TestCase
 from student.tests.factories import UserFactory

lms/djangoapps/instructor/views/api.py

@@ -14,7 +14,7 @@ from django.core.urlresolvers import reverse
 from django.http import HttpResponse, HttpResponseBadRequest
 
 from courseware.courses import get_course_with_access
-from django.contrib.auth.models import User, Group
+from django.contrib.auth.models import User
 from django_comment_common.models import (Role,
                                           FORUM_ROLE_ADMINISTRATOR,
                                           FORUM_ROLE_MODERATOR,
@@ -30,11 +30,11 @@ import analytics.distributions
 import analytics.csvs
 
 
-def common_exceptions_400(fn):
+def common_exceptions_400(func):
     """ Catches common exceptions and renders matching 400 errors. (decorator) """
     def wrapped(*args, **kwargs):
         try:
-            return fn(*args, **kwargs)
+            return func(*args, **kwargs)
         except User.DoesNotExist:
             return HttpResponseBadRequest("User does not exist.")
     return wrapped