Merge pull request #14118 from edx/efischer/hackathon_emails

Bulk Email for course modes

common/djangoapps/enrollment/api.py

@@ -192,7 +192,7 @@ def add_enrollment(user_id, course_id, mode=None, is_active=True):
     """
     if mode is None:
         mode = _default_course_mode(course_id)
-    _validate_course_mode(course_id, mode, is_active=is_active)
+    validate_course_mode(course_id, mode, is_active=is_active)
     return _data_api().create_course_enrollment(user_id, course_id, mode, is_active)
 
 
@@ -252,7 +252,7 @@ def update_enrollment(user_id, course_id, mode=None, is_active=None, enrollment_
         mode=mode,
     ))
     if mode is not None:
-        _validate_course_mode(course_id, mode, is_active=is_active, include_expired=include_expired)
+        validate_course_mode(course_id, mode, is_active=is_active, include_expired=include_expired)
     enrollment = _data_api().update_course_enrollment(user_id, course_id, mode=mode, is_active=is_active)
     if enrollment is None:
         msg = u"Course Enrollment not found for user {user} in course {course}".format(user=user_id, course=course_id)
@@ -409,7 +409,7 @@ def _default_course_mode(course_id):
     return CourseMode.DEFAULT_MODE_SLUG
 
 
-def _validate_course_mode(course_id, mode, is_active=None, include_expired=False):
+def validate_course_mode(course_id, mode, is_active=None, include_expired=False):
     """Checks to see if the specified course mode is valid for the course.
 
     If the requested course mode is not available for the course, raise an error with corresponding

lms/djangoapps/bulk_email/migrations/0006_course_mode_targets.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('course_modes', '0007_coursemode_bulk_sku'),
+        ('bulk_email', '0005_move_target_data'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='CourseModeTarget',
+            fields=[
+                ('target_ptr', models.OneToOneField(parent_link=True, auto_created=True, primary_key=True, serialize=False, to='bulk_email.Target')),
+                ('track', models.ForeignKey(to='course_modes.CourseMode')),
+            ],
+            bases=('bulk_email.target',),
+        ),
+        migrations.AlterField(
+            model_name='target',
+            name='target_type',
+            field=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'), (b'track', b'Specific course mode')]),
+        ),
+    ]

lms/djangoapps/bulk_email/models.py

@@ -13,6 +13,9 @@ 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 course_modes.models import CourseMode
+from enrollment.api import validate_course_mode
+from enrollment.errors import CourseModeNotFoundError
 from student.roles import CourseStaffRole, CourseInstructorRole
 
 from openedx.core.djangoapps.xmodule_django.models import CourseKeyField
@@ -45,9 +48,10 @@ SEND_TO_MYSELF = 'myself'
 SEND_TO_STAFF = 'staff'
 SEND_TO_LEARNERS = 'learners'
 SEND_TO_COHORT = 'cohort'
+SEND_TO_TRACK = 'track'
 EMAIL_TARGET_CHOICES = zip(
-    [SEND_TO_MYSELF, SEND_TO_STAFF, SEND_TO_LEARNERS, SEND_TO_COHORT],
-    ['Myself', 'Staff and instructors', 'All students', 'Specific cohort']
+    [SEND_TO_MYSELF, SEND_TO_STAFF, SEND_TO_LEARNERS, SEND_TO_COHORT, SEND_TO_TRACK],
+    ['Myself', 'Staff and instructors', 'All students', 'Specific cohort', 'Specific course mode']
 )
 EMAIL_TARGETS = {target[0] for target in EMAIL_TARGET_CHOICES}
 
@@ -78,6 +82,8 @@ class Target(models.Model):
         """
         if self.target_type == SEND_TO_COHORT:
             return self.cohorttarget.short_display()  # pylint: disable=no-member
+        elif self.target_type == SEND_TO_TRACK:
+            return self.coursemodetarget.short_display()  # pylint: disable=no-member
         else:
             return self.target_type
 
@@ -87,6 +93,8 @@ class Target(models.Model):
         """
         if self.target_type == SEND_TO_COHORT:
             return self.cohorttarget.long_display()  # pylint: disable=no-member
+        elif self.target_type == SEND_TO_TRACK:
+            return self.coursemodetarget.long_display()  # pylint: disable=no-member
         else:
             return self.get_target_type_display()
 
@@ -115,6 +123,12 @@ class Target(models.Model):
             return use_read_replica_if_available(enrollment_qset.exclude(id__in=staff_instructor_qset))
         elif self.target_type == SEND_TO_COHORT:
             return self.cohorttarget.cohort.users.filter(id__in=enrollment_qset)  # pylint: disable=no-member
+        elif self.target_type == SEND_TO_TRACK:
+            return use_read_replica_if_available(
+                enrollment_qset.filter(
+                    courseenrollment__mode=self.coursemodetarget.track.mode_slug
+                )
+            )
         else:
             raise ValueError("Unrecognized target type {}".format(self.target_type))
 
@@ -132,6 +146,9 @@ class CohortTarget(Target):
         kwargs['target_type'] = SEND_TO_COHORT
         super(CohortTarget, self).__init__(*args, **kwargs)
 
+    def __unicode__(self):
+        return self.short_display()
+
     def short_display(self):
         return "{}-{}".format(self.target_type, self.cohort.name)
 
@@ -159,6 +176,53 @@ class CohortTarget(Target):
         return cohort
 
 
+class CourseModeTarget(Target):
+    """
+    Subclass of Target, specifically for course modes.
+    """
+    track = models.ForeignKey('course_modes.CourseMode')
+
+    class Meta:
+        app_label = "bulk_email"
+
+    def __init__(self, *args, **kwargs):
+        kwargs['target_type'] = SEND_TO_TRACK
+        super(CourseModeTarget, self).__init__(*args, **kwargs)
+
+    def __unicode__(self):
+        return self.short_display()
+
+    def short_display(self):
+        return "{}-{}".format(self.target_type, self.track.mode_slug)  # pylint: disable=no-member
+
+    def long_display(self):
+        all_modes = CourseMode.objects.filter(
+            course_id=self.track.course_id,
+            mode_slug=self.track.mode_slug,  # pylint: disable=no-member
+        )
+        return "Course mode: {}, Currencies: {}".format(
+            self.track.mode_display_name,  # pylint: disable=no-member
+            ", ".join([mode.currency for mode in all_modes])
+        )
+
+    @classmethod
+    def ensure_valid_mode(cls, mode_slug, course_id):
+        """
+        Ensures mode_slug is a valid mode for course_id. Will raise an error if invalid.
+        """
+        if mode_slug is None:
+            raise ValueError("Cannot create a CourseModeTarget without specifying a mode_slug.")
+        try:
+            validate_course_mode(unicode(course_id), mode_slug)
+        except CourseModeNotFoundError:
+            raise ValueError(
+                "Track {track} does not exist in course {course_id}".format(
+                    track=mode_slug,
+                    course_id=course_id
+                )
+            )
+
+
 class CourseEmail(Email):
     """
     Stores information for an email to a course.
@@ -179,7 +243,7 @@ class CourseEmail(Email):
     @classmethod
     def create(
             cls, course_id, sender, targets, subject, html_message,
-            text_message=None, template_name=None, from_addr=None, cohort_name=None):
+            text_message=None, template_name=None, from_addr=None):
         """
         Create an instance of CourseEmail.
         """
@@ -189,7 +253,7 @@ class CourseEmail(Email):
 
         new_targets = []
         for target in targets:
-            # split target, to handle cohort:cohort_name
+            # split target, to handle cohort:cohort_name and track:mode_slug
             target_split = target.split(':', 1)
             # Ensure our desired target exists
             if target_split[0] not in EMAIL_TARGETS:
@@ -200,6 +264,14 @@ class CourseEmail(Email):
                 # target_split[1] will contain the cohort name
                 cohort = CohortTarget.ensure_valid_cohort(target_split[1], course_id)
                 new_target, _ = CohortTarget.objects.get_or_create(target_type=target_split[0], cohort=cohort)
+            elif target_split[0] == SEND_TO_TRACK:
+                # target_split[1] contains the desired mode slug
+                CourseModeTarget.ensure_valid_mode(target_split[1], course_id)
+
+                # There could exist multiple CourseModes that match this query, due to differing currency types.
+                # The currencies do not affect user lookup though, so we can just use the first result.
+                mode = CourseMode.objects.filter(course_id=course_id, mode_slug=target_split[1])[0]
+                new_target, _ = CourseModeTarget.objects.get_or_create(target_type=target_split[0], track=mode)
             else:
                 new_target, _ = Target.objects.get_or_create(target_type=target_split[0])
             new_targets.append(new_target)

lms/djangoapps/bulk_email/tests/test_email.py

@@ -20,7 +20,9 @@ from bulk_email.models import Optout, BulkEmailFlag
 from bulk_email.tasks import _get_source_address, _get_course_email_context
 from openedx.core.djangoapps.course_groups.models import CourseCohort
 from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort
+from course_modes.models import CourseMode
 from courseware.tests.factories import StaffFactory, InstructorFactory
+from enrollment.api import update_enrollment
 from lms.djangoapps.instructor_task.subtasks import update_subtask_status
 from student.roles import CourseStaffRole
 from student.models import CourseEnrollment
@@ -239,6 +241,27 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase)
         self.assertEquals(len(mail.outbox), len(self.students) - 1)
         self.assertNotIn(self.students[-1].email, [e.to[0] for e in mail.outbox])
 
+    def test_send_to_track(self):
+        """
+        Make sure email sent to a registration track goes there.
+        """
+        CourseMode.objects.create(mode_slug='test', course_id=self.course.id)
+        for student in self.students:
+            update_enrollment(student, unicode(self.course.id), 'test')
+        test_email = {
+            'action': 'Send email',
+            'send_to': '["track:test"]',
+            'subject': 'test subject for test track',
+            'message': 'test message for test track',
+        }
+        response = self.client.post(self.send_mail_url, test_email)
+        self.assertEquals(json.loads(response.content), self.success_content)
+
+        self.assertItemsEqual(
+            [e.to[0] for e in mail.outbox],
+            [s.email for s in self.students]
+        )
+
     def test_send_to_all(self):
         """
         Make sure email send to all goes there.

lms/djangoapps/bulk_email/tests/test_err_handling.py

@@ -5,6 +5,7 @@ Unit tests for handling email sending errors
 from itertools import cycle
 
 from celery.states import SUCCESS, RETRY  # pylint: disable=no-name-in-module, import-error
+import ddt
 from django.conf import settings
 from django.core.management import call_command
 from django.core.urlresolvers import reverse
@@ -36,6 +37,7 @@ class EmailTestException(Exception):
     pass
 
 
+@ddt.ddt
 @attr(shard=1)
 @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message', autospec=True))
 class TestEmailErrors(ModuleStoreTestCase):
@@ -213,15 +215,16 @@ class TestEmailErrors(ModuleStoreTestCase):
                 "dummy body goes here"
             )
 
-    def test_nonexistent_cohort(self):
+    @ddt.data('track', 'cohort')
+    def test_nonexistent_grouping(self, target_type):
         """
-        Tests exception when the cohort doesn't exist
+        Tests exception when the cohort or course mode doesn't exist
         """
-        with self.assertRaisesRegexp(ValueError, 'Cohort IDONTEXIST does not exist *'):
+        with self.assertRaisesRegexp(ValueError, '.* IDONTEXIST does not exist .*'):
             email = CourseEmail.create(  # pylint: disable=unused-variable
                 self.course.id,
                 self.instructor,
-                ["cohort:IDONTEXIST"],
+                ["{}:IDONTEXIST".format(target_type)],
                 "re: subject",
                 "dummy body goes here"
             )

lms/djangoapps/bulk_email/tests/test_models.py

@@ -12,11 +12,13 @@ from nose.plugins.attrib import attr
 from bulk_email.models import (
     CourseEmail,
     SEND_TO_COHORT,
+    SEND_TO_TRACK,
     SEND_TO_STAFF,
     CourseEmailTemplate,
     CourseAuthorization,
     BulkEmailFlag
 )
+from course_modes.models import CourseMode
 from openedx.core.djangoapps.course_groups.models import CourseCohort
 from opaque_keys.edx.keys import CourseKey
 
@@ -67,6 +69,22 @@ class CourseEmailTest(TestCase):
         with self.assertRaises(ValueError):
             CourseEmail.create(course_id, sender, to_option, subject, html_message)
 
+    def test_track_target(self):
+        course_id = CourseKey.from_string('abc/123/doremi')
+        sender = UserFactory.create()
+        to_option = 'track:test'
+        subject = "dummy subject"
+        html_message = "<html>dummy message</html>"
+        CourseMode.objects.create(mode_slug='test', mode_display_name='Test', course_id=course_id)
+        with patch('bulk_email.models.validate_course_mode'):
+            # we don't have a real course, so validation will fail. Mock it out!
+            email = CourseEmail.create(course_id, sender, [to_option], subject, html_message)
+        self.assertEqual(len(email.targets.all()), 1)
+        target = email.targets.all()[0]
+        self.assertEqual(target.target_type, SEND_TO_TRACK)
+        self.assertEqual(target.short_display(), 'track-test')
+        self.assertEqual(target.long_display(), 'Course mode: Test, Currencies: usd')
+
     def test_cohort_target(self):
         course_id = CourseKey.from_string('abc/123/doremi')
         sender = UserFactory.create()

lms/djangoapps/instructor/views/instructor_dashboard.py

@@ -633,6 +633,10 @@ def _section_send_email(course, access):
     cohorts = []
     if is_course_cohorted(course_key):
         cohorts = get_course_cohorts(course)
+    course_modes = []
+    from verified_track_content.models import VerifiedTrackCohortedCourse
+    if not VerifiedTrackCohortedCourse.is_verified_track_cohort_enabled(course_key):
+        course_modes = CourseMode.modes_for_course(course_key)
     email_editor = fragment.content
     section_data = {
         'section_key': 'send_email',
@@ -641,6 +645,7 @@ def _section_send_email(course, access):
         'send_email': reverse('send_email', kwargs={'course_id': unicode(course_key)}),
         'editor': email_editor,
         'cohorts': cohorts,
+        'course_modes': course_modes,
         'default_cohort_name': DEFAULT_COHORT_NAME,
         'list_instructor_tasks_url': reverse(
             'list_instructor_tasks', kwargs={'course_id': unicode(course_key)}

lms/static/js/instructor_dashboard/send_email.js

@@ -41,6 +41,7 @@
             this.$emailEditor = XBlock.initializeBlock($('.xblock-studio_view'));
             this.$send_to = this.$container.find("input[name='send_to']");
             this.$cohort_targets = this.$send_to.filter('[value^="cohort:"]');
+            this.$course_mode_targets = this.$send_to.filter('[value^="track:"]');
             this.$subject = this.$container.find("input[name='subject']");
             this.$btn_send = this.$container.find("input[name='send']");
             this.$task_response = this.$container.find('.request-response');
@@ -85,9 +86,12 @@
                             return gettext('Everyone who has staff privileges in this course');
                         } else if (value === 'learners') {
                             return gettext('All learners who are enrolled in this course');
-                        } else {
+                        } else if (value.startsWith('cohort')) {
                             return gettext('All learners in the {cohort_name} cohort')
                                 .replace('{cohort_name}', value.slice(value.indexOf(':') + 1));
+                        } else if (value.startsWith('track')) {
+                            return gettext('All learners in the {track_name} track')
+                                .replace('{track_name}', value.slice(value.indexOf(':') + 1));
                         }
                     };
                     successMessage = gettext('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.');  // eslint-disable-line max-len
@@ -179,17 +183,21 @@
             });
             this.$send_to.change(function() {
                 var targets;
+                var inputDisable = function() {
+                    this.checked = false;
+                    this.disabled = true;
+                    return true;
+                };
+                var inputEnable = function() {
+                    this.disabled = false;
+                    return true;
+                };
                 if ($('input#target_learners:checked').length) {
-                    sendemail.$cohort_targets.each(function() {
-                        this.checked = false;
-                        this.disabled = true;
-                        return true;
-                    });
+                    sendemail.$cohort_targets.each(inputDisable);
+                    sendemail.$course_mode_targets.each(inputDisable);
                 } else {
-                    sendemail.$cohort_targets.each(function() {
-                        this.disabled = false;
-                        return true;
-                    });
+                    sendemail.$cohort_targets.each(inputEnable);
+                    sendemail.$cohort_targets.each(inputEnable);
                 }
                 targets = [];
                 $('input[name="send_to"]:checked+label').each(function() {

lms/templates/instructor/instructor_dashboard_2/send_email.html

@@ -45,6 +45,19 @@ from openedx.core.djangolib.markup import HTML
                 %endfor
             </ul>
             %endif
+            ## If there's only 1 course mode, everyone is in it and you should just use "All Learners"
+            %if len(section_data['course_modes']) > 1:
+            <ul role="group" class="email-targets-secondary">
+                %for track in section_data['course_modes']:
+                <li>
+                    <input type="checkbox" name="send_to" value="track:${track.slug}" id="target_track_${track.slug}">
+                    <label for="target_track_${track.slug}">
+                        ${_("Learners in the {track_name} Track").format(track_name=track.name)}
+                    </label>
+                </li>
+                %endfor
+            </ul>
+            %endif
         </fieldset>
         </li>
         <li class="field">