Merge pull request #2927 from edx/adam/allow_usernames_or_email

make widget accept usernames or emails (LMS-1438)

lms/djangoapps/instructor/tests/test_api.py

@@ -135,7 +135,7 @@ class TestInstructorAPIDenyLevels(ModuleStoreTestCase, LoginEnrollmentTestCase):
         ]
         # Endpoints that only Instructors can access
         self.instructor_level_endpoints = [
-            ('modify_access', {'email': self.user.email, 'rolename': 'beta', 'action': 'allow'}),
+            ('modify_access', {'unique_student_identifier': self.user.email, 'rolename': 'beta', 'action': 'allow'}),
             ('list_course_role_members', {'rolename': 'beta'}),
             ('rescore_problem', {'problem_to_reset': self.problem_urlname, 'unique_student_identifier': self.user.email}),
         ]
@@ -639,7 +639,7 @@ class TestInstructorAPILevelsAccess(ModuleStoreTestCase, LoginEnrollmentTestCase
         """ Test with an invalid action parameter. """
         url = reverse('modify_access', kwargs={'course_id': self.course.id})
         response = self.client.get(url, {
-            'email': self.other_staff.email,
+            'unique_student_identifier': self.other_staff.email,
             'rolename': 'staff',
             'action': 'robot-not-an-action',
         })
@@ -649,7 +649,7 @@ class TestInstructorAPILevelsAccess(ModuleStoreTestCase, LoginEnrollmentTestCase
         """ Test with an invalid action parameter. """
         url = reverse('modify_access', kwargs={'course_id': self.course.id})
         response = self.client.get(url, {
-            'email': self.other_staff.email,
+            'unique_student_identifier': self.other_staff.email,
             'rolename': 'robot-not-a-roll',
             'action': 'revoke',
         })
@@ -658,7 +658,16 @@ class TestInstructorAPILevelsAccess(ModuleStoreTestCase, LoginEnrollmentTestCase
     def test_modify_access_allow(self):
         url = reverse('modify_access', kwargs={'course_id': self.course.id})
         response = self.client.get(url, {
-            'email': self.other_instructor.email,
+            'unique_student_identifier': self.other_instructor.email,
+            'rolename': 'staff',
+            'action': 'allow',
+        })
+        self.assertEqual(response.status_code, 200)
+
+    def test_modify_access_allow_with_uname(self):
+        url = reverse('modify_access', kwargs={'course_id': self.course.id})
+        response = self.client.get(url, {
+            'unique_student_identifier': self.other_instructor.username,
             'rolename': 'staff',
             'action': 'allow',
         })
@@ -667,7 +676,16 @@ class TestInstructorAPILevelsAccess(ModuleStoreTestCase, LoginEnrollmentTestCase
     def test_modify_access_revoke(self):
         url = reverse('modify_access', kwargs={'course_id': self.course.id})
         response = self.client.get(url, {
-            'email': self.other_staff.email,
+            'unique_student_identifier': self.other_staff.email,
+            'rolename': 'staff',
+            'action': 'revoke',
+        })
+        self.assertEqual(response.status_code, 200)
+
+    def test_modify_access_revoke_with_username(self):
+        url = reverse('modify_access', kwargs={'course_id': self.course.id})
+        response = self.client.get(url, {
+            'unique_student_identifier': self.other_staff.username,
             'rolename': 'staff',
             'action': 'revoke',
         })
@@ -677,7 +695,7 @@ class TestInstructorAPILevelsAccess(ModuleStoreTestCase, LoginEnrollmentTestCase
         """ Test revoking access that a user does not have. """
         url = reverse('modify_access', kwargs={'course_id': self.course.id})
         response = self.client.get(url, {
-            'email': self.other_staff.email,
+            'unique_student_identifier': self.other_staff.email,
             'rolename': 'instructor',
             'action': 'revoke',
         })
@@ -689,7 +707,7 @@ class TestInstructorAPILevelsAccess(ModuleStoreTestCase, LoginEnrollmentTestCase
         """
         url = reverse('modify_access', kwargs={'course_id': self.course.id})
         response = self.client.get(url, {
-            'email': self.instructor.email,
+            'unique_student_identifier': self.instructor.email,
             'rolename': 'instructor',
             'action': 'revoke',
         })

lms/djangoapps/instructor/views/api.py

@@ -281,7 +281,7 @@ def students_update_enrollment(request, course_id):
 @require_level('instructor')
 @common_exceptions_400
 @require_query_params(
-    email="user email",
+    unique_student_identifier="email or username of user to change access",
     rolename="'instructor', 'staff', or 'beta'",
     action="'allow' or 'revoke'"
 )
@@ -293,7 +293,7 @@ def modify_access(request, course_id):
     NOTE: instructors cannot remove their own instructor access.
 
     Query parameters:
-    email is the target users email
+    unique_student_identifer is the target user's username or email
     rolename is one of ['instructor', 'staff', 'beta']
     action is one of ['allow', 'revoke']
     """
@@ -301,7 +301,7 @@ def modify_access(request, course_id):
         request.user, course_id, 'instructor', depth=None
     )
 
-    email = strip_if_string(request.GET.get('email'))
+    user = get_student_from_identifier(request.GET.get('unique_student_identifier'))
     rolename = request.GET.get('rolename')
     action = request.GET.get('action')
 
@@ -310,8 +310,6 @@ def modify_access(request, course_id):
             "unknown rolename '{}'".format(rolename)
         ))
 
-    user = User.objects.get(email=email)
-
     # disallow instructors from removing their own instructor access.
     if rolename == 'instructor' and user == request.user and action != 'allow':
         return HttpResponseBadRequest(
@@ -328,7 +326,7 @@ def modify_access(request, course_id):
         ))
 
     response_payload = {
-        'email': email,
+        'unique_student_identifier': user.username,
         'rolename': rolename,
         'action': action,
         'success': 'yes',

lms/static/coffee/src/instructor_dashboard/membership.coffee

@@ -70,7 +70,7 @@ class AuthListWidget extends MemberListWidget
       title: $container.data 'display-name'
       info: $container.data 'info-text'
       labels: ["Username", "Email", "Revoke access"]
-      add_placeholder: "Enter email"
+      add_placeholder: "Enter username or email"
       add_btn_label: $container.data 'add-button-label'
       add_handler: (input) => @add_handler input
 
@@ -98,7 +98,7 @@ class AuthListWidget extends MemberListWidget
         @clear_input()
         @reload_list()
     else
-      @show_errors "Enter an email."
+      @show_errors gettext "Please enter a username or email."
 
   # reload the list of members
   reload_list: ->
@@ -140,22 +140,24 @@ class AuthListWidget extends MemberListWidget
       url: @list_endpoint
       data: rolename: @rolename
       success: (data) => cb? null, data[@rolename]
-      error: std_ajax_err => cb? "Error fetching list for role '#{@rolename}'"
+      error: std_ajax_err => 
+        `// Translators: A rolename appears this sentence.`
+        cb? gettext("Error fetching list for role") + " '#{@rolename}'"
 
   # send ajax request to modify access
   # (add or remove them from the list)
   # `action` can be 'allow' or 'revoke'
   # `cb` is called with cb(error, data)
-  modify_member_access: (email, action, cb) ->
+  modify_member_access: (unique_student_identifier, action, cb) ->
     $.ajax
       dataType: 'json'
       url: @modify_endpoint
       data:
-        email: email
+        unique_student_identifier: unique_student_identifier
         rolename: @rolename
         action: action
       success: (data) => cb? null, data
-      error: std_ajax_err => cb? "Error changing user's permissions."
+      error: std_ajax_err => cb? gettext "Error changing user's permissions."
 
 
 # Wrapper for the batch enrollment subsection.
@@ -204,7 +206,7 @@ class BatchEnrollment
         url: @$btn_unenroll.data 'endpoint'
         data: send_data
         success: (data) => @display_response data
-        error: std_ajax_err => @fail_with_error "Error enrolling/unenrolling students."
+        error: std_ajax_err => @fail_with_error gettext "Error enrolling/unenrolling students."
 
 
   fail_with_error: (msg) ->
@@ -310,37 +312,45 @@ class BatchEnrollment
       render_list gettext("Successfully enrolled and sent email to the following students:"), (sr.email for sr in enrolled)
 
     if enrolled.length and not emailStudents
+      `// Translators: A list of students appears after this sentence.`
       render_list gettext("Successfully enrolled the following students:"), (sr.email for sr in enrolled)
 
     # Student hasn't registered so we allow them to enroll
     if allowed.length and emailStudents
+      `// Translators: A list of students appears after this sentence.`
       render_list gettext("Successfully sent enrollment emails to the following students. They will be allowed to enroll once they register:"),
         (sr.email for sr in allowed)
 
     # Student hasn't registered so we allow them to enroll
     if allowed.length and not emailStudents
+      `// Translators: A list of students appears after this sentence.`
       render_list gettext("These students will be allowed to enroll once they register:"),
         (sr.email for sr in allowed)
 
     # Student hasn't registered so we allow them to enroll with autoenroll
     if autoenrolled.length and emailStudents
+      `// Translators: A list of students appears after this sentence.`
       render_list gettext("Successfully sent enrollment emails to the following students. They will be enrolled once they register:"),
         (sr.email for sr in autoenrolled)
 
     # Student hasn't registered so we allow them to enroll with autoenroll
     if autoenrolled.length and not emailStudents
+      `// Translators: A list of students appears after this sentence.`
       render_list gettext("These students will be enrolled once they register:"),
         (sr.email for sr in autoenrolled)
 
     if notenrolled.length and emailStudents
+      `// Translators: A list of students appears after this sentence.`
       render_list gettext("Emails successfully sent. The following students are no longer enrolled in the course:"),
         (sr.email for sr in notenrolled)
 
     if notenrolled.length and not emailStudents
+      `// Translators: A list of students appears after this sentence.`
       render_list gettext("The following students are no longer enrolled in the course:"),
         (sr.email for sr in notenrolled)
 
     if notunenrolled.length
+      `// Translators: A list of students appears after this sentence.`
       render_list gettext("These students were not affliliated with the course so could not be unenrolled:"),
         (sr.email for sr in notunenrolled)
 
@@ -451,7 +461,7 @@ class AuthList
         rolename: @rolename
         action: action
       success: (data) -> cb?(data)
-      error: std_ajax_err => @$request_response_error.text "Error changing user's permissions."
+      error: std_ajax_err => @$request_response_error.text gettext "Error changing user's permissions."
 
 
 # Membership Section