From d7c9491f233f3fcc1f7db514e2478f6e070770ed Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Fri, 7 Mar 2014 14:37:16 -0500 Subject: [PATCH 1/2] Bulk beta tester add/remove on instructor dashboard LMS-1287 --- lms/djangoapps/instructor/views/api.py | 48 +++++++++ lms/djangoapps/instructor/views/api_urls.py | 2 + .../instructor/views/instructor_dashboard.py | 1 + .../instructor_dashboard/membership.coffee | 99 +++++++++++++------ .../sass/course/instructor/_instructor_2.scss | 2 +- .../instructor_dashboard_2/membership.html | 35 ++++++- 6 files changed, 154 insertions(+), 33 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index b02396429a..f6ac3fe1ca 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -278,6 +278,54 @@ def students_update_enrollment(request, course_id): return JsonResponse(response_payload) +@ensure_csrf_cookie +@cache_control(no_cache=True, no_store=True, must_revalidate=True) +@require_level('instructor') +@common_exceptions_400 +@require_query_params( + emails="stringified list of emails", + action="add or remove", +) +def bulk_modify_access(request, course_id): + action = request.GET.get('action') + emails_raw = request.GET.get('emails') + emails = _split_input_list(emails_raw) + results = [] + rolename = 'beta' + course = get_course_by_id(course_id) + for email in emails: + try: + user = User.objects.get(email=email) + if action == 'add': + allow_access(course, user, rolename) + elif action == 'remove': + revoke_access(course, user, rolename) + else: + return HttpResponseBadRequest(strip_tags( + "Unrecognized action '{}'".format(action) + )) + results.append({ + 'email': email, + 'error': False, + }) + + # catch and log any exceptions + # so that one error doesn't cause a 500. + except Exception as exc: # pylint: disable=W0703 + log.exception("Error while #{}ing student") + log.exception(exc) + results.append({ + 'email': email, + 'error': True, + }) + + response_payload = { + 'action': action, + 'results': results, + } + return JsonResponse(response_payload) + + @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) @require_level('instructor') diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index 4ab668fb24..e2fbeba3c8 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -11,6 +11,8 @@ urlpatterns = patterns('', # nopep8 'instructor.views.api.list_course_role_members', name="list_course_role_members"), url(r'^modify_access$', 'instructor.views.api.modify_access', name="modify_access"), + url(r'^bulk_modify_access$', + 'instructor.views.api.bulk_modify_access', name="bulk_modify_access"), url(r'^get_grading_config$', 'instructor.views.api.get_grading_config', name="get_grading_config"), url(r'^get_students_features(?P/csv)?$', diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 56ec3f7325..ce532c1e94 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -146,6 +146,7 @@ def _section_membership(course_id, access): 'access': access, 'enroll_button_url': reverse('students_update_enrollment', kwargs={'course_id': course_id}), 'unenroll_button_url': reverse('students_update_enrollment', kwargs={'course_id': course_id}), + 'modify_beta_testers_button_url': reverse('bulk_modify_access', kwargs={'course_id': course_id}), 'list_course_role_members_url': reverse('list_course_role_members', kwargs={'course_id': course_id}), 'modify_access_url': reverse('modify_access', kwargs={'course_id': course_id}), 'list_forum_members_url': reverse('list_forum_members', kwargs={'course_id': course_id}), diff --git a/lms/static/coffee/src/instructor_dashboard/membership.coffee b/lms/static/coffee/src/instructor_dashboard/membership.coffee index 64d860e474..849ecd92c7 100644 --- a/lms/static/coffee/src/instructor_dashboard/membership.coffee +++ b/lms/static/coffee/src/instructor_dashboard/membership.coffee @@ -160,50 +160,90 @@ class AuthListWidget extends MemberListWidget error: std_ajax_err => cb? gettext "Error changing user's permissions." +class BetaTesterBulkAddition + constructor: (@$container) -> + # gather elements + @$emails_input = @$container.find("textarea[name='student-emails-for-beta']") + @$btn_beta_testers = @$container.find("input[name='beta-testers']") + # @$checkbox_emailstudents = @$container.find("input[name='email-students']") + @$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') + send_data = + action: $(event.target).data('action') + emails: @$emails_input.val() + email_students: emailStudents + + $.ajax + dataType: 'json' + url: @$btn_beta_testers.data 'endpoint' + data: send_data + success: (data) => @display_response data + # error: std_ajax_err => @fail_with_error "Error enrolling/unenrolling students." + + # success: (data) => @display_response data + + # fail_with_error: (msg) -> + # console.warn msg + # @$task_response.empty() + # @$request_response_error.empty() + # @$request_response_error.text msg + + display_response: (data_from_server) -> + @$task_response.empty() + # @$request_response_error.empty() + errors = [] + sucesses = [] + for student_results in data_from_server.results + if student_results.error + errors.push student_results + else + sucesses.push student_results + + console.log(sr.email for sr in sucesses) + + render_list = (label, emails) => + task_res_section = $ '
', class: 'request-res-section' + task_res_section.append $ '

', text: label + email_list = $ '
    ' + task_res_section.append email_list + + for email in emails + email_list.append $ '
  • ', text: email + + @$task_response.append task_res_section + + render_list gettext("these students were added as beta testers"), (sr.email for sr in sucesses) + render_list gettext("these students were not added as beta testers"), (sr.email for sr in errors) + # Wrapper for the batch enrollment subsection. # This object handles buttons, success and failure reporting, # and server communication. class BatchEnrollment constructor: (@$container) -> # gather elements - @$emails_input = @$container.find("textarea[name='student-emails']'") - @$btn_enroll = @$container.find("input[name='enroll']'") - @$btn_unenroll = @$container.find("input[name='unenroll']'") - @$checkbox_autoenroll = @$container.find("input[name='auto-enroll']'") - @$checkbox_emailstudents = @$container.find("input[name='email-students']'") + @$emails_input = @$container.find("textarea[name='student-emails']") + @$enrollment_button = @$container.find(".enrollment-button") + @$checkbox_autoenroll = @$container.find("input[name='auto-enroll']") + @$checkbox_emailstudents = @$container.find("input[name='email-students']") @$task_response = @$container.find(".request-response") @$request_response_error = @$container.find(".request-response-error") - # attach click handlers - - @$btn_enroll.click => - emailStudents = @$checkbox_emailstudents.is(':checked') - + # attach click handler for enrollment buttons + @$enrollment_button.click => + emailStudents: @$checkbox_emailstudents.is(':checked') send_data = - action: 'enroll' + action: $(event.target).data('action') # 'enroll' or 'unenroll' emails: @$emails_input.val() auto_enroll: @$checkbox_autoenroll.is(':checked') email_students: emailStudents $.ajax dataType: 'json' - url: @$btn_enroll.data 'endpoint' - data: send_data - success: (data) => @display_response data - error: std_ajax_err => @fail_with_error "Error enrolling/unenrolling students." - - @$btn_unenroll.click => - emailStudents = @$checkbox_emailstudents.is(':checked') - - send_data = - action: 'unenroll' - emails: @$emails_input.val() - auto_enroll: @$checkbox_autoenroll.is(':checked') - email_students: emailStudents - - $.ajax - dataType: 'json' - url: @$btn_unenroll.data 'endpoint' + url: $(event.target).data 'endpoint' data: send_data success: (data) => @display_response data error: std_ajax_err => @fail_with_error gettext "Error enrolling/unenrolling students." @@ -475,6 +515,9 @@ class Membership # isolate # initialize BatchEnrollment subsection plantTimeout 0, => new BatchEnrollment @$section.find '.batch-enrollment' + + # initialize BetaTesterBulkAddition subsection + plantTimeout 0, => new BetaTesterBulkAddition @$section.find '.batch-beta-testers' # gather elements @$list_selector = @$section.find 'select#member-lists-selector' diff --git a/lms/static/sass/course/instructor/_instructor_2.scss b/lms/static/sass/course/instructor/_instructor_2.scss index eff6a56c4a..ab73566b32 100644 --- a/lms/static/sass/course/instructor/_instructor_2.scss +++ b/lms/static/sass/course/instructor/_instructor_2.scss @@ -292,7 +292,7 @@ section.instructor-dashboard-content-2 { margin-right: 0; } - .batch-enrollment { + .batch-enrollment, .batch-beta-testers { textarea { margin-top: 0.2em; height: auto; diff --git a/lms/templates/instructor/instructor_dashboard_2/membership.html b/lms/templates/instructor/instructor_dashboard_2/membership.html index 5fb595add2..14634892ba 100644 --- a/lms/templates/instructor/instructor_dashboard_2/membership.html +++ b/lms/templates/instructor/instructor_dashboard_2/membership.html @@ -26,7 +26,8 @@

-
+
+

${_("Batch Enrollment")}

@@ -51,15 +52,41 @@

${_("If email students is checked students will receive an email notification.")}

+ +
+ + +
+
+
+
-
- - +%if section_data['access']['instructor']: +
+

${_("Add Beta Testers")}

+

${_("Enter student emails separated by new lines or commas.")}

+ +
+ +
+ + + +
+ +
+ +
+%endif +

${_("Administration List Management")}

From 8f24a5fd5ea3fb0d94a9c6abd2816a9c781a6bae Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Thu, 13 Mar 2014 22:58:05 -0400 Subject: [PATCH 2/2] Batch add/remove beta testers on beta dash LMS-1287 --- lms/djangoapps/instructor/enrollment.py | 34 +++- lms/djangoapps/instructor/tests/test_api.py | 192 +++++++++++++++++- .../instructor/tests/test_enrollment.py | 26 ++- lms/djangoapps/instructor/views/api.py | 47 ++++- lms/djangoapps/instructor/views/api_urls.py | 4 +- .../instructor/views/instructor_dashboard.py | 2 +- .../instructor_dashboard/membership.coffee | 91 +++++---- .../sass/course/instructor/_instructor_2.scss | 8 +- .../emails/add_beta_tester_email_message.txt | 17 ++ .../emails/add_beta_tester_email_subject.txt | 5 + .../remove_beta_tester_email_message.txt | 18 ++ .../remove_beta_tester_email_subject.txt | 5 + .../instructor_dashboard_2/membership.html | 47 +++-- 13 files changed, 420 insertions(+), 76 deletions(-) create mode 100644 lms/templates/emails/add_beta_tester_email_message.txt create mode 100644 lms/templates/emails/add_beta_tester_email_subject.txt create mode 100644 lms/templates/emails/remove_beta_tester_email_message.txt create mode 100644 lms/templates/emails/remove_beta_tester_email_subject.txt diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index e301a41630..d9627e03ff 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -140,6 +140,30 @@ def unenroll_email(course_id, student_email, email_students=False, email_params= return previous_state, after_state +def send_beta_role_email(action, user, email_params): + """ + Send an email to a user added or removed as a beta tester. + + `action` is one of 'add' or 'remove' + `user` is the User affected + `email_params` parameters used while parsing email templates (a `dict`). + """ + if action == 'add': + email_params['message'] = 'add_beta_tester' + email_params['email_address'] = user.email + email_params['full_name'] = user.profile.name + + elif action == 'remove': + email_params['message'] = 'remove_beta_tester' + email_params['email_address'] = user.email + email_params['full_name'] = user.profile.name + + else: + raise ValueError("Unexpected action received '{}' - expected 'add' or 'remove'".format(action)) + + send_mail_to_student(user.email, email_params) + + def reset_student_attempts(course_id, student, module_state_key, delete_module=False): """ Reset student attempts for a problem. Optionally deletes all student state for the specified problem. @@ -257,7 +281,15 @@ def send_mail_to_student(student, param_dict): 'enrolled_unenroll': ( 'emails/unenroll_email_subject.txt', 'emails/unenroll_email_enrolledmessage.txt' - ) + ), + 'add_beta_tester': ( + 'emails/add_beta_tester_email_subject.txt', + 'emails/add_beta_tester_email_message.txt' + ), + 'remove_beta_tester': ( + 'emails/remove_beta_tester_email_subject.txt', + 'emails/remove_beta_tester_email_message.txt' + ), } subject_template, message_template = email_template_dict.get(message_type, (None, None)) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 352f5c93ad..754034e487 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- """ Unit tests for instructor.api methods. """ @@ -23,7 +24,8 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from courseware.tests.helpers import LoginEnrollmentTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from student.tests.factories import UserFactory -from courseware.tests.factories import StaffFactory, InstructorFactory +from courseware.tests.factories import StaffFactory, InstructorFactory, BetaTesterFactory +from student.roles import CourseBetaTesterRole from student.models import CourseEnrollment, CourseEnrollmentAllowed from courseware.models import StudentModule @@ -135,6 +137,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'}), ('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}), @@ -606,6 +609,193 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): ) +@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) +class TestInstructorAPIBulkBetaEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): + """ + Test bulk beta modify access endpoint. + """ + def setUp(self): + self.course = CourseFactory.create() + self.instructor = InstructorFactory(course=self.course.location) + self.client.login(username=self.instructor.username, password='test') + + self.beta_tester = BetaTesterFactory(course=self.course.location) + CourseEnrollment.enroll( + self.beta_tester, + self.course.id + ) + + self.notenrolled_student = UserFactory(username='NotEnrolledStudent') + + self.notregistered_email = 'robot-not-an-email-yet@robot.org' + self.assertEqual(User.objects.filter(email=self.notregistered_email).count(), 0) + + # uncomment to enable enable printing of large diffs + # from failed assertions in the event of a test failure. + # (comment because pylint C0103) + # self.maxDiff = None + + def test_missing_params(self): + """ Test missing all query parameters. """ + url = reverse('bulk_beta_modify_access', kwargs={'course_id': self.course.id}) + response = self.client.get(url) + self.assertEqual(response.status_code, 400) + + def test_bad_action(self): + """ 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}) + 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) + + self.assertTrue(CourseBetaTesterRole(self.course.location).has_user(self.notenrolled_student)) + # test the response data + expected = { + "action": "add", + "results": [ + { + "email": self.notenrolled_student.email, + "error": False, + "userDoesNotExist": False + } + ] + } + + res_json = json.loads(response.content) + self.assertEqual(res_json, expected) + + # Check the outbox + self.assertEqual(len(mail.outbox), 0) + + 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}) + 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, + "error": False, + "userDoesNotExist": False + } + ] + } + res_json = json.loads(response.content) + self.assertEqual(res_json, expected) + + # Check the outbox + self.assertEqual(len(mail.outbox), 1) + self.assertEqual( + mail.outbox[0].subject, + 'You have been invited to a beta test for Robot Super Course' + ) + self.assertEqual( + mail.outbox[0].body, + u"Dear {0}\n\nYou have been invited to be a beta tester " + "for Robot Super Course at edx.org by a member of the course staff.\n\n" + "Visit https://edx.org/courses/MITx/999/Robot_Super_Course/about to join " + "the course and begin the beta test.\n\n----\n" + "This email was automatically sent from edx.org to {1}".format( + self.notenrolled_student.profile.name, + self.notenrolled_student.email + ) + ) + + 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}) + self.assertEqual(response.status_code, 200) + # test the response data + expected = { + "action": "add", + "results": [ + { + "email": self.notregistered_email, + "error": True, + "userDoesNotExist": True + } + ] + } + res_json = json.loads(response.content) + self.assertEqual(res_json, expected) + + # Check the outbox + self.assertEqual(len(mail.outbox), 0) + + 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}) + self.assertEqual(response.status_code, 200) + + self.assertFalse(CourseBetaTesterRole(self.course.location).has_user(self.beta_tester)) + + # test the response data + expected = { + "action": "remove", + "results": [ + { + "email": self.beta_tester.email, + "error": False, + "userDoesNotExist": False + } + ] + } + res_json = json.loads(response.content) + self.assertEqual(res_json, expected) + + # Check the outbox + self.assertEqual(len(mail.outbox), 0) + + 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}) + self.assertEqual(response.status_code, 200) + + self.assertFalse(CourseBetaTesterRole(self.course.location).has_user(self.beta_tester)) + + # test the response data + expected = { + "action": "remove", + "results": [ + { + "email": self.beta_tester.email, + "error": False, + "userDoesNotExist": False + } + ] + } + res_json = json.loads(response.content) + self.assertEqual(res_json, expected) + # Check the outbox + self.assertEqual(len(mail.outbox), 1) + self.assertEqual( + mail.outbox[0].subject, + 'You have been removed from a beta test for Robot Super Course' + ) + self.assertEqual( + mail.outbox[0].body, + "Dear {full_name}\n\nYou have been removed as a beta tester for " + "Robot Super Course at edx.org by a member of the course staff. " + "The course will remain on your dashboard, but you will no longer " + "be part of the beta testing group.\n\n" + "Your other courses have not been affected.\n\n----\n" + "This email was automatically sent from edx.org to {email_address}".format( + full_name=self.beta_tester.profile.name, + email_address=self.beta_tester.email + ) + ) + + @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) class TestInstructorAPILevelsAccess(ModuleStoreTestCase, LoginEnrollmentTestCase): """ diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index d8202bf534..98b91732cf 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -9,9 +9,13 @@ from django.test import TestCase from student.tests.factories import UserFactory from student.models import CourseEnrollment, CourseEnrollmentAllowed -from instructor.enrollment import (EmailEnrollmentState, - enroll_email, unenroll_email, - reset_student_attempts) +from instructor.enrollment import ( + EmailEnrollmentState, + enroll_email, + reset_student_attempts, + send_beta_role_email, + unenroll_email +) class TestSettableEnrollmentState(TestCase): @@ -365,3 +369,19 @@ class SettableEnrollmentState(EmailEnrollmentState): return EnrollmentObjects(email, None, None, cea) else: return EnrollmentObjects(email, None, None, None) + + +class TestSendBetaRoleEmail(TestCase): + """ + Test edge cases for `send_beta_role_email` + """ + + def setUp(self): + self.user = UserFactory.create() + self.email_params = {'course': 'Robot Super Course'} + + def test_bad_action(self): + bad_action = 'beta_tester' + error_msg = "Unexpected action received '{}' - expected 'add' or 'remove'".format(bad_action) + with self.assertRaisesRegexp(ValueError, error_msg): + send_beta_role_email(bad_action, self.user, self.email_params) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index f6ac3fe1ca..a8bb467a13 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -37,7 +37,12 @@ from instructor_task.api_helper import AlreadyRunningError from instructor_task.views import get_task_completion_info from instructor_task.models import ReportStore import instructor.enrollment as enrollment -from instructor.enrollment import enroll_email, unenroll_email, get_email_params +from instructor.enrollment import ( + enroll_email, + get_email_params, + send_beta_role_email, + unenroll_email +) from instructor.access import list_with_level, allow_access, revoke_access, update_forum_role import analytics.basic import analytics.distributions @@ -286,16 +291,32 @@ def students_update_enrollment(request, course_id): emails="stringified list of emails", action="add or remove", ) -def bulk_modify_access(request, course_id): +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. + - action is one of ['add', 'remove'] + """ action = request.GET.get('action') emails_raw = request.GET.get('emails') emails = _split_input_list(emails_raw) + email_students = request.GET.get('email_students') in ['true', 'True', True] results = [] rolename = 'beta' course = get_course_by_id(course_id) + + email_params = {} + if email_students: + email_params = get_email_params(course, auto_enroll=False) + for email in emails: try: + error = False + user_does_not_exist = False user = User.objects.get(email=email) + if action == 'add': allow_access(course, user, rolename) elif action == 'remove': @@ -304,19 +325,25 @@ def bulk_modify_access(request, course_id): return HttpResponseBadRequest(strip_tags( "Unrecognized action '{}'".format(action) )) - results.append({ - 'email': email, - 'error': False, - }) - - # catch and log any exceptions + except User.DoesNotExist: + error = True + user_does_not_exist = True + # catch and log any unexpected exceptions # so that one error doesn't cause a 500. - except Exception as exc: # pylint: disable=W0703 + except Exception as exc: # pylint: disable=broad-except log.exception("Error while #{}ing student") log.exception(exc) + error = True + else: + # If no exception thrown, see if we should send an email + if email_students: + send_beta_role_email(action, user, email_params) + finally: + # Tabulate the action result of this email address results.append({ 'email': email, - 'error': True, + 'error': error, + 'userDoesNotExist': user_does_not_exist }) response_payload = { diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index e2fbeba3c8..96064d2135 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -11,8 +11,8 @@ urlpatterns = patterns('', # nopep8 'instructor.views.api.list_course_role_members', name="list_course_role_members"), url(r'^modify_access$', 'instructor.views.api.modify_access', name="modify_access"), - url(r'^bulk_modify_access$', - 'instructor.views.api.bulk_modify_access', name="bulk_modify_access"), + url(r'^bulk_beta_modify_access$', + 'instructor.views.api.bulk_beta_modify_access', name="bulk_beta_modify_access"), url(r'^get_grading_config$', 'instructor.views.api.get_grading_config', name="get_grading_config"), url(r'^get_students_features(?P/csv)?$', diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index ce532c1e94..33a385beec 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -146,7 +146,7 @@ def _section_membership(course_id, access): 'access': access, 'enroll_button_url': reverse('students_update_enrollment', kwargs={'course_id': course_id}), 'unenroll_button_url': reverse('students_update_enrollment', kwargs={'course_id': course_id}), - 'modify_beta_testers_button_url': reverse('bulk_modify_access', kwargs={'course_id': course_id}), + 'modify_beta_testers_button_url': reverse('bulk_beta_modify_access', kwargs={'course_id': course_id}), 'list_course_role_members_url': reverse('list_course_role_members', kwargs={'course_id': course_id}), 'modify_access_url': reverse('modify_access', kwargs={'course_id': course_id}), 'list_forum_members_url': reverse('list_forum_members', kwargs={'course_id': course_id}), diff --git a/lms/static/coffee/src/instructor_dashboard/membership.coffee b/lms/static/coffee/src/instructor_dashboard/membership.coffee index 849ecd92c7..cd0379c2cd 100644 --- a/lms/static/coffee/src/instructor_dashboard/membership.coffee +++ b/lms/static/coffee/src/instructor_dashboard/membership.coffee @@ -165,15 +165,15 @@ class BetaTesterBulkAddition # gather elements @$emails_input = @$container.find("textarea[name='student-emails-for-beta']") @$btn_beta_testers = @$container.find("input[name='beta-testers']") - # @$checkbox_emailstudents = @$container.find("input[name='email-students']") + @$checkbox_emailstudents = @$container.find("input[name='email-students']") @$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') + emailStudents = @$checkbox_emailstudents.is(':checked') send_data = - action: $(event.target).data('action') + action: $(event.target).data('action') # 'add' or 'remove' emails: @$emails_input.val() email_students: emailStudents @@ -182,28 +182,29 @@ class BetaTesterBulkAddition url: @$btn_beta_testers.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 adding/removing user(s) as beta tester(s)." - # success: (data) => @display_response data - - # fail_with_error: (msg) -> - # console.warn msg - # @$task_response.empty() - # @$request_response_error.empty() - # @$request_response_error.text msg + fail_with_error: (msg) -> + console.warn msg + @$task_response.empty() + @$request_response_error.empty() + @$request_response_error.text msg display_response: (data_from_server) -> @$task_response.empty() - # @$request_response_error.empty() + @$request_response_error.empty() errors = [] - sucesses = [] + successes = [] + no_users = [] for student_results in data_from_server.results - if student_results.error + if student_results.userDoesNotExist + no_users.push student_results + else if student_results.error errors.push student_results else - sucesses.push student_results + successes.push student_results - console.log(sr.email for sr in sucesses) + console.log(sr.email for sr in successes) render_list = (label, emails) => task_res_section = $ '
', class: 'request-res-section' @@ -216,8 +217,26 @@ class BetaTesterBulkAddition @$task_response.append task_res_section - render_list gettext("these students were added as beta testers"), (sr.email for sr in sucesses) - render_list gettext("these students were not added as beta testers"), (sr.email for sr in errors) + if successes.length and data_from_server.action is 'add' + `// Translators: A list of users appears after this sentence` + render_list gettext("These user(s) were successfully added as beta tester(s):"), (sr.email 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 user(s) were successfully removed as beta tester(s):"), (sr.email 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 user(s) were not added as beta tester(s):"), (sr.email 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 user(s) were not removed as beta tester(s):"), (sr.email 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.") + `// Translators: A list of email addresses appears after this sentence` + render_list gettext("Could not find users associated with the following email addresses:"), (sr.email for sr in no_users) # Wrapper for the batch enrollment subsection. # This object handles buttons, success and failure reporting, @@ -246,7 +265,7 @@ class BatchEnrollment url: $(event.target).data 'endpoint' data: send_data success: (data) => @display_response data - error: std_ajax_err => @fail_with_error gettext "Error enrolling/unenrolling students." + error: std_ajax_err => @fail_with_error gettext "Error enrolling/unenrolling user(s)." fail_with_error: (msg) -> @@ -349,49 +368,49 @@ class BatchEnrollment render_list errors_label, (sr.email for sr in errors) if enrolled.length and emailStudents - render_list gettext("Successfully enrolled and sent email to the following students:"), (sr.email for sr in enrolled) + render_list gettext("Successfully enrolled and sent email to the following user(s):"), (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) + `// Translators: A list of users appears after this sentence` + render_list gettext("Successfully enrolled the following user(s):"), (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:"), + `// Translators: A list of users appears after this sentence` + render_list gettext("Successfully sent enrollment emails to the following user(s). 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:"), + `// Translators: A list of users appears after this sentence` + render_list gettext("These user(s) 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:"), + `// Translators: A list of users appears after this sentence` + render_list gettext("Successfully sent enrollment emails to the following user(s). 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:"), + `// Translators: A list of users appears after this sentence` + render_list gettext("These user(s) 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:"), + `// Translators: A list of users appears after this sentence` + render_list gettext("Emails successfully sent. The following user(s) 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:"), + `// Translators: A list of users appears after this sentence` + render_list gettext("The following user(s) 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:"), + `// Translators: A list of users appears after this sentence` + render_list gettext("These user(s) were not affliliated with the course so could not be unenrolled:"), (sr.email for sr in notunenrolled) # Wrapper for auth list subsection. diff --git a/lms/static/sass/course/instructor/_instructor_2.scss b/lms/static/sass/course/instructor/_instructor_2.scss index ab73566b32..7affd0c26e 100644 --- a/lms/static/sass/course/instructor/_instructor_2.scss +++ b/lms/static/sass/course/instructor/_instructor_2.scss @@ -323,7 +323,7 @@ section.instructor-dashboard-content-2 { } .enroll-option { - margin-bottom: ($baseline/2); + margin: ($baseline/2) 0; position: relative; label { @@ -339,6 +339,7 @@ section.instructor-dashboard-content-2 { padding: ($baseline/2); width: 50%; background-color: $light-gray; + box-shadow: 2px 2px 3px $shadow; .hint-caret { display: block; @@ -362,6 +363,11 @@ section.instructor-dashboard-content-2 { display: block; } + label[for="email-students-beta"]:hover + .email-students-beta-hint { + width: 30%; + display: block; + } + .enroll-actions { margin-top: $baseline; } diff --git a/lms/templates/emails/add_beta_tester_email_message.txt b/lms/templates/emails/add_beta_tester_email_message.txt new file mode 100644 index 0000000000..c6b3f3cff4 --- /dev/null +++ b/lms/templates/emails/add_beta_tester_email_message.txt @@ -0,0 +1,17 @@ +<%! from django.utils.translation import ugettext as _ %> + +${_("Dear {full_name}").format(full_name=full_name)} + +${_("You have been invited to be a beta tester for {course_name} at {site_name} by a " + "member of the course staff.").format( + course_name=course.display_name_with_default, + site_name=site_name + )} + +${_("Visit {course_about_url} to join the course and begin the beta test.").format(course_about_url=course_about_url)} + +---- +${_("This email was automatically sent from {site_name} to " + "{email_address}").format( + site_name=site_name, email_address=email_address + )} diff --git a/lms/templates/emails/add_beta_tester_email_subject.txt b/lms/templates/emails/add_beta_tester_email_subject.txt new file mode 100644 index 0000000000..e147d87bad --- /dev/null +++ b/lms/templates/emails/add_beta_tester_email_subject.txt @@ -0,0 +1,5 @@ +<%! from django.utils.translation import ugettext as _ %> + +${_("You have been invited to a beta test for {course_name}").format( + course_name=course.display_name_with_default + )} diff --git a/lms/templates/emails/remove_beta_tester_email_message.txt b/lms/templates/emails/remove_beta_tester_email_message.txt new file mode 100644 index 0000000000..3a7b4df827 --- /dev/null +++ b/lms/templates/emails/remove_beta_tester_email_message.txt @@ -0,0 +1,18 @@ +<%! from django.utils.translation import ugettext as _ %> + +${_("Dear {full_name}").format(full_name=full_name)} + +${_("You have been removed as a beta tester for {course_name} at {site_name} by a " + "member of the course staff. The course will remain on your dashboard, but " + "you will no longer be part of the beta testing group.").format( + course_name=course.display_name_with_default, + site_name=site_name + )} + +${_("Your other courses have not been affected.")} + +---- +${_("This email was automatically sent from {site_name} to " + "{email_address}").format( + site_name=site_name, email_address=email_address + )} diff --git a/lms/templates/emails/remove_beta_tester_email_subject.txt b/lms/templates/emails/remove_beta_tester_email_subject.txt new file mode 100644 index 0000000000..c81f606f8d --- /dev/null +++ b/lms/templates/emails/remove_beta_tester_email_subject.txt @@ -0,0 +1,5 @@ +<%! from django.utils.translation import ugettext as _ %> + +${_("You have been removed from a beta test for {course_name}").format( + course_name=course.display_name_with_default + )} diff --git a/lms/templates/instructor/instructor_dashboard_2/membership.html b/lms/templates/instructor/instructor_dashboard_2/membership.html index 14634892ba..990b665f7f 100644 --- a/lms/templates/instructor/instructor_dashboard_2/membership.html +++ b/lms/templates/instructor/instructor_dashboard_2/membership.html @@ -30,29 +30,29 @@

${_("Batch Enrollment")}

- - + +

- +
-

${_("If auto enroll is checked, students who have not yet registered for edX will be automatically enrolled.")} - ${_("If auto enroll is left unchecked, students who have not yet registered for edX will not be enrolled, but will be allowed to enroll.")}

+

${_("If this option is checked, users who have not yet registered for {platform_name} will be automatically enrolled.").format(platform_name=settings.PLATFORM_NAME)} + ${_("If this option is left unchecked, users who have not yet registered for {platform_name} will not be enrolled, but will be allowed to enroll once they make an account.").format(platform_name=settings.PLATFORM_NAME)}

- +
- +
@@ -63,23 +63,28 @@ %if section_data['access']['instructor']:
-

${_("Add Beta Testers")}

-

${_("Enter student emails separated by new lines or commas.")}

- -
- -
+

${_("Batch Beta Testers")}

+

+ + + +

+ +
- - - +
- - + +