Takes care of Victor's comments on the PR.

Note, the students_lc list is used in _do_enroll_students when overload
= True so it needs to be left in.

lms/djangoapps/instructor/tests/test_enrollment.py

@@ -1,6 +1,5 @@
 '''
 Unit tests for enrollment methods in views.py
-
 '''
 
 from django.test.utils import override_settings
@@ -11,6 +10,7 @@ from courseware.tests.tests import LoginEnrollmentTestCase, TEST_DATA_XML_MODULE
 from xmodule.modulestore.django import modulestore
 import xmodule.modulestore.django
 from student.models import CourseEnrollment, CourseEnrollmentAllowed
+from instructor.views import get_and_clean_student_list
 
 
 @override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
@@ -68,7 +68,7 @@ class TestInstructorEnrollsStudent(LoginEnrollmentTestCase):
 
          #Check the page output
         self.assertContains(response, '<td>student1@test.com</td>')
-        self.assertContains(response, '<td>student1@test.com</td>')
+        self.assertContains(response, '<td>student2@test.com</td>')
         self.assertContains(response, '<td>un-enrolled</td>')
 
         #Check the enrollment table
@@ -166,3 +166,12 @@ 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))
+
+    def test_get_and_clean_student_list(self):
+        '''
+        Clean user input test
+        '''
+        
+        string = "abc@test.com, def@test.com ghi@test.com \n \n jkl@test.com      "
+        cleaned_string, cleaned_string_lc = get_and_clean_student_list(string)
+        self.assertEqual(cleaned_string, ['abc@test.com', 'def@test.com', 'ghi@test.com', 'jkl@test.com'])

lms/djangoapps/instructor/views.py

@@ -542,7 +542,7 @@ 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) is not False
+        auto_enroll = bool(request.POST.get('auto_enroll'))
         ret = _do_enroll_students(course, course_id, students, auto_enroll=auto_enroll)
         datatable = ret['datatable']
 
@@ -1000,11 +1000,12 @@ def _do_enroll_students(course, course_id, students, overload=False, auto_enroll
             if cea:
                 cea[0].auto_enroll = auto_enroll
                 cea[0].save()
-                status[student] = 'user does not exist, enrollment already allowed, pending with auto enrollment ' + ("off", "on")[auto_enroll]
+                status[student] = 'user does not exist, enrollment already allowed, pending with auto enrollment ' \
+                    + ('on' if auto_enroll else 'off')
                 continue
             cea = CourseEnrollmentAllowed(email=student, course_id=course_id, auto_enroll=auto_enroll)
             cea.save()
-            status[student] = 'user does not exist, enrollment allowed, pending with auto enrollment ' + ("off", "on")[auto_enroll]
+            status[student] = 'user does not exist, enrollment allowed, pending with auto enrollment ' + ('on' if auto_enroll else 'off')
             continue
 
         if CourseEnrollment.objects.filter(user=user, course_id=course_id):
@@ -1018,7 +1019,7 @@ def _do_enroll_students(course, course_id, students, overload=False, auto_enroll
             status[student] = 'rejected'
 
     datatable = {'header': ['StudentEmail', 'action']}
-    datatable['data'] = [[x, status[x]] for x in status]
+    datatable['data'] = [[x, status[x]] for x in sorted(status)]
     datatable['title'] = 'Enrollment of students'
 
     def sf(stat):
@@ -1050,7 +1051,7 @@ def _do_unenroll_students(course_id, students):
 
         try:
             user = User.objects.get(email=student)
-        except User.DoesNotExist:    
+        except User.DoesNotExist:
             continue
 
         nce = CourseEnrollment.objects.filter(user=user, course_id=course_id)
@@ -1064,21 +1065,29 @@ def _do_unenroll_students(course_id, students):
                     status[student] = "Error!  Failed to un-enroll"
 
     datatable = {'header': ['StudentEmail', 'action']}
-    datatable['data'] = [[x, status[x]] for x in status]
+    datatable['data'] = [[x, status[x]] for x in sorted(status)]
     datatable['title'] = 'Un-enrollment of students'
 
     data = dict(datatable=datatable)
     return data
 
+
 def get_and_clean_student_list(students):
-    
+    """
+    Separate out individual student email from the comma, or space separated string.
+
+    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
+    """
+
     students = split_by_comma_and_whitespace(students)
     students = [str(s.strip()) for s in students]
+    students = [s for s in students if s != '']
     students_lc = [x.lower() for x in students]
 
-    if '' in students:
-        students.remove('')
-
     return students, students_lc
 
 #-----------------------------------------------------------------------------