From 6a1a907339ece6231dd681f4f96fac144eef098a Mon Sep 17 00:00:00 2001 From: dcadams Date: Tue, 30 Apr 2013 11:48:23 -0700 Subject: [PATCH] Fix pep8 violations. --- .../instructor/tests/test_enrollment.py | 69 +++++++++++-------- lms/djangoapps/instructor/views.py | 26 +++---- 2 files changed, 52 insertions(+), 43 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index a33e224e5e..d2e31d3eb6 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -1,4 +1,6 @@ -""" Unit tests for enrollment methods in views.py """ +''' +Unit tests for enrollment methods in views.py +''' from django.test.utils import override_settings from django.contrib.auth.models import Group, User @@ -12,9 +14,11 @@ from student.models import CourseEnrollment, CourseEnrollmentAllowed @override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE) class TestInstructorEnrollsStudent(LoginEnrollmentTestCase): + ''' + Check Enrollment/Unenrollment with/without auto-enrollment on activation + ''' def setUp(self): - xmodule.modulestore.django._MODULESTORES = {} self.full = modulestore().get_course("edX/full/6.002_Spring_2012") self.toy = modulestore().get_course("edX/toy/2012_Fall") @@ -42,112 +46,118 @@ class TestInstructorEnrollsStudent(LoginEnrollmentTestCase): self.logout() self.login(self.student1, self.password) self.enroll(self.toy) - + self.logout() self.login(self.student2, self.password) self.enroll(self.toy) - + #Enroll Instructor self.logout() self.login(self.instructor, self.password) self.enroll(self.toy) - def test_unenrollment(self): - #Do un-enrollment + ''' + Do un-enrollment test + ''' + course = self.toy url = reverse('instructor_dashboard', kwargs={'course_id': course.id}) response = self.client.post(url, {'action': 'Unenroll multiple students', 'multiple_students': 'student1@test.com, student2@test.com'}) - + #Check the page output self.assertContains(response, 'student1@test.com') self.assertContains(response, 'student1@test.com') self.assertContains(response, 'un-enrolled') - + #Check the enrollment table user = User.objects.get(email='student1@test.com') ce = CourseEnrollment.objects.filter(course_id=course.id, user=user) self.assertEqual(0, len(ce)) - + user = User.objects.get(email='student2@test.com') ce = CourseEnrollment.objects.filter(course_id=course.id, user=user) - self.assertEqual(0, len(ce)) - + self.assertEqual(0, len(ce)) def test_enrollmemt_new_student_autoenroll_on(self): - + ''' + Do auto-enroll on test + ''' + #Run the Enroll students command course = self.toy url = reverse('instructor_dashboard', kwargs={'course_id': course.id}) response = self.client.post(url, {'action': 'Enroll multiple students', 'multiple_students': 'test1_1@student.com, test1_2@student.com', 'auto_enroll': 'on'}) - + #Check the page output self.assertContains(response, 'test1_1@student.com') self.assertContains(response, 'test1_2@student.com') self.assertContains(response, 'user does not exist, enrollment allowed, pending with auto enrollment on') - + #Check the enrollmentallowed db entries cea = CourseEnrollmentAllowed.objects.filter(email='test1_1@student.com', course_id=course.id) self.assertEqual(1, cea[0].auto_enroll) cea = CourseEnrollmentAllowed.objects.filter(email='test1_2@student.com', course_id=course.id) self.assertEqual(1, cea[0].auto_enroll) - + #Check there is no enrollment db entry other than for the setup instructor and students ce = CourseEnrollment.objects.filter(course_id=course.id) self.assertEqual(3, len(ce)) - + #Create and activate student accounts with same email self.student1 = 'test1_1@student.com' self.password = 'bar' self.create_account('s1_1', self.student1, self.password) self.activate_user(self.student1) - + self.student2 = 'test1_2@student.com' self.create_account('s1_2', self.student2, self.password) self.activate_user(self.student2) - + #Check students are enrolled user = User.objects.get(email='test1_1@student.com') ce = CourseEnrollment.objects.filter(course_id=course.id, user=user) self.assertEqual(1, len(ce)) - + user = User.objects.get(email='test1_2@student.com') ce = CourseEnrollment.objects.filter(course_id=course.id, user=user) - self.assertEqual(1, len(ce)) - + self.assertEqual(1, len(ce)) def test_enrollmemt_new_student_autoenroll_off(self): - + ''' + Do auto-enroll off test + ''' + #Run the Enroll students command course = self.toy url = reverse('instructor_dashboard', kwargs={'course_id': course.id}) response = self.client.post(url, {'action': 'Enroll multiple students', 'multiple_students': 'test2_1@student.com, test2_2@student.com'}) - + #Check the page output self.assertContains(response, 'test2_1@student.com') self.assertContains(response, 'test2_2@student.com') self.assertContains(response, 'user does not exist, enrollment allowed, pending with auto enrollment off') - + #Check the enrollmentallowed db entries cea = CourseEnrollmentAllowed.objects.filter(email='test2_1@student.com', course_id=course.id) self.assertEqual(0, cea[0].auto_enroll) cea = CourseEnrollmentAllowed.objects.filter(email='test2_2@student.com', course_id=course.id) self.assertEqual(0, cea[0].auto_enroll) - + #Check there is no enrollment db entry other than for the setup instructor and students ce = CourseEnrollment.objects.filter(course_id=course.id) self.assertEqual(3, len(ce)) - + #Create and activate student accounts with same email self.student = 'test2_1@student.com' self.password = 'bar' self.create_account('s2_1', self.student, self.password) self.activate_user(self.student) - + self.student = 'test2_2@student.com' self.create_account('s2_2', self.student, self.password) - self.activate_user(self.student) - + self.activate_user(self.student) + #Check students are not enrolled user = User.objects.get(email='test2_1@student.com') ce = CourseEnrollment.objects.filter(course_id=course.id, user=user) @@ -155,4 +165,3 @@ class TestInstructorEnrollsStudent(LoginEnrollmentTestCase): user = User.objects.get(email='test2_2@student.com') ce = CourseEnrollment.objects.filter(course_id=course.id, user=user) self.assertEqual(0, len(ce)) - \ No newline at end of file diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index 606f875eb8..756d1a6204 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -542,12 +542,12 @@ def instructor_dashboard(request, course_id): elif action == 'Enroll multiple students': students = request.POST.get('multiple_students', '') - auto_enroll = request.POST.get('auto_enroll', False) != False - ret = _do_enroll_students(course_id, students, auto_enroll=auto_enroll) + auto_enroll = request.POST.get('auto_enroll', False) is not False + ret = _do_enroll_students(course, course_id, students, auto_enroll=auto_enroll) datatable = ret['datatable'] elif action == 'Unenroll multiple students': - + students = request.POST.get('multiple_students', '') ret = _do_unenroll_students(course_id, students) datatable = ret['datatable'] @@ -967,11 +967,11 @@ def grade_summary(request, course_id): #----------------------------------------------------------------------------- # enrollment -def _do_enroll_students(course_id, students, overload=False, auto_enroll=False): +def _do_enroll_students(course, course_id, students, overload=False, auto_enroll=False): """Do the actual work of enrolling multiple students, presented as a string of emails separated by commas or returns""" - new_students = get_and_clean_student_list(students) + new_students, new_students_lc = get_and_clean_student_list(students) status = dict([x, 'unprocessed'] for x in new_students) if overload: # delete all but staff @@ -991,10 +991,10 @@ def _do_enroll_students(course_id, students, overload=False, auto_enroll=False): try: user = User.objects.get(email=student) except User.DoesNotExist: - + #User not signed up yet, put in pending enrollment allowed table cea = CourseEnrollmentAllowed.objects.filter(email=student, course_id=course_id) - + #If enrollmentallowed already exists, update auto_enroll flag to however it was set in UI #Will be 0 or 1 records as there is a unique key on email + course_id if cea: @@ -1035,11 +1035,11 @@ def _do_unenroll_students(course_id, students): """Do the actual work of un-enrolling multiple students, presented as a string of emails separated by commas or returns""" - old_students = get_and_clean_student_list(students) + old_students, old_students_lc = get_and_clean_student_list(students) status = dict([x, 'unprocessed'] for x in old_students) - + for student in old_students: - + isok = False cea = CourseEnrollmentAllowed.objects.filter(course_id=course_id, email=student) #Will be 0 or 1 records as there is a unique key on email + course_id @@ -1047,7 +1047,7 @@ def _do_unenroll_students(course_id, students): cea[0].delete() status[student] = "un-enrolled" isok = True - + try: user = User.objects.get(email=student) except User.DoesNotExist: @@ -1066,7 +1066,7 @@ def _do_unenroll_students(course_id, students): datatable = {'header': ['StudentEmail', 'action']} datatable['data'] = [[x, status[x]] for x in status] datatable['title'] = 'Un-enrollment of students' - + data = dict(datatable=datatable) return data @@ -1079,7 +1079,7 @@ def get_and_clean_student_list(students): if '' in students: students.remove('') - return students + return students, students_lc #----------------------------------------------------------------------------- # answer distribution