From 096f7dcd2380f7a0b9ed1b3ea0a776d70eac95ad Mon Sep 17 00:00:00 2001 From: Miles Steele Date: Wed, 3 Jul 2013 13:43:36 -0400 Subject: [PATCH] add docstrings, cleanup (minor) --- lms/djangoapps/analytics/basic.py | 4 +- lms/djangoapps/instructor/access.py | 7 ++-- lms/djangoapps/instructor/enrollment.py | 27 ++++++++------ .../instructor/tests/test_access.py | 37 +------------------ .../instructor/tests/test_enrollment.py | 1 - lms/djangoapps/instructor/views/api.py | 6 +-- 6 files changed, 24 insertions(+), 58 deletions(-) diff --git a/lms/djangoapps/analytics/basic.py b/lms/djangoapps/analytics/basic.py index 8d5bff2c16..c51b96bc2d 100644 --- a/lms/djangoapps/analytics/basic.py +++ b/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): diff --git a/lms/djangoapps/instructor/access.py b/lms/djangoapps/instructor/access.py index d2e2952c02..c8d9abbb3d 100644 --- a/lms/djangoapps/instructor/access.py +++ b/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) diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index fc4a3fec2c..ac66bead4c 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/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"] diff --git a/lms/djangoapps/instructor/tests/test_access.py b/lms/djangoapps/instructor/tests/test_access.py index 98cc8cb159..5895504ee1 100644 --- a/lms/djangoapps/instructor/tests/test_access.py +++ b/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): diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index 857098952f..59858dabaa 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/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 diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 55e0809c38..1e3835a3a4 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/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