diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 132a350840..352f5c93ad 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/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', }) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 8fbd22a4c5..68c9e8e68c 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/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', diff --git a/lms/static/coffee/src/instructor_dashboard/membership.coffee b/lms/static/coffee/src/instructor_dashboard/membership.coffee index a7d17aa163..64d860e474 100644 --- a/lms/static/coffee/src/instructor_dashboard/membership.coffee +++ b/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