Skip to content

E
edx-platform
Commit faa7f544 by Eric Fischer
Options

Bulk Email Cohorts (#12602) 

Adds cohorts as valid bulk email targets.
parent 740266bd
Showing with 328 additions and 117 deletions
lms/djangoapps/bulk_email/models.py
"""
Models for bulk email
WE'RE USING MIGRATIONS!
If you make changes to this model, be sure to create an appropriate migration
file and check it in at the same time as your model changes. To do that,
1. Go to the edx-platform dir
2. ./manage.py lms schemamigration bulk_email --auto description_of_your_change
3. Add the migration file created in edx-platform/lms/djangoapps/bulk_email/migrations/
"""
import logging
import markupsafe
......@@ -19,6 +9,7 @@ from django.contrib.auth.models import User
from django.db import models
from openedx.core.djangoapps.course_groups.models import CourseUserGroup
from openedx.core.djangoapps.course_groups.cohorts import get_cohort_by_name
from openedx.core.lib.html_to_text import html_to_text
from openedx.core.lib.mail_utils import wrap_message
......@@ -55,14 +46,24 @@ SEND_TO_MYSELF = 'myself'
SEND_TO_STAFF = 'staff'
SEND_TO_LEARNERS = 'learners'
SEND_TO_COHORT = 'cohort'
EMAIL_TARGETS = [SEND_TO_MYSELF, SEND_TO_STAFF, SEND_TO_LEARNERS, SEND_TO_COHORT]
EMAIL_TARGET_DESCRIPTIONS = ['Myself', 'Staff and instructors', 'All students', 'Specific cohort']
EMAIL_TARGET_CHOICES = zip(EMAIL_TARGETS, EMAIL_TARGET_DESCRIPTIONS)
EMAIL_TARGET_CHOICES = zip(
[SEND_TO_MYSELF, SEND_TO_STAFF, SEND_TO_LEARNERS, SEND_TO_COHORT],
['Myself', 'Staff and instructors', 'All students', 'Specific cohort']
)
EMAIL_TARGETS = {target[0] for target in EMAIL_TARGET_CHOICES}
class Target(models.Model):
"""
A way to refer to a particular group (within a course) as a "Send to:" target.
Django hackery in this class - polymorphism does not work well in django, for reasons relating to how
each class is represented by its own database table. Due to this, we can't just override
methods of Target in CohortTarget and get the child method, as one would expect. The
workaround is to check to see that a given target is a CohortTarget (self.target_type ==
SEND_TO_COHORT), then explicitly call the method on self.cohorttarget, which is created
by django as part of this inheritance setup. These calls require pylint disable no-member in
several locations in this class.
"""
target_type = models.CharField(max_length=64, choices=EMAIL_TARGET_CHOICES)
......@@ -70,7 +71,25 @@ class Target(models.Model):
app_label = "bulk_email"
def __unicode__(self):
return "CourseEmail Target for: {}".format(self.target_type)
return "CourseEmail Target: {}".format(self.short_display())
def short_display(self):
"""
Returns a short display name
"""
if self.target_type == SEND_TO_COHORT:
return self.cohorttarget.short_display() # pylint: disable=no-member
else:
return self.target_type
def long_display(self):
"""
Returns a long display name
"""
if self.target_type == SEND_TO_COHORT:
return self.cohorttarget.long_display() # pylint: disable=no-member
else:
return self.get_target_type_display()
def get_users(self, course_id, user_id=None):
"""
......@@ -96,7 +115,7 @@ class Target(models.Model):
elif self.target_type == SEND_TO_LEARNERS:
return use_read_replica_if_available(enrollment_qset.exclude(id__in=staff_instructor_qset))
elif self.target_type == SEND_TO_COHORT:
return User.objects.none() # TODO: cohorts aren't hooked up, put that logic here
return self.cohorttarget.cohort.users.filter(id__in=enrollment_qset) # pylint: disable=no-member
else:
raise ValueError("Unrecognized target type {}".format(self.target_type))
......@@ -114,8 +133,11 @@ class CohortTarget(Target):
kwargs['target_type'] = SEND_TO_COHORT
super(CohortTarget, self).__init__(*args, **kwargs)
def __unicode__(self):
return "CourseEmail CohortTarget: {}".format(self.cohort)
def short_display(self):
return "{}-{}".format(self.target_type, self.cohort.name)
def long_display(self):
return "Cohort: {}".format(self.cohort.name)
@classmethod
def ensure_valid_cohort(cls, cohort_name, course_id):
......@@ -127,7 +149,7 @@ class CohortTarget(Target):
if cohort_name is None:
raise ValueError("Cannot create a CohortTarget without specifying a cohort_name.")
try:
cohort = CourseUserGroup.get(name=cohort_name, course_id=course_id)
cohort = get_cohort_by_name(name=cohort_name, course_key=course_id)
except CourseUserGroup.DoesNotExist:
raise ValueError(
"Cohort {cohort} does not exist in course {course_id}".format(
......@@ -168,16 +190,19 @@ class CourseEmail(Email):
new_targets = []
for target in targets:
# split target, to handle cohort:cohort_name
target_split = target.split(':', 1)
# Ensure our desired target exists
if target not in EMAIL_TARGETS:
if target_split[0] not in EMAIL_TARGETS:
fmt = 'Course email being sent to unrecognized target: "{target}" for "{course}", subject "{subject}"'
msg = fmt.format(target=target, course=course_id, subject=subject)
raise ValueError(msg)
elif target == SEND_TO_COHORT:
cohort = CohortTarget.ensure_valid_cohort(cohort_name, course_id)
new_target, _ = CohortTarget.objects.get_or_create(target_type=target, cohort=cohort)
elif target_split[0] == SEND_TO_COHORT:
# 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)
else:
new_target, _ = Target.objects.get_or_create(target_type=target)
new_target, _ = Target.objects.get_or_create(target_type=target_split[0])
new_targets.append(new_target)
# create the task, then save it immediately:
......
lms/djangoapps/bulk_email/tasks.py
......@@ -195,6 +195,13 @@ def perform_delegate_email_batches(entry_id, course_id, task_input, action_name)
if total_recipients <= settings.BULK_EMAIL_JOB_SIZE_THRESHOLD:
routing_key = settings.BULK_EMAIL_ROUTING_KEY_SMALL_JOBS
# Weird things happen if we allow empty querysets as input to emailing subtasks
# The task appears to hang at "0 out of 0 completed" and never finishes.
if total_recipients == 0:
msg = u"Bulk Email Task: Empty recipient set"
log.warning(msg)
raise ValueError(msg)
def _create_send_email_subtask(to_list, initial_subtask_status):
"""Creates a subtask to send email to a given recipient list."""
subtask_id = initial_subtask_status.task_id
......
lms/djangoapps/bulk_email/tests/test_email.py
......@@ -18,6 +18,8 @@ from django.test.utils import override_settings
from bulk_email.models import Optout, BulkEmailFlag
from bulk_email.tasks import _get_source_address
from openedx.core.djangoapps.course_groups.models import CourseCohort
from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort
from courseware.tests.factories import StaffFactory, InstructorFactory
from instructor_task.subtasks import update_subtask_status
from student.roles import CourseStaffRole
......@@ -113,6 +115,7 @@ class EmailSendFromDashboardTestCase(SharedModuleStoreTestCase):
self.login_as_user(self.instructor)
# Pulling up the instructor dash email view here allows us to test sending emails in tests
self.goto_instructor_dash_email_view()
self.send_mail_url = reverse(
'send_email', kwargs={'course_id': unicode(self.course.id)}
......@@ -153,15 +156,12 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase)
"""
Make sure email send to myself goes to myself.
"""
# Now we know we have pulled up the instructor dash's email view
# (in the setUp method), we can test sending an email.
test_email = {
'action': 'send',
'send_to': '["myself"]',
'subject': 'test subject for myself',
'message': 'test message for myself'
}
# Post the email to the instructor dashboard API
response = self.client.post(self.send_mail_url, test_email)
self.assertEquals(json.loads(response.content), self.success_content)
......@@ -182,15 +182,12 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase)
"""
Make sure email send to staff and instructors goes there.
"""
# Now we know we have pulled up the instructor dash's email view
# (in the setUp method), we can test sending an email.
test_email = {
'action': 'Send email',
'send_to': '["staff"]',
'subject': 'test subject for staff',
'message': 'test message for subject'
}
# Post the email to the instructor dashboard API
response = self.client.post(self.send_mail_url, test_email)
self.assertEquals(json.loads(response.content), self.success_content)
......@@ -201,12 +198,51 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase)
[self.instructor.email] + [s.email for s in self.staff]
)
def test_send_to_cohort(self):
"""
Make sure email sent to a cohort goes there.
"""
cohort = CourseCohort.create(cohort_name='test cohort', course_id=self.course.id)
for student in self.students:
add_user_to_cohort(cohort.course_user_group, student.username)
test_email = {
'action': 'Send email',
'send_to': '["cohort:{}"]'.format(cohort.course_user_group.name),
'subject': 'test subject for cohort',
'message': 'test message for cohort'
}
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_cohort_unenrolled(self):
"""
Make sure email sent to a cohort does not go to unenrolled members of the cohort.
"""
self.students.append(UserFactory()) # user will be added to cohort, but not enrolled in course
cohort = CourseCohort.create(cohort_name='test cohort', course_id=self.course.id)
for student in self.students:
add_user_to_cohort(cohort.course_user_group, student.username)
test_email = {
'action': 'Send email',
'send_to': '["cohort:{}"]'.format(cohort.course_user_group.name),
'subject': 'test subject for cohort',
'message': 'test message for cohort'
}
response = self.client.post(self.send_mail_url, test_email)
self.assertEquals(json.loads(response.content), self.success_content)
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_all(self):
"""
Make sure email send to all goes there.
"""
# Now we know we have pulled up the instructor dash's email view
# (in the setUp method), we can test sending an email.
test_email = {
'action': 'Send email',
......@@ -214,7 +250,6 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase)
'subject': 'test subject for all',
'message': 'test message for all'
}
# Post the email to the instructor dashboard API
response = self.client.post(self.send_mail_url, test_email)
self.assertEquals(json.loads(response.content), self.success_content)
......@@ -265,8 +300,6 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase)
"""
Make sure email (with Unicode characters) send to all goes there.
"""
# Now we know we have pulled up the instructor dash's email view
# (in the setUp method), we can test sending an email.
uni_subject = u'téśt śúbjéćt főŕ áĺĺ'
test_email = {
......@@ -275,7 +308,6 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase)
'subject': uni_subject,
'message': 'test message for all'
}
# Post the email to the instructor dashboard API
response = self.client.post(self.send_mail_url, test_email)
self.assertEquals(json.loads(response.content), self.success_content)
......@@ -290,8 +322,6 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase)
"""
Make sure email (with Unicode characters) send to all goes there.
"""
# Now we know we have pulled up the instructor dash's email view
# (in the setUp method), we can test sending an email.
# Create a student with Unicode in their first & last names
unicode_user = UserFactory(first_name=u'Ⓡⓞⓑⓞⓣ', last_name=u'ՇﻉรՇ')
......@@ -304,7 +334,6 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase)
'subject': 'test subject for all',
'message': 'test message for all'
}
# Post the email to the instructor dashboard API
response = self.client.post(self.send_mail_url, test_email)
self.assertEquals(json.loads(response.content), self.success_content)
......@@ -401,7 +430,6 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase)
'subject': 'test subject for all',
'message': 'test message for all'
}
# Post the email to the instructor dashboard API
response = self.client.post(self.send_mail_url, test_email)
self.assertEquals(json.loads(response.content), self.success_content)
......@@ -429,8 +457,6 @@ class TestEmailSendFromDashboard(EmailSendFromDashboardTestCase):
"""
Make sure email (with Unicode characters) send to all goes there.
"""
# Now we know we have pulled up the instructor dash's email view
# (in the setUp method), we can test sending an email.
uni_message = u'ẗëṡẗ ṁëṡṡäġë ḟöṛ äḷḷ ｲ乇丂ｲ ﾶ乇丂丂ﾑg乇 ｷo尺 ﾑﾚﾚ тэѕт мэѕѕаБэ fоѓ аll'
test_email = {
......@@ -439,7 +465,6 @@ class TestEmailSendFromDashboard(EmailSendFromDashboardTestCase):
'subject': 'test subject for all',
'message': uni_message
}
# Post the email to the instructor dashboard API
response = self.client.post(self.send_mail_url, test_email)
self.assertEquals(json.loads(response.content), self.success_content)
......
lms/djangoapps/bulk_email/tests/test_err_handling.py
......@@ -204,7 +204,7 @@ class TestEmailErrors(ModuleStoreTestCase):
"""
Tests exception when the to_option in the email doesn't exist
"""
with self.assertRaisesRegexp(Exception, 'Course email being sent to unrecognized target: "IDONTEXIST" *'):
with self.assertRaisesRegexp(ValueError, 'Course email being sent to unrecognized target: "IDONTEXIST" *'):
email = CourseEmail.create( # pylint: disable=unused-variable
self.course.id,
self.instructor,
......@@ -213,6 +213,19 @@ class TestEmailErrors(ModuleStoreTestCase):
"dummy body goes here"
)
def test_nonexistent_cohort(self):
"""
Tests exception when the cohort doesn't exist
"""
with self.assertRaisesRegexp(ValueError, 'Cohort IDONTEXIST does not exist *'):
email = CourseEmail.create( # pylint: disable=unused-variable
self.course.id,
self.instructor,
["cohort:IDONTEXIST"],
"re: subject",
"dummy body goes here"
)
def test_wrong_course_id_in_task(self):
"""
Tests exception when the course_id in task is not the same as one explicitly passed in.
......
lms/djangoapps/bulk_email/tests/test_models.py
......@@ -10,8 +10,16 @@ from student.tests.factories import UserFactory
from mock import patch, Mock
from nose.plugins.attrib import attr
from bulk_email.models import CourseEmail, SEND_TO_STAFF, CourseEmailTemplate, CourseAuthorization, BulkEmailFlag
from opaque_keys.edx.locations import SlashSeparatedCourseKey
from bulk_email.models import (
CourseEmail,
SEND_TO_COHORT,
SEND_TO_STAFF,
CourseEmailTemplate,
CourseAuthorization,
BulkEmailFlag
)
from openedx.core.djangoapps.course_groups.models import CourseCohort
from opaque_keys.edx.keys import CourseKey
@attr('shard_1')
......@@ -20,20 +28,20 @@ class CourseEmailTest(TestCase):
"""Test the CourseEmail model."""
def test_creation(self):
course_id = SlashSeparatedCourseKey('abc', '123', 'doremi')
course_id = CourseKey.from_string('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.assertEqual(email.course_id, course_id)
self.assertIn(SEND_TO_STAFF, [target.target_type for target in email.targets.all()])
self.assertEquals(email.subject, subject)
self.assertEquals(email.html_message, html_message)
self.assertEquals(email.sender, sender)
self.assertEqual(email.subject, subject)
self.assertEqual(email.html_message, html_message)
self.assertEqual(email.sender, sender)
def test_creation_with_optional_attributes(self):
course_id = SlashSeparatedCourseKey('abc', '123', 'doremi')
course_id = CourseKey.from_string('abc/123/doremi')
sender = UserFactory.create()
to_option = SEND_TO_STAFF
subject = "dummy subject"
......@@ -43,16 +51,16 @@ class CourseEmailTest(TestCase):
email = CourseEmail.create(
course_id, sender, [to_option], subject, html_message, template_name=template_name, from_addr=from_addr
)
self.assertEquals(email.course_id, course_id)
self.assertEquals(email.targets.all()[0].target_type, SEND_TO_STAFF)
self.assertEquals(email.subject, subject)
self.assertEquals(email.html_message, html_message)
self.assertEquals(email.sender, sender)
self.assertEquals(email.template_name, template_name)
self.assertEquals(email.from_addr, from_addr)
self.assertEqual(email.course_id, course_id)
self.assertEqual(email.targets.all()[0].target_type, SEND_TO_STAFF)
self.assertEqual(email.subject, subject)
self.assertEqual(email.html_message, html_message)
self.assertEqual(email.sender, sender)
self.assertEqual(email.template_name, template_name)
self.assertEqual(email.from_addr, from_addr)
def test_bad_to_option(self):
course_id = SlashSeparatedCourseKey('abc', '123', 'doremi')
course_id = CourseKey.from_string('abc/123/doremi')
sender = UserFactory.create()
to_option = "fake"
subject = "dummy subject"
......@@ -60,6 +68,20 @@ class CourseEmailTest(TestCase):
with self.assertRaises(ValueError):
CourseEmail.create(course_id, sender, to_option, subject, html_message)
def test_cohort_target(self):
course_id = CourseKey.from_string('abc/123/doremi')
sender = UserFactory.create()
to_option = 'cohort:test cohort'
subject = "dummy subject"
html_message = "<html>dummy message</html>"
CourseCohort.create(cohort_name='test cohort', course_id=course_id)
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_COHORT)
self.assertEqual(target.short_display(), 'cohort-test cohort')
self.assertEqual(target.long_display(), 'Cohort: test cohort')
@attr('shard_1')
class NoCourseEmailTemplateTest(TestCase):
......@@ -179,7 +201,7 @@ class CourseAuthorizationTest(TestCase):
def test_creation_auth_on(self):
BulkEmailFlag.objects.create(enabled=True, require_course_email_auth=True)
course_id = SlashSeparatedCourseKey('abc', '123', 'doremi')
course_id = CourseKey.from_string('abc/123/doremi')
# Test that course is not authorized by default
self.assertFalse(BulkEmailFlag.feature_enabled(course_id))
......@@ -188,7 +210,7 @@ class CourseAuthorizationTest(TestCase):
cauth.save()
# Now, course should be authorized
self.assertTrue(BulkEmailFlag.feature_enabled(course_id))
self.assertEquals(
self.assertEqual(
cauth.__unicode__(),
"Course 'abc/123/doremi': Instructor Email Enabled"
)
......@@ -198,14 +220,14 @@ class CourseAuthorizationTest(TestCase):
cauth.save()
# Test that course is now unauthorized
self.assertFalse(BulkEmailFlag.feature_enabled(course_id))
self.assertEquals(
self.assertEqual(
cauth.__unicode__(),
"Course 'abc/123/doremi': Instructor Email Not Enabled"
)
def test_creation_auth_off(self):
BulkEmailFlag.objects.create(enabled=True, require_course_email_auth=False)
course_id = SlashSeparatedCourseKey('blahx', 'blah101', 'ehhhhhhh')
course_id = CourseKey.from_string('blahx/blah101/ehhhhhhh')
# Test that course is authorized by default, since auth is turned off
self.assertTrue(BulkEmailFlag.feature_enabled(course_id))
......
lms/djangoapps/instructor/tests/test_email.py
......@@ -56,7 +56,7 @@ class TestNewInstructorDashboardEmailViewMongoBacked(SharedModuleStoreTestCase):
response = self.client.get(self.url)
self.assertIn(self.email_link, response.content)
send_to_label = '<ul role="group" aria-label="Send to:">'
send_to_label = '<div class="send_to_list">Send to:</div>'
self.assertTrue(send_to_label in response.content)
self.assertEqual(response.status_code, 200)
......
lms/djangoapps/instructor/tests/utils.py
......@@ -31,7 +31,7 @@ class FakeContentTask(FakeInfo):
def __init__(self, email_id, num_sent, num_failed, sent_to):
super(FakeContentTask, self).__init__()
self.task_input = {'email_id': email_id, 'to_option': sent_to}
self.task_input = {'email_id': email_id}
self.task_input = json.dumps(self.task_input)
self.task_output = {'succeeded': num_sent, 'failed': num_failed}
self.task_output = json.dumps(self.task_output)
......@@ -51,20 +51,6 @@ class FakeEmail(FakeInfo):
'created',
]
class FakeTarget(object):
""" Corresponding fake target for a fake email """
target_type = "expected"
def get_target_type_display(self):
""" Mocks out a django method """
return self.target_type
class FakeTargetGroup(object):
""" Helps to mock out a django M2M relationship """
def all(self):
""" Mocks out a django method """
return [FakeEmail.FakeTarget()]
def __init__(self, email_id):
super(FakeEmail, self).__init__()
self.id = unicode(email_id) # pylint: disable=invalid-name
......@@ -75,7 +61,23 @@ class FakeEmail(FakeInfo):
hour = random.randint(0, 23)
minute = random.randint(0, 59)
self.created = datetime.datetime(year, month, day, hour, minute, tzinfo=utc)
self.targets = FakeEmail.FakeTargetGroup()
self.targets = FakeTargetGroup()
class FakeTarget(object):
""" Corresponding fake target for a fake email """
target_type = "expected"
def long_display(self):
""" Mocks out a class method """
return self.target_type
class FakeTargetGroup(object):
""" Mocks out the M2M relationship between FakeEmail and FakeTarget """
def all(self):
""" Mocks out a django method """
return [FakeTarget()]
class FakeEmailInfo(FakeInfo):
......
lms/djangoapps/instructor/views/api.py
......@@ -2489,7 +2489,7 @@ def list_forum_members(request, course_id):
@require_post_params(send_to="sending to whom", subject="subject line", message="message text")
def send_email(request, course_id):
"""
Send an email to self, staff, or everyone involved in a course.
Send an email to self, staff, cohorts, or everyone involved in a course.
Query Parameters:
- 'send_to' specifies what group the email should be sent to
Options are defined by the CourseEmail model in
......
lms/djangoapps/instructor/views/instructor_dashboard.py
......@@ -33,6 +33,7 @@ from courseware.access import has_access
from courseware.courses import get_course_by_id, get_studio_url
from django_comment_client.utils import has_forum_access
from django_comment_common.models import FORUM_ROLE_ADMINISTRATOR
from openedx.core.djangoapps.course_groups.cohorts import get_course_cohorts, is_course_cohorted, DEFAULT_COHORT_NAME
from student.models import CourseEnrollment
from shoppingcart.models import Coupon, PaidCourseRegistration, CourseRegCodeItem
from course_modes.models import CourseMode, CourseModesArchive
......@@ -609,6 +610,9 @@ def _section_send_email(course, access):
# xblock rendering.
request_token=uuid.uuid1().get_hex()
)
cohorts = []
if is_course_cohorted(course_key):
cohorts = get_course_cohorts(course)
email_editor = fragment.content
section_data = {
'section_key': 'send_email',
......@@ -616,6 +620,8 @@ def _section_send_email(course, access):
'access': access,
'send_email': reverse('send_email', kwargs={'course_id': unicode(course_key)}),
'editor': email_editor,
'cohorts': cohorts,
'default_cohort_name': DEFAULT_COHORT_NAME,
'list_instructor_tasks_url': reverse(
'list_instructor_tasks', kwargs={'course_id': unicode(course_key)}
),
......
lms/djangoapps/instructor/views/instructor_task_helpers.py
......@@ -33,7 +33,7 @@ def extract_email_features(email_task):
From the given task, extract email content information
Expects that the given task has the following attributes:
* task_input (dict containing email_id and to_option)
* task_input (dict containing email_id)
* task_output (optional, dict containing total emails sent)
* requester, the user who executed the task
......@@ -57,7 +57,7 @@ def extract_email_features(email_task):
email = CourseEmail.objects.get(id=task_input_information['email_id'])
email_feature_dict = {
'created': get_default_time_display(email.created),
'sent_to': [target.get_target_type_display() for target in email.targets.all()],
'sent_to': [target.long_display() for target in email.targets.all()],
'requester': str(email_task.requester),
}
features = ['subject', 'html_message', 'id']
......
lms/djangoapps/instructor_task/api.py
......@@ -6,6 +6,7 @@ already been submitted, filtered either by running state or input
arguments.
"""
from collections import Counter
import hashlib
from celery.states import READY_STATES
......@@ -279,12 +280,18 @@ def submit_bulk_course_email(request, course_key, email_id):
# We also pull out the targets argument here, so that is displayed in
# the InstructorTask status.
email_obj = CourseEmail.objects.get(id=email_id)
targets = [target.target_type for target in email_obj.targets.all()]
# task_input has a limit to the size it can store, so any target_type with count > 1 is combined and counted
targets = Counter([target.target_type for target in email_obj.targets.all()])
targets = [
target if count <= 1 else
"{} {}".format(count, target)
for target, count in targets.iteritems()
]
task_type = 'bulk_course_email'
task_class = send_bulk_course_email
task_input = {'email_id': email_id, 'to_option': targets}
task_key_stub = "{email_id}".format(email_id=email_id)
task_key_stub = str(email_id)
# create the key value by using MD5 hash:
task_key = hashlib.md5(task_key_stub).hexdigest()
return submit_task(request, task_type, task_class, course_key, task_input, task_key)
......
lms/static/coffee/spec/instructor_dashboard/send_email_spec.coffee
......@@ -77,3 +77,16 @@ describe "Bulk Email Queueing", ->
@send_email.$btn_send.click()
expect($('.request-response-error').text()).toEqual('Error sending email.')
expect(console.warn).toHaveBeenCalled()
it 'selecting all learners disables cohort selections', ->
@send_email.$send_to.filter("[value='learners']").click
@send_email.$cohort_targets.each ->
expect(this.disabled).toBe(true)
@send_email.$send_to.filter("[value='learners']").click
@send_email.$cohort_targets.each ->
expect(this.disabled).toBe(false)
it 'selected targets are listed after "send to:"', ->
@send_email.$send_to.click
$('input[name="send_to"]:checked+label').each ->
expect($('.send_to_list'.text())).toContain(this.innerText.replace(/\s*\n.*/g,''))
lms/static/coffee/src/instructor_dashboard/send_email.coffee
......@@ -20,6 +20,7 @@ class @SendEmail
# gather elements
@$emailEditor = XBlock.initializeBlock($('.xblock-studio_view'));
@$send_to = @$container.find("input[name='send_to']")
@$cohort_targets = @$send_to.filter('[value^="cohort:"]')
@$subject = @$container.find("input[name='subject']")
@$btn_send = @$container.find("input[name='send']")
@$task_response = @$container.find(".request-response")
......@@ -60,15 +61,19 @@ class @SendEmail
alert message
return
target_map = {
"myself": gettext("Yourself"),
"staff": gettext("Everyone who has staff privileges in this course"),
"learners": gettext("All learners who are enrolled in this course"),
}
display_target = (value) ->
if value == "myself"
gettext("Yourself")
else if value == "staff"
gettext("Everyone who has staff privileges in this course")
else if value == "learners"
gettext("All learners who are enrolled in this course")
else
gettext("All learners in the {cohort_name} cohort").replace('{cohort_name}', value.slice(value.indexOf(':')+1))
success_message = 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.")
confirm_message = gettext("You are sending an email message with the subject {subject} to the following recipients.")
for target in targets
confirm_message += "\n-" + target_map[target]
confirm_message += "\n-" + display_target(target)
confirm_message += "\n\n" + gettext("Is this OK?")
full_confirm_message = confirm_message.replace('{subject}', subject)
......@@ -127,6 +132,27 @@ class @SendEmail
error: std_ajax_err =>
@$content_request_response_error.text gettext("There was an error obtaining email content history for this course.")
@$send_to.change =>
# Ensure invalid combinations are disabled
if $('input#target_learners:checked').length
# If all is selected, cohorts can't be
@$cohort_targets.each ->
this.checked = false
this.disabled = true
true
else
@$cohort_targets.each ->
this.disabled = false
true
# Also, keep the sent_to_list div updated
targets = []
$('input[name="send_to"]:checked+label').each ->
# Only use the first line, even if a subheading is present
targets.push(this.innerText.replace(/\s*\n.*/g,''))
$(".send_to_list").text(gettext("Send to:") + " " + targets.join(", "))
fail_with_error: (msg) ->
console.warn msg
@$task_response.empty()
......
lms/static/sass/course/instructor/_instructor_2.scss
......@@ -406,6 +406,11 @@
// view - bulk email
// --------------------
.instructor-dashboard-wrapper-2 section.idash-section#send_email {
h2 {
// override forced uppercase
text-transform: none;
}
// form fields
.list-fields {
list-style: none;
......@@ -416,12 +421,58 @@
margin-bottom: $baseline;
padding: 0;
.label {
ul {
list-style-type: none;
}
.send_to_list {
line-height: 1.3em;
}
input[name="send_to"] {
float: left;
margin-top: .3em;
margin-left: .1em;
}
input[name="send_to"]+label {
display: block;
padding-left: 1.3em;
margin-bottom: 0;
}
ul {
list-style-type: none;
input[name="send_to"]:checked+label {
// "bolds" the text without causing a width recalculation
text-shadow: 1px 0px 0px;
}
input[name="send_to"]:focus+label, input[name="send_to"]:hover:not([disabled])+label {
background-color: #EFEFEF;
* {
background-color: #EFEFEF;
}
}
input[name="send_to"]:disabled+label {
font-weight: lighter;
background-color: $light-gray3
}
.email-targets-primary {
display: table-cell;
margin: 0;
width: 20%;
}
.email-targets-secondary {
display: table-cell;
margin: 0;
@include columns(2);
.subheading {
font-size: .9em;
}
}
&:last-child {
......
lms/templates/instructor/instructor_dashboard_2/send_email.html
......@@ -9,30 +9,44 @@ from openedx.core.djangolib.markup import HTML
<div class="request-response msg msg-confirm copy" id="request-response"></div>
<ul class="list-fields">
<li class="field">
${_("Send to:")}
<ul role="group" aria-label="${_('Send to:')}">
<div class="send_to_list">${_("Send to:")}</div>
</li>
<li class="field">
<fieldset>
<legend class="sr">${_("Send to:")}</legend>
<ul role="group" class="email-targets-primary">
<li>
<label>
<input type="checkbox" name="send_to" value="myself">
${_("Myself")}
</input>
</label>
<input type="checkbox" name="send_to" value="myself" id="target_myself">
<label for="target_myself">${_("Myself")}</label>
</li>
<li>
<label>
<input type="checkbox" name="send_to" value="staff">
${_("Staff and Admin")}
</input>
</label>
<input type="checkbox" name="send_to" value="staff" id="target_staff">
<label for="target_staff">${_("Staff and Administrators")}</label>
</li>
<li>
<label>
<input type="checkbox" name="send_to" value="learners">
${_("All Learners")}
</input>
<input type="checkbox" name="send_to" value="learners" id="target_learners">
<label for="target_learners">${_("All Learners")}</label>
</li>
</ul>
%if len(section_data['cohorts']) > 0:
<ul role="group" class="email-targets-secondary">
%for cohort in section_data['cohorts']:
<li>
<input type="checkbox" name="send_to" value="cohort:${cohort.name}" id="target_cohort_${cohort.id}">
<label for="target_cohort_${cohort.id}">
${_("Cohort: ") + cohort.name}
%if cohort.name == section_data['default_cohort_name']:
<br/>
<div class="subheading">
${_("(Students without cohort assignment)")}
</div>
%endif
</label>
</li>
%endfor
</ul>
%endif
</fieldset>
</li>
<li class="field">
<label>
......@@ -63,12 +77,12 @@ from openedx.core.djangolib.markup import HTML
<p class="copy"><span style="color: red;"><b>${_("CAUTION!")}</b></span>
${_("When you select Send Email, your email message is added to the queue for sending, and cannot be cancelled.")}
</p>
</div>
</div>
<br />
<input type="button" name="send" value="${_("Send Email")}" data-endpoint="${ section_data['send_email'] }" >
<div class="request-response-error"></div>
%if settings.FEATURES.get('ENABLE_INSTRUCTOR_BACKGROUND_TASKS'):
%if settings.FEATURES.get('ENABLE_INSTRUCTOR_BACKGROUND_TASKS'):
<div class="running-tasks-container action-type-container">
<hr>
<h2> ${_("Pending Tasks")} </h2>
......@@ -82,14 +96,14 @@ from openedx.core.djangolib.markup import HTML
</div>
<hr>
<hr>
<div class="vert-left email-background" id="section-task-history">
<div class="vert-left email-background" id="section-task-history">
<h2> ${_("Email Task History")} </h2>
<div>
<p>${_("To see the content of previously sent emails, click this button:")}</p>
<br/>
<input type="button" name="task-history-email-content" value="${_("Sent Email History")}" data-endpoint="${ section_data['email_content_history_url'] }" >
<input type="button" name="task-history-email-content" value="${_("Show Sent Email History")}" data-endpoint="${ section_data['email_content_history_url'] }" >
<div class="content-request-response-error msg msg-warning copy"></div>
<p>
<div class="content-history-email-table">
......@@ -106,7 +120,7 @@ from openedx.core.djangolib.markup import HTML
<div class="history-request-response-error msg msg-warning copy"></div>
<div class="task-history-email-table"></div>
</div>
</div>
%endif
</div>
%endif
</div> <!-- end section send-email -->
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment