Bulk Email Multiselect (#12301)

TNL-4356

Allows multiple bulk email targets to be specified at once.

-The previous "All" option has been split into "Staff" and "Learners"
-The backend changes made here lay the groundwork for cohort emailing
-The data migration, 0005, is somewhat large and requires deploy attention
-Tests have been updated
-Numerous safe-commit-linter fixes are included

common/djangoapps/util/json_request.py

@@ -66,7 +66,7 @@ class JsonResponse(HttpResponse):
         elif isinstance(resp_obj, QuerySet):
             content = serialize('json', resp_obj)
         else:
-            content = json.dumps(resp_obj, cls=encoder, indent=2, ensure_ascii=False)
+            content = json.dumps(resp_obj, cls=encoder, indent=2, ensure_ascii=True)
         kwargs.setdefault("content_type", "application/json")
         if status:
             kwargs["status"] = status

common/test/acceptance/pages/lms/instructor_dashboard.py

@@ -127,12 +127,10 @@ class BulkEmailPage(PageObject):
         """
         Selects the specified recipient from the selector. Assumes that recipient is not None.
         """
-        recipient_selector_css = "select[name='send_to']"
-        select_option_by_text(
-            self.q(css=self._bounded_selector(recipient_selector_css)), recipient
-        )
+        recipient_selector_css = "input[name='send_to'][value='{}']".format(recipient)
+        self.q(css=self._bounded_selector(recipient_selector_css))[0].click()
 
-    def send_message(self, recipient):
+    def send_message(self, recipients):
         """
         Send a test message to the specified recipient.
         """
@@ -140,7 +138,8 @@ class BulkEmailPage(PageObject):
         test_subject = "Hello"
         test_body = "This is a test email"
 
-        self._select_recipient(recipient)
+        for recipient in recipients:
+            self._select_recipient(recipient)
         self.q(css=self._bounded_selector("input[name='subject']")).fill(test_subject)
         self.q(css=self._bounded_selector("iframe#mce_0_ifr"))[0].click()
         self.q(css=self._bounded_selector("iframe#mce_0_ifr"))[0].send_keys(test_body)
@@ -156,7 +155,7 @@ class BulkEmailPage(PageObject):
         is covered by the bulk_email unit tests.
         """
         confirmation_selector = self._bounded_selector(".msg-confirm")
-        expected_text = u"Your email was successfully queued for sending."
+        expected_text = u"Your email message was successfully queued for sending."
         EmptyPromise(
             lambda: expected_text in self.q(css=confirmation_selector)[0].text,
             "Message Queued Confirmation"

common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py

@@ -58,7 +58,7 @@ class BulkEmailTest(BaseInstructorDashboardTest):
         instructor_dashboard_page = self.visit_instructor_dashboard()
         self.send_email_page = instructor_dashboard_page.select_bulk_email()
 
-    @ddt.data("Myself", "Staff and admins", "All (students, staff, and admins)")
+    @ddt.data(["myself"], ["staff"], ["learners"], ["myself", "staff", "learners"])
     def test_email_queued_for_sending(self, recipient):
         self.assertTrue(self.send_email_page.is_browser_on_page())
         self.send_email_page.send_message(recipient)

lms/djangoapps/bulk_email/migrations/0004_add_email_targets.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('course_groups', '0001_initial'),
+        ('bulk_email', '0003_config_model_feature_flag'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='Target',
+            fields=[
+                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
+                ('target_type', models.CharField(max_length=64, choices=[(b'myself', b'Myself'), (b'staff', b'Staff and instructors'), (b'learners', b'All students'), (b'cohort', b'Specific cohort')])),
+            ],
+        ),
+        migrations.AlterField(
+            model_name='courseemail',
+            name='to_option',
+            field=models.CharField(max_length=64, choices=[(b'deprecated', b'deprecated')]),
+        ),
+        migrations.CreateModel(
+            name='CohortTarget',
+            fields=[
+                ('target_ptr', models.OneToOneField(parent_link=True, auto_created=True, primary_key=True, serialize=False, to='bulk_email.Target')),
+                ('cohort', models.ForeignKey(to='course_groups.CourseUserGroup')),
+            ],
+            bases=('bulk_email.target',),
+        ),
+        migrations.AddField(
+            model_name='courseemail',
+            name='targets',
+            field=models.ManyToManyField(to='bulk_email.Target'),
+        ),
+    ]

lms/djangoapps/bulk_email/migrations/0005_move_target_data.py

+# -*- coding: utf-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+from django.db.utils import DatabaseError
+from bulk_email.models import EMAIL_TARGETS, SEND_TO_MYSELF
+
+def to_option_to_targets(apps, schema_editor):
+    CourseEmail = apps.get_model("bulk_email", "CourseEmail")
+    Target = apps.get_model("bulk_email", "Target")
+    db_alias = schema_editor.connection.alias
+    try:
+        for email in CourseEmail.objects.using(db_alias).all().iterator():
+            new_target, created = Target.objects.using(db_alias).get_or_create(
+                target_type=email.to_option
+            )
+            email.targets.add(new_target)
+            email.save()
+    except DatabaseError:
+        # Student module history table will fail this migration otherwise
+        pass
+
+def targets_to_to_option(apps, schema_editor):
+    CourseEmail = apps.get_model("bulk_email", "CourseEmail")
+    db_alias = schema_editor.connection.alias
+    try:
+        for email in CourseEmail.objects.using(db_alias).all().iterator():
+            # Note this is not a perfect 1:1 backwards migration - targets can hold more information than to_option can.
+            # We use the first valid value from targets, or 'myself' if none can be found
+            email.to_option = next(
+                (
+                    t_type for t_type in (
+                        target.target_type for target in email.targets.all()
+                    ) if t_type in EMAIL_TARGETS
+                ),
+                SEND_TO_MYSELF
+            )
+            email.save()
+    except DatabaseError:
+        # Student module history table will fail this migration otherwise
+        pass
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('bulk_email', '0004_add_email_targets'),
+    ]
+
+    operations = [
+        migrations.RunPython(to_option_to_targets, targets_to_to_option),
+    ]

lms/djangoapps/bulk_email/models.py

@@ -13,27 +13,25 @@ file and check it in at the same time as your model changes. To do that,
 """
 import logging
 import markupsafe
+
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.db import models
 
+from openedx.core.djangoapps.course_groups.models import CourseUserGroup
 from openedx.core.lib.html_to_text import html_to_text
 from openedx.core.lib.mail_utils import wrap_message
 
 from config_models.models import ConfigurationModel
+from student.roles import CourseStaffRole, CourseInstructorRole
 
 from xmodule_django.models import CourseKeyField
+
 from util.keyword_substitution import substitute_keywords_with_data
+from util.query import use_read_replica_if_available
 
 log = logging.getLogger(__name__)
 
-# Bulk email to_options - the send to options that users can
-# select from when they send email.
-SEND_TO_MYSELF = 'myself'
-SEND_TO_STAFF = 'staff'
-SEND_TO_ALL = 'all'
-TO_OPTIONS = [SEND_TO_MYSELF, SEND_TO_STAFF, SEND_TO_ALL]
-
 
 class Email(models.Model):
     """
@@ -52,6 +50,94 @@ class Email(models.Model):
         abstract = True
 
 
+# Bulk email targets - the send to options that users can select from when they send email.
+SEND_TO_MYSELF = 'myself'
+SEND_TO_STAFF = 'staff'
+SEND_TO_LEARNERS = 'learners'
+SEND_TO_COHORT = 'cohort'
+EMAIL_TARGETS = [SEND_TO_MYSELF, SEND_TO_STAFF, SEND_TO_LEARNERS, SEND_TO_COHORT]
+EMAIL_TARGET_DESCRIPTIONS = ['Myself', 'Staff and instructors', 'All students', 'Specific cohort']
+EMAIL_TARGET_CHOICES = zip(EMAIL_TARGETS, EMAIL_TARGET_DESCRIPTIONS)
+
+
+class Target(models.Model):
+    """
+    A way to refer to a particular group (within a course) as a "Send to:" target.
+    """
+    target_type = models.CharField(max_length=64, choices=EMAIL_TARGET_CHOICES)
+
+    class Meta(object):
+        app_label = "bulk_email"
+
+    def __unicode__(self):
+        return "CourseEmail Target for: {}".format(self.target_type)
+
+    def get_users(self, course_id, user_id=None):
+        """
+        Gets the users for a given target.
+
+        Result is returned in the form of a queryset, and may contain duplicates.
+        """
+        staff_qset = CourseStaffRole(course_id).users_with_role()
+        instructor_qset = CourseInstructorRole(course_id).users_with_role()
+        staff_instructor_qset = (staff_qset | instructor_qset)
+        enrollment_qset = User.objects.filter(
+            is_active=True,
+            courseenrollment__course_id=course_id,
+            courseenrollment__is_active=True
+        )
+        if self.target_type == SEND_TO_MYSELF:
+            if user_id is None:
+                raise ValueError("Must define self user to send email to self.")
+            user = User.objects.filter(id=user_id)
+            return use_read_replica_if_available(user)
+        elif self.target_type == SEND_TO_STAFF:
+            return use_read_replica_if_available(staff_instructor_qset)
+        elif self.target_type == SEND_TO_LEARNERS:
+            return use_read_replica_if_available(enrollment_qset.exclude(id__in=staff_instructor_qset))
+        elif self.target_type == SEND_TO_COHORT:
+            return User.objects.none()  # TODO: cohorts aren't hooked up, put that logic here
+        else:
+            raise ValueError("Unrecognized target type {}".format(self.target_type))
+
+
+class CohortTarget(Target):
+    """
+    Subclass of Target, specifically referring to a cohort.
+    """
+    cohort = models.ForeignKey('course_groups.CourseUserGroup')
+
+    class Meta:
+        app_label = "bulk_email"
+
+    def __init__(self, *args, **kwargs):
+        kwargs['target_type'] = SEND_TO_COHORT
+        super(CohortTarget, self).__init__(*args, **kwargs)
+
+    def __unicode__(self):
+        return "CourseEmail CohortTarget: {}".format(self.cohort)
+
+    @classmethod
+    def ensure_valid_cohort(cls, cohort_name, course_id):
+        """
+        Ensures cohort_name is a valid cohort for course_id.
+
+        Returns the cohort if valid, raises an error otherwise.
+        """
+        if cohort_name is None:
+            raise ValueError("Cannot create a CohortTarget without specifying a cohort_name.")
+        try:
+            cohort = CourseUserGroup.get(name=cohort_name, course_id=course_id)
+        except CourseUserGroup.DoesNotExist:
+            raise ValueError(
+                "Cohort {cohort} does not exist in course {course_id}".format(
+                    cohort=cohort_name,
+                    course_id=course_id
+                )
+            )
+        return cohort
+
+
 class CourseEmail(Email):
     """
     Stores information for an email to a course.
@@ -59,22 +145,10 @@ class CourseEmail(Email):
     class Meta(object):
         app_label = "bulk_email"
 
-    # Three options for sending that we provide from the instructor dashboard:
-    # * Myself: This sends an email to the staff member that is composing the email.
-    #
-    # * Staff and instructors: This sends an email to anyone in the staff group and
-    #   anyone in the instructor group
-    #
-    # * All: This sends an email to anyone enrolled in the course, with any role
-    #   (student, staff, or instructor)
-    #
-    TO_OPTION_CHOICES = (
-        (SEND_TO_MYSELF, 'Myself'),
-        (SEND_TO_STAFF, 'Staff and instructors'),
-        (SEND_TO_ALL, 'All')
-    )
     course_id = CourseKeyField(max_length=255, db_index=True)
-    to_option = models.CharField(max_length=64, choices=TO_OPTION_CHOICES, default=SEND_TO_MYSELF)
+    # to_option is deprecated and unused, but dropping db columns is hard so it's still here for legacy reasons
+    to_option = models.CharField(max_length=64, choices=[("deprecated", "deprecated")])
+    targets = models.ManyToManyField(Target)
     template_name = models.CharField(null=True, max_length=255)
     from_addr = models.CharField(null=True, max_length=255)
 
@@ -83,8 +157,8 @@ class CourseEmail(Email):
 
     @classmethod
     def create(
-            cls, course_id, sender, to_option, subject, html_message,
-            text_message=None, template_name=None, from_addr=None):
+            cls, course_id, sender, targets, subject, html_message,
+            text_message=None, template_name=None, from_addr=None, cohort_name=None):
         """
         Create an instance of CourseEmail.
         """
@@ -92,23 +166,32 @@ class CourseEmail(Email):
         if text_message is None:
             text_message = html_to_text(html_message)
 
-        # perform some validation here:
-        if to_option not in TO_OPTIONS:
-            fmt = 'Course email being sent to unrecognized to_option: "{to_option}" for "{course}", subject "{subject}"'
-            msg = fmt.format(to_option=to_option, course=course_id, subject=subject)
-            raise ValueError(msg)
+        new_targets = []
+        for target in targets:
+            # Ensure our desired target exists
+            if target not in EMAIL_TARGETS:
+                fmt = 'Course email being sent to unrecognized target: "{target}" for "{course}", subject "{subject}"'
+                msg = fmt.format(target=target, course=course_id, subject=subject)
+                raise ValueError(msg)
+            elif target == SEND_TO_COHORT:
+                cohort = CohortTarget.ensure_valid_cohort(cohort_name, course_id)
+                new_target, _ = CohortTarget.objects.get_or_create(target_type=target, cohort=cohort)
+            else:
+                new_target, _ = Target.objects.get_or_create(target_type=target)
+            new_targets.append(new_target)
 
         # create the task, then save it immediately:
         course_email = cls(
             course_id=course_id,
             sender=sender,
-            to_option=to_option,
             subject=subject,
             html_message=html_message,
             text_message=text_message,
             template_name=template_name,
             from_addr=from_addr,
         )
+        course_email.save()  # Must exist in db before setting M2M relationship values
+        course_email.targets.add(*new_targets)
         course_email.save()
 
         return course_email
@@ -295,7 +378,7 @@ class BulkEmailFlag(ConfigurationModel):
 
     def __unicode__(self):
         current_model = BulkEmailFlag.current()
-        return u"<BulkEmailFlag: enabled {}, require_course_email_auth: {}>".format(
+        return u"BulkEmailFlag: enabled {}, require_course_email_auth: {}".format(
             current_model.is_enabled(),
             current_model.require_course_email_auth
         )

lms/djangoapps/bulk_email/tasks.py

@@ -37,9 +37,7 @@ from django.core.mail.message import forbid_multi_line_headers
 from django.core.urlresolvers import reverse
 
 from bulk_email.models import (
-    CourseEmail, Optout,
-    SEND_TO_MYSELF, SEND_TO_ALL, TO_OPTIONS,
-    SEND_TO_STAFF,
+    CourseEmail, Optout, Target
 )
 from courseware.courses import get_course
 from openedx.core.lib.courses import course_image_url
@@ -99,53 +97,6 @@ BULK_EMAIL_FAILURE_ERRORS = (
 )
 
 
-def _get_recipient_querysets(user_id, to_option, course_id):
-    """
-    Returns a list of query sets of email recipients corresponding to the
-    requested `to_option` category.
-
-    `to_option` is either SEND_TO_MYSELF, SEND_TO_STAFF, or SEND_TO_ALL.
-
-    Recipients who are in more than one category (e.g. enrolled in the course
-    and are staff or self) will be properly deduped.
-    """
-    if to_option not in TO_OPTIONS:
-        log.error("Unexpected bulk email TO_OPTION found: %s", to_option)
-        raise Exception("Unexpected bulk email TO_OPTION found: {0}".format(to_option))
-
-    if to_option == SEND_TO_MYSELF:
-        user = User.objects.filter(id=user_id)
-        return [use_read_replica_if_available(user)]
-    else:
-        staff_qset = CourseStaffRole(course_id).users_with_role()
-        instructor_qset = CourseInstructorRole(course_id).users_with_role()
-        staff_instructor_qset = (staff_qset | instructor_qset).distinct()
-        if to_option == SEND_TO_STAFF:
-            return [use_read_replica_if_available(staff_instructor_qset)]
-
-        if to_option == SEND_TO_ALL:
-            # We also require students to have activated their accounts to
-            # provide verification that the provided email address is valid.
-            enrollment_qset = User.objects.filter(
-                is_active=True,
-                courseenrollment__course_id=course_id,
-                courseenrollment__is_active=True
-            )
-
-            # to avoid duplicates, we only want to email unenrolled course staff
-            # members here
-            unenrolled_staff_qset = staff_instructor_qset.exclude(
-                courseenrollment__course_id=course_id, courseenrollment__is_active=True
-            )
-
-            # use read_replica if available
-            recipient_qsets = [
-                use_read_replica_if_available(unenrolled_staff_qset),
-                use_read_replica_if_available(enrollment_qset),
-            ]
-            return recipient_qsets
-
-
 def _get_course_email_context(course):
     """
     Returns context arguments to apply to all emails, independent of recipient.
@@ -220,16 +171,23 @@ def perform_delegate_email_batches(entry_id, course_id, task_input, action_name)
     course = get_course(course_id)
 
     # Get arguments that will be passed to every subtask.
-    to_option = email_obj.to_option
+    targets = email_obj.targets.all()
     global_email_context = _get_course_email_context(course)
 
-    recipient_qsets = _get_recipient_querysets(user_id, to_option, course_id)
+    recipient_qsets = [
+        target.get_users(course_id, user_id)
+        for target in targets
+    ]
+    combined_set = User.objects.none()
+    for qset in recipient_qsets:
+        combined_set |= qset
+    combined_set = combined_set.distinct()
     recipient_fields = ['profile__name', 'email']
 
-    log.info(u"Task %s: Preparing to queue subtasks for sending emails for course %s, email %s, to_option %s",
-             task_id, course_id, email_id, to_option)
+    log.info(u"Task %s: Preparing to queue subtasks for sending emails for course %s, email %s",
+             task_id, course_id, email_id)
 
-    total_recipients = sum([recipient_queryset.count() for recipient_queryset in recipient_qsets])
+    total_recipients = combined_set.count()
 
     routing_key = settings.BULK_EMAIL_ROUTING_KEY
     # if there are few enough emails, send them through a different queue
@@ -257,7 +215,7 @@ def perform_delegate_email_batches(entry_id, course_id, task_input, action_name)
         entry,
         action_name,
         _create_send_email_subtask,
-        recipient_qsets,
+        [combined_set],
         recipient_fields,
         settings.BULK_EMAIL_EMAILS_PER_TASK,
         total_recipients,

lms/djangoapps/bulk_email/tests/test_course_optout.py

@@ -75,15 +75,16 @@ class TestOptoutCourseEmails(ModuleStoreTestCase):
 
         test_email = {
             'action': 'Send email',
-            'send_to': 'all',
+            'send_to': '["myself", "staff", "learners"]',
             'subject': 'test subject for all',
             'message': 'test message for all'
         }
         response = self.client.post(self.send_mail_url, test_email)
         self.assertEquals(json.loads(response.content), self.success_content)
 
-        # Assert that self.student.email not in mail.to, outbox should be empty
-        self.assertEqual(len(mail.outbox), 0)
+        # Assert that self.student.email not in mail.to, outbox should only contain "myself" target
+        self.assertEqual(len(mail.outbox), 1)
+        self.assertEqual(mail.outbox[0].to[0], self.instructor.email)
 
     def test_optin_course(self):
         """
@@ -102,14 +103,15 @@ class TestOptoutCourseEmails(ModuleStoreTestCase):
 
         test_email = {
             'action': 'Send email',
-            'send_to': 'all',
+            'send_to': '["myself", "staff", "learners"]',
             'subject': 'test subject for all',
             'message': 'test message for all'
         }
         response = self.client.post(self.send_mail_url, test_email)
         self.assertEquals(json.loads(response.content), self.success_content)
 
-        # Assert that self.student.email in mail.to
-        self.assertEqual(len(mail.outbox), 1)
-        self.assertEqual(len(mail.outbox[0].to), 1)
-        self.assertEquals(mail.outbox[0].to[0], self.student.email)
+        # Assert that self.student.email in mail.to, along with "myself" target
+        self.assertEqual(len(mail.outbox), 2)
+        sent_addresses = [message.to[0] for message in mail.outbox]
+        self.assertIn(self.student.email, sent_addresses)
+        self.assertIn(self.instructor.email, sent_addresses)

lms/djangoapps/bulk_email/tests/test_email.py

@@ -140,7 +140,7 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase)
         BulkEmailFlag.objects.create(enabled=True, require_course_email_auth=True)
         test_email = {
             'action': 'Send email',
-            'send_to': 'myself',
+            'send_to': '["myself"]',
             'subject': 'test subject for myself',
             'message': 'test message for myself'
         }
@@ -157,7 +157,7 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase)
         # (in the setUp method), we can test sending an email.
         test_email = {
             'action': 'send',
-            'send_to': 'myself',
+            'send_to': '["myself"]',
             'subject': 'test subject for myself',
             'message': 'test message for myself'
         }
@@ -186,7 +186,7 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase)
         # (in the setUp method), we can test sending an email.
         test_email = {
             'action': 'Send email',
-            'send_to': 'staff',
+            'send_to': '["staff"]',
             'subject': 'test subject for staff',
             'message': 'test message for subject'
         }
@@ -210,7 +210,7 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase)
 
         test_email = {
             'action': 'Send email',
-            'send_to': 'all',
+            'send_to': '["myself", "staff", "learners"]',
             'subject': 'test subject for all',
             'message': 'test message for all'
         }
@@ -271,7 +271,7 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase)
         uni_subject = u'téśt śúbjéćt főŕ áĺĺ'
         test_email = {
             'action': 'Send email',
-            'send_to': 'all',
+            'send_to': '["myself", "staff", "learners"]',
             'subject': uni_subject,
             'message': 'test message for all'
         }
@@ -300,7 +300,7 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase)
 
         test_email = {
             'action': 'Send email',
-            'send_to': 'all',
+            'send_to': '["myself", "staff", "learners"]',
             'subject': 'test subject for all',
             'message': 'test message for all'
         }
@@ -324,7 +324,7 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase)
         """
         test_email = {
             'action': 'Send email',
-            'send_to': 'myself',
+            'send_to': '["myself", "staff", "learners"]',
             'subject': 'test subject for self',
             'message': 'test message for self'
         }
@@ -397,7 +397,7 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase)
 
         test_email = {
             'action': 'Send email',
-            'send_to': 'all',
+            'send_to': '["myself", "staff", "learners"]',
             'subject': 'test subject for all',
             'message': 'test message for all'
         }
@@ -435,7 +435,7 @@ class TestEmailSendFromDashboard(EmailSendFromDashboardTestCase):
         uni_message = u'ẗëṡẗ ṁëṡṡäġë ḟöṛ äḷḷ ｲ乇丂ｲ ﾶ乇丂丂ﾑg乇 ｷo尺 ﾑﾚﾚ тэѕт мэѕѕаБэ fоѓ аll'
         test_email = {
             'action': 'Send email',
-            'send_to': 'all',
+            'send_to': '["myself", "staff", "learners"]',
             'subject': 'test subject for all',
             'message': uni_message
         }

lms/djangoapps/bulk_email/tests/test_err_handling.py

@@ -14,7 +14,7 @@ from mock import patch, Mock
 from nose.plugins.attrib import attr
 from smtplib import SMTPDataError, SMTPServerDisconnected, SMTPConnectError
 
-from bulk_email.models import CourseEmail, SEND_TO_ALL, BulkEmailFlag
+from bulk_email.models import CourseEmail, SEND_TO_MYSELF, BulkEmailFlag
 from bulk_email.tasks import perform_delegate_email_batches, send_course_email
 from instructor_task.models import InstructorTask
 from instructor_task.subtasks import (
@@ -60,10 +60,15 @@ class TestEmailErrors(ModuleStoreTestCase):
             'course_id': self.course.id.to_deprecated_string(),
             'success': True,
         }
+
+    @classmethod
+    def setUpClass(cls):
+        super(TestEmailErrors, cls).setUpClass()
         BulkEmailFlag.objects.create(enabled=True, require_course_email_auth=False)
 
-    def tearDown(self):
-        super(TestEmailErrors, self).tearDown()
+    @classmethod
+    def tearDownClass(cls):
+        super(TestEmailErrors, cls).tearDownClass()
         BulkEmailFlag.objects.all().delete()
 
     @patch('bulk_email.tasks.get_connection', autospec=True)
@@ -75,7 +80,7 @@ class TestEmailErrors(ModuleStoreTestCase):
         get_conn.return_value.send_messages.side_effect = SMTPDataError(455, "Throttling: Sending rate exceeded")
         test_email = {
             'action': 'Send email',
-            'send_to': 'myself',
+            'send_to': '["myself"]',
             'subject': 'test subject for myself',
             'message': 'test message for myself'
         }
@@ -98,13 +103,14 @@ class TestEmailErrors(ModuleStoreTestCase):
         # have every fourth email fail due to blacklisting:
         get_conn.return_value.send_messages.side_effect = cycle([SMTPDataError(554, "Email address is blacklisted"),
                                                                  None, None, None])
-        students = [UserFactory() for _ in xrange(settings.BULK_EMAIL_EMAILS_PER_TASK)]
+        # Don't forget to account for the "myself" instructor user
+        students = [UserFactory() for _ in xrange(settings.BULK_EMAIL_EMAILS_PER_TASK - 1)]
         for student in students:
             CourseEnrollmentFactory.create(user=student, course_id=self.course.id)
 
         test_email = {
             'action': 'Send email',
-            'send_to': 'all',
+            'send_to': '["myself", "staff", "learners"]',
             'subject': 'test subject for all',
             'message': 'test message for all'
         }
@@ -129,7 +135,7 @@ class TestEmailErrors(ModuleStoreTestCase):
         get_conn.return_value.open.side_effect = SMTPServerDisconnected(425, "Disconnecting")
         test_email = {
             'action': 'Send email',
-            'send_to': 'myself',
+            'send_to': '["myself"]',
             'subject': 'test subject for myself',
             'message': 'test message for myself'
         }
@@ -151,7 +157,7 @@ class TestEmailErrors(ModuleStoreTestCase):
 
         test_email = {
             'action': 'Send email',
-            'send_to': 'myself',
+            'send_to': '["myself"]',
             'subject': 'test subject for myself',
             'message': 'test message for myself'
         }
@@ -198,19 +204,26 @@ class TestEmailErrors(ModuleStoreTestCase):
         """
         Tests exception when the to_option in the email doesn't exist
         """
-        email = CourseEmail(course_id=self.course.id, to_option="IDONTEXIST")
-        email.save()
-        entry = InstructorTask.create(self.course.id, "task_type", "task_key", "task_input", self.instructor)
-        task_input = {"email_id": email.id}
-        with self.assertRaisesRegexp(Exception, 'Unexpected bulk email TO_OPTION found: IDONTEXIST'):
-            perform_delegate_email_batches(entry.id, self.course.id, task_input, "action_name")
+        with self.assertRaisesRegexp(Exception, 'Course email being sent to unrecognized target: "IDONTEXIST" *'):
+            email = CourseEmail.create(  # pylint: disable=unused-variable
+                self.course.id,
+                self.instructor,
+                ["IDONTEXIST"],
+                "re: subject",
+                "dummy body goes here"
+            )
 
     def test_wrong_course_id_in_task(self):
         """
         Tests exception when the course_id in task is not the same as one explicitly passed in.
         """
-        email = CourseEmail(course_id=self.course.id, to_option=SEND_TO_ALL)
-        email.save()
+        email = CourseEmail.create(
+            self.course.id,
+            self.instructor,
+            [SEND_TO_MYSELF],
+            "re: subject",
+            "dummy body goes here"
+        )
         entry = InstructorTask.create("bogus/task/id", "task_type", "task_key", "task_input", self.instructor)
         task_input = {"email_id": email.id}
         with self.assertRaisesRegexp(ValueError, 'does not match task value'):
@@ -220,8 +233,13 @@ class TestEmailErrors(ModuleStoreTestCase):
         """
         Tests exception when the course_id in CourseEmail is not the same as one explicitly passed in.
         """
-        email = CourseEmail(course_id=SlashSeparatedCourseKey("bogus", "course", "id"), to_option=SEND_TO_ALL)
-        email.save()
+        email = CourseEmail.create(
+            SlashSeparatedCourseKey("bogus", "course", "id"),
+            self.instructor,
+            [SEND_TO_MYSELF],
+            "re: subject",
+            "dummy body goes here"
+        )
         entry = InstructorTask.create(self.course.id, "task_type", "task_key", "task_input", self.instructor)
         task_input = {"email_id": email.id}
         with self.assertRaisesRegexp(ValueError, 'does not match email value'):

lms/djangoapps/bulk_email/tests/test_models.py

@@ -25,9 +25,9 @@ class CourseEmailTest(TestCase):
         to_option = SEND_TO_STAFF
         subject = "dummy subject"
         html_message = "<html>dummy message</html>"
-        email = CourseEmail.create(course_id, sender, to_option, subject, html_message)
+        email = CourseEmail.create(course_id, sender, [to_option], subject, html_message)
         self.assertEquals(email.course_id, course_id)
-        self.assertEquals(email.to_option, SEND_TO_STAFF)
+        self.assertIn(SEND_TO_STAFF, [target.target_type for target in email.targets.all()])
         self.assertEquals(email.subject, subject)
         self.assertEquals(email.html_message, html_message)
         self.assertEquals(email.sender, sender)
@@ -41,10 +41,10 @@ class CourseEmailTest(TestCase):
         template_name = "branded_template"
         from_addr = "branded@branding.com"
         email = CourseEmail.create(
-            course_id, sender, to_option, subject, html_message, template_name=template_name, from_addr=from_addr
+            course_id, sender, [to_option], subject, html_message, template_name=template_name, from_addr=from_addr
         )
         self.assertEquals(email.course_id, course_id)
-        self.assertEquals(email.to_option, SEND_TO_STAFF)
+        self.assertEquals(email.targets.all()[0].target_type, SEND_TO_STAFF)
         self.assertEquals(email.subject, subject)
         self.assertEquals(email.html_message, html_message)
         self.assertEquals(email.sender, sender)

lms/djangoapps/bulk_email/tests/test_tasks.py

@@ -32,7 +32,7 @@ from django.core.management import call_command
 
 from xmodule.modulestore.tests.factories import CourseFactory
 
-from bulk_email.models import CourseEmail, Optout, SEND_TO_ALL
+from bulk_email.models import CourseEmail, Optout, SEND_TO_MYSELF, SEND_TO_STAFF, SEND_TO_LEARNERS
 
 from instructor_task.tasks import send_bulk_course_email
 from instructor_task.subtasks import update_subtask_status, SubtaskStatus
@@ -94,10 +94,10 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
 
         Overrides the base class version in that this creates CourseEmail.
         """
-        to_option = SEND_TO_ALL
+        targets = [SEND_TO_MYSELF, SEND_TO_STAFF, SEND_TO_LEARNERS]
         course_id = course_id or self.course.id
         course_email = CourseEmail.create(
-            course_id, self.instructor, to_option, "Test Subject", "<p>This is a test message</p>"
+            course_id, self.instructor, targets, "Test Subject", "<p>This is a test message</p>"
         )
         task_input = {'email_id': course_email.id}
         task_id = str(uuid4())
@@ -427,8 +427,9 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
         course_image = u'在淡水測試.jpg'
         self.course = CourseFactory.create(course_image=course_image)
 
-        num_emails = 1
-        self._create_students(num_emails)
+        num_emails = 2
+        # We also send email to the instructor:
+        self._create_students(num_emails - 1)
 
         with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn:
             get_conn.return_value.send_messages.side_effect = cycle([None])

lms/djangoapps/instructor/tests/test_api.py

@@ -238,7 +238,7 @@ class TestInstructorAPIDenyLevels(SharedModuleStoreTestCase, LoginEnrollmentTest
             ('update_forum_role_membership',
              {'unique_student_identifier': self.user.email, 'rolename': 'Moderator', 'action': 'allow'}),
             ('list_forum_members', {'rolename': FORUM_ROLE_COMMUNITY_TA}),
-            ('send_email', {'send_to': 'staff', 'subject': 'test', 'message': 'asdf'}),
+            ('send_email', {'send_to': '["staff"]', 'subject': 'test', 'message': 'asdf'}),
             ('list_instructor_tasks', {}),
             ('list_background_email_tasks', {}),
             ('list_report_downloads', {}),
@@ -3415,7 +3415,7 @@ class TestInstructorSendEmail(SharedModuleStoreTestCase, LoginEnrollmentTestCase
         test_subject = u'\u1234 test subject'
         test_message = u'\u6824 test message'
         cls.full_test_message = {
-            'send_to': 'staff',
+            'send_to': '["myself", "staff"]',
             'subject': test_subject,
             'message': test_message,
         }
@@ -3464,10 +3464,19 @@ class TestInstructorSendEmail(SharedModuleStoreTestCase, LoginEnrollmentTestCase
         })
         self.assertEqual(response.status_code, 400)
 
+    def test_send_email_invalid_sendto(self):
+        url = reverse('send_email', kwargs={'course_id': self.course.id.to_deprecated_string()})
+        response = self.client.post(url, {
+            'send_to': '["invalid_target", "staff"]',
+            'subject': 'test subject',
+            'message': 'test message',
+        })
+        self.assertEqual(response.status_code, 400)
+
     def test_send_email_no_subject(self):
         url = reverse('send_email', kwargs={'course_id': self.course.id.to_deprecated_string()})
         response = self.client.post(url, {
-            'send_to': 'staff',
+            'send_to': '["staff"]',
             'message': 'test message',
         })
         self.assertEqual(response.status_code, 400)
@@ -3475,7 +3484,7 @@ class TestInstructorSendEmail(SharedModuleStoreTestCase, LoginEnrollmentTestCase
     def test_send_email_no_message(self):
         url = reverse('send_email', kwargs={'course_id': self.course.id.to_deprecated_string()})
         response = self.client.post(url, {
-            'send_to': 'staff',
+            'send_to': '["staff"]',
             'subject': 'test subject',
         })
         self.assertEqual(response.status_code, 400)

lms/djangoapps/instructor/tests/test_email.py

@@ -56,7 +56,7 @@ class TestNewInstructorDashboardEmailViewMongoBacked(SharedModuleStoreTestCase):
         response = self.client.get(self.url)
         self.assertIn(self.email_link, response.content)
 
-        send_to_label = '<label for="id_to">Send to:</label>'
+        send_to_label = '<ul role="group" aria-label="Send to:">'
         self.assertTrue(send_to_label in response.content)
         self.assertEqual(response.status_code, 200)
 

lms/djangoapps/instructor/tests/utils.py

@@ -51,6 +51,20 @@ class FakeEmail(FakeInfo):
         'created',
     ]
 
+    class FakeTarget(object):
+        """ Corresponding fake target for a fake email """
+        target_type = "expected"
+
+        def get_target_type_display(self):
+            """ Mocks out a django method """
+            return self.target_type
+
+    class FakeTargetGroup(object):
+        """ Helps to mock out a django M2M relationship """
+        def all(self):
+            """ Mocks out a django method """
+            return [FakeEmail.FakeTarget()]
+
     def __init__(self, email_id):
         super(FakeEmail, self).__init__()
         self.id = unicode(email_id)  # pylint: disable=invalid-name
@@ -61,6 +75,7 @@ class FakeEmail(FakeInfo):
         hour = random.randint(0, 23)
         minute = random.randint(0, 59)
         self.created = datetime.datetime(year, month, day, hour, minute, tzinfo=utc)
+        self.targets = FakeEmail.FakeTargetGroup()
 
 
 class FakeEmailInfo(FakeInfo):
@@ -91,3 +106,4 @@ class FakeEmailInfo(FakeInfo):
         fake_email_dict = fake_email.to_dict()
         self.email = {feature: fake_email_dict[feature] for feature in self.EMAIL_FEATURES}
         self.requester = u'expected'
+        self.sent_to = [u'expected']

lms/djangoapps/instructor/views/api.py

@@ -84,6 +84,7 @@ import instructor_analytics.distributions
 import instructor_analytics.csvs
 import csv
 from openedx.core.djangoapps.user_api.preferences.api import get_user_preference, set_user_preference
+from openedx.core.djangolib.markup import HTML, Text
 from instructor.views import INVOICE_KEY
 
 from submissions import api as sub_api  # installed from the edx-submissions repository
@@ -2312,7 +2313,7 @@ def list_report_downloads(_request, course_id):
 
     response_payload = {
         'downloads': [
-            dict(name=name, url=url, link='<a href="{}">{}</a>'.format(url, name))
+            dict(name=name, url=url, link=HTML('<a href="{}">{}</a>').format(HTML(url), Text(name)))
             for name, url in report_store.links_for(course_id)
         ]
     }
@@ -2332,7 +2333,7 @@ def list_financial_report_downloads(_request, course_id):
 
     response_payload = {
         'downloads': [
-            dict(name=name, url=url, link='<a href="{}">{}</a>'.format(url, name))
+            dict(name=name, url=url, link=HTML('<a href="{}">{}</a>').format(HTML(url), Text(name)))
             for name, url in report_store.links_for(course_id)
         ]
     }
@@ -2494,7 +2495,7 @@ def send_email(request, course_id):
     if not BulkEmailFlag.feature_enabled(course_id):
         return HttpResponseForbidden("Email is not enabled for this course.")
 
-    send_to = request.POST.get("send_to")
+    targets = json.loads(request.POST.get("send_to"))
     subject = request.POST.get("subject")
     message = request.POST.get("message")
 
@@ -2509,14 +2510,17 @@ def send_email(request, course_id):
     # Create the CourseEmail object.  This is saved immediately, so that
     # any transaction that has been pending up to this point will also be
     # committed.
-    email = CourseEmail.create(
-        course_id,
-        request.user,
-        send_to,
-        subject, message,
-        template_name=template_name,
-        from_addr=from_addr
-    )
+    try:
+        email = CourseEmail.create(
+            course_id,
+            request.user,
+            targets,
+            subject, message,
+            template_name=template_name,
+            from_addr=from_addr
+        )
+    except ValueError as err:
+        return HttpResponseBadRequest(repr(err))
 
     # Submit the task, so that the correct InstructorTask object gets created (for monitoring purposes)
     instructor_task.api.submit_bulk_course_email(request, course_id, email.id)

lms/djangoapps/instructor/views/instructor_task_helpers.py

@@ -57,7 +57,7 @@ def extract_email_features(email_task):
     email = CourseEmail.objects.get(id=task_input_information['email_id'])
     email_feature_dict = {
         'created': get_default_time_display(email.created),
-        'sent_to': task_input_information['to_option'],
+        'sent_to': [target.get_target_type_display() for target in email.targets.all()],
         'requester': str(email_task.requester),
     }
     features = ['subject', 'html_message', 'id']

lms/djangoapps/instructor_task/api.py

@@ -276,19 +276,15 @@ def submit_bulk_course_email(request, course_key, email_id):
     """
     # Assume that the course is defined, and that the user has already been verified to have
     # appropriate access to the course. But make sure that the email exists.
-    # We also pull out the To argument here, so that is displayed in
+    # We also pull out the targets argument here, so that is displayed in
     # the InstructorTask status.
     email_obj = CourseEmail.objects.get(id=email_id)
-    to_option = email_obj.to_option
+    targets = [target.target_type for target in email_obj.targets.all()]
 
     task_type = 'bulk_course_email'
     task_class = send_bulk_course_email
-    # Pass in the to_option as a separate argument, even though it's (currently)
-    # in the CourseEmail.  That way it's visible in the progress status.
-    # (At some point in the future, we might take the recipient out of the CourseEmail,
-    # so that the same saved email can be sent to different recipients, as it is tested.)
-    task_input = {'email_id': email_id, 'to_option': to_option}
-    task_key_stub = "{email_id}_{to_option}".format(email_id=email_id, to_option=to_option)
+    task_input = {'email_id': email_id, 'to_option': targets}
+    task_key_stub = "{email_id}".format(email_id=email_id)
     # create the key value by using MD5 hash:
     task_key = hashlib.md5(task_key_stub).hexdigest()
     return submit_task(request, task_type, task_class, course_key, task_input, task_key)

lms/djangoapps/instructor_task/tests/test_api.py

@@ -3,7 +3,7 @@ Test for LMS instructor background task queue management
 """
 from mock import patch, Mock, MagicMock
 from nose.plugins.attrib import attr
-from bulk_email.models import CourseEmail, SEND_TO_ALL
+from bulk_email.models import CourseEmail, SEND_TO_MYSELF, SEND_TO_STAFF, SEND_TO_LEARNERS
 from courseware.tests.factories import UserFactory
 from xmodule.modulestore.exceptions import ItemNotFoundError
 
@@ -188,7 +188,13 @@ class InstructorTaskCourseSubmitTest(TestReportMixin, InstructorTaskCourseTestCa
 
     def _define_course_email(self):
         """Create CourseEmail object for testing."""
-        course_email = CourseEmail.create(self.course.id, self.instructor, SEND_TO_ALL, "Test Subject", "<p>This is a test message</p>")
+        course_email = CourseEmail.create(
+            self.course.id,
+            self.instructor,
+            [SEND_TO_MYSELF, SEND_TO_STAFF, SEND_TO_LEARNERS],
+            "Test Subject",
+            "<p>This is a test message</p>"
+        )
         return course_email.id
 
     def _test_resubmission(self, api_call):

lms/static/coffee/fixtures/send_email.html

+<div class="send-email">
+    <div class="request-response msg msg-confirm copy" id="request-response"></div>
+    <ul>
+        <li><ul>
+            <li><input type="checkbox" name="send_to" value="myself"></li>
+            <li><input type="checkbox" name="send_to" value="staff"></li>
+            <li><input type="checkbox" name="send_to" value="learners"></li>
+        </ul></li>
+        <li><input type="text" id="id_subject" name="subject" maxlength="128" size="75"></li>
+        <li><input type="button" name="send"></li>
+    </ul>
+    <div class="request-response-error"></div>
+</div>

lms/static/coffee/spec/instructor_dashboard/send_email_spec.coffee

+describe "Bulk Email Queueing", ->
+    beforeEach ->
+        testSubject = "Test Subject"
+        testBody =  "Hello, World! This is a test email message!"
+        loadFixtures 'coffee/fixtures/send_email.html'
+        @send_email = new SendEmail $('.send-email')
+        @send_email.$subject.val(testSubject)
+        @send_email.$send_to.first().prop("checked", true)
+        @send_email.$emailEditor =
+            save: ->
+                {"data": testBody}
+        @ajax_params = {
+            type: "POST",
+            dataType: "json",
+            url: undefined,
+            data: {
+                action: "send",
+                send_to: JSON.stringify([@send_email.$send_to.first().val()]),
+                subject: testSubject,
+                message: testBody,
+            },
+            success: jasmine.any(Function),
+            error: jasmine.any(Function),
+        }
+
+    it 'cannot send an email with no target', ->
+        spyOn(window, "alert")
+        spyOn($, "ajax")
+        for target in @send_email.$send_to
+            target.checked = false
+        @send_email.$btn_send.click()
+        expect(window.alert).toHaveBeenCalledWith("Your message must have at least one target.")
+        expect($.ajax).not.toHaveBeenCalled()
+
+    it 'cannot send an email with no subject', ->
+        spyOn(window, "alert")
+        spyOn($, "ajax")
+        @send_email.$subject.val("")
+        @send_email.$btn_send.click()
+        expect(window.alert).toHaveBeenCalledWith("Your message must have a subject.")
+        expect($.ajax).not.toHaveBeenCalled()
+
+    it 'cannot send an email with no message', ->
+        spyOn(window, "alert")
+        spyOn($, "ajax")
+        @send_email.$emailEditor =
+            save: ->
+                {"data": ""}
+        @send_email.$btn_send.click()
+        expect(window.alert).toHaveBeenCalledWith("Your message cannot be blank.")
+        expect($.ajax).not.toHaveBeenCalled()
+
+    it 'can send a simple message to a single target', ->
+        spyOn($, "ajax").and.callFake((params) =>
+          params.success()
+        )
+        @send_email.$btn_send.click()
+        expect($('.msg-confirm').text()).toEqual('Your email message was successfully queued for sending. In courses with a large number of learners, email messages to learners might take up to an hour to be sent.')
+        expect($.ajax).toHaveBeenCalledWith(@ajax_params)
+
+    it 'can send a simple message to a multiple targets', ->
+        spyOn($, "ajax").and.callFake((params) =>
+            params.success()
+        )
+        @ajax_params.data.send_to = JSON.stringify(target.value for target in @send_email.$send_to)
+        for target in @send_email.$send_to
+            target.checked = true
+        @send_email.$btn_send.click()
+        expect($('.msg-confirm').text()).toEqual('Your email message was successfully queued for sending. In courses with a large number of learners, email messages to learners might take up to an hour to be sent.')
+        expect($.ajax).toHaveBeenCalledWith(@ajax_params)
+
+    it 'can handle an error result from the bulk email api', ->
+        spyOn($, "ajax").and.callFake((params) =>
+            params.error()
+        )
+        spyOn(console, "warn")
+        @send_email.$btn_send.click()
+        expect($('.request-response-error').text()).toEqual('Error sending email.')
+        expect(console.warn).toHaveBeenCalled()

lms/static/coffee/src/instructor_dashboard/util.coffee

@@ -116,26 +116,46 @@ find_and_assert = ($root, selector) ->
   table_data = tasks_data
 
   $table_placeholder = $ '<div/>', class: 'slickgrid'
-  $table_tasks.append $table_placeholder
+  $table_tasks.append($table_placeholder)
   grid = new Slick.Grid($table_placeholder, table_data, columns, options)
 
 # Formats the subject field for email content history table
 subject_formatter = (row, cell, value, columnDef, dataContext) ->
   if value is null then return gettext("An error occurred retrieving your email. Please try again later, and contact technical support if the problem persists.")
   subject_text = $('<span>').text(value['subject']).html()
-  return '<p><a href="#email_message_' + value['id']+ '" id="email_message_' + value['id'] + '_trig">' + subject_text + '</a></p>'
-
-# Formats the author field for the email content history table
-sent_by_formatter = (row, cell, value, columnDef, dataContext) ->
-  if value is null then return "<p>" + gettext("Unknown") + "</p>" else return '<p>' + value + '</p>'
-
-# Formats the created field for the email content history table
-created_formatter = (row, cell, value, columnDef, dataContext) ->
-  if value is null then return "<p>" + gettext("Unknown") + "</p>" else return '<p>' + value + '</p>'
+  return edx.HtmlUtils.joinHtml(
+    edx.HtmlUtils.HTML('<p><a href="#email_message_'),
+    value['id'],
+    edx.HtmlUtils.HTML('" id="email_message_'),
+    value['id'],
+    edx.HtmlUtils.HTML('_trig">'),
+    subject_text,
+    edx.HtmlUtils.HTML('</a></p>'),
+  )
+
+p_wrapper = (value) ->
+  edx.HtmlUtils.joinHtml(
+    edx.HtmlUtils.HTML('<p>'),
+    value,
+    edx.HtmlUtils.HTML('</p>'),
+  )
+
+unknown_p = () ->
+  p_wrapper(gettext('Unknown'))
+
+# Since sent_to is a json array, it needs some extra attention
+sent_to_formatter = (row, cell, value, columnDef, dataContext) ->
+  if value is null
+    return unknown_p()
+  else
+    return p_wrapper(value.join(", "))
 
-# Formats the number sent field for the email content history table
-number_sent_formatter = (row, cell, value, columndDef, dataContext) ->
-  if value is null then return "<p>" + gettext("Unknown") + "</p>" else return '<p>' + value + '</p>'
+# Formats the author, created, and number sent fields for the email content history table
+unknown_if_null_formatter = (row, cell, value, columnDef, dataContext) ->
+  if value is null
+    return unknown_p()
+  else
+    return p_wrapper(value)
 
 # Creates a table to display the content of bulk course emails
 # sent in the past
@@ -164,14 +184,22 @@ create_email_content_table = ($table_emails, $table_emails_inner, email_data) ->
       minWidth: 80
       maxWidth: 100
       cssClass: "email-content-cell"
-      formatter: sent_by_formatter
+      formatter: unknown_if_null_formatter
+    ,
+      id: 'sent_to'
+      field: 'sent_to'
+      name: gettext('Sent To')
+      minWidth: 80
+      maxWidth: 100
+      cssClass: "email-content-cell"
+      formatter: sent_to_formatter
     ,
       id: 'created'
       field: 'created'
       name: gettext('Time Sent')
       minWidth: 80
       cssClass: "email-content-cell"
-      formatter: created_formatter
+      formatter: unknown_if_null_formatter
     ,
       id: 'number_sent'
       field: 'number_sent'
@@ -179,16 +207,16 @@ create_email_content_table = ($table_emails, $table_emails_inner, email_data) ->
       minwidth: 100
       maxWidth: 150
       cssClass: "email-content-cell"
-      formatter: number_sent_formatter
+      formatter: unknown_if_null_formatter
     ,
     ]
 
     table_data = email_data
 
     $table_placeholder = $ '<div/>', class: 'slickgrid'
-    $table_emails_inner.append $table_placeholder
+    $table_emails_inner.append($table_placeholder)
     grid = new Slick.Grid($table_placeholder, table_data, columns, options)
-    $table_emails.append $ '<br/>'
+    $table_emails.append($('<br/>'))
 
 # Creates the modal windows linked to each email in the email history
 # Displayed when instructor clicks an email's subject in the content history table
@@ -206,31 +234,55 @@ create_email_message_views = ($messages_wrapper, emails) ->
     $email_header = $ '<div>', class: 'email-content-header'
 
     # Add copy email body button
-    $email_header.append $('<input>', type: "button", name: "copy-email-body-text", value: gettext("Copy Email To Editor"), id: "copy_email_" + email_id)
+    $email_header.append($('<input>', type: "button", name: "copy-email-body-text", value: gettext("Copy Email To Editor"), id: "copy_email_" + email_id))
 
     $close_button = $ '<a>', href: '#', class: "close-modal"
-    $close_button.append $ '<i>', class: 'icon fa fa-times'
-    $email_header.append $close_button
-
-    # HTML escape the subject line
-    subject_text = $('<span>').text(email_info.email['subject']).html()
-    $email_header.append $('<h2>', class: "message-bold").html('<em>' + gettext('Subject:') + '</em> ' + subject_text)
-    $email_header.append $('<h2>', class: "message-bold").html('<em>' + gettext('Sent By:') + '</em> ' + email_info.requester)
-    $email_header.append $('<h2>', class: "message-bold").html('<em>' + gettext('Time Sent:') + '</em> ' + email_info.created)
-    $email_header.append $('<h2>', class: "message-bold").html('<em>' + gettext('Sent To:') + '</em> ' + email_info.sent_to)
-    $email_wrapper.append $email_header
-
-    $email_wrapper.append $ '<hr>'
+    $close_button.append($('<i>', class: 'icon fa fa-times'))
+    $email_header.append($close_button)
+
+    # HTML escape things
+    interpolate_header = (title, value) ->
+      edx.HtmlUtils.setHtml(
+        $('<h2>', class: 'message-bold'),
+        edx.HtmlUtils.joinHtml(
+          edx.HtmlUtils.HTML('<em>'),
+          title
+          edx.HtmlUtils.HTML('</em>'),
+          value,
+        )
+      )
+    $subject = interpolate_header(gettext('Subject:'), email_info.email['subject'])
+    $requester = interpolate_header(gettext('Sent By:'), email_info.requester)
+    $created = interpolate_header(gettext('Time Sent:'), email_info.created)
+    $sent_to = interpolate_header(gettext('Sent To:'), email_info.sent_to.join(", "))
+    $email_header.append($subject)
+    $email_header.append($requester)
+    $email_header.append($created)
+    $email_header.append($sent_to)
+    $email_wrapper.append($email_header)
+
+    $email_wrapper.append($('<hr>'))
 
     # Last, add email content section
     $email_content = $ '<div>', class: 'email-content-message'
-    $email_content.append $('<h2>', class: "message-bold").html("<em>" + gettext("Message:") + "</em>")
-    $message = $('<div>').html(email_info.email['html_message'])
-    $email_content.append $message
-    $email_wrapper.append $email_content
+    $email_content_header = edx.HtmlUtils.setHtml(
+      $('<h2>', class: "message-bold"),
+      edx.HtmlUtils.joinHtml(
+        edx.HtmlUtils.HTML('<em>'),
+        gettext("Message:"),
+        edx.HtmlUtils.HTML('</em>'),
+      )
+    )
+    $email_content.append($email_content_header)
+    $message = edx.HtmlUtils.setHtml(
+      $('<div>'),
+      edx.HtmlUtils.HTML(email_info.email['html_message'])
+    )
+    $email_content.append($message)
+    $email_wrapper.append($email_content)
 
-    $message_content.append $email_wrapper
-    $messages_wrapper.append $message_content
+    $message_content.append($email_wrapper)
+    $messages_wrapper.append($message_content)
 
     # Setup buttons to open modal window and copy an email message
     $('#email_message_' + email_info.email['id'] + '_trig').leanModal({closeButton: ".close-modal", copyEmailButton: "#copy_email_" + email_id})
@@ -295,7 +347,7 @@ class @PendingInstructorTasks
           console.log "No pending tasks to display"
           @$running_tasks_section.hide()
           @$no_tasks_message.empty()
-          @$no_tasks_message.append $('<p>').text gettext("No tasks currently running.")
+          @$no_tasks_message.append($('<p>').text(gettext("No tasks currently running.")))
           @$no_tasks_message.show()
       error: std_ajax_err => console.error "Error finding pending tasks to display"
     ### /Pending Instructor Tasks Section ####
@@ -367,11 +419,17 @@ class ReportDownloads
       minWidth: 150
       cssClass: "file-download-link"
       formatter: (row, cell, value, columnDef, dataContext) ->
-        '<a target="_blank" href="' + dataContext['url'] + '">' + dataContext['name'] + '</a>'
+        edx.HtmlUtils.joinHtml(
+          edx.HtmlUtils.HTML('<a target="_blank" href="'),
+          dataContext['url'],
+          edx.HtmlUtils.HTML('">'),
+          dataContext['name'],
+          edx.HtmlUtils.HTML('</a>')
+        )
     ]
 
     $table_placeholder = $ '<div/>', class: 'slickgrid'
-    @$report_downloads_table.append $table_placeholder
+    @$report_downloads_table.append($table_placeholder)
     grid = new Slick.Grid($table_placeholder, report_downloads_data, columns, options)
     grid.onClick.subscribe(
         (event) =>

lms/static/js/spec/main.js

@@ -287,6 +287,21 @@
                 exports: 'AjaxPrefix',
                 deps: ['coffee/src/ajax_prefix']
             },
+            'coffee/src/instructor_dashboard/util': {
+                exports: 'coffee/src/instructor_dashboard/util',
+                deps: ['jquery', 'gettext'],
+                init: function() {
+                    // Set global variables that the util code is expecting to be defined
+                    require([
+                        'edx-ui-toolkit/js/utils/html-utils',
+                        'edx-ui-toolkit/js/utils/string-utils'
+                    ], function (HtmlUtils, StringUtils) {
+                        window.edx = edx || {};
+                        window.edx.HtmlUtils = HtmlUtils;
+                        window.edx.StringUtils = StringUtils;
+                    });
+                }
+            },
             'coffee/src/instructor_dashboard/student_admin': {
                 exports: 'coffee/src/instructor_dashboard/student_admin',
                 deps: ['jquery', 'underscore', 'coffee/src/instructor_dashboard/util', 'string_utils']

lms/static/sass/course/instructor/_instructor_2.scss

@@ -160,9 +160,13 @@
     color: $error-color;
   }
 
+  .submit-email-warning {
+    margin-top: ($baseline);
+  }
+
   .slickgrid {
     @include margin-left(1px);
-    @include font-size(12px);
+    @include font-size(14);
     font-family: verdana,arial,sans-serif;
     color: #333333;
 
@@ -412,6 +416,14 @@
       margin-bottom: $baseline;
       padding: 0;
 
+      .label {
+        display: block;
+      }
+
+      ul {
+        list-style-type: none;
+      }
+
       &:last-child {
         margin-bottom: 0;
       }
@@ -419,7 +431,7 @@
         display: block;
         margin-top: ($baseline/4);
         @include font-size(12);
-        color: tint(rgb(127,127,127),50%);
+        color: $gray-d1;
       }
     }
   }

lms/templates/instructor/instructor_dashboard_2/send_email.html

@@ -9,50 +9,60 @@ from openedx.core.djangolib.markup import HTML
   <div class="request-response msg msg-confirm copy" id="request-response"></div>
   <ul class="list-fields">
     <li class="field">
-      <label for="id_to">${_("Send to:")}</label><br/>
-      <select id="id_to" name="send_to">
-        <option value="myself">${_("Myself")}</option>
-        %if to_option == "staff":
-        <option value="staff" selected="selected">${_("Staff and admins")}</option>
-        %else:
-        <option value="staff">${_("Staff and admins")}</option>
-          %endif
-          %if to_option == "all":
-            <option value="all" selected="selected">${_("All (students, staff, and admins)")}</option>
-          %else:
-      <option value="all">${_("All (students, staff, and admins)")}</option>
-      %endif
-      </select>
+        ${_("Send to:")}
+        <ul role="group" aria-label="${_('Send to:')}">
+            <li>
+                <label>
+                    <input type="checkbox" name="send_to" value="myself">
+                        ${_("Myself")}
+                    </input>
+                </label>
+            </li>
+            <li>
+                <label>
+                    <input type="checkbox" name="send_to" value="staff">
+                        ${_("Staff and Admin")}
+                    </input>
+                </label>
+            </li>
+            <li>
+                <label>
+                    <input type="checkbox" name="send_to" value="learners">
+                        ${_("All Learners")}
+                    </input>
+                </label>
+            </li>
+        </ul>
+    </li>
+    <li class="field">
+        <label>
+            ${_("Subject: ")}
+            <br/>
+            %if subject:
+                <input type="text" id="id_subject" name="subject" maxlength="128" size="75" value="${subject}">
+            %else:
+                <input type="text" id="id_subject" name="subject" maxlength="128" size="75">
+            %endif
+            <span class="tip">${_("(Maximum 128 characters)")}</span>
+        </label>
+    </li>
+    <li>
+        <label>
+            ${_("Message:")}
+            <div class="email-editor">
+                ${ HTML(section_data['editor']) }
+            </div>
+            <input type="hidden" name="message" value="">
+        </label>
     </li>
-  <br/>
-  <li class="field">
-    <label for="id_subject">${_("Subject: ")}</label><br/>
-     %if subject:
-      <input type="text" id="id_subject" name="subject" maxlength="128" size="75" value="${subject}">
-     %else:
-      <input type="text" id="id_subject" name="subject" maxlength="128" size="75">
-     %endif
-     <span class="tip">${_("(Max 128 characters)")}</span>
-  </li>
-  <li class="field">
-    <label>${_("Message:")}</label>
-      <div class="email-editor">
-      ${ HTML(section_data['editor']) }
-      </div>
-    <input type="hidden" name="message" value="">
-  </li>
   </ul>
  <div class="submit-email-action">
-  <p class="copy">${_("Please try not to email students more than once per week. Before sending your email, consider:")}</p>
-  <ul class="list-advice">
-   <li class="item">${_("Have you read over the email to make sure it says everything you want to say?")}</li>
-   <li class="item">${_("Have you sent the email to yourself first to make sure you're happy with how it's displayed, and that embedded links and images work properly?")}</li>
-  </ul>
+  <p class="copy">${_("We recommend sending learners no more than one email message per week. Before you send your email, review the text carefully and send it to yourself first, so that you can preview the formatting and make sure embedded images and links work correctly.")}</p>
  </div>
  <div class="submit-email-warning">
    <p class="copy"><span style="color: red;"><b>${_("CAUTION!")}</b></span>
-     ${_("Once the 'Send Email' button is clicked, your email will be queued for sending.")}
-   <b>${_("A queued email CANNOT be cancelled.")}</b></p>
+     ${_("When you select Send Email, your email message is added to the queue for sending, and cannot be cancelled.")}
+   </p>
 </div>
  <br />
   <input type="button" name="send" value="${_("Send Email")}" data-endpoint="${ section_data['send_email'] }" >
@@ -77,20 +87,20 @@ from openedx.core.djangolib.markup import HTML
 <div class="vert-left email-background" id="section-task-history">
   <h2> ${_("Email Task History")} </h2>
   <div>
-    <p>${_("To see the content of all previously sent emails, click this button:")}</p>
+    <p>${_("To see the content of previously sent emails, click this button:")}</p>
     <br/>
     <input type="button" name="task-history-email-content" value="${_("Sent Email History")}" data-endpoint="${ section_data['email_content_history_url'] }" >
     <div class="content-request-response-error msg msg-warning copy"></div>
     <p>
     <div class="content-history-email-table">
-      <p><em>${_("To read an email, click its subject.")}</em></p>
+      <p><em>${_("To read a sent email message, click its subject.")}</em></p>
       <br/>
       <div class="content-history-table-inner"></div>
     </div>
     <div class="email-messages-wrapper"></div>
   </div>
   <div>
-    <p>${_("To see the status for all bulk email tasks ever submitted for this course, click on this button:")}</p>
+    <p>${_("To see the status for all email tasks submitted for this course, click this button:")}</p>
     <br/>
     <input type="button" name="task-history-email" value="${_("Show Email Task History")}" data-endpoint="${ section_data['email_background_tasks_url'] }" >
     <div class="history-request-response-error msg msg-warning copy"></div>