Merge pull request #2941 from edx/sarina/lms-1287

Sarina/lms 1287

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))

lms/djangoapps/instructor/tests/test_api.py

+# -*- 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}),
@@ -607,6 +610,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):
     """
     Test endpoints whereby instructors can change permissions

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)

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
@@ -283,6 +288,76 @@ def students_update_enrollment(request, course_id):
 @require_level('instructor')
 @common_exceptions_400
 @require_query_params(
+    emails="stringified list of emails",
+    action="add or remove",
+)
+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':
+                revoke_access(course, user, rolename)
+            else:
+                return HttpResponseBadRequest(strip_tags(
+                    "Unrecognized action '{}'".format(action)
+                ))
+        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=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': error,
+                'userDoesNotExist': user_does_not_exist
+            })
+
+    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')
+@common_exceptions_400
+@require_query_params(
     unique_student_identifier="email or username of user to change access",
     rolename="'instructor', 'staff', or 'beta'",
     action="'allow' or 'revoke'"

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_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>/csv)?$',

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_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}),

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;
@@ -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;
   }

lms/templates/emails/add_beta_tester_email_message.txt

+<%! 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
+	)}

lms/templates/emails/add_beta_tester_email_subject.txt

+<%! 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
+	)}

lms/templates/emails/remove_beta_tester_email_message.txt

+<%! 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
+	)}

lms/templates/emails/remove_beta_tester_email_subject.txt

+<%! 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
+	)}

lms/templates/instructor/instructor_dashboard_2/membership.html

@@ -26,40 +26,72 @@
   </div>
 </script>
 
-<div class="vert-left batch-enrollment">
+<div class="vert-left">
+<div class="batch-enrollment">
   <h2> ${_("Batch Enrollment")} </h2>
   <p>
-    <label for="student-emails">${_("Enter student emails separated by new lines or commas.")} </label>
-    <textarea rows="6" name="student-emails" placeholder="${_("Student Emails")}" spellcheck="false"></textarea>
+    <label for="student-emails">${_("Enter email addresses separated by new lines or commas.")} </label>
+    <textarea rows="6" name="student-emails" placeholder="${_("Email Addresses")}" spellcheck="false"></textarea>
   </p>
 
   <div class="enroll-option">
-    <input type="checkbox" name="auto-enroll" value="Auto-Enroll" style="margin-top: 1em;">
+    <input type="checkbox" name="auto-enroll" value="Auto-Enroll">
     <label for="auto-enroll">${_("Auto Enroll")}</label>
     <div class="hint auto-enroll-hint">
       <span class="hint-caret"></span>
-      <p> ${_("If auto enroll is <em>checked</em>, students who have not yet registered for edX will be automatically enrolled.")}
-          ${_("If auto enroll is left <em>unchecked</em>, students who have not yet registered for edX will not be enrolled, but will be allowed to enroll.")}</p>
+      <p> ${_("If this option is <em>checked</em>, users who have not yet registered for {platform_name} will be automatically enrolled.").format(platform_name=settings.PLATFORM_NAME)}
+          ${_("If this option is left <em>unchecked</em>, 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)}</p>
     </div>
   </div>
 
   <div class="enroll-option">
     <input type="checkbox" name="email-students" value="Notify-students-by-email">
-    <label for="email-students">${_("Notify students by email")}</label>
+    <label for="email-students">${_("Notify users by email")}</label>
     <div class="hint email-students-hint">
       <span class="hint-caret"></span>
-      <p> ${_("If email students is <em>checked</em> students will receive an email notification.")}</p>
+      <p> ${_("If this option is <em>checked</em>, users will receive an email notification.")}</p>
     </div>
   </div>
 
-  <div class="enroll-actions actions">
-    <input type="button" name="enroll" value="${_("Enroll")}" data-endpoint="${ section_data['enroll_button_url'] }" >
-    <input type="button" name="unenroll" value="${_("Unenroll")}" data-endpoint="${ section_data['unenroll_button_url'] }" >
+  <div>
+    <input type="button" name="enrollment-button" class="enrollment-button" value="${_("Enroll")}" data-endpoint="${ section_data['enroll_button_url'] }" data-action="enroll" >
+    <input type="button" name="enrollment-button" class="enrollment-button" value="${_("Unenroll")}" data-endpoint="${ section_data['unenroll_button_url'] }" data-action="unenroll" >
+  </div>
+  <div class="request-response"></div>
+  <div class="request-response-error"></div>
+</div>
+
+%if section_data['access']['instructor']:
+<div class="batch-beta-testers">
+  <h2> ${_("Batch Beta Testers")} </h2>
+  <p>
+    <label for="student-emails-for-beta">
+      ${_("Enter email addresses separated by new lines or commas.")}<br/>
+      ${_("Note: Users must have an activated {platform_name} account before they can be enrolled as a beta tester.").format(platform_name=settings.PLATFORM_NAME)}
+    </label>
+
+  <textarea rows="6" cols="50" name="student-emails-for-beta" placeholder="${_("Email addresses")}" spellcheck="false"></textarea>
+  </p>
+
+  <div class="enroll-option">
+    <input type="checkbox" name="email-students" value="Notify-students-by-email">
+    <label for="email-students-beta">${_("Notify users by email")}</label>
+    <div class="hint email-students-beta-hint">
+      <span class="hint-caret"></span>
+      <p> ${_("If this option is <em>checked</em>, users will receive an email notification.")}</p>
+    </div>
+  </div>
+
+  <div>
+    <input type="button" name="beta-testers" class="enrollment-button" value="${_("Add beta tester(s)")}" data-endpoint="${ section_data['modify_beta_testers_button_url'] }" data-action="add" >
+    <input type="button" name="beta-testers" class="enrollment-button" value="${_("Remove beta tester(s)")}" data-endpoint="${ section_data['modify_beta_testers_button_url'] }" data-action="remove" >
   </div>
 
   <div class="request-response"></div>
   <div class="request-response-error"></div>
 </div>
+%endif
+</div>
 
 <div class="vert-right member-lists-management">
   <h2> ${_("Administration List Management")} </h2>