Simplify the content enrollment tab in the CCX Dashboard.

Signed-off-by: Tomasz Gargas <[email protected]>

Author: tomaszgy

lms/djangoapps/ccx/tests/test_views.py

@@ -592,10 +592,8 @@ def test_save_without_min_count(self):
         self.assertEqual(response.status_code, 200)
 
     @ddt.data(
-        ('ccx_invite', True, 1, 'student-ids', ('enrollment-button', 'Enroll')),
-        ('ccx_invite', False, 0, 'student-ids', ('enrollment-button', 'Enroll')),
-        ('ccx_manage_student', True, 1, 'student-id', ('student-action', 'add')),
-        ('ccx_manage_student', False, 0, 'student-id', ('student-action', 'add')),
+        ('ccx-manage-students', True, 1, 'student-ids', ('enrollment-button', 'Enroll')),
+        ('ccx-manage-students', False, 0, 'student-ids', ('enrollment-button', 'Enroll')),
     )
     @ddt.unpack
     def test_enroll_member_student(self, view_name, send_email, outbox_count, student_form_input_name, button_tuple):
@@ -656,7 +654,7 @@ def test_ccx_invite_enroll_up_to_limit(self):
         ]
 
         url = reverse(
-            'ccx_invite',
+            'ccx-manage-students',
             kwargs={'course_id': ccx_course_key}
         )
         data = {
@@ -687,81 +685,11 @@ def test_ccx_invite_enroll_up_to_limit(self):
             CourseEnrollment.objects.filter(course_id=ccx_course_key, user=students[5]).exists()
         )
 
-    def test_manage_student_enrollment_limit(self):
-        """
-        Enroll students up to the enrollment limit.
-
-        This test is specific to one of the enrollment views: the reason is because
-        the view used in this test cannot perform bulk enrollments.
-        """
-        students_limit = 1
-        self.make_coach()
-        staff = self.make_staff()
-        ccx = self.make_ccx(max_students_allowed=students_limit)
-        ccx_course_key = CCXLocator.from_course_locator(self.course.id, ccx.id)
-        students = [
-            UserFactory.create(is_staff=False) for _ in range(2)
-        ]
-
-        url = reverse(
-            'ccx_manage_student',
-            kwargs={'course_id': CCXLocator.from_course_locator(self.course.id, ccx.id)}
-        )
-        # enroll the first student
-        data = {
-            'student-action': 'add',
-            'student-id': students[0].email,
-        }
-        response = self.client.post(url, data=data, follow=True)
-        self.assertEqual(response.status_code, 200)
-        # a CcxMembership exists for this student
-        self.assertTrue(
-            CourseEnrollment.objects.filter(course_id=ccx_course_key, user=students[0]).exists()
-        )
-
-        # try to enroll the second student without success
-        # enroll the first student
-        data = {
-            'student-action': 'add',
-            'student-id': students[1].email,
-        }
-        response = self.client.post(url, data=data, follow=True)
-        self.assertEqual(response.status_code, 200)
-        # a CcxMembership does not exist for this student
-        self.assertFalse(
-            CourseEnrollment.objects.filter(course_id=ccx_course_key, user=students[1]).exists()
-        )
-        error_message = 'The course is full: the limit is {students_limit}'.format(
-            students_limit=students_limit
-        )
-        self.assertContains(response, error_message, status_code=200)
-
-        # try to enroll the 3rd student which is staff
-        data = {
-            'student-action': 'add',
-            'student-id': staff.email,
-        }
-        response = self.client.post(url, data=data, follow=True)
-        self.assertEqual(response.status_code, 200)
-        # staff gets enroll
-        self.assertTrue(
-            CourseEnrollment.objects.filter(course_id=ccx_course_key, user=staff).exists()
-        )
-
-        self.assertEqual(CourseEnrollment.objects.num_enrolled_in_exclude_admins(ccx_course_key), 1)
-
-        # asert that number of enroll is still 0 because staff and instructor do not count.
-        CourseEnrollment.enroll(staff, self.course.id)
-        self.assertEqual(CourseEnrollment.objects.num_enrolled_in_exclude_admins(self.course.id), 0)
-        # assert that handles  wrong ccx id code
-        ccx_course_key_fake = CCXLocator.from_course_locator(self.course.id, 55)
-        self.assertEqual(CourseEnrollment.objects.num_enrolled_in_exclude_admins(ccx_course_key_fake), 0)
-
     @ddt.data(
-        ('ccx_invite', True, 1, 'student-ids', ('enrollment-button', 'Unenroll')),
-        ('ccx_invite', False, 0, 'student-ids', ('enrollment-button', 'Unenroll')),
-        ('ccx_manage_student', True, 1, 'student-id', ('student-action', 'revoke')),
-        ('ccx_manage_student', False, 0, 'student-id', ('student-action', 'revoke')),
+        ('ccx-manage-students', True, 1, 'student-ids', ('enrollment-button', 'Unenroll')),
+        ('ccx-manage-students', False, 0, 'student-ids', ('enrollment-button', 'Unenroll')),
+        ('ccx-manage-students', True, 1, 'student-id', ('student-action', 'revoke')),
+        ('ccx-manage-students', False, 0, 'student-id', ('student-action', 'revoke')),
     )
     @ddt.unpack
     def test_unenroll_member_student(self, view_name, send_email, outbox_count, student_form_input_name, button_tuple):
@@ -803,14 +731,10 @@ def test_unenroll_member_student(self, view_name, send_email, outbox_count, stud
         )
 
     @ddt.data(
-        ('ccx_invite', True, 1, 'student-ids', ('enrollment-button', 'Enroll'), '[email protected]'),
-        ('ccx_invite', False, 0, 'student-ids', ('enrollment-button', 'Enroll'), '[email protected]'),
-        ('ccx_invite', True, 0, 'student-ids', ('enrollment-button', 'Enroll'), 'nobody'),
-        ('ccx_invite', False, 0, 'student-ids', ('enrollment-button', 'Enroll'), 'nobody'),
-        ('ccx_manage_student', True, 0, 'student-id', ('student-action', 'add'), 'dummy_student_id'),
-        ('ccx_manage_student', False, 0, 'student-id', ('student-action', 'add'), 'dummy_student_id'),
-        ('ccx_manage_student', True, 1, 'student-id', ('student-action', 'add'), '[email protected]'),
-        ('ccx_manage_student', False, 0, 'student-id', ('student-action', 'add'), '[email protected]'),
+        ('ccx-manage-students', True, 1, 'student-ids', ('enrollment-button', 'Enroll'), '[email protected]'),
+        ('ccx-manage-students', False, 0, 'student-ids', ('enrollment-button', 'Enroll'), '[email protected]'),
+        ('ccx-manage-students', True, 0, 'student-ids', ('enrollment-button', 'Enroll'), 'nobody'),
+        ('ccx-manage-students', False, 0, 'student-ids', ('enrollment-button', 'Enroll'), 'nobody'),
     )
     @ddt.unpack
     def test_enroll_non_user_student(
@@ -860,10 +784,10 @@ def test_enroll_non_user_student(
             )
 
     @ddt.data(
-        ('ccx_invite', True, 0, 'student-ids', ('enrollment-button', 'Unenroll'), '[email protected]'),
-        ('ccx_invite', False, 0, 'student-ids', ('enrollment-button', 'Unenroll'), '[email protected]'),
-        ('ccx_invite', True, 0, 'student-ids', ('enrollment-button', 'Unenroll'), 'nobody'),
-        ('ccx_invite', False, 0, 'student-ids', ('enrollment-button', 'Unenroll'), 'nobody'),
+        ('ccx-manage-students', True, 0, 'student-ids', ('enrollment-button', 'Unenroll'), '[email protected]'),
+        ('ccx-manage-students', False, 0, 'student-ids', ('enrollment-button', 'Unenroll'), '[email protected]'),
+        ('ccx-manage-students', True, 0, 'student-ids', ('enrollment-button', 'Unenroll'), 'nobody'),
+        ('ccx-manage-students', False, 0, 'student-ids', ('enrollment-button', 'Unenroll'), 'nobody'),
     )
     @ddt.unpack
     def test_unenroll_non_user_student(

lms/djangoapps/ccx/urls.py

@@ -11,12 +11,10 @@
         'ccx.views.create_ccx', name='create_ccx'),
     url(r'^save_ccx$',
         'ccx.views.save_ccx', name='save_ccx'),
-    url(r'^ccx_invite$',
-        'ccx.views.ccx_invite', name='ccx_invite'),
     url(r'^ccx_schedule$',
         'ccx.views.ccx_schedule', name='ccx_schedule'),
-    url(r'^ccx_manage_student$',
-        'ccx.views.ccx_student_management', name='ccx_manage_student'),
+    url(r'^ccx-manage-students$',
+        'ccx.views.ccx_students_management', name='ccx-manage-students'),
 
     # Grade book
     url(r'^ccx_gradebook$',

lms/djangoapps/ccx/utils.py

@@ -208,16 +208,10 @@ def get_valid_student_with_email(identifier):
 
 def ccx_students_enrolling_center(action, identifiers, email_students, course_key, email_params, coach):
     """
-    Function to enroll/add or unenroll/revoke students.
-
-    This function exists for backwards compatibility: in CCX there are
-    two different views to manage students that used to implement
-    a different logic. Now the logic has been reconciled at the point that
-    this function can be used by both.
-    The two different views can be merged after some UI refactoring.
+    Function to enroll or unenroll/revoke students.
 
     Arguments:
-        action (str): type of action to perform (add, Enroll, revoke, Unenroll)
+        action (str): type of action to perform (Enroll, Unenroll/revoke)
         identifiers (list): list of students username/email
         email_students (bool): Flag to send an email to students
         course_key (CCXLocator): a CCX course key
@@ -229,7 +223,7 @@ def ccx_students_enrolling_center(action, identifiers, email_students, course_ke
     """
     errors = []
 
-    if action == 'Enroll' or action == 'add':
+    if action == 'Enroll':
         ccx_course_overview = CourseOverview.get_from_id(course_key)
         course_locator = course_key.to_course_locator()
         staff = CourseStaffRole(course_locator).users_with_role()

lms/djangoapps/ccx/views.py

@@ -460,40 +460,26 @@ def ccx_schedule(request, course, ccx=None):  # pylint: disable=unused-argument
 @ensure_csrf_cookie
 @cache_control(no_cache=True, no_store=True, must_revalidate=True)
 @coach_dashboard
-def ccx_invite(request, course, ccx=None):
+def ccx_students_management(request, course, ccx=None):
     """
-    Invite users to new ccx
+    Manage the enrollment of the students in a CCX
     """
     if not ccx:
         raise Http404
 
-    action = request.POST.get('enrollment-button')
-    identifiers_raw = request.POST.get('student-ids')
-    identifiers = _split_input_list(identifiers_raw)
-    email_students = 'email-students' in request.POST
-    course_key = CCXLocator.from_course_locator(course.id, unicode(ccx.id))
-    email_params = get_email_params(course, auto_enroll=True, course_key=course_key, display_name=ccx.display_name)
-
-    ccx_students_enrolling_center(action, identifiers, email_students, course_key, email_params, ccx.coach)
-
-    url = reverse('ccx_coach_dashboard', kwargs={'course_id': course_key})
-    return redirect(url)
-
+    list_action = request.POST.get('student-action', None)
+    batch_action = request.POST.get('enrollment-button', None)
 
-@ensure_csrf_cookie
-@cache_control(no_cache=True, no_store=True, must_revalidate=True)
-@coach_dashboard
-def ccx_student_management(request, course, ccx=None):
-    """
-    Manage the enrollment of individual students in a CCX
-    """
-    if not ccx:
-        raise Http404
+    if list_action:
+        action = list_action
+        student_id = request.POST.get('student-id', '')
+        identifiers = [student_id]
+    elif batch_action:
+        action = batch_action
+        identifiers_raw = request.POST.get('student-ids')
+        identifiers = _split_input_list(identifiers_raw)
 
-    action = request.POST.get('student-action', None)
-    student_id = request.POST.get('student-id', '')
     email_students = 'email-students' in request.POST
-    identifiers = [student_id]
     course_key = CCXLocator.from_course_locator(course.id, unicode(ccx.id))
     email_params = get_email_params(course, auto_enroll=True, course_key=course_key, display_name=ccx.display_name)
 

lms/templates/ccx/coach_dashboard.html

@@ -112,34 +112,12 @@ <h2 class="hd hd-2" id="header-ccx-dashboard">${_("CCX Coach Dashboard")}</h2>
 
   function setup_management_form() {
 
-    $(".member-lists-management form").on("submit", function (event) {
-      var target, action;
-      target = $(event.target);
-      if (target.serialize().indexOf('student-action') < 0) {
-        action = $('<input />', {
-          type: 'hidden',
-          name: 'student-action',
-          value: 'add'
-        });
-        target.append(action);
-      }
-    });
-
-    $(".member-lists-management form .add, .member-lists-management form .revoke").on("click", function(event) {
+    $(".member-lists-management form .revoke").on("click", function(event) {
         var target, form, action, studentId, selectedStudent;
         event.preventDefault();
         target = $(event.target);
         form = target.parents('form').first();
-        if (target.hasClass('add')) {
-          // adding a new student, add the student-action input and submit
-          action = $('<input />', {
-            type: 'hidden',
-            name: 'student-action',
-            // this is untenable, tied to a translated value.  Fix it.
-            value: 'add'
-          });
-          form.append(action).submit();
-        } else if (target.hasClass('revoke')) {
+        if (target.hasClass('revoke')) {
           // revoking access for a student, get set form values and submit
           // get the email address of the student, since they might not be 'enrolled' yet.
           selectedStudent = target.parent('td').siblings().last().text();

lms/templates/ccx/enrollment.html

@@ -5,15 +5,15 @@
 %>
 
 <h2 class="hd hd-2">${_("Batch Enrollment")}</h2>
-<div class="batch-enrollment" style="float:left;width:50%">
-  <form method="POST" action="ccx_invite">
+<div class="batch-enrollment" style="float:left;width:100%">
+  <form method="POST" action="ccx-manage-students">
   <input type="hidden" name="csrfmiddlewaretoken" value="${ csrf_token }">
   <label for="student-ids" class="sr">${_("Email Addresses/Usernames")}</label>
   <p id="label_student_ids" class="text-helper">
       ${_("Enter email addresses and/or usernames separated by new lines or commas.")}
       ${_("You will not get notification for emails that bounce, so please double-check spelling.")}
   </p>
-  <textarea rows="6" name="student-ids" id="student-ids" aria-describedby="label_student_ids" placeholder="${_("Email Addresses/Usernames")}" spellcheck="false"></textarea>
+  <textarea rows="6" name="student-ids" id="student-ids" aria-describedby="label_student_ids" placeholder="${_("Email Addresses/Usernames")}" spellcheck="false" style="width:100%;"></textarea>
 
 
   <div class="enroll-option">
@@ -61,8 +61,8 @@ <h2 class="hd hd-2">${_("Batch Enrollment")}</h2>
   </form>
 </div>
 
-<div class="member-lists-management" style="float:left;width:50%">
-  <form method="POST" action="ccx_manage_student" class="ccx-manage-student-form">
+<div class="member-lists-management" style="float:left;width:100%">
+  <form method="POST" action="ccx-manage-students" class="ccx-manage-student-form">
   <input type="hidden" name="csrfmiddlewaretoken" value="${ csrf_token }">
   <div class="auth-list-container active">
     <div class="member-list-widget">
@@ -95,45 +95,6 @@ <h2 class="hd hd-2">${_("Student List Management")}</h2>
           </tbody>
         </table>
       </div>
-      <div class="bottom-bar">
-        <label for="student-id" class="sr">${_("Enter username or email")}</label>
-        <input name="student-id" id="student-id" class="add-field" placeholder="${_("Enter username or email")}" type="text">
-        <input name="student-action" class="add" value="${_("Add Student")}" type="button">
-        <div class="enroll-option">
-            <input type="checkbox" name="auto-enroll" id="auto-enroll" value="Auto-Enroll" checked="yes" aria-describedby="auto-enroll-helper" disabled>
-            <label style="display:inline" for="auto-enroll">${_("Auto Enroll")}</label>
-            <div class="hint auto-enroll-hint">
-              <span class="hint-caret"></span>
-              <p class="text-helper" id="auto-enroll-helper">
-                ${Text(_("If this option is {em_start}checked{em_end}, users who have not yet registered for {platform_name} will be automatically enrolled.")).format(
-                    em_start=HTML('<em>'),
-                    em_end=HTML('</em>'),
-                    platform_name=settings.PLATFORM_NAME,
-                )}
-                ${Text(_("If this option is left {em_start}unchecked{em_end}, users who have not yet registered for {platform_name} will not be enrolled, but will be allowed to enroll once they make an account.")).format(
-                    em_start=HTML('<em>'),
-                    em_end=HTML('</em>'),
-                    platform_name=settings.PLATFORM_NAME,
-                )}
-                <br /><br />
-                ${_("Checking this box has no effect if 'Revoke' is clicked.")}
-              </p>
-            </div>
-        </div>
-        <div class="enroll-option ccx-notify-user">
-            <input type="checkbox" name="email-students" id="email-students" value="Notify-students-by-email" checked="yes" aria-describedby="email-students-helper">
-            <label style="display:inline" for="email-students">${_("Notify users by email")}</label>
-            <div class="hint email-students-hint">
-              <span class="hint-caret"></span>
-              <p class="text-helper" id="email-students-helper">
-                ${Text(_("If this option is {em_start}checked{em_end}, users will receive an email notification.")).format(
-                    em_start=HTML('<em>'),
-                    em_end=HTML('</em>'),
-                )}
-              </p>
-            </div>
-        </div>
-      </div>
     </div>
   </div>
   </form>