Merge pull request #1359 from edx/brian/bulk-email-rc

Bulk Email improvements for release

AUTHORS

@@ -89,3 +89,4 @@ Akshay Jagadeesh <akjags@gmail.com>
 Nick Parlante <nick.parlante@cs.stanford.edu>
 Marko Seric <marko.seric@math.uzh.ch>
 Felipe Montoya <felipe.montoya@edunext.co>
+Julia Hansbrough <julia@edx.org>

CHANGELOG.rst

@@ -5,10 +5,14 @@ These are notable changes in edx-platform.  This is a rolling list of changes,
 in roughly chronological order, most recent first.  Add your entries at or near
 the top.  Include a label indicating the component affected.
 
-
 LMS: Fix issue with CourseMode expiration dates
 
-LMS: Add PaidCourseRegistration mode, where payment is required before course registration.
+LMS: Ported bulk emailing to the beta instructor dashboard.
+
+LMS: Add monitoring of bulk email subtasks to display progress on instructor dash.
+
+LMS: Add PaidCourseRegistration mode, where payment is required before course
+registration.
 
 LMS: Add split testing functionality for internal use.
 

common/djangoapps/terrain/course_helpers.py

@@ -2,7 +2,7 @@
 # pylint: disable=W0621
 
 from lettuce import world
-from django.contrib.auth.models import User
+from django.contrib.auth.models import User, Group
 from student.models import CourseEnrollment
 from xmodule.modulestore.django import editable_modulestore
 from xmodule.contentstore.django import contentstore
@@ -41,15 +41,30 @@ def log_in(username='robot', password='test', email='robot@edx.org', name='Robot
 
 
 @world.absorb
-def register_by_course_id(course_id, is_staff=False):
-    create_user('robot', 'password')
-    u = User.objects.get(username='robot')
+def register_by_course_id(course_id, username='robot', password='test', is_staff=False):
+    create_user(username, password)
+    user = User.objects.get(username=username)
     if is_staff:
         u.is_staff = True
         u.save()
     CourseEnrollment.enroll(u, course_id)
 
 
+@world.absorb
+def add_to_course_staff(username, course_num):
+    """
+    Add the user with `username` to the course staff group
+    for `course_num`.
+    """
+    # Based on code in lms/djangoapps/courseware/access.py
+    group_name = "instructor_{}".format(course_num)
+    group, _ = Group.objects.get_or_create(name=group_name)
+    group.save()
+
+    user = User.objects.get(username=username)
+    user.groups.add(group)
+
+
 @world.absorb
 def clear_courses():
     # Flush and initialize the module store

common/djangoapps/terrain/ui_helpers.py

@@ -45,8 +45,46 @@ def is_css_not_present(css_selector, wait_time=5):
         world.browser.driver.implicitly_wait(world.IMPLICIT_WAIT)
 
 @world.absorb
-def css_has_text(css_selector, text, index=0):
-    return world.css_text(css_selector, index=index) == text
+def css_has_text(css_selector, text, index=0, strip=False):
+    """
+    Return a boolean indicating whether the element with `css_selector`
+    has `text`.
+
+    If `strip` is True, strip whitespace at beginning/end of both
+    strings before comparing.
+
+    If there are multiple elements matching the css selector,
+    use `index` to indicate which one.
+    """
+    # If we're expecting a non-empty string, give the page
+    # a chance to fill in text fields.
+    if text:
+        world.wait_for(lambda _: world.css_text(css_selector, index=index))
+
+    actual_text = world.css_text(css_selector, index=index)
+
+    if strip:
+        actual_text = actual_text.strip()
+        text = text.strip()
+
+    return actual_text == text
+
+
+@world.absorb
+def css_has_value(css_selector, value, index=0):
+    """
+    Return a boolean indicating whether the element with
+    `css_selector` has the specified `value`.
+
+    If there are multiple elements matching the css selector,
+    use `index` to indicate which one.
+    """
+    # If we're expecting a non-empty string, give the page
+    # a chance to fill in values
+    if value:
+        world.wait_for(lambda _: world.css_value(css_selector, index=index))
+
+    return world.css_value(css_selector, index=index) == value
 
 
 @world.absorb

lms/djangoapps/bulk_email/admin.py

@@ -3,8 +3,8 @@ Django admin page for bulk email models
 """
 from django.contrib import admin
 
-from bulk_email.models import CourseEmail, Optout, CourseEmailTemplate
-from bulk_email.forms import CourseEmailTemplateForm
+from bulk_email.models import CourseEmail, Optout, CourseEmailTemplate, CourseAuthorization
+from bulk_email.forms import CourseEmailTemplateForm, CourseAuthorizationAdminForm
 
 
 class CourseEmailAdmin(admin.ModelAdmin):
@@ -57,6 +57,23 @@ unsupported tags will cause email sending to fail.
         return False
 
 
+class CourseAuthorizationAdmin(admin.ModelAdmin):
+    """Admin for enabling email on a course-by-course basis."""
+    form = CourseAuthorizationAdminForm
+    fieldsets = (
+        (None, {
+            'fields': ('course_id', 'email_enabled'),
+            'description': '''
+Enter a course id in the following form: Org/Course/CourseRun, eg MITx/6.002x/2012_Fall
+Do not enter leading or trailing slashes. There is no need to surround the course ID with quotes.
+Validation will be performed on the course name, and if it is invalid, an error message will display.
+
+To enable email for the course, check the "Email enabled" box, then click "Save".
+'''
+        }),
+    )
+
 admin.site.register(CourseEmail, CourseEmailAdmin)
 admin.site.register(Optout, OptoutAdmin)
 admin.site.register(CourseEmailTemplate, CourseEmailTemplateAdmin)
+admin.site.register(CourseAuthorization, CourseAuthorizationAdmin)

lms/djangoapps/bulk_email/forms.py

@@ -6,12 +6,16 @@ import logging
 from django import forms
 from django.core.exceptions import ValidationError
 
-from bulk_email.models import CourseEmailTemplate, COURSE_EMAIL_MESSAGE_BODY_TAG
+from bulk_email.models import CourseEmailTemplate, COURSE_EMAIL_MESSAGE_BODY_TAG, CourseAuthorization
+
+from courseware.courses import get_course_by_id
+from xmodule.modulestore import MONGO_MODULESTORE_TYPE
+from xmodule.modulestore.django import modulestore
 
 log = logging.getLogger(__name__)
 
 
-class CourseEmailTemplateForm(forms.ModelForm):
+class CourseEmailTemplateForm(forms.ModelForm):  # pylint: disable=R0924
     """Form providing validation of CourseEmail templates."""
 
     class Meta:  # pylint: disable=C0111
@@ -43,3 +47,32 @@ class CourseEmailTemplateForm(forms.ModelForm):
         template = self.cleaned_data["plain_template"]
         self._validate_template(template)
         return template
+
+
+class CourseAuthorizationAdminForm(forms.ModelForm):  # pylint: disable=R0924
+    """Input form for email enabling, allowing us to verify data."""
+
+    class Meta:  # pylint: disable=C0111
+        model = CourseAuthorization
+
+    def clean_course_id(self):
+        """Validate the course id"""
+        course_id = self.cleaned_data["course_id"]
+        try:
+            # Just try to get the course descriptor.
+            # If we can do that, it's a real course.
+            get_course_by_id(course_id, depth=1)
+        except Exception as exc:
+            msg = 'Error encountered ({0})'.format(str(exc).capitalize())
+            msg += ' --- Entered course id was: "{0}". '.format(course_id)
+            msg += 'Please recheck that you have supplied a course id in the format: ORG/COURSE/RUN'
+            raise forms.ValidationError(msg)
+
+        # Now, try and discern if it is a Studio course - HTML editor doesn't work with XML courses
+        is_studio_course = modulestore().get_modulestore_type(course_id) == MONGO_MODULESTORE_TYPE
+        if not is_studio_course:
+            msg = "Course Email feature is only available for courses authored in Studio. "
+            msg += '"{0}" appears to be an XML backed course.'.format(course_id)
+            raise forms.ValidationError(msg)
+
+        return course_id

lms/djangoapps/bulk_email/migrations/0008_add_course_authorizations.py

+# -*- coding: utf-8 -*-
+import datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Adding model 'CourseAuthorization'
+        db.create_table('bulk_email_courseauthorization', (
+            ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)),
+            ('course_id', self.gf('django.db.models.fields.CharField')(max_length=255, db_index=True)),
+            ('email_enabled', self.gf('django.db.models.fields.BooleanField')(default=False)),
+        ))
+        db.send_create_signal('bulk_email', ['CourseAuthorization'])
+
+
+    def backwards(self, orm):
+        # Deleting model 'CourseAuthorization'
+        db.delete_table('bulk_email_courseauthorization')
+
+
+    models = {
+        'auth.group': {
+            'Meta': {'object_name': 'Group'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
+            'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
+        },
+        'auth.permission': {
+            'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
+            'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+        },
+        'auth.user': {
+            'Meta': {'object_name': 'User'},
+            'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}),
+            'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+            'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}),
+            'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'})
+        },
+        'bulk_email.courseauthorization': {
+            'Meta': {'object_name': 'CourseAuthorization'},
+            'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'email_enabled': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'})
+        },
+        'bulk_email.courseemail': {
+            'Meta': {'object_name': 'CourseEmail'},
+            'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
+            'html_message': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'modified': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'blank': 'True'}),
+            'sender': ('django.db.models.fields.related.ForeignKey', [], {'default': '1', 'to': "orm['auth.User']", 'null': 'True', 'blank': 'True'}),
+            'slug': ('django.db.models.fields.CharField', [], {'max_length': '128', 'db_index': 'True'}),
+            'subject': ('django.db.models.fields.CharField', [], {'max_length': '128', 'blank': 'True'}),
+            'text_message': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}),
+            'to_option': ('django.db.models.fields.CharField', [], {'default': "'myself'", 'max_length': '64'})
+        },
+        'bulk_email.courseemailtemplate': {
+            'Meta': {'object_name': 'CourseEmailTemplate'},
+            'html_template': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'plain_template': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'})
+        },
+        'bulk_email.optout': {
+            'Meta': {'unique_together': "(('user', 'course_id'),)", 'object_name': 'Optout'},
+            'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'null': 'True'})
+        },
+        'contenttypes.contenttype': {
+            'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
+            'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
+        }
+    }
+
+    complete_apps = ['bulk_email']
\ No newline at end of file

lms/djangoapps/bulk_email/models.py

@@ -12,11 +12,21 @@ file and check it in at the same time as your model changes. To do that,
 
 """
 import logging
-from django.db import models
+from django.db import models, transaction
 from django.contrib.auth.models import User
+from html_to_text import html_to_text
+
+from django.conf import settings
 
 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):
     """
@@ -33,12 +43,8 @@ class Email(models.Model):
     class Meta:  # pylint: disable=C0111
         abstract = True
 
-SEND_TO_MYSELF = 'myself'
-SEND_TO_STAFF = 'staff'
-SEND_TO_ALL = 'all'
-
 
-class CourseEmail(Email, models.Model):
+class CourseEmail(Email):
     """
     Stores information for an email to a course.
     """
@@ -51,17 +57,66 @@ class CourseEmail(Email, models.Model):
     # * All: This sends an email to anyone enrolled in the course, with any role
     #   (student, staff, or instructor)
     #
-    TO_OPTIONS = (
+    TO_OPTION_CHOICES = (
         (SEND_TO_MYSELF, 'Myself'),
         (SEND_TO_STAFF, 'Staff and instructors'),
         (SEND_TO_ALL, 'All')
     )
     course_id = models.CharField(max_length=255, db_index=True)
-    to_option = models.CharField(max_length=64, choices=TO_OPTIONS, default=SEND_TO_MYSELF)
+    to_option = models.CharField(max_length=64, choices=TO_OPTION_CHOICES, default=SEND_TO_MYSELF)
 
     def __unicode__(self):
         return self.subject
 
+    @classmethod
+    def create(cls, course_id, sender, to_option, subject, html_message, text_message=None):
+        """
+        Create an instance of CourseEmail.
+
+        The CourseEmail.save_now method makes sure the CourseEmail entry is committed.
+        When called from any view that is wrapped by TransactionMiddleware,
+        and thus in a "commit-on-success" transaction, an autocommit buried within here
+        will cause any pending transaction to be committed by a successful
+        save here.  Any future database operations will take place in a
+        separate transaction.
+        """
+        # automatically generate the stripped version of the text from the HTML markup:
+        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)
+
+        # 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,
+        )
+        course_email.save_now()
+
+        return course_email
+
+    @transaction.autocommit
+    def save_now(self):
+        """
+        Writes CourseEmail immediately, ensuring the transaction is committed.
+
+        Autocommit annotation makes sure the database entry is committed.
+        When called from any view that is wrapped by TransactionMiddleware,
+        and thus in a "commit-on-success" transaction, this autocommit here
+        will cause any pending transaction to be committed by a successful
+        save here.  Any future database operations will take place in a
+        separate transaction.
+        """
+        self.save()
+
 
 class Optout(models.Model):
     """
@@ -101,7 +156,11 @@ class CourseEmailTemplate(models.Model):
 
         If one isn't stored, an exception is thrown.
         """
-        return CourseEmailTemplate.objects.get()
+        try:
+            return CourseEmailTemplate.objects.get()
+        except CourseEmailTemplate.DoesNotExist:
+            log.exception("Attempting to fetch a non-existent course email template")
+            raise
 
     @staticmethod
     def _render(format_string, message_body, context):
@@ -153,3 +212,38 @@ class CourseEmailTemplate(models.Model):
         stored HTML template and the provided `context` dict.
         """
         return CourseEmailTemplate._render(self.html_template, htmltext, context)
+
+
+class CourseAuthorization(models.Model):
+    """
+    Enable the course email feature on a course-by-course basis.
+    """
+    # The course that these features are attached to.
+    course_id = models.CharField(max_length=255, db_index=True)
+
+    # Whether or not to enable instructor email
+    email_enabled = models.BooleanField(default=False)
+
+    @classmethod
+    def instructor_email_enabled(cls, course_id):
+        """
+        Returns whether or not email is enabled for the given course id.
+
+        If email has not been explicitly enabled, returns False.
+        """
+        # If settings.MITX_FEATURES['REQUIRE_COURSE_EMAIL_AUTH'] is
+        # set to False, then we enable email for every course.
+        if not settings.MITX_FEATURES['REQUIRE_COURSE_EMAIL_AUTH']:
+            return True
+
+        try:
+            record = cls.objects.get(course_id=course_id)
+            return record.email_enabled
+        except cls.DoesNotExist:
+            return False
+
+    def __unicode__(self):
+        not_en = "Not "
+        if self.email_enabled:
+            not_en = ""
+        return u"Course '{}': Instructor Email {}Enabled".format(self.course_id, not_en)

lms/djangoapps/bulk_email/tests/test_course_optout.py

@@ -59,7 +59,7 @@ class TestOptoutCourseEmails(ModuleStoreTestCase):
         selected_email_link = '<a href="#" onclick="goto(\'Email\')" class="selectedmode">Email</a>'
         self.assertTrue(selected_email_link in response.content)
 
-    @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True})
+    @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False})
     def test_optout_course(self):
         """
         Make sure student does not receive course email after opting out.
@@ -88,7 +88,7 @@ class TestOptoutCourseEmails(ModuleStoreTestCase):
         # Assert that self.student.email not in mail.to, outbox should be empty
         self.assertEqual(len(mail.outbox), 0)
 
-    @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True})
+    @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False})
     def test_optin_course(self):
         """
         Make sure student receives course email after opting in.

lms/djangoapps/bulk_email/tests/test_email.py

@@ -2,6 +2,8 @@
 """
 Unit tests for sending course email
 """
+from mock import patch
+
 from django.conf import settings
 from django.core import mail
 from django.core.urlresolvers import reverse
@@ -12,11 +14,8 @@ from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
 from student.tests.factories import UserFactory, GroupFactory, CourseEnrollmentFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
-
-from bulk_email.tasks import delegate_email_batches, course_email
-from bulk_email.models import CourseEmail, Optout
-
-from mock import patch
+from bulk_email.models import Optout
+from instructor_task.subtasks import increment_subtask_status
 
 STAFF_COUNT = 3
 STUDENT_COUNT = 10
@@ -30,13 +29,13 @@ class MockCourseEmailResult(object):
     """
     emails_sent = 0
 
-    def get_mock_course_email_result(self):
+    def get_mock_increment_subtask_status(self):
         """Wrapper for mock email function."""
-        def mock_course_email_result(sent, failed, output, **kwargs):  # pylint: disable=W0613
+        def mock_increment_subtask_status(original_status, **kwargs):  # pylint: disable=W0613
             """Increments count of number of emails sent."""
-            self.emails_sent += sent
-            return True
-        return mock_course_email_result
+            self.emails_sent += kwargs.get('succeeded', 0)
+            return increment_subtask_status(original_status, **kwargs)
+        return mock_increment_subtask_status
 
 
 @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
@@ -45,7 +44,7 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase):
     Test that emails send correctly.
     """
 
-    @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True})
+    @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False})
     def setUp(self):
         self.course = CourseFactory.create()
         self.instructor = UserFactory.create(username="instructor", email="robot+instructor@edx.org")
@@ -244,14 +243,14 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase):
             [self.instructor.email] + [s.email for s in self.staff] + [s.email for s in self.students]
         )
 
-    @override_settings(EMAILS_PER_TASK=3, EMAILS_PER_QUERY=7)
-    @patch('bulk_email.tasks.course_email_result')
+    @override_settings(BULK_EMAIL_EMAILS_PER_TASK=3, BULK_EMAIL_EMAILS_PER_QUERY=7)
+    @patch('bulk_email.tasks.increment_subtask_status')
     def test_chunked_queries_send_numerous_emails(self, email_mock):
         """
         Test sending a large number of emails, to test the chunked querying
         """
         mock_factory = MockCourseEmailResult()
-        email_mock.side_effect = mock_factory.get_mock_course_email_result()
+        email_mock.side_effect = mock_factory.get_mock_increment_subtask_status()
         added_users = []
         for _ in xrange(LARGE_NUM_EMAILS):
             user = UserFactory()
@@ -281,14 +280,3 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase):
                                 [s.email for s in self.students] +
                                 [s.email for s in added_users if s not in optouts])
         self.assertItemsEqual(outbox_contents, should_send_contents)
-
-
-@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
-class TestEmailSendExceptions(ModuleStoreTestCase):
-    """
-    Test that exceptions are handled correctly.
-    """
-    def test_no_course_email_obj(self):
-        # Make sure course_email handles CourseEmail.DoesNotExist exception.
-        with self.assertRaises(CourseEmail.DoesNotExist):
-            course_email(101, [], "_", "_", "_", False)

lms/djangoapps/bulk_email/tests/test_err_handling.py

@@ -2,21 +2,24 @@
 Unit tests for handling email sending errors
 """
 from itertools import cycle
+from mock import patch
+from smtplib import SMTPDataError, SMTPServerDisconnected, SMTPConnectError
+
 from django.test.utils import override_settings
 from django.conf import settings
 from django.core.management import call_command
 from django.core.urlresolvers import reverse
 
+
 from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 from student.tests.factories import UserFactory, AdminFactory, CourseEnrollmentFactory
 
-from bulk_email.models import CourseEmail
-from bulk_email.tasks import delegate_email_batches
-
-from mock import patch, Mock
-from smtplib import SMTPDataError, SMTPServerDisconnected, SMTPConnectError
+from bulk_email.models import CourseEmail, SEND_TO_ALL
+from bulk_email.tasks import perform_delegate_email_batches, send_course_email
+from instructor_task.models import InstructorTask
+from instructor_task.subtasks import create_subtask_status, initialize_subtask_info
 
 
 class EmailTestException(Exception):
@@ -43,7 +46,7 @@ class TestEmailErrors(ModuleStoreTestCase):
         patch.stopall()
 
     @patch('bulk_email.tasks.get_connection', autospec=True)
-    @patch('bulk_email.tasks.course_email.retry')
+    @patch('bulk_email.tasks.send_course_email.retry')
     def test_data_err_retry(self, retry, get_conn):
         """
         Test that celery handles transient SMTPDataErrors by retrying.
@@ -64,15 +67,16 @@ class TestEmailErrors(ModuleStoreTestCase):
         self.assertTrue(type(exc) == SMTPDataError)
 
     @patch('bulk_email.tasks.get_connection', autospec=True)
-    @patch('bulk_email.tasks.course_email_result')
-    @patch('bulk_email.tasks.course_email.retry')
+    @patch('bulk_email.tasks.increment_subtask_status')
+    @patch('bulk_email.tasks.send_course_email.retry')
     def test_data_err_fail(self, retry, result, get_conn):
         """
         Test that celery handles permanent SMTPDataErrors by failing and not retrying.
         """
+        # have every fourth email fail due to blacklisting:
         get_conn.return_value.send_messages.side_effect = cycle([SMTPDataError(554, "Email address is blacklisted"),
-                                                                 None])
-        students = [UserFactory() for _ in xrange(settings.EMAILS_PER_TASK)]
+                                                                 None, None, None])
+        students = [UserFactory() for _ in xrange(settings.BULK_EMAIL_EMAILS_PER_TASK)]
         for student in students:
             CourseEnrollmentFactory.create(user=student, course_id=self.course.id)
 
@@ -87,13 +91,14 @@ class TestEmailErrors(ModuleStoreTestCase):
         # We shouldn't retry when hitting a 5xx error
         self.assertFalse(retry.called)
         # Test that after the rejected email, the rest still successfully send
-        ((sent, fail, optouts), _) = result.call_args
-        self.assertEquals(optouts, 0)
-        self.assertEquals(fail, settings.EMAILS_PER_TASK / 2)
-        self.assertEquals(sent, settings.EMAILS_PER_TASK / 2)
+        ((_initial_results), kwargs) = result.call_args
+        self.assertEquals(kwargs['skipped'], 0)
+        expected_fails = int((settings.BULK_EMAIL_EMAILS_PER_TASK + 3) / 4.0)
+        self.assertEquals(kwargs['failed'], expected_fails)
+        self.assertEquals(kwargs['succeeded'], settings.BULK_EMAIL_EMAILS_PER_TASK - expected_fails)
 
     @patch('bulk_email.tasks.get_connection', autospec=True)
-    @patch('bulk_email.tasks.course_email.retry')
+    @patch('bulk_email.tasks.send_course_email.retry')
     def test_disconn_err_retry(self, retry, get_conn):
         """
         Test that celery handles SMTPServerDisconnected by retrying.
@@ -113,7 +118,7 @@ class TestEmailErrors(ModuleStoreTestCase):
         self.assertTrue(type(exc) == SMTPServerDisconnected)
 
     @patch('bulk_email.tasks.get_connection', autospec=True)
-    @patch('bulk_email.tasks.course_email.retry')
+    @patch('bulk_email.tasks.send_course_email.retry')
     def test_conn_err_retry(self, retry, get_conn):
         """
         Test that celery handles SMTPConnectError by retrying.
@@ -133,67 +138,107 @@ class TestEmailErrors(ModuleStoreTestCase):
         exc = kwargs['exc']
         self.assertTrue(type(exc) == SMTPConnectError)
 
-    @patch('bulk_email.tasks.course_email_result')
-    @patch('bulk_email.tasks.course_email.retry')
+    @patch('bulk_email.tasks.increment_subtask_status')
     @patch('bulk_email.tasks.log')
-    @patch('bulk_email.tasks.get_connection', Mock(return_value=EmailTestException))
-    def test_general_exception(self, mock_log, retry, result):
-        """
-        Tests the if the error is not SMTP-related, we log and reraise
-        """
-        test_email = {
-            'action': 'Send email',
-            'to_option': 'myself',
-            'subject': 'test subject for myself',
-            'message': 'test message for myself'
-        }
-        # For some reason (probably the weirdness of testing with celery tasks) assertRaises doesn't work here
-        # so we assert on the arguments of log.exception
-        self.client.post(self.url, test_email)
-        ((log_str, email_id, to_list), _) = mock_log.exception.call_args
-        self.assertTrue(mock_log.exception.called)
-        self.assertIn('caused course_email task to fail with uncaught exception.', log_str)
-        self.assertEqual(email_id, 1)
-        self.assertEqual(to_list, [self.instructor.email])
-        self.assertFalse(retry.called)
-        self.assertFalse(result.called)
-
-    @patch('bulk_email.tasks.course_email_result')
-    @patch('bulk_email.tasks.delegate_email_batches.retry')
-    @patch('bulk_email.tasks.log')
-    def test_nonexist_email(self, mock_log, retry, result):
+    def test_nonexistent_email(self, mock_log, result):
         """
         Tests retries when the email doesn't exist
         """
-        delegate_email_batches.delay(-1, self.instructor.id)
-        ((log_str, email_id, _num_retries), _) = mock_log.warning.call_args
+        # create an InstructorTask object to pass through
+        course_id = self.course.id
+        entry = InstructorTask.create(course_id, "task_type", "task_key", "task_input", self.instructor)
+        task_input = {"email_id": -1}
+        with self.assertRaises(CourseEmail.DoesNotExist):
+            perform_delegate_email_batches(entry.id, course_id, task_input, "action_name")  # pylint: disable=E1101
+        ((log_str, _, email_id), _) = mock_log.warning.call_args
         self.assertTrue(mock_log.warning.called)
         self.assertIn('Failed to get CourseEmail with id', log_str)
         self.assertEqual(email_id, -1)
-        self.assertTrue(retry.called)
         self.assertFalse(result.called)
 
-    @patch('bulk_email.tasks.log')
-    def test_nonexist_course(self, mock_log):
+    def test_nonexistent_course(self):
         """
         Tests exception when the course in the email doesn't exist
         """
-        email = CourseEmail(course_id="I/DONT/EXIST")
+        course_id = "I/DONT/EXIST"
+        email = CourseEmail(course_id=course_id)
         email.save()
-        delegate_email_batches.delay(email.id, self.instructor.id)
-        ((log_str, _), _) = mock_log.exception.call_args
-        self.assertTrue(mock_log.exception.called)
-        self.assertIn('get_course_by_id failed:', log_str)
+        entry = InstructorTask.create(course_id, "task_type", "task_key", "task_input", self.instructor)
+        task_input = {"email_id": email.id}  # pylint: disable=E1101
+        with self.assertRaisesRegexp(ValueError, "Course not found"):
+            perform_delegate_email_batches(entry.id, course_id, task_input, "action_name")  # pylint: disable=E1101
 
-    @patch('bulk_email.tasks.log')
-    def test_nonexist_to_option(self, mock_log):
+    def test_nonexistent_to_option(self):
         """
         Tests exception when the to_option in the email doesn't exist
         """
         email = CourseEmail(course_id=self.course.id, to_option="IDONTEXIST")
         email.save()
-        delegate_email_batches.delay(email.id, self.instructor.id)
-        ((log_str, opt_str), _) = mock_log.error.call_args
-        self.assertTrue(mock_log.error.called)
-        self.assertIn('Unexpected bulk email TO_OPTION found', log_str)
-        self.assertEqual("IDONTEXIST", opt_str)
+        entry = InstructorTask.create(self.course.id, "task_type", "task_key", "task_input", self.instructor)
+        task_input = {"email_id": email.id}  # pylint: disable=E1101
+        with self.assertRaisesRegexp(Exception, 'Unexpected bulk email TO_OPTION found: IDONTEXIST'):
+            perform_delegate_email_batches(entry.id, self.course.id, task_input, "action_name")  # pylint: disable=E1101
+
+    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()
+        entry = InstructorTask.create("bogus_task_id", "task_type", "task_key", "task_input", self.instructor)
+        task_input = {"email_id": email.id}  # pylint: disable=E1101
+        with self.assertRaisesRegexp(ValueError, 'does not match task value'):
+            perform_delegate_email_batches(entry.id, self.course.id, task_input, "action_name")  # pylint: disable=E1101
+
+    def test_wrong_course_id_in_email(self):
+        """
+        Tests exception when the course_id in CourseEmail is not the same as one explicitly passed in.
+        """
+        email = CourseEmail(course_id="bogus_course_id", to_option=SEND_TO_ALL)
+        email.save()
+        entry = InstructorTask.create(self.course.id, "task_type", "task_key", "task_input", self.instructor)
+        task_input = {"email_id": email.id}  # pylint: disable=E1101
+        with self.assertRaisesRegexp(ValueError, 'does not match email value'):
+            perform_delegate_email_batches(entry.id, self.course.id, task_input, "action_name")  # pylint: disable=E1101
+
+    def test_send_email_undefined_subtask(self):
+        # test at a lower level, to ensure that the course gets checked down below too.
+        entry = InstructorTask.create(self.course.id, "task_type", "task_key", "task_input", self.instructor)
+        entry_id = entry.id  # pylint: disable=E1101
+        to_list = ['test@test.com']
+        global_email_context = {'course_title': 'dummy course'}
+        subtask_id = "subtask-id-value"
+        subtask_status = create_subtask_status(subtask_id)
+        email_id = 1001
+        with self.assertRaisesRegexp(ValueError, 'unable to find email subtasks of instructor task'):
+            send_course_email(entry_id, email_id, to_list, global_email_context, subtask_status)
+
+    def test_send_email_missing_subtask(self):
+        # test at a lower level, to ensure that the course gets checked down below too.
+        entry = InstructorTask.create(self.course.id, "task_type", "task_key", "task_input", self.instructor)
+        entry_id = entry.id  # pylint: disable=E1101
+        to_list = ['test@test.com']
+        global_email_context = {'course_title': 'dummy course'}
+        subtask_id = "subtask-id-value"
+        initialize_subtask_info(entry, "emailed", 100, [subtask_id])
+        different_subtask_id = "bogus-subtask-id-value"
+        subtask_status = create_subtask_status(different_subtask_id)
+        bogus_email_id = 1001
+        with self.assertRaisesRegexp(ValueError, 'unable to find status for email subtask of instructor task'):
+            send_course_email(entry_id, bogus_email_id, to_list, global_email_context, subtask_status)
+
+    def dont_test_send_email_undefined_email(self):
+        # test at a lower level, to ensure that the course gets checked down below too.
+        entry = InstructorTask.create(self.course.id, "task_type", "task_key", "task_input", self.instructor)
+        entry_id = entry.id  # pylint: disable=E1101
+        to_list = ['test@test.com']
+        global_email_context = {'course_title': 'dummy course'}
+        subtask_id = "subtask-id-value"
+        initialize_subtask_info(entry, "emailed", 100, [subtask_id])
+        subtask_status = create_subtask_status(subtask_id)
+        bogus_email_id = 1001
+        with self.assertRaises(CourseEmail.DoesNotExist):
+            # we skip the call that updates subtask status, since we've not set up the InstructorTask
+            # for the subtask, and it's not important to the test.
+            with patch('bulk_email.tasks.update_subtask_status'):
+                send_course_email(entry_id, bogus_email_id, to_list, global_email_context, subtask_status)

lms/djangoapps/bulk_email/tests/test_forms.py

+"""
+Unit tests for bulk-email-related forms.
+"""
+from django.test.utils import override_settings
+from django.conf import settings
+
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory
+from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
+from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
+
+from xmodule.modulestore.django import modulestore
+from xmodule.modulestore import MONGO_MODULESTORE_TYPE
+
+from mock import patch
+
+from bulk_email.models import CourseAuthorization
+from bulk_email.forms import CourseAuthorizationAdminForm
+
+
+@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
+class CourseAuthorizationFormTest(ModuleStoreTestCase):
+    """Test the CourseAuthorizationAdminForm form for Mongo-backed courses."""
+
+    def setUp(self):
+        # Make a mongo course
+        self.course = CourseFactory.create()
+
+    def tearDown(self):
+        """
+        Undo all patches.
+        """
+        patch.stopall()
+
+    @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True})
+    def test_authorize_mongo_course(self):
+        # Initially course shouldn't be authorized
+        self.assertFalse(CourseAuthorization.instructor_email_enabled(self.course.id))
+        # Test authorizing the course, which should totally work
+        form_data = {'course_id': self.course.id, 'email_enabled': True}
+        form = CourseAuthorizationAdminForm(data=form_data)
+        # Validation should work
+        self.assertTrue(form.is_valid())
+        form.save()
+        # Check that this course is authorized
+        self.assertTrue(CourseAuthorization.instructor_email_enabled(self.course.id))
+
+    @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True})
+    def test_form_typo(self):
+        # Munge course id
+        bad_id = self.course.id + '_typo'
+
+        form_data = {'course_id': bad_id, 'email_enabled': True}
+        form = CourseAuthorizationAdminForm(data=form_data)
+        # Validation shouldn't work
+        self.assertFalse(form.is_valid())
+
+        msg = u'Error encountered (Course not found.)'
+        msg += ' --- Entered course id was: "{0}". '.format(bad_id)
+        msg += 'Please recheck that you have supplied a course id in the format: ORG/COURSE/RUN'
+        self.assertEquals(msg, form._errors['course_id'][0])  # pylint: disable=protected-access
+
+        with self.assertRaisesRegexp(ValueError, "The CourseAuthorization could not be created because the data didn't validate."):
+            form.save()
+
+    @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True})
+    def test_course_name_only(self):
+        # Munge course id - common
+        bad_id = self.course.id.split('/')[-1]
+
+        form_data = {'course_id': bad_id, 'email_enabled': True}
+        form = CourseAuthorizationAdminForm(data=form_data)
+        # Validation shouldn't work
+        self.assertFalse(form.is_valid())
+
+        msg = u'Error encountered (Need more than 1 value to unpack)'
+        msg += ' --- Entered course id was: "{0}". '.format(bad_id)
+        msg += 'Please recheck that you have supplied a course id in the format: ORG/COURSE/RUN'
+        self.assertEquals(msg, form._errors['course_id'][0])  # pylint: disable=protected-access
+
+        with self.assertRaisesRegexp(ValueError, "The CourseAuthorization could not be created because the data didn't validate."):
+            form.save()
+
+
+@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
+class CourseAuthorizationXMLFormTest(ModuleStoreTestCase):
+    """Check that XML courses cannot be authorized for email."""
+
+    @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True})
+    def test_xml_course_authorization(self):
+        course_id = 'edX/toy/2012_Fall'
+        # Assert this is an XML course
+        self.assertTrue(modulestore().get_modulestore_type(course_id) != MONGO_MODULESTORE_TYPE)
+
+        form_data = {'course_id': course_id, 'email_enabled': True}
+        form = CourseAuthorizationAdminForm(data=form_data)
+        # Validation shouldn't work
+        self.assertFalse(form.is_valid())
+
+        msg = u"Course Email feature is only available for courses authored in Studio. "
+        msg += '"{0}" appears to be an XML backed course.'.format(course_id)
+        self.assertEquals(msg, form._errors['course_id'][0])  # pylint: disable=protected-access
+
+        with self.assertRaisesRegexp(ValueError, "The CourseAuthorization could not be created because the data didn't validate."):
+            form.save()

lms/djangoapps/bulk_email/tests/test_models.py

+"""
+Unit tests for bulk-email-related models.
+"""
+from django.test import TestCase
+from django.core.management import call_command
+from django.conf import settings
+
+from student.tests.factories import UserFactory
+
+from mock import patch
+
+from bulk_email.models import CourseEmail, SEND_TO_STAFF, CourseEmailTemplate, CourseAuthorization
+
+
+class CourseEmailTest(TestCase):
+    """Test the CourseEmail model."""
+
+    def test_creation(self):
+        course_id = 'abc/123/doremi'
+        sender = UserFactory.create()
+        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)
+        self.assertEquals(email.course_id, course_id)
+        self.assertEquals(email.to_option, SEND_TO_STAFF)
+        self.assertEquals(email.subject, subject)
+        self.assertEquals(email.html_message, html_message)
+        self.assertEquals(email.sender, sender)
+
+    def test_bad_to_option(self):
+        course_id = 'abc/123/doremi'
+        sender = UserFactory.create()
+        to_option = "fake"
+        subject = "dummy subject"
+        html_message = "<html>dummy message</html>"
+        with self.assertRaises(ValueError):
+            CourseEmail.create(course_id, sender, to_option, subject, html_message)
+
+
+class NoCourseEmailTemplateTest(TestCase):
+    """Test the CourseEmailTemplate model without loading the template data."""
+
+    def test_get_missing_template(self):
+        with self.assertRaises(CourseEmailTemplate.DoesNotExist):
+            CourseEmailTemplate.get_template()
+
+
+class CourseEmailTemplateTest(TestCase):
+    """Test the CourseEmailTemplate model."""
+
+    def setUp(self):
+        # load initial content (since we don't run migrations as part of tests):
+        call_command("loaddata", "course_email_template.json")
+
+    def _get_sample_plain_context(self):
+        """Provide sample context sufficient for rendering plaintext template"""
+        context = {
+            'course_title': "Bogus Course Title",
+            'course_url': "/location/of/course/url",
+            'account_settings_url': "/location/of/account/settings/url",
+            'platform_name': 'edX',
+            'email': 'your-email@test.com',
+        }
+        return context
+
+    def _get_sample_html_context(self):
+        """Provide sample context sufficient for rendering HTML template"""
+        context = self._get_sample_plain_context()
+        context['course_image_url'] = "/location/of/course/image/url"
+        return context
+
+    def test_get_template(self):
+        template = CourseEmailTemplate.get_template()
+        self.assertIsNotNone(template.html_template)
+        self.assertIsNotNone(template.plain_template)
+
+    def test_render_html_without_context(self):
+        template = CourseEmailTemplate.get_template()
+        base_context = self._get_sample_html_context()
+        for keyname in base_context:
+            context = dict(base_context)
+            del context[keyname]
+            with self.assertRaises(KeyError):
+                template.render_htmltext("My new html text.", context)
+
+    def test_render_plaintext_without_context(self):
+        template = CourseEmailTemplate.get_template()
+        base_context = self._get_sample_plain_context()
+        for keyname in base_context:
+            context = dict(base_context)
+            del context[keyname]
+            with self.assertRaises(KeyError):
+                template.render_plaintext("My new plain text.", context)
+
+    def test_render_html(self):
+        template = CourseEmailTemplate.get_template()
+        context = self._get_sample_html_context()
+        template.render_htmltext("My new html text.", context)
+
+    def test_render_plain(self):
+        template = CourseEmailTemplate.get_template()
+        context = self._get_sample_plain_context()
+        template.render_plaintext("My new plain text.", context)
+
+
+class CourseAuthorizationTest(TestCase):
+    """Test the CourseAuthorization model."""
+
+    @patch.dict(settings.MITX_FEATURES, {'REQUIRE_COURSE_EMAIL_AUTH': True})
+    def test_creation_auth_on(self):
+        course_id = 'abc/123/doremi'
+        # Test that course is not authorized by default
+        self.assertFalse(CourseAuthorization.instructor_email_enabled(course_id))
+
+        # Authorize
+        cauth = CourseAuthorization(course_id=course_id, email_enabled=True)
+        cauth.save()
+        # Now, course should be authorized
+        self.assertTrue(CourseAuthorization.instructor_email_enabled(course_id))
+        self.assertEquals(
+            cauth.__unicode__(),
+            "Course 'abc/123/doremi': Instructor Email Enabled"
+        )
+
+        # Unauthorize by explicitly setting email_enabled to False
+        cauth.email_enabled = False
+        cauth.save()
+        # Test that course is now unauthorized
+        self.assertFalse(CourseAuthorization.instructor_email_enabled(course_id))
+        self.assertEquals(
+            cauth.__unicode__(),
+            "Course 'abc/123/doremi': Instructor Email Not Enabled"
+        )
+
+    @patch.dict(settings.MITX_FEATURES, {'REQUIRE_COURSE_EMAIL_AUTH': False})
+    def test_creation_auth_off(self):
+        course_id = 'blahx/blah101/ehhhhhhh'
+        # Test that course is authorized by default, since auth is turned off
+        self.assertTrue(CourseAuthorization.instructor_email_enabled(course_id))
+
+        # Use the admin interface to unauthorize the course
+        cauth = CourseAuthorization(course_id=course_id, email_enabled=False)
+        cauth.save()
+
+        # Now, course should STILL be authorized!
+        self.assertTrue(CourseAuthorization.instructor_email_enabled(course_id))

lms/djangoapps/bulk_email/tests/test_tasks.py

+"""
+Unit tests for LMS instructor-initiated background tasks.
+
+Runs tasks on answers to course problems to validate that code
+paths actually work.
+
+"""
+import json
+from uuid import uuid4
+from itertools import cycle, chain, repeat
+from mock import patch, Mock
+from smtplib import SMTPServerDisconnected, SMTPDataError, SMTPConnectError, SMTPAuthenticationError
+from boto.ses.exceptions import (
+    SESDailyQuotaExceededError,
+    SESMaxSendingRateExceededError,
+    SESAddressBlacklistedError,
+    SESIllegalAddressError,
+    SESLocalAddressCharacterError,
+)
+from boto.exception import AWSConnectionError
+
+from celery.states import SUCCESS, FAILURE
+
+from django.conf import settings
+from django.core.management import call_command
+
+from bulk_email.models import CourseEmail, Optout, SEND_TO_ALL
+
+from instructor_task.tasks import send_bulk_course_email
+from instructor_task.subtasks import update_subtask_status
+from instructor_task.models import InstructorTask
+from instructor_task.tests.test_base import InstructorTaskCourseTestCase
+from instructor_task.tests.factories import InstructorTaskFactory
+
+
+class TestTaskFailure(Exception):
+    """Dummy exception used for unit tests."""
+    pass
+
+
+def my_update_subtask_status(entry_id, current_task_id, new_subtask_status):
+    """
+    Check whether a subtask has been updated before really updating.
+
+    Check whether a subtask which has been retried
+    has had the retry already write its results here before the code
+    that was invoking the retry had a chance to update this status.
+
+    This is the norm in "eager" mode (used by tests) where the retry is called
+    and run to completion before control is returned to the code that
+    invoked the retry.  If the retries eventually end in failure (e.g. due to
+    a maximum number of retries being attempted), the "eager" code will return
+    the error for each retry as it is popped off the stack.  We want to just ignore
+    the later updates that are called as the result of the earlier retries.
+
+    This should not be an issue in production, where status is updated before
+    a task is retried, and is then updated afterwards if the retry fails.
+    """
+    entry = InstructorTask.objects.get(pk=entry_id)
+    subtask_dict = json.loads(entry.subtasks)
+    subtask_status_info = subtask_dict['status']
+    current_subtask_status = subtask_status_info[current_task_id]
+
+    def _get_retry_count(subtask_result):
+        """Return the number of retries counted for the given subtask."""
+        retry_count = subtask_result.get('retried_nomax', 0)
+        retry_count += subtask_result.get('retried_withmax', 0)
+        return retry_count
+
+    current_retry_count = _get_retry_count(current_subtask_status)
+    new_retry_count = _get_retry_count(new_subtask_status)
+    if current_retry_count <= new_retry_count:
+        update_subtask_status(entry_id, current_task_id, new_subtask_status)
+
+
+class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
+    """Tests instructor task that send bulk email."""
+
+    def setUp(self):
+        super(TestBulkEmailInstructorTask, self).setUp()
+        self.initialize_course()
+        self.instructor = self.create_instructor('instructor')
+
+        # load initial content (since we don't run migrations as part of tests):
+        call_command("loaddata", "course_email_template.json")
+
+    def _create_input_entry(self, course_id=None):
+        """
+        Creates a InstructorTask entry for testing.
+
+        Overrides the base class version in that this creates CourseEmail.
+        """
+        to_option = SEND_TO_ALL
+        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>")
+        task_input = {'email_id': course_email.id}  # pylint: disable=E1101
+        task_id = str(uuid4())
+        instructor_task = InstructorTaskFactory.create(
+            course_id=course_id,
+            requester=self.instructor,
+            task_input=json.dumps(task_input),
+            task_key='dummy value',
+            task_id=task_id,
+        )
+        return instructor_task
+
+    def _run_task_with_mock_celery(self, task_class, entry_id, task_id):
+        """Submit a task and mock how celery provides a current_task."""
+        mock_current_task = Mock()
+        mock_current_task.max_retries = settings.BULK_EMAIL_MAX_RETRIES
+        mock_current_task.default_retry_delay = settings.BULK_EMAIL_DEFAULT_RETRY_DELAY
+        task_args = [entry_id, {}]
+
+        with patch('bulk_email.tasks._get_current_task') as mock_get_task:
+            mock_get_task.return_value = mock_current_task
+            return task_class.apply(task_args, task_id=task_id).get()
+
+    def test_email_missing_current_task(self):
+        task_entry = self._create_input_entry()
+        with self.assertRaises(ValueError):
+            send_bulk_course_email(task_entry.id, {})
+
+    def test_email_undefined_course(self):
+        # Check that we fail when passing in a course that doesn't exist.
+        task_entry = self._create_input_entry(course_id="bogus/course/id")
+        with self.assertRaises(ValueError):
+            self._run_task_with_mock_celery(send_bulk_course_email, task_entry.id, task_entry.task_id)
+
+    def test_bad_task_id_on_update(self):
+        task_entry = self._create_input_entry()
+
+        def dummy_update_subtask_status(entry_id, _current_task_id, new_subtask_status):
+            """Passes a bad value for task_id to test update_subtask_status"""
+            bogus_task_id = "this-is-bogus"
+            update_subtask_status(entry_id, bogus_task_id, new_subtask_status)
+
+        with self.assertRaises(ValueError):
+            with patch('bulk_email.tasks.update_subtask_status', dummy_update_subtask_status):
+                send_bulk_course_email(task_entry.id, {})  # pylint: disable=E1101
+
+    def _create_students(self, num_students):
+        """Create students for testing"""
+        return [self.create_student('robot%d' % i) for i in xrange(num_students)]
+
+    def _assert_single_subtask_status(self, entry, succeeded, failed=0, skipped=0, retried_nomax=0, retried_withmax=0):
+        """Compare counts with 'subtasks' entry in InstructorTask table."""
+        subtask_info = json.loads(entry.subtasks)
+        # verify subtask-level counts:
+        self.assertEquals(subtask_info.get('total'), 1)
+        self.assertEquals(subtask_info.get('succeeded'), 1 if succeeded > 0 else 0)
+        self.assertEquals(subtask_info.get('failed'), 0 if succeeded > 0 else 1)
+        # verify individual subtask status:
+        subtask_status_info = subtask_info.get('status')
+        task_id_list = subtask_status_info.keys()
+        self.assertEquals(len(task_id_list), 1)
+        task_id = task_id_list[0]
+        subtask_status = subtask_status_info.get(task_id)
+        print("Testing subtask status: {}".format(subtask_status))
+        self.assertEquals(subtask_status.get('task_id'), task_id)
+        self.assertEquals(subtask_status.get('attempted'), succeeded + failed)
+        self.assertEquals(subtask_status.get('succeeded'), succeeded)
+        self.assertEquals(subtask_status.get('skipped'), skipped)
+        self.assertEquals(subtask_status.get('failed'), failed)
+        self.assertEquals(subtask_status.get('retried_nomax'), retried_nomax)
+        self.assertEquals(subtask_status.get('retried_withmax'), retried_withmax)
+        self.assertEquals(subtask_status.get('state'), SUCCESS if succeeded > 0 else FAILURE)
+
+    def _test_run_with_task(self, task_class, action_name, total, succeeded, failed=0, skipped=0, retried_nomax=0, retried_withmax=0):
+        """Run a task and check the number of emails processed."""
+        task_entry = self._create_input_entry()
+        parent_status = self._run_task_with_mock_celery(task_class, task_entry.id, task_entry.task_id)
+
+        # check return value
+        self.assertEquals(parent_status.get('total'), total)
+        self.assertEquals(parent_status.get('action_name'), action_name)
+
+        # compare with task_output entry in InstructorTask table:
+        entry = InstructorTask.objects.get(id=task_entry.id)
+        status = json.loads(entry.task_output)
+        self.assertEquals(status.get('attempted'), succeeded + failed)
+        self.assertEquals(status.get('succeeded'), succeeded)
+        self.assertEquals(status.get('skipped'), skipped)
+        self.assertEquals(status.get('failed'), failed)
+        self.assertEquals(status.get('total'), total)
+        self.assertEquals(status.get('action_name'), action_name)
+        self.assertGreater(status.get('duration_ms'), 0)
+        self.assertEquals(entry.task_state, SUCCESS)
+        self._assert_single_subtask_status(entry, succeeded, failed, skipped, retried_nomax, retried_withmax)
+        return entry
+
+    def test_successful(self):
+        # Select number of emails to fit into a single subtask.
+        num_emails = settings.BULK_EMAIL_EMAILS_PER_TASK
+        # 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])
+            self._test_run_with_task(send_bulk_course_email, 'emailed', num_emails, num_emails)
+
+    def test_successful_twice(self):
+        # Select number of emails to fit into a single subtask.
+        num_emails = settings.BULK_EMAIL_EMAILS_PER_TASK
+        # 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])
+            task_entry = self._test_run_with_task(send_bulk_course_email, 'emailed', num_emails, num_emails)
+
+        # submit the same task a second time, and confirm that it is not run again.
+        with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn:
+            get_conn.return_value.send_messages.side_effect = cycle([Exception("This should not happen!")])
+            parent_status = self._run_task_with_mock_celery(send_bulk_course_email, task_entry.id, task_entry.task_id)
+        self.assertEquals(parent_status.get('total'), num_emails)
+        self.assertEquals(parent_status.get('succeeded'), num_emails)
+        self.assertEquals(parent_status.get('failed'), 0)
+
+    def test_unactivated_user(self):
+        # Select number of emails to fit into a single subtask.
+        num_emails = settings.BULK_EMAIL_EMAILS_PER_TASK
+        # We also send email to the instructor:
+        students = self._create_students(num_emails - 1)
+        # mark a student as not yet having activated their email:
+        student = students[0]
+        student.is_active = False
+        student.save()
+        with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn:
+            get_conn.return_value.send_messages.side_effect = cycle([None])
+            self._test_run_with_task(send_bulk_course_email, 'emailed', num_emails - 1, num_emails - 1)
+
+    def test_skipped(self):
+        # Select number of emails to fit into a single subtask.
+        num_emails = settings.BULK_EMAIL_EMAILS_PER_TASK
+        # We also send email to the instructor:
+        students = self._create_students(num_emails - 1)
+        # have every fourth student optout:
+        expected_skipped = int((num_emails + 3) / 4.0)
+        expected_succeeds = num_emails - expected_skipped
+        for index in range(0, num_emails, 4):
+            Optout.objects.create(user=students[index], course_id=self.course.id)
+        # mark some students as opting out
+        with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn:
+            get_conn.return_value.send_messages.side_effect = cycle([None])
+            self._test_run_with_task(send_bulk_course_email, 'emailed', num_emails, expected_succeeds, skipped=expected_skipped)
+
+    def _test_email_address_failures(self, exception):
+        """Test that celery handles bad address errors by failing and not retrying."""
+        # Select number of emails to fit into a single subtask.
+        num_emails = settings.BULK_EMAIL_EMAILS_PER_TASK
+        # We also send email to the instructor:
+        self._create_students(num_emails - 1)
+        expected_fails = int((num_emails + 3) / 4.0)
+        expected_succeeds = num_emails - expected_fails
+        with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn:
+            # have every fourth email fail due to some address failure:
+            get_conn.return_value.send_messages.side_effect = cycle([exception, None, None, None])
+            self._test_run_with_task(send_bulk_course_email, 'emailed', num_emails, expected_succeeds, failed=expected_fails)
+
+    def test_smtp_blacklisted_user(self):
+        # Test that celery handles permanent SMTPDataErrors by failing and not retrying.
+        self._test_email_address_failures(SMTPDataError(554, "Email address is blacklisted"))
+
+    def test_ses_blacklisted_user(self):
+        # Test that celery handles permanent SMTPDataErrors by failing and not retrying.
+        self._test_email_address_failures(SESAddressBlacklistedError(554, "Email address is blacklisted"))
+
+    def test_ses_illegal_address(self):
+        # Test that celery handles permanent SMTPDataErrors by failing and not retrying.
+        self._test_email_address_failures(SESIllegalAddressError(554, "Email address is illegal"))
+
+    def test_ses_local_address_character_error(self):
+        # Test that celery handles permanent SMTPDataErrors by failing and not retrying.
+        self._test_email_address_failures(SESLocalAddressCharacterError(554, "Email address contains a bad character"))
+
+    def _test_retry_after_limited_retry_error(self, exception):
+        """Test that celery handles connection failures by retrying."""
+        # If we want the batch to succeed, we need to send fewer emails
+        # than the max retries, so that the max is not triggered.
+        num_emails = settings.BULK_EMAIL_MAX_RETRIES
+        # We also send email to the instructor:
+        self._create_students(num_emails - 1)
+        expected_fails = 0
+        expected_succeeds = num_emails
+        with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn:
+            # Have every other mail attempt fail due to disconnection.
+            get_conn.return_value.send_messages.side_effect = cycle([exception, None])
+            self._test_run_with_task(
+                send_bulk_course_email,
+                'emailed',
+                num_emails,
+                expected_succeeds,
+                failed=expected_fails,
+                retried_withmax=num_emails
+            )
+
+    def _test_max_retry_limit_causes_failure(self, exception):
+        """Test that celery can hit a maximum number of retries."""
+        # Doesn't really matter how many recipients, since we expect
+        # to fail on the first.
+        num_emails = 10
+        # We also send email to the instructor:
+        self._create_students(num_emails - 1)
+        expected_fails = num_emails
+        expected_succeeds = 0
+        with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn:
+            # always fail to connect, triggering repeated retries until limit is hit:
+            get_conn.return_value.send_messages.side_effect = cycle([exception])
+            with patch('bulk_email.tasks.update_subtask_status', my_update_subtask_status):
+                self._test_run_with_task(
+                    send_bulk_course_email,
+                    'emailed',
+                    num_emails,
+                    expected_succeeds,
+                    failed=expected_fails,
+                    retried_withmax=(settings.BULK_EMAIL_MAX_RETRIES + 1)
+                )
+
+    def test_retry_after_smtp_disconnect(self):
+        self._test_retry_after_limited_retry_error(SMTPServerDisconnected(425, "Disconnecting"))
+
+    def test_max_retry_after_smtp_disconnect(self):
+        self._test_max_retry_limit_causes_failure(SMTPServerDisconnected(425, "Disconnecting"))
+
+    def test_retry_after_smtp_connect_error(self):
+        self._test_retry_after_limited_retry_error(SMTPConnectError(424, "Bad Connection"))
+
+    def test_max_retry_after_smtp_connect_error(self):
+        self._test_max_retry_limit_causes_failure(SMTPConnectError(424, "Bad Connection"))
+
+    def test_retry_after_aws_connect_error(self):
+        self._test_retry_after_limited_retry_error(AWSConnectionError("Unable to provide secure connection through proxy"))
+
+    def test_max_retry_after_aws_connect_error(self):
+        self._test_max_retry_limit_causes_failure(AWSConnectionError("Unable to provide secure connection through proxy"))
+
+    def test_retry_after_general_error(self):
+        self._test_retry_after_limited_retry_error(Exception("This is some random exception."))
+
+    def test_max_retry_after_general_error(self):
+        self._test_max_retry_limit_causes_failure(Exception("This is some random exception."))
+
+    def _test_retry_after_unlimited_retry_error(self, exception):
+        """Test that celery handles throttling failures by retrying."""
+        num_emails = 8
+        # We also send email to the instructor:
+        self._create_students(num_emails - 1)
+        expected_fails = 0
+        expected_succeeds = num_emails
+        # Note that because celery in eager mode will call retries synchronously,
+        # each retry will increase the stack depth.  It turns out that there is a
+        # maximum depth at which a RuntimeError is raised ("maximum recursion depth
+        # exceeded").  The maximum recursion depth is 90, so
+        # num_emails * expected_retries < 90.
+        expected_retries = 10
+        with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn:
+            # Cycle through N throttling errors followed by a success.
+            get_conn.return_value.send_messages.side_effect = cycle(
+                chain(repeat(exception, expected_retries), [None])
+            )
+            self._test_run_with_task(
+                send_bulk_course_email,
+                'emailed',
+                num_emails,
+                expected_succeeds,
+                failed=expected_fails,
+                retried_nomax=(expected_retries * num_emails)
+            )
+
+    def test_retry_after_smtp_throttling_error(self):
+        self._test_retry_after_unlimited_retry_error(SMTPDataError(455, "Throttling: Sending rate exceeded"))
+
+    def test_retry_after_ses_throttling_error(self):
+        self._test_retry_after_unlimited_retry_error(SESMaxSendingRateExceededError(455, "Throttling: Sending rate exceeded"))
+
+    def _test_immediate_failure(self, exception):
+        """Test that celery can hit a maximum number of retries."""
+        # Doesn't really matter how many recipients, since we expect
+        # to fail on the first.
+        num_emails = 10
+        # We also send email to the instructor:
+        self._create_students(num_emails - 1)
+        expected_fails = num_emails
+        expected_succeeds = 0
+        with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn:
+            # always fail to connect, triggering repeated retries until limit is hit:
+            get_conn.return_value.send_messages.side_effect = cycle([exception])
+            self._test_run_with_task(
+                send_bulk_course_email,
+                'emailed',
+                num_emails,
+                expected_succeeds,
+                failed=expected_fails,
+            )
+
+    def test_failure_on_unhandled_smtp(self):
+        self._test_immediate_failure(SMTPAuthenticationError(403, "That password doesn't work!"))
+
+    def test_failure_on_ses_quota_exceeded(self):
+        self._test_immediate_failure(SESDailyQuotaExceededError(403, "You're done for the day!"))

lms/djangoapps/courseware/courses.py

@@ -36,11 +36,31 @@ def get_request_for_thread():
         del frame
 
 
+def get_course(course_id, depth=0):
+    """
+    Given a course id, return the corresponding course descriptor.
+
+    If course_id is not valid, raises a ValueError.  This is appropriate
+    for internal use.
+
+    depth: The number of levels of children for the modulestore to cache.
+    None means infinite depth.  Default is to fetch no children.
+    """
+    try:
+        course_loc = CourseDescriptor.id_to_location(course_id)
+        return modulestore().get_instance(course_id, course_loc, depth=depth)
+    except (KeyError, ItemNotFoundError):
+        raise ValueError("Course not found: {}".format(course_id))
+    except InvalidLocationError:
+        raise ValueError("Invalid location: {}".format(course_id))
+
+
 def get_course_by_id(course_id, depth=0):
     """
     Given a course id, return the corresponding course descriptor.
 
     If course_id is not valid, raises a 404.
+
     depth: The number of levels of children for the modulestore to cache. None means infinite depth
     """
     try:
@@ -51,6 +71,7 @@ def get_course_by_id(course_id, depth=0):
     except InvalidLocationError:
         raise Http404("Invalid location")
 
+
 def get_course_with_access(user, course_id, action, depth=0):
     """
     Given a course_id, look up the corresponding course descriptor,
@@ -182,7 +203,6 @@ def get_course_about_section(course, section_key):
     raise KeyError("Invalid about key " + str(section_key))
 
 
-
 def get_course_info_section(request, course, section_key):
     """
     This returns the snippet of html to be rendered on the course info page,
@@ -194,8 +214,6 @@ def get_course_info_section(request, course, section_key):
     - updates
     - guest_updates
     """
-
-
     loc = Location(course.location.tag, course.location.org, course.location.course, 'course_info', section_key)
 
     # Use an empty cache

lms/djangoapps/courseware/tests/test_courses.py

@@ -2,15 +2,18 @@
 from django.test import TestCase
 from django.http import Http404
 from django.test.utils import override_settings
-from courseware.courses import get_course_by_id, get_cms_course_link_by_id
+from courseware.courses import get_course_by_id, get_course, get_cms_course_link_by_id
 
 CMS_BASE_TEST = 'testcms'
 
+
 class CoursesTest(TestCase):
+    """Test methods related to fetching courses."""
+
     def test_get_course_by_id_invalid_chars(self):
         """
         Test that `get_course_by_id` throws a 404, rather than
-        an exception, when faced with unexpected characters 
+        an exception, when faced with unexpected characters
         (such as unicode characters, and symbols such as = and ' ')
         """
         with self.assertRaises(Http404):
@@ -18,6 +21,17 @@ class CoursesTest(TestCase):
             get_course_by_id('MITx/foobar/business and management')
             get_course_by_id('MITx/foobar/NiñøJoséMaríáßç')
 
+    def test_get_course_invalid_chars(self):
+        """
+        Test that `get_course` throws a ValueError, rather than
+        a 404, when faced with unexpected characters
+        (such as unicode characters, and symbols such as = and ' ')
+        """
+        with self.assertRaises(ValueError):
+            get_course('MITx/foobar/statistics=introduction')
+            get_course('MITx/foobar/business and management')
+            get_course('MITx/foobar/NiñøJoséMaríáßç')
+
     @override_settings(CMS_BASE=CMS_BASE_TEST)
     def test_get_cms_course_link_by_id(self):
         """

lms/djangoapps/instructor/features/bulk_email.feature

+@shard_2
+Feature: Bulk Email
+    As an instructor or course staff,
+    In order to communicate with students and staff
+    I want to send email to staff and students in a course.
+
+    Scenario: Send bulk email
+    Given I am "<Role>" for a course
+    When I send email to "<Recipient>"
+    Then Email is sent to "<Recipient>"
+
+    Examples:
+    | Role          |  Recipient     |
+    | instructor    |  myself        |
+    | instructor    |  course staff  |
+    | instructor    |  students, staff, and instructors  |
+    | staff         |  myself        |
+    | staff         |  course staff  |
+    | staff         |  students, staff, and instructors  |
+    

lms/djangoapps/instructor/features/bulk_email.py

+"""
+Define steps for bulk email acceptance test.
+"""
+
+#pylint: disable=C0111
+#pylint: disable=W0621
+
+from lettuce import world, step
+from lettuce.django import mail
+from nose.tools import assert_in, assert_true, assert_equal  # pylint: disable=E0611
+from django.core.management import call_command
+from django.conf import settings
+
+
+@step(u'I am an instructor for a course')
+def i_am_an_instructor(step):  # pylint: disable=W0613
+
+    # Clear existing courses to avoid conflicts
+    world.clear_courses()
+
+    # Register the instructor as staff for the course
+    world.register_by_course_id(
+        'edx/999/Test_Course',
+        username='instructor',
+        password='password',
+        is_staff=True
+    )
+    world.add_to_course_staff('instructor', '999')
+
+    # Register another staff member
+    world.register_by_course_id(
+        'edx/999/Test_Course',
+        username='staff',
+        password='password',
+        is_staff=True
+    )
+    world.add_to_course_staff('staff', '999')
+
+    # Register a student
+    world.register_by_course_id(
+        'edx/999/Test_Course',
+        username='student',
+        password='password',
+        is_staff=False
+    )
+
+    # Log in as the instructor for the course
+    world.log_in(
+        username='instructor',
+        password='password',
+        email="instructor@edx.org",
+        name="Instructor"
+    )
+
+
+# Dictionary mapping a description of the email recipient
+# to the corresponding <option> value in the UI.
+SEND_TO_OPTIONS = {
+    'myself': 'myself',
+    'course staff': 'staff',
+    'students, staff, and instructors': 'all'
+}
+
+
+@step(u'I send email to "([^"]*)"')
+def when_i_send_an_email(recipient):
+
+    # Check that the recipient is valid
+    assert_in(
+        recipient, SEND_TO_OPTIONS,
+        msg="Invalid recipient: {}".format(recipient)
+    )
+
+    # Because we flush the database before each run,
+    # we need to ensure that the email template fixture
+    # is re-loaded into the database
+    call_command('loaddata', 'course_email_template.json')
+
+    # Go to the email section of the instructor dash
+    world.visit('/courses/edx/999/Test_Course')
+    world.css_click('a[href="/courses/edx/999/Test_Course/instructor"]')
+    world.css_click('div.beta-button-wrapper>a')
+    world.css_click('a[data-section="send_email"]')
+
+    # Select the recipient
+    world.select_option('send_to', SEND_TO_OPTIONS[recipient])
+
+    # Enter subject and message
+    world.css_fill('input#id_subject', 'Hello')
+
+    with world.browser.get_iframe('mce_0_ifr') as iframe:
+        editor = iframe.find_by_id('tinymce')[0]
+        editor.fill('test message')
+
+    # Click send
+    world.css_click('input[name="send"]')
+
+    # Expect to see a message that the email was sent
+    expected_msg = "Your email was successfully queued for sending."
+    assert_true(
+        world.css_has_text('div.request-response', expected_msg, '#request-response', allow_blank=False),
+        msg="Could not find email success message."
+    )
+
+
+# Dictionaries mapping description of email recipient
+# to the expected recipient email addresses
+EXPECTED_ADDRESSES = {
+    'myself': ['instructor@edx.org'],
+    'course staff': ['instructor@edx.org', 'staff@edx.org'],
+    'students, staff, and instructors': ['instructor@edx.org', 'staff@edx.org', 'student@edx.org']
+}
+
+UNSUBSCRIBE_MSG = 'To stop receiving email like this'
+
+
+@step(u'Email is sent to "([^"]*)"')
+def then_the_email_is_sent(recipient):
+
+    # Check that the recipient is valid
+    assert_in(
+        recipient, SEND_TO_OPTIONS,
+        msg="Invalid recipient: {}".format(recipient)
+    )
+
+    # Retrieve messages.  Because we are using celery in "always eager"
+    # mode, we expect all messages to be sent by this point.
+    messages = []
+    while not mail.queue.empty():  # pylint: disable=E1101
+        messages.append(mail.queue.get())  # pylint: disable=E1101
+
+    # Check that we got the right number of messages
+    assert_equal(
+        len(messages), len(EXPECTED_ADDRESSES[recipient]),
+        msg="Received {0} instead of {1} messages for {2}".format(
+            len(messages), len(EXPECTED_ADDRESSES[recipient]), recipient
+        )
+    )
+
+    # Check that the message properties were correct
+    recipients = []
+    for msg in messages:
+        assert_in('Hello', msg.subject)
+        assert_in(settings.DEFAULT_BULK_FROM_EMAIL, msg.from_email)
+
+        # Message body should have the message we sent
+        # and an unsubscribe message
+        assert_in('test message', msg.body)
+        assert_in(UNSUBSCRIBE_MSG, msg.body)
+
+        # Should have alternative HTML form
+        assert_equal(len(msg.alternatives), 1)
+        content = msg.alternatives[0]
+        assert_in('test message', content)
+        assert_in(UNSUBSCRIBE_MSG, content)
+
+        # Store the recipient address so we can verify later
+        recipients.extend(msg.recipients())
+
+    # Check that the messages were sent to the right people
+    for addr in EXPECTED_ADDRESSES[recipient]:
+        assert_in(addr, recipients)

lms/djangoapps/instructor/tests/test_api.py

@@ -6,7 +6,6 @@ import unittest
 import json
 import requests
 from urllib import quote
-from django.conf import settings
 from django.test import TestCase
 from nose.tools import raises
 from mock import Mock, patch
@@ -125,6 +124,7 @@ class TestInstructorAPIDenyLevels(ModuleStoreTestCase, LoginEnrollmentTestCase):
             'list_forum_members',
             'update_forum_role_membership',
             'proxy_legacy_analytics',
+            'send_email',
         ]
         for endpoint in staff_level_endpoints:
             url = reverse(endpoint, kwargs={'course_id': self.course.id})
@@ -280,8 +280,8 @@ class TestInstructorAPILevelsAccess(ModuleStoreTestCase, LoginEnrollmentTestCase
     This test does NOT test whether the actions had an effect on the
     database, that is the job of test_access.
     This tests the response and action switch.
-    Actually, modify_access does not having a very meaningful
-        response yet, so only the status code is tested.
+    Actually, modify_access does not have a very meaningful
+    response yet, so only the status code is tested.
     """
     def setUp(self):
         self.instructor = AdminFactory.create()
@@ -691,7 +691,74 @@ class TestInstructorAPIRegradeTask(ModuleStoreTestCase, LoginEnrollmentTestCase)
         })
         print response.content
         self.assertEqual(response.status_code, 200)
-        self.assertTrue(act.called)
+
+
+@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
+class TestInstructorSendEmail(ModuleStoreTestCase, LoginEnrollmentTestCase):
+    """
+    Checks that only instructors have access to email endpoints, and that
+    these endpoints are only accessible with courses that actually exist,
+    only with valid email messages.
+    """
+    def setUp(self):
+        self.instructor = AdminFactory.create()
+        self.course = CourseFactory.create()
+        self.client.login(username=self.instructor.username, password='test')
+        test_subject = u'\u1234 test subject'
+        test_message = u'\u6824 test message'
+        self.full_test_message = {
+            'send_to': 'staff',
+            'subject': test_subject,
+            'message': test_message,
+        }
+
+    def test_send_email_as_logged_in_instructor(self):
+        url = reverse('send_email', kwargs={'course_id': self.course.id})
+        response = self.client.post(url, self.full_test_message)
+        self.assertEqual(response.status_code, 200)
+
+    def test_send_email_but_not_logged_in(self):
+        self.client.logout()
+        url = reverse('send_email', kwargs={'course_id': self.course.id})
+        response = self.client.post(url, self.full_test_message)
+        self.assertEqual(response.status_code, 403)
+
+    def test_send_email_but_not_staff(self):
+        self.client.logout()
+        student = UserFactory()
+        self.client.login(username=student.username, password='test')
+        url = reverse('send_email', kwargs={'course_id': self.course.id})
+        response = self.client.post(url, self.full_test_message)
+        self.assertEqual(response.status_code, 403)
+
+    def test_send_email_but_course_not_exist(self):
+        url = reverse('send_email', kwargs={'course_id': 'GarbageCourse/DNE/NoTerm'})
+        response = self.client.post(url, self.full_test_message)
+        self.assertNotEqual(response.status_code, 200)
+
+    def test_send_email_no_sendto(self):
+        url = reverse('send_email', kwargs={'course_id': self.course.id})
+        response = self.client.post(url, {
+            '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})
+        response = self.client.post(url, {
+            'send_to': 'staff',
+            'message': 'test message',
+        })
+        self.assertEqual(response.status_code, 400)
+
+    def test_send_email_no_message(self):
+        url = reverse('send_email', kwargs={'course_id': self.course.id})
+        response = self.client.post(url, {
+            'send_to': 'staff',
+            'subject': 'test subject',
+        })
+        self.assertEqual(response.status_code, 400)
 
 
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)