Modifications to instructor dashboard - enrollment tab:

- Remove single enrollment/unenrollment
- Make unenrollment work with multi-email listbox.
- Provide checkbox for auto-enroll on activate if user not yet signed
up.
- Enroll student when activating if auto-enroll flag set.

common/djangoapps/student/models.py

@@ -662,6 +662,7 @@ class CourseEnrollmentAllowed(models.Model):
     """
     email = models.CharField(max_length=255, db_index=True)
     course_id = models.CharField(max_length=255, db_index=True)
+    auto_enroll = models.BooleanField(default=0)
 
     created = models.DateTimeField(auto_now_add=True, null=True, db_index=True)
 

common/djangoapps/student/views.py

@@ -31,7 +31,7 @@ from student.models import (Registration, UserProfile, TestCenterUser, TestCente
                             TestCenterRegistration, TestCenterRegistrationForm,
                             PendingNameChange, PendingEmailChange,
                             CourseEnrollment, unique_id_for_user,
-                            get_testcenter_registration)
+                            get_testcenter_registration, CourseEnrollmentAllowed)
 
 from certificates.models import CertificateStatuses, certificate_status_for_student
 
@@ -825,6 +825,15 @@ def activate_account(request, key):
         if not r[0].user.is_active:
             r[0].activate()
             already_active = False
+        
+        #Enroll student in any pending courses he/she may have if auto_enroll flag is set
+        student = request.user
+        ceas = CourseEnrollmentAllowed.objects.filter(email=student.email)
+        for cea in ceas:
+            if cea.auto_enroll:
+                course_id = cea.course_id
+                enrollment, created = CourseEnrollment.objects.get_or_create(user_id=student.id, course_id=course_id)
+        
         resp = render_to_response("registration/activation_complete.html", {'user_logged_in': user_logged_in, 'already_active': already_active})
         return resp
     if len(r) == 0:

lms/djangoapps/instructor/tests/test_enrollment.py

+""" Unit tests for enrollment methods in views.py """
+
+from django.test.utils import override_settings
+from django.contrib.auth.models import Group, User
+from django.core.urlresolvers import reverse
+from courseware.access import _course_staff_group_name
+from courseware.tests.tests import LoginEnrollmentTestCase, TEST_DATA_XML_MODULESTORE, get_user
+from xmodule.modulestore.django import modulestore
+import xmodule.modulestore.django
+from student.models import CourseEnrollment, CourseEnrollmentAllowed
+
+
+@override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
+class TestInstructorEnrollsStudent(LoginEnrollmentTestCase):
+
+    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")
+
+        #Create instructor and student accounts
+        self.instructor = 'instructor1@test.com'
+        self.student1 = 'student1@test.com'
+        self.student2 = 'student2@test.com'
+        self.password = 'foo'
+        self.create_account('it1', self.instructor, self.password)
+        self.create_account('st1', self.student1, self.password)
+        self.create_account('st2', self.student2, self.password)
+        self.activate_user(self.instructor)
+        self.activate_user(self.student1)
+        self.activate_user(self.student2)
+
+        def make_instructor(course):
+            group_name = _course_staff_group_name(course.location)
+            g = Group.objects.create(name=group_name)
+            g.user_set.add(get_user(self.instructor))
+
+        make_instructor(self.toy)
+
+        #Enroll Students
+        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
+        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, '<td>student1@test.com</td>')
+        self.assertContains(response, '<td>student1@test.com</td>')
+        self.assertContains(response, '<td>un-enrolled</td>')
+        
+        #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))  
+        
+
+    def test_enrollmemt_new_student_autoenroll_on(self):
+        
+        #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, '<td>test1_1@student.com</td>')
+        self.assertContains(response, '<td>test1_2@student.com</td>')
+        self.assertContains(response, '<td>user does not exist, enrollment allowed, pending with auto enrollment on</td>')
+        
+        #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.password = 'bar'
+        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)) 
+
+
+    def test_enrollmemt_new_student_autoenroll_off(self):
+        
+        #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, '<td>test2_1@student.com</td>')
+        self.assertContains(response, '<td>test2_2@student.com</td>')
+        self.assertContains(response, '<td>user does not exist, enrollment allowed, pending with auto enrollment off</td>')
+        
+        #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.password = 'bar'
+        self.create_account('s2_2', self.student, self.password)
+        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)
+        self.assertEqual(0, len(ce))
+        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

lms/djangoapps/instructor/views.py

@@ -539,35 +539,17 @@ def instructor_dashboard(request, course_id):
         datatable['data'] = [[x.email] for x in ceaset]
         datatable['title'] = action
 
-    elif action == 'Enroll student':
+    elif action == 'Enroll multiple students':
 
-        student = request.POST.get('enstudent', '')
-        ret = _do_enroll_students(course, course_id, student)
+        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)
         datatable = ret['datatable']
 
-    elif action == 'Un-enroll student':
-
-        student = request.POST.get('enstudent', '')
-        datatable = {}
-        isok = False
-        cea = CourseEnrollmentAllowed.objects.filter(course_id=course_id, email=student)
-        if cea:
-            cea.delete()
-            msg += "Un-enrolled student with email '%s'" % student
-            isok = True
-        try:
-            nce = CourseEnrollment.objects.get(user=User.objects.get(email=student), course_id=course_id)
-            nce.delete()
-            msg += "Un-enrolled student with email '%s'" % student
-        except Exception as err:
-            if not isok:
-                msg += "Error!  Failed to un-enroll student with email '%s'\n" % student
-                msg += str(err) + '\n'
-
-    elif action == 'Enroll multiple students':
-
-        students = request.POST.get('enroll_multiple', '')
-        ret = _do_enroll_students(course, course_id, students)
+    elif action == 'Unenroll multiple students':
+        
+        students = request.POST.get('multiple_students', '')
+        ret = _do_unenroll_students(course_id, students)
         datatable = ret['datatable']
 
     elif action == 'List sections available in remote gradebook':
@@ -985,17 +967,11 @@ def grade_summary(request, course_id):
 #-----------------------------------------------------------------------------
 # enrollment
 
-def _do_enroll_students(course, course_id, students, overload=False):
+def _do_enroll_students(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 = split_by_comma_and_whitespace(students)
-    new_students = [str(s.strip()) for s in new_students]
-    new_students_lc = [x.lower() for x in new_students]
-
-    if '' in new_students:
-        new_students.remove('')
-
+    new_students = get_and_clean_student_list(students)
     status = dict([x, 'unprocessed'] for x in new_students)
 
     if overload:  	# delete all but staff
@@ -1015,13 +991,20 @@ def _do_enroll_students(course, course_id, students, overload=False):
         try:
             user = User.objects.get(email=student)
         except User.DoesNotExist:
-            # user not signed up yet, put in pending enrollment allowed table
-            if CourseEnrollmentAllowed.objects.filter(email=student, course_id=course_id):
-                status[student] = 'user does not exist, enrollment already allowed, pending'
+            
+            #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:
+                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]
                 continue
-            cea = CourseEnrollmentAllowed(email=student, course_id=course_id)
+            cea = CourseEnrollmentAllowed(email=student, course_id=course_id, auto_enroll=auto_enroll)
             cea.save()
-            status[student] = 'user does not exist, enrollment allowed, pending'
+            status[student] = 'user does not exist, enrollment allowed, pending with auto enrollment ' + ("off", "on")[auto_enroll]
             continue
 
         if CourseEnrollment.objects.filter(user=user, course_id=course_id):
@@ -1077,6 +1060,57 @@ def enroll_students(request, course_id):
                                                        'debug': new_students})
 
 
+#Unenrollment
+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)
+    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)
+        #Safe to get the first entry as there is a unique key on email + course_id
+        if cea:
+            cea[0].delete()
+            status[student] = "un-enrolled"
+            isok = True
+        
+        try:
+            user = User.objects.get(email=student)
+        except User.DoesNotExist:    
+            continue
+
+        nce = CourseEnrollment.objects.filter(user=user, course_id=course_id)
+        #Safe to get the first entry as there is a unique key on user + course_id
+        if nce:
+            try:
+                nce[0].delete()
+                status[student] = "un-enrolled"
+            except Exception as err:
+                if not isok:
+                    status[student] = "Error!  Failed to un-enroll"
+
+    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
+
+def get_and_clean_student_list(students):
+    
+    students = split_by_comma_and_whitespace(students)
+    students = [str(s.strip()) for s in students]
+    students_lc = [x.lower() for x in students]
+
+    if '' in students:
+        students.remove('')
+
+    return students
+
 #-----------------------------------------------------------------------------
 # answer distribution
 

lms/templates/courseware/instructor_dashboard.html

@@ -296,9 +296,6 @@ function goto( mode)
     <p>
     <input type="submit" name="action" value="List enrolled students">
     <input type="submit" name="action" value="List students who may enroll but may not have yet signed up">
-    <p>
-    Student Email: <input type="text" name="enstudent"> <input type="submit" name="action" value="Un-enroll student">
-    <input type="submit" name="action" value="Enroll student">
     <hr width="40%" style="align:left">
 
   %if settings.MITX_FEATURES.get('REMOTE_GRADEBOOK_URL','') and instructor_access:
@@ -320,9 +317,13 @@ function goto( mode)
 
   %endif
 
-  <p>Add students: enter emails, separated by new lines or commas;</p>
-  <textarea rows="6" cols="70" name="enroll_multiple"></textarea>
+  <p>Enroll or un-enroll one or many students: enter emails, separated by new lines or commas;</p>
+  <textarea rows="6" cols="70" name="multiple_students"></textarea>
+  <p>
+  <input type="checkbox" name="auto_enroll"> Auto-enroll students when they activate
   <input type="submit" name="action" value="Enroll multiple students">
+  <p>
+  <input type="submit" name="action" value="Unenroll multiple students">
 
 %endif