diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 227d26c15e..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,21 +748,28 @@ 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 test_add_notenrolled(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}) - self.assertEqual(response.status_code, 200) + 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 '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. + Additionally asserts no email was sent. + """ + self.assertEqual(response.status_code, 200) self.assertTrue(CourseBetaTesterRole(self.course.location).has_user(self.notenrolled_student)) # test the response data expected = { "action": "add", "results": [ { - "email": self.notenrolled_student.email, + "identifier": identifier, "error": False, "userDoesNotExist": False } @@ -726,9 +782,33 @@ class TestInstructorAPIBulkBetaEnrollment(ModuleStoreTestCase, LoginEnrollmentTe # Check the outbox self.assertEqual(len(mail.outbox), 0) + def test_add_notenrolled_email(self): + url = reverse('bulk_beta_modify_access', kwargs={'course_id': self.course.id}) + 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, {'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, {'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, {'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)) @@ -737,7 +817,7 @@ class TestInstructorAPIBulkBetaEnrollment(ModuleStoreTestCase, LoginEnrollmentTe "action": "add", "results": [ { - "email": self.notenrolled_student.email, + "identifier": self.notenrolled_student.email, "error": False, "userDoesNotExist": False } @@ -769,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( @@ -786,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 } @@ -807,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)) @@ -817,7 +897,7 @@ class TestInstructorAPIBulkBetaEnrollment(ModuleStoreTestCase, LoginEnrollmentTe "action": "remove", "results": [ { - "email": self.beta_tester.email, + "identifier": self.beta_tester.email, "error": False, "userDoesNotExist": False } @@ -831,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)) @@ -841,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 bd1573845b..ef5ef690b0 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -33,7 +33,7 @@ from django_comment_common.models import ( ) from courseware.models import StudentModule -from student.models import unique_id_for_user +from student.models import unique_id_for_user, CourseEnrollment import instructor_task.api from instructor_task.api_helper import AlreadyRunningError from instructor_task.views import get_task_completion_info @@ -204,7 +204,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. @@ -212,7 +212,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. @@ -244,8 +244,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] @@ -255,23 +255,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': @@ -281,20 +281,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 = { @@ -310,7 +319,7 @@ def students_update_enrollment(request, course_id): @require_level('instructor') @common_exceptions_400 @require_query_params( - emails="stringified list of emails", + identifiers="stringified list of emails and/or usernames", action="add or remove", ) def bulk_beta_modify_access(request, course_id): @@ -318,13 +327,15 @@ 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 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. - 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 = [] rolename = 'beta' course = get_course_by_id(course_id) @@ -333,11 +344,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 = User.objects.get(email=email) + user = get_student_from_identifier(identifier) if action == 'add': allow_access(course, user, rolename) @@ -360,10 +371,16 @@ def bulk_beta_modify_access(request, course_id): # If no exception thrown, see if we should send an email if email_students: send_beta_role_email(action, user, email_params) + # See if we should autoenroll the student + if auto_enroll: + # Check if student is already enrolled + if not CourseEnrollment.is_enrolled(user, course_id): + CourseEnrollment.enroll(user, course_id) + finally: # Tabulate the action result of this email address results.append({ - 'email': email, + 'identifier': identifier, 'error': error, 'userDoesNotExist': user_does_not_exist }) @@ -543,8 +560,9 @@ def get_students_features(request, course_id, csv=False): # pylint: disable=W06 student_data = analytics.basic.enrolled_students_features(course_id, query_features) - # Scrape the query features for i18n - can't translate here because it breaks further queries - # and how the coffeescript works. The actual translation will be done in data_download.coffee + # Provide human-friendly and translatable names for these features. These names + # will be displayed in the table generated in data_download.coffee. It is not (yet) + # used as the header row in the CSV, but could be in the future. query_features_names = { 'username': _('Username'), 'name': _('Name'), @@ -1086,7 +1104,7 @@ def update_forum_role_membership(request, course_id): target_is_instructor = has_access(user, course, 'instructor') # cannot revoke instructor if target_is_instructor and action == 'revoke' and rolename == FORUM_ROLE_ADMINISTRATOR: - return HttpResponseBadRequest("Cannot revoke instructor forum admin privelages.") + return HttpResponseBadRequest("Cannot revoke instructor forum admin privileges.") try: update_forum_role(course_id, user, rolename, action) diff --git a/lms/static/coffee/src/instructor_dashboard/membership.coffee b/lms/static/coffee/src/instructor_dashboard/membership.coffee index 78311441e0..bdd63a1089 100644 --- a/lms/static/coffee/src/instructor_dashboard/membership.coffee +++ b/lms/static/coffee/src/instructor_dashboard/membership.coffee @@ -178,19 +178,22 @@ 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']") @$task_response = @$container.find(".request-response") @$request_response_error = @$container.find(".request-response-error") # click handlers @$btn_beta_testers.click => emailStudents = @$checkbox_emailstudents.is(':checked') + 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 $.ajax dataType: 'json' @@ -199,13 +202,22 @@ class BetaTesterBulkAddition success: (data) => @display_response data error: std_ajax_err => @fail_with_error gettext "Error adding/removing users as beta testers." + # clear the input text field + clear_input: -> + @$identifier_input.val '' + # default for the checkboxes should be checked + @$checkbox_emailstudents.attr('checked', true) + @$checkbox_autoenroll.attr('checked', true) + fail_with_error: (msg) -> console.warn msg + @clear_input() @$task_response.empty() @$request_response_error.empty() @$request_response_error.text msg display_response: (data_from_server) -> + @clear_input() @$task_response.empty() @$request_response_error.empty() errors = [] @@ -219,39 +231,37 @@ class BetaTesterBulkAddition else successes.push student_results - console.log(sr.email for sr in successes) - - render_list = (label, emails) => + render_list = (label, ids) => task_res_section = $ '
', class: 'request-res-section' task_res_section.append $ '

', text: label - email_list = $ '