Accept usernames in Batch Enrollment widget

Respond to pull request comments

lms/djangoapps/instructor/views/api.py

@@ -200,7 +200,7 @@ def require_level(level):
 @ensure_csrf_cookie
 @cache_control(no_cache=True, no_store=True, must_revalidate=True)
 @require_level('staff')
-@require_query_params(action="enroll or unenroll", emails="stringified list of emails")
+@require_query_params(action="enroll or unenroll", identifiers="stringified list of emails and/or usernames")
 def students_update_enrollment(request, course_id):
     """
     Enroll or unenroll students by email.
@@ -208,7 +208,7 @@ def students_update_enrollment(request, course_id):
 
     Query Parameters:
     - action in ['enroll', 'unenroll']
-    - emails is string containing a list of emails separated by anything split_input_list can handle.
+    - identifiers is string containing a list of emails and/or usernames separated by anything split_input_list can handle.
     - auto_enroll is a boolean (defaults to false)
         If auto_enroll is false, students will be allowed to enroll.
         If auto_enroll is true, students will be enrolled as soon as they register.
@@ -240,8 +240,8 @@ def students_update_enrollment(request, course_id):
     """
 
     action = request.GET.get('action')
-    emails_raw = request.GET.get('emails')
-    emails = _split_input_list(emails_raw)
+    identifiers_raw = request.GET.get('identifiers')
+    identifiers = _split_input_list(identifiers_raw)
     auto_enroll = request.GET.get('auto_enroll') in ['true', 'True', True]
     email_students = request.GET.get('email_students') in ['true', 'True', True]
 
@@ -251,23 +251,23 @@ def students_update_enrollment(request, course_id):
         email_params = get_email_params(course, auto_enroll)
 
     results = []
-    for email in emails:
+    for identifier in identifiers:
+        # First try to get a user object from the identifer
+        user = None
+        email = None
+        try:
+            user = get_student_from_identifier(identifier)
+        except User.DoesNotExist:
+            email = identifier
+        else:
+            email = user.email
+
         try:
             # Use django.core.validators.validate_email to check email address
             # validity (obviously, cannot check if email actually /exists/,
             # simply that it is plausibly valid)
-            validate_email(email)
-        except ValidationError:
-            # Flag this email as an error if invalid, but continue checking
-            # the remaining in the list
-            results.append({
-                'email': email,
-                'error': True,
-                'invalidEmail': True,
-            })
-            continue
+            validate_email(email)  # Raises ValidationError if invalid
 
-        try:
             if action == 'enroll':
                 before, after = enroll_email(course_id, email, auto_enroll, email_students, email_params)
             elif action == 'unenroll':
@@ -277,20 +277,29 @@ def students_update_enrollment(request, course_id):
                     "Unrecognized action '{}'".format(action)
                 ))
 
+        except ValidationError:
+            # Flag this email as an error if invalid, but continue checking
+            # the remaining in the list
             results.append({
-                'email': email,
-                'before': before.to_dict(),
-                'after': after.to_dict(),
+                'identifier': identifier,
+                'invalidIdentifier': True,
             })
-        # catch and log any exceptions
-        # so that one error doesn't cause a 500.
+
         except Exception as exc:  # pylint: disable=W0703
+            # catch and log any exceptions
+            # so that one error doesn't cause a 500.
             log.exception("Error while #{}ing student")
             log.exception(exc)
             results.append({
-                'email': email,
+                'identifier': identifier,
                 'error': True,
-                'invalidEmail': False,
+            })
+
+        else:
+            results.append({
+                'identifier': identifier,
+                'before': before.to_dict(),
+                'after': after.to_dict(),
             })
 
     response_payload = {
@@ -306,7 +315,7 @@ def students_update_enrollment(request, course_id):
 @require_level('instructor')
 @common_exceptions_400
 @require_query_params(
-    emails="stringified list of emails or usernames",
+    identifiers="stringified list of emails and/or usernames",
     action="add or remove",
 )
 def bulk_beta_modify_access(request, course_id):
@@ -314,13 +323,13 @@ def bulk_beta_modify_access(request, course_id):
     Enroll or unenroll users in beta testing program.
 
     Query parameters:
-    - emails is string containing a list of emails or usernames separated by
+    - identifiers is string containing a list of emails and/or usernames separated by
       anything split_input_list can handle.
     - action is one of ['add', 'remove']
     """
     action = request.GET.get('action')
-    emails_raw = request.GET.get('emails')
-    emails = _split_input_list(emails_raw)
+    identifiers_raw = request.GET.get('identifiers')
+    identifiers = _split_input_list(identifiers_raw)
     email_students = request.GET.get('email_students') in ['true', 'True', True]
     auto_enroll = request.GET.get('auto_enroll') in ['true', 'True', True]
     results = []
@@ -331,11 +340,11 @@ def bulk_beta_modify_access(request, course_id):
     if email_students:
         email_params = get_email_params(course, auto_enroll=False)
 
-    for email in emails:
+    for identifier in identifiers:
         try:
             error = False
             user_does_not_exist = False
-            user = get_student_from_identifier(email)
+            user = get_student_from_identifier(identifier)
 
             if action == 'add':
                 allow_access(course, user, rolename)
@@ -367,7 +376,7 @@ def bulk_beta_modify_access(request, course_id):
         finally:
             # Tabulate the action result of this email address
             results.append({
-                'email': email,
+                'identifier': identifier,
                 'error': error,
                 'userDoesNotExist': user_does_not_exist
             })

lms/templates/instructor/instructor_dashboard_2/membership.html

@@ -30,9 +30,10 @@
 <div class="batch-enrollment">
   <h2> ${_("Batch Enrollment")} </h2>
   <p>
-    <label for="student-emails">${_("Enter email addresses separated by new lines or commas.")}
+    <label for="student-ids">
+      ${_("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.")} </label>
-    <textarea rows="6" name="student-emails" placeholder="${_("Email Addresses")}" spellcheck="false"></textarea>
+    <textarea rows="6" name="student-ids" placeholder="${_("Email Addresses/Usernames")}" spellcheck="false"></textarea>
   </p>
 
   <div class="enroll-option">
@@ -70,14 +71,14 @@
 
 %if section_data['access']['instructor']:
 <div class="batch-beta-testers">
-  <h2> ${_("Batch Beta Testers")} </h2>
+  <h2> ${_("Batch Beta Tester Addition")} </h2>
   <p>
-    <label for="student-emails-for-beta">
+    <label for="student-ids-for-beta">
       ${_("Enter email addresses and/or usernames separated by new lines or commas.")}<br/>
       ${_("Note: Users must have an activated {platform_name} account before they can be enrolled as a beta tester.").format(platform_name=settings.PLATFORM_NAME)}
     </label>
 
-  <textarea rows="6" cols="50" name="student-emails-for-beta" placeholder="${_("Email Addresses/Usernames")}" spellcheck="false"></textarea>
+  <textarea rows="6" cols="50" name="student-ids-for-beta" placeholder="${_("Email Addresses/Usernames")}" spellcheck="false"></textarea>
   </p>
 
   <div class="enroll-option">
@@ -86,7 +87,7 @@
     <div class="hint auto-enroll-beta-hint">
       <span class="hint-caret"></span>
       <p>
-	${_("If this option is <em>checked</em>, users who have not enrolled in your course will be automatically enrolled.").format(platform_name=settings.PLATFORM_NAME)}
+	${_("If this option is <em>checked</em>, users who have not enrolled in your course will be automatically enrolled.")}
 	<br /><br />
 	${_("Checking this box has no effect if 'Remove beta testers' is selected.")}
       </p>