From bddae213f65cc3c0cc0d239b9dd3b39833314553 Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Fri, 28 Mar 2014 18:43:43 -0400 Subject: [PATCH] Accept usernames in Batch Enrollment widget Respond to pull request comments --- lms/djangoapps/instructor/tests/test_api.py | 133 ++++++++++++------ lms/djangoapps/instructor/views/api.py | 69 +++++---- .../instructor_dashboard/membership.coffee | 83 +++++------ .../instructor_dashboard_2/membership.html | 13 +- 4 files changed, 180 insertions(+), 118 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 57412f905f..5753c30423 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -121,7 +121,7 @@ class TestInstructorAPIDenyLevels(ModuleStoreTestCase, LoginEnrollmentTestCase): # Endpoints that only Staff or Instructors can access self.staff_level_endpoints = [ - ('students_update_enrollment', {'emails': 'foo@example.org', 'action': 'enroll'}), + ('students_update_enrollment', {'identifiers': 'foo@example.org', 'action': 'enroll'}), ('get_grading_config', {}), ('get_students_features', {}), ('get_distribution', {}), @@ -138,7 +138,7 @@ class TestInstructorAPIDenyLevels(ModuleStoreTestCase, LoginEnrollmentTestCase): ] # Endpoints that only Instructors can access self.instructor_level_endpoints = [ - ('bulk_beta_modify_access', {'emails': 'foo@example.org', 'action': 'add'}), + ('bulk_beta_modify_access', {'identifiers': 'foo@example.org', 'action': 'add'}), ('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}), @@ -291,13 +291,12 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): """ Test with an invalid action. """ action = 'robot-not-an-action' url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.enrolled_student.email, 'action': action}) + response = self.client.get(url, {'identifiers': self.enrolled_student.email, 'action': action}) self.assertEqual(response.status_code, 400) - def test_enroll_with_username(self): - # Test with an invalid email address (eg, a username). + def test_invalid_email(self): url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.notenrolled_student.username, 'action': 'enroll', 'email_students': False}) + response = self.client.get(url, {'identifiers': 'percivaloctavius@', 'action': 'enroll', 'email_students': False}) self.assertEqual(response.status_code, 200) # test the response data @@ -306,9 +305,59 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): 'auto_enroll': False, "results": [ { - "email": self.notenrolled_student.username, - "error": True, - "invalidEmail": True + "identifier": 'percivaloctavius@', + "invalidIdentifier": True, + } + ] + } + + res_json = json.loads(response.content) + self.assertEqual(res_json, expected) + + def test_invalid_username(self): + url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) + response = self.client.get(url, {'identifiers': 'percivaloctavius', 'action': 'enroll', 'email_students': False}) + self.assertEqual(response.status_code, 200) + + # test the response data + expected = { + "action": "enroll", + 'auto_enroll': False, + "results": [ + { + "identifier": 'percivaloctavius', + "invalidIdentifier": True, + } + ] + } + + res_json = json.loads(response.content) + self.assertEqual(res_json, expected) + + def test_enroll_with_username(self): + url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) + response = self.client.get(url, {'identifiers': self.notenrolled_student.username, 'action': 'enroll', 'email_students': False}) + self.assertEqual(response.status_code, 200) + + # test the response data + expected = { + "action": "enroll", + 'auto_enroll': False, + "results": [ + { + "identifier": self.notenrolled_student.username, + "before": { + "enrollment": False, + "auto_enroll": False, + "user": True, + "allowed": False, + }, + "after": { + "enrollment": True, + "auto_enroll": False, + "user": True, + "allowed": False, + } } ] } @@ -318,7 +367,7 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): def test_enroll_without_email(self): url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.notenrolled_student.email, 'action': 'enroll', 'email_students': False}) + response = self.client.get(url, {'identifiers': self.notenrolled_student.email, 'action': 'enroll', 'email_students': False}) print "type(self.notenrolled_student.email): {}".format(type(self.notenrolled_student.email)) self.assertEqual(response.status_code, 200) @@ -332,7 +381,7 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): "auto_enroll": False, "results": [ { - "email": self.notenrolled_student.email, + "identifier": self.notenrolled_student.email, "before": { "enrollment": False, "auto_enroll": False, @@ -357,7 +406,7 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): def test_enroll_with_email(self): url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.notenrolled_student.email, 'action': 'enroll', 'email_students': True}) + response = self.client.get(url, {'identifiers': self.notenrolled_student.email, 'action': 'enroll', 'email_students': True}) print "type(self.notenrolled_student.email): {}".format(type(self.notenrolled_student.email)) self.assertEqual(response.status_code, 200) @@ -371,7 +420,7 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): "auto_enroll": False, "results": [ { - "email": self.notenrolled_student.email, + "identifier": self.notenrolled_student.email, "before": { "enrollment": False, "auto_enroll": False, @@ -409,7 +458,7 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): def test_enroll_with_email_not_registered(self): url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.notregistered_email, 'action': 'enroll', 'email_students': True}) + response = self.client.get(url, {'identifiers': self.notregistered_email, 'action': 'enroll', 'email_students': True}) self.assertEqual(response.status_code, 200) # Check the outbox @@ -432,7 +481,7 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) # Try with marketing site enabled with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): - response = self.client.get(url, {'emails': self.notregistered_email, 'action': 'enroll', 'email_students': True}) + response = self.client.get(url, {'identifiers': self.notregistered_email, 'action': 'enroll', 'email_students': True}) self.assertEqual(response.status_code, 200) self.assertEqual( @@ -446,7 +495,7 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): def test_enroll_with_email_not_registered_autoenroll(self): url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.notregistered_email, 'action': 'enroll', 'email_students': True, 'auto_enroll': True}) + response = self.client.get(url, {'identifiers': self.notregistered_email, 'action': 'enroll', 'email_students': True, 'auto_enroll': True}) print "type(self.notregistered_email): {}".format(type(self.notregistered_email)) self.assertEqual(response.status_code, 200) @@ -467,7 +516,7 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): def test_unenroll_without_email(self): url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.enrolled_student.email, 'action': 'unenroll', 'email_students': False}) + response = self.client.get(url, {'identifiers': self.enrolled_student.email, 'action': 'unenroll', 'email_students': False}) print "type(self.enrolled_student.email): {}".format(type(self.enrolled_student.email)) self.assertEqual(response.status_code, 200) @@ -481,7 +530,7 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): "auto_enroll": False, "results": [ { - "email": self.enrolled_student.email, + "identifier": self.enrolled_student.email, "before": { "enrollment": True, "auto_enroll": False, @@ -506,7 +555,7 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): def test_unenroll_with_email(self): url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.enrolled_student.email, 'action': 'unenroll', 'email_students': True}) + response = self.client.get(url, {'identifiers': self.enrolled_student.email, 'action': 'unenroll', 'email_students': True}) print "type(self.enrolled_student.email): {}".format(type(self.enrolled_student.email)) self.assertEqual(response.status_code, 200) @@ -520,7 +569,7 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): "auto_enroll": False, "results": [ { - "email": self.enrolled_student.email, + "identifier": self.enrolled_student.email, "before": { "enrollment": True, "auto_enroll": False, @@ -557,7 +606,7 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): def test_unenroll_with_email_allowed_student(self): url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.allowed_email, 'action': 'unenroll', 'email_students': True}) + response = self.client.get(url, {'identifiers': self.allowed_email, 'action': 'unenroll', 'email_students': True}) print "type(self.allowed_email): {}".format(type(self.allowed_email)) self.assertEqual(response.status_code, 200) @@ -567,7 +616,7 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): "auto_enroll": False, "results": [ { - "email": self.allowed_email, + "identifier": self.allowed_email, "before": { "enrollment": False, "auto_enroll": False, @@ -605,7 +654,7 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): mock_uses_shib.return_value = True url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.notregistered_email, 'action': 'enroll', 'email_students': True}) + response = self.client.get(url, {'identifiers': self.notregistered_email, 'action': 'enroll', 'email_students': True}) self.assertEqual(response.status_code, 200) # Check the outbox @@ -629,7 +678,7 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) # Try with marketing site enabled with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): - response = self.client.get(url, {'emails': self.notregistered_email, 'action': 'enroll', 'email_students': True}) + response = self.client.get(url, {'identifiers': self.notregistered_email, 'action': 'enroll', 'email_students': True}) self.assertEqual(response.status_code, 200) self.assertEqual( @@ -644,7 +693,7 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): mock_uses_shib.return_value = True url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.notregistered_email, 'action': 'enroll', 'email_students': True, 'auto_enroll': True}) + response = self.client.get(url, {'identifiers': self.notregistered_email, 'action': 'enroll', 'email_students': True, 'auto_enroll': True}) print "type(self.notregistered_email): {}".format(type(self.notregistered_email)) self.assertEqual(response.status_code, 200) @@ -699,13 +748,15 @@ class TestInstructorAPIBulkBetaEnrollment(ModuleStoreTestCase, LoginEnrollmentTe """ Test with an invalid action. """ action = 'robot-not-an-action' url = reverse('bulk_beta_modify_access', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.beta_tester.email, 'action': action}) + response = self.client.get(url, {'identifiers': self.beta_tester.email, 'action': action}) self.assertEqual(response.status_code, 400) def add_notenrolled(self, response, identifier): """ + Test Helper Method (not a test, called by other tests) + Takes a client response from a call to bulk_beta_modify_access with 'email_students': False, - and the student identifier (email or username) given as 'emails' in the request. + and the student identifier (email or username) given as 'identifiers' in the request. Asserts the reponse returns cleanly, that the student was added as a beta tester, and the response properly contains their identifier, 'error': False, and 'userDoesNotExist': False. @@ -718,7 +769,7 @@ class TestInstructorAPIBulkBetaEnrollment(ModuleStoreTestCase, LoginEnrollmentTe "action": "add", "results": [ { - "email": identifier, + "identifier": identifier, "error": False, "userDoesNotExist": False } @@ -733,31 +784,31 @@ class TestInstructorAPIBulkBetaEnrollment(ModuleStoreTestCase, LoginEnrollmentTe def test_add_notenrolled_email(self): url = reverse('bulk_beta_modify_access', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.notenrolled_student.email, 'action': 'add', 'email_students': False}) + response = self.client.get(url, {'identifiers': self.notenrolled_student.email, 'action': 'add', 'email_students': False}) self.add_notenrolled(response, self.notenrolled_student.email) self.assertFalse(CourseEnrollment.is_enrolled(self.notenrolled_student, self.course.id)) def test_add_notenrolled_email_autoenroll(self): url = reverse('bulk_beta_modify_access', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.notenrolled_student.email, 'action': 'add', 'email_students': False, 'auto_enroll': True}) + response = self.client.get(url, {'identifiers': self.notenrolled_student.email, 'action': 'add', 'email_students': False, 'auto_enroll': True}) self.add_notenrolled(response, self.notenrolled_student.email) self.assertTrue(CourseEnrollment.is_enrolled(self.notenrolled_student, self.course.id)) def test_add_notenrolled_username(self): url = reverse('bulk_beta_modify_access', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.notenrolled_student.username, 'action': 'add', 'email_students': False}) + response = self.client.get(url, {'identifiers': self.notenrolled_student.username, 'action': 'add', 'email_students': False}) self.add_notenrolled(response, self.notenrolled_student.username) self.assertFalse(CourseEnrollment.is_enrolled(self.notenrolled_student, self.course.id)) def test_add_notenrolled_username_autoenroll(self): url = reverse('bulk_beta_modify_access', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.notenrolled_student.username, 'action': 'add', 'email_students': False, 'auto_enroll': True}) + response = self.client.get(url, {'identifiers': self.notenrolled_student.username, 'action': 'add', 'email_students': False, 'auto_enroll': True}) self.add_notenrolled(response, self.notenrolled_student.username) self.assertTrue(CourseEnrollment.is_enrolled(self.notenrolled_student, self.course.id)) def test_add_notenrolled_with_email(self): url = reverse('bulk_beta_modify_access', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.notenrolled_student.email, 'action': 'add', 'email_students': True}) + response = self.client.get(url, {'identifiers': self.notenrolled_student.email, 'action': 'add', 'email_students': True}) self.assertEqual(response.status_code, 200) self.assertTrue(CourseBetaTesterRole(self.course.location).has_user(self.notenrolled_student)) @@ -766,7 +817,7 @@ class TestInstructorAPIBulkBetaEnrollment(ModuleStoreTestCase, LoginEnrollmentTe "action": "add", "results": [ { - "email": self.notenrolled_student.email, + "identifier": self.notenrolled_student.email, "error": False, "userDoesNotExist": False } @@ -798,7 +849,7 @@ class TestInstructorAPIBulkBetaEnrollment(ModuleStoreTestCase, LoginEnrollmentTe url = reverse('bulk_beta_modify_access', kwargs={'course_id': self.course.id}) # Try with marketing site enabled with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): - response = self.client.get(url, {'emails': self.notenrolled_student.email, 'action': 'add', 'email_students': True}) + response = self.client.get(url, {'identifiers': self.notenrolled_student.email, 'action': 'add', 'email_students': True}) self.assertEqual(response.status_code, 200) self.assertEqual( @@ -815,14 +866,14 @@ class TestInstructorAPIBulkBetaEnrollment(ModuleStoreTestCase, LoginEnrollmentTe def test_enroll_with_email_not_registered(self): # User doesn't exist url = reverse('bulk_beta_modify_access', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.notregistered_email, 'action': 'add', 'email_students': True}) + response = self.client.get(url, {'identifiers': self.notregistered_email, 'action': 'add', 'email_students': True}) self.assertEqual(response.status_code, 200) # test the response data expected = { "action": "add", "results": [ { - "email": self.notregistered_email, + "identifier": self.notregistered_email, "error": True, "userDoesNotExist": True } @@ -836,7 +887,7 @@ class TestInstructorAPIBulkBetaEnrollment(ModuleStoreTestCase, LoginEnrollmentTe def test_remove_without_email(self): url = reverse('bulk_beta_modify_access', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.beta_tester.email, 'action': 'remove', 'email_students': False}) + response = self.client.get(url, {'identifiers': self.beta_tester.email, 'action': 'remove', 'email_students': False}) self.assertEqual(response.status_code, 200) self.assertFalse(CourseBetaTesterRole(self.course.location).has_user(self.beta_tester)) @@ -846,7 +897,7 @@ class TestInstructorAPIBulkBetaEnrollment(ModuleStoreTestCase, LoginEnrollmentTe "action": "remove", "results": [ { - "email": self.beta_tester.email, + "identifier": self.beta_tester.email, "error": False, "userDoesNotExist": False } @@ -860,7 +911,7 @@ class TestInstructorAPIBulkBetaEnrollment(ModuleStoreTestCase, LoginEnrollmentTe def test_remove_with_email(self): url = reverse('bulk_beta_modify_access', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.beta_tester.email, 'action': 'remove', 'email_students': True}) + response = self.client.get(url, {'identifiers': self.beta_tester.email, 'action': 'remove', 'email_students': True}) self.assertEqual(response.status_code, 200) self.assertFalse(CourseBetaTesterRole(self.course.location).has_user(self.beta_tester)) @@ -870,7 +921,7 @@ class TestInstructorAPIBulkBetaEnrollment(ModuleStoreTestCase, LoginEnrollmentTe "action": "remove", "results": [ { - "email": self.beta_tester.email, + "identifier": self.beta_tester.email, "error": False, "userDoesNotExist": False } diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 170f6044e4..5da85632fa 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/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 }) diff --git a/lms/static/coffee/src/instructor_dashboard/membership.coffee b/lms/static/coffee/src/instructor_dashboard/membership.coffee index d7426734e6..bdd63a1089 100644 --- a/lms/static/coffee/src/instructor_dashboard/membership.coffee +++ b/lms/static/coffee/src/instructor_dashboard/membership.coffee @@ -178,7 +178,7 @@ class AuthListWidget extends MemberListWidget class BetaTesterBulkAddition constructor: (@$container) -> # gather elements - @$emails_input = @$container.find("textarea[name='student-emails-for-beta']") + @$identifier_input = @$container.find("textarea[name='student-ids-for-beta']") @$btn_beta_testers = @$container.find("input[name='beta-testers']") @$checkbox_emailstudents = @$container.find("input[name='email-students']") @$checkbox_autoenroll = @$container.find("input[name='auto-enroll']") @@ -191,7 +191,7 @@ class BetaTesterBulkAddition autoEnroll = @$checkbox_autoenroll.is(':checked') send_data = action: $(event.target).data('action') # 'add' or 'remove' - emails: @$emails_input.val() + identifiers: @$identifier_input.val() email_students: emailStudents auto_enroll: autoEnroll @@ -204,7 +204,7 @@ class BetaTesterBulkAddition # clear the input text field clear_input: -> - @$emails_input.val '' + @$identifier_input.val '' # default for the checkboxes should be checked @$checkbox_emailstudents.attr('checked', true) @$checkbox_autoenroll.attr('checked', true) @@ -231,37 +231,37 @@ class BetaTesterBulkAddition else successes.push student_results - render_list = (label, emails) => + render_list = (label, ids) => task_res_section = $ '
', class: 'request-res-section' task_res_section.append $ '

', text: label - email_list = $ '
    ' - task_res_section.append email_list + ids_list = $ '
      ' + task_res_section.append ids_list - for email in emails - email_list.append $ '
    • ', text: email + for identifier in ids + ids_list.append $ '
    • ', text: identifier @$task_response.append task_res_section if successes.length and data_from_server.action is 'add' `// Translators: A list of users appears after this sentence` - render_list gettext("These users were successfully added as beta testers:"), (sr.email for sr in successes) + render_list gettext("These users were successfully added as beta testers:"), (sr.identifier for sr in successes) if successes.length and data_from_server.action is 'remove' `// Translators: A list of users appears after this sentence` - render_list gettext("These users were successfully removed as beta testers:"), (sr.email for sr in successes) + render_list gettext("These users were successfully removed as beta testers:"), (sr.identifier for sr in successes) if errors.length and data_from_server.action is 'add' `// Translators: A list of users appears after this sentence` - render_list gettext("These users were not added as beta testers:"), (sr.email for sr in errors) + render_list gettext("These users were not added as beta testers:"), (sr.identifier for sr in errors) if errors.length and data_from_server.action is 'remove' `// Translators: A list of users appears after this sentence` - render_list gettext("These users were not removed as beta testers:"), (sr.email for sr in errors) + render_list gettext("These users were not removed as beta testers:"), (sr.identifier for sr in errors) if no_users.length - no_users.push gettext("Users must create and activate their account before they can be promoted to beta tester.") + no_users.push $ gettext("Users must create and activate their account before they can be promoted to beta tester.") `// Translators: A list of identifiers (which are email addresses and/or usernames) appears after this sentence` - render_list gettext("Could not find users associated with the following identifiers:"), (sr.email for sr in no_users) + render_list gettext("Could not find users associated with the following identifiers:"), (sr.identifier for sr in no_users) # Wrapper for the batch enrollment subsection. # This object handles buttons, success and failure reporting, @@ -269,7 +269,7 @@ class BetaTesterBulkAddition class BatchEnrollment constructor: (@$container) -> # gather elements - @$emails_input = @$container.find("textarea[name='student-emails']") + @$identifier_input = @$container.find("textarea[name='student-ids']") @$enrollment_button = @$container.find(".enrollment-button") @$checkbox_autoenroll = @$container.find("input[name='auto-enroll']") @$checkbox_emailstudents = @$container.find("input[name='email-students']") @@ -281,7 +281,7 @@ class BatchEnrollment emailStudents: @$checkbox_emailstudents.is(':checked') send_data = action: $(event.target).data('action') # 'enroll' or 'unenroll' - emails: @$emails_input.val() + identifiers: @$identifier_input.val() auto_enroll: @$checkbox_autoenroll.is(':checked') email_students: emailStudents @@ -295,7 +295,7 @@ class BatchEnrollment # clear the input text field clear_input: -> - @$emails_input.val '' + @$identifier_input.val '' # default for the checkboxes should be checked @$checkbox_emailstudents.attr('checked', true) @$checkbox_autoenroll.attr('checked', true) @@ -315,8 +315,8 @@ class BatchEnrollment # these results arrays contain student_results # only populated arrays will be rendered # - # invalid email addresses - invalid_email = [] + # invalid identifiers + invalid_identifier = [] # students for which there was an error during the action errors = [] # students who are now enrolled in the course @@ -334,7 +334,7 @@ class BatchEnrollment for student_results in data_from_server.results # for a successful action. # student_results is of the form { - # "email": "jd405@edx.org", + # "identifier": "jd405@edx.org", # "before": { # "enrollment": true, # "auto_enroll": false, @@ -351,13 +351,14 @@ class BatchEnrollment # # for an action error. # student_results is of the form { - # 'email': email, + # 'identifier': identifier, + # # then one of: # 'error': True, - # 'invalidEmail': True, # if email doesn't match "[^@]+@[^@]+\.[^@]+" + # 'invalidIdentifier': True # if identifier can't find a valid User object and doesn't pass validate_email # } - if student_results.invalidEmail - invalid_email.push student_results + if student_results.invalidIdentifier + invalid_identifier.push student_results else if student_results.error errors.push student_results @@ -383,19 +384,19 @@ class BatchEnrollment console.warn student_results # render populated result arrays - render_list = (label, emails) => + render_list = (label, ids) => task_res_section = $ '
      ', class: 'request-res-section' task_res_section.append $ '

      ', text: label - email_list = $ '
        ' - task_res_section.append email_list + ids_list = $ '
          ' + task_res_section.append ids_list - for email in emails - email_list.append $ '
        • ', text: email + for identifier in ids + ids_list.append $ '
        • ', text: identifier @$task_response.append task_res_section - if invalid_email.length - render_list gettext("The following email addresses are invalid:"), (sr.email for sr in invalid_email) + if invalid_identifier.length + render_list gettext("The following email addresses and/or usernames are invalid:"), (sr.identifier for sr in invalid_identifier) if errors.length errors_label = do -> @@ -408,53 +409,53 @@ class BatchEnrollment "There was an error processing:" for student_results in errors - render_list errors_label, (sr.email for sr in errors) + render_list errors_label, (sr.identifier for sr in errors) if enrolled.length and emailStudents - render_list gettext("Successfully enrolled and sent email to the following users:"), (sr.email for sr in enrolled) + render_list gettext("Successfully enrolled and sent email to the following users:"), (sr.identifier for sr in enrolled) if enrolled.length and not emailStudents `// Translators: A list of users appears after this sentence` - render_list gettext("Successfully enrolled the following users:"), (sr.email for sr in enrolled) + render_list gettext("Successfully enrolled the following users:"), (sr.identifier for sr in enrolled) # Student hasn't registered so we allow them to enroll if allowed.length and emailStudents `// Translators: A list of users appears after this sentence` render_list gettext("Successfully sent enrollment emails to the following users. They will be allowed to enroll once they register:"), - (sr.email for sr in allowed) + (sr.identifier for sr in allowed) # Student hasn't registered so we allow them to enroll if allowed.length and not emailStudents `// Translators: A list of users appears after this sentence` render_list gettext("These users will be allowed to enroll once they register:"), - (sr.email for sr in allowed) + (sr.identifier 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 users appears after this sentence` render_list gettext("Successfully sent enrollment emails to the following users. They will be enrolled once they register:"), - (sr.email for sr in autoenrolled) + (sr.identifier 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 users appears after this sentence` render_list gettext("These users will be enrolled once they register:"), - (sr.email for sr in autoenrolled) + (sr.identifier for sr in autoenrolled) if notenrolled.length and emailStudents `// Translators: A list of users appears after this sentence` render_list gettext("Emails successfully sent. The following users are no longer enrolled in the course:"), - (sr.email for sr in notenrolled) + (sr.identifier for sr in notenrolled) if notenrolled.length and not emailStudents `// Translators: A list of users appears after this sentence` render_list gettext("The following users are no longer enrolled in the course:"), - (sr.email for sr in notenrolled) + (sr.identifier for sr in notenrolled) if notunenrolled.length `// Translators: A list of users appears after this sentence` render_list gettext("These users were not affiliated with the course so could not be unenrolled:"), - (sr.email for sr in notunenrolled) + (sr.identifier for sr in notunenrolled) # Wrapper for auth list subsection. # manages a list of users who have special access. diff --git a/lms/templates/instructor/instructor_dashboard_2/membership.html b/lms/templates/instructor/instructor_dashboard_2/membership.html index 9c336d380c..38e78c0f1c 100644 --- a/lms/templates/instructor/instructor_dashboard_2/membership.html +++ b/lms/templates/instructor/instructor_dashboard_2/membership.html @@ -30,9 +30,10 @@

          ${_("Batch Enrollment")}

          -

          @@ -70,14 +71,14 @@ %if section_data['access']['instructor']:
          -

          ${_("Batch Beta Testers")}

          +

          ${_("Batch Beta Tester Addition")}

          -

          @@ -86,7 +87,7 @@

          - ${_("If this option is checked, users who have not enrolled in your course will be automatically enrolled.").format(platform_name=settings.PLATFORM_NAME)} + ${_("If this option is checked, users who have not enrolled in your course will be automatically enrolled.")}

          ${_("Checking this box has no effect if 'Remove beta testers' is selected.")}