Skip to content

E
edx-platform
Commit 3b32d421 by Kevin Luo Committed by Sarina Canelake
Options

Add delay to course bulk email task and use SITE_NAME for site url 

 Delay for possible race condition with fetching course email object.
 Use settings.SITE_NAME for host name to generate email footer url.
parent 3ea2b24b
Showing with 108 additions and 49 deletions
lms/djangoapps/bulk_email/admin.py
"""
Django admin page for bulk email models
"""
from django.contrib import admin
from bulk_email.models import CourseEmail, Optout
admin.site.register(Optout)
class CourseEmailAdmin(admin.ModelAdmin):
"""Admin for course email."""
readonly_fields = ('sender',)
class OptoutAdmin(admin.ModelAdmin):
"""Admin for optouts."""
list_display = ('email', 'course_id')
admin.site.register(CourseEmail, CourseEmailAdmin)
admin.site.register(Optout, OptoutAdmin)
lms/djangoapps/bulk_email/models.py
"""
Models for bulk email
"""
from django.db import models
from django.contrib.auth.models import User
......
lms/djangoapps/bulk_email/tasks.py
"""
This module contains celery task functions for handling the sending of bulk email
to a course.
"""
import logging
import math
import re
......@@ -20,24 +24,30 @@ from mitxmako.shortcuts import render_to_string
log = logging.getLogger(__name__)
@task()
def delegate_email_batches(hash_for_msg, recipient, course_id, course_url, user_id):
'''
@task(default_retry_delay=10, max_retries=5) # pylint: disable=E1102
def delegate_email_batches(hash_for_msg, to_option, course_id, course_url, user_id):
"""
Delegates emails by querying for the list of recipients who should
get the mail, chopping up into batches of settings.EMAILS_PER_TASK size,
and queueing up worker jobs.
Recipient is {'students', 'staff', or 'all'}
`to_option` is {'students', 'staff', or 'all'}
Returns the number of batches (workers) kicked off.
'''
"""
try:
course = get_course_by_id(course_id)
except Http404 as exc:
log.error("get_course_by_id failed: " + exc.args[0])
raise Exception("get_course_by_id failed: " + exc.args[0])
if recipient == "myself":
try:
CourseEmail.objects.get(hash=hash_for_msg)
except CourseEmail.DoesNotExist as exc:
log.warning("Failed to get CourseEmail with hash %s, retry %d", hash_for_msg, current_task.request.retries)
raise delegate_email_batches.retry(arg=[hash_for_msg, to_option, course_id, course_url, user_id], exc=exc)
if to_option == "myself":
recipient_qset = User.objects.filter(id=user_id).values('profile__name', 'email')
else:
staff_grpname = _course_staff_group_name(course.location)
......@@ -48,9 +58,9 @@ def delegate_email_batches(hash_for_msg, recipient, course_id, course_url, user_
instructor_qset = instructor_group.user_set.values('profile__name', 'email')
recipient_qset = staff_qset | instructor_qset
if recipient == "all":
#Execute two queries per performance considerations for MySQL
#https://docs.djangoproject.com/en/1.2/ref/models/querysets/#in
if to_option == "all":
# Two queries are executed per performance considerations for MySQL.
# See https://docs.djangoproject.com/en/1.2/ref/models/querysets/#in.
course_optouts = Optout.objects.filter(course_id=course_id).values_list('email', flat=True)
enrollment_qset = User.objects.filter(courseenrollment__course_id=course_id).exclude(email__in=list(course_optouts)).values('profile__name', 'email')
recipient_qset = recipient_qset | enrollment_qset
......@@ -67,7 +77,7 @@ def delegate_email_batches(hash_for_msg, recipient, course_id, course_url, user_
return num_workers
@task(default_retry_delay=15, max_retries=5)
@task(default_retry_delay=15, max_retries=5) # pylint: disable=E1102
def course_email(hash_for_msg, to_list, course_title, course_url, throttle=False):
"""
Takes a subject and an html formatted email and sends it from
......@@ -127,7 +137,7 @@ def course_email(hash_for_msg, to_list, course_title, course_url, throttle=False
raise exc # this will cause the outer handler to catch the exception and retry the entire task
else:
#this will fall through and not retry the message, since it will be popped
log.warn('Email with hash ' + hash_for_msg + ' not delivered to ' + email + ' due to error: ' + exc.smtp_error)
log.warning('Email with hash ' + hash_for_msg + ' not delivered to ' + email + ' due to error: ' + exc.smtp_error)
num_error += 1
to_list.pop()
......@@ -140,6 +150,7 @@ def course_email(hash_for_msg, to_list, course_title, course_url, throttle=False
raise course_email.retry(arg=[hash_for_msg, to_list, course_title, course_url, current_task.request.retries > 0], exc=exc, countdown=(2 ** current_task.request.retries) * 15)
#This string format code is wrapped in this function to allow mocking for a unit test
# This string format code is wrapped in this function to allow mocking for a unit test
def course_email_result(num_sent, num_error):
return "Sent %d, Fail %d" % (num_sent, num_error)
"""Return the formatted result of course_email sending."""
return "Sent {0}, Fail {1}".format(num_sent, num_error)
lms/djangoapps/bulk_email/tests/fake_smtp.py
......@@ -51,11 +51,18 @@ class FakeSMTPServer(smtpd.SMTPServer):
def __init__(self, *args, **kwargs):
smtpd.SMTPServer.__init__(self, *args, **kwargs)
self.errtype = None
self.reply = None
self.response = None
def set_errtype(self, errtype, reply=''):
def set_errtype(self, errtype, response=''):
"""Specify the type of error to cause smptlib to raise, with optional response string.
`errtype` -- "DATA": The server will cause smptlib to throw SMTPDataError.
"CONN": The server will cause smptlib to throw SMTPConnectError.
"DISCONN": The server will cause smptlib to throw SMTPServerDisconnected.
"""
self.errtype = errtype
self.reply = reply
self.response = response
def handle_accept(self):
if self.errtype == "DISCONN":
......@@ -70,11 +77,12 @@ class FakeSMTPServer(smtpd.SMTPServer):
def process_message(self, *_args, **_kwargs):
if self.errtype == "DATA":
#after failing on the first email, succeed on rest
# After failing on the first email, succeed on the rest.
self.errtype = None
return self.reply
return self.response
else:
return None
def serve_forever(self):
"""Start the server running until close() is called on the server."""
asyncore.loop()
lms/djangoapps/bulk_email/tests/smtp_server_thread.py
"""
Defines a class for a thread that runs a Fake SMTP server, used for testing
error handling from sending email.
"""
import threading
from bulk_email.tests.fake_smtp import FakeSMTPServer
class FakeSMTPServerThread(threading.Thread):
"""
Thread for running a fake SMTP server for testing email
Thread for running a fake SMTP server
"""
def __init__(self, host, port):
self.host = host
......@@ -19,21 +23,25 @@ class FakeSMTPServerThread(threading.Thread):
super(FakeSMTPServerThread, self).start()
self.is_ready.wait()
if self.error:
raise self.error
raise self.error # pylint: disable=E0702
def stop(self):
"""
Stop the thread by closing the server instance.
Wait for the server thread to terminate.
"""
if hasattr(self, 'server'):
self.server.close()
self.join()
def run(self):
"""
Sets up the test smtp server and handle requests
Sets up the test smtp server and handle requests.
"""
try:
self.server = FakeSMTPServer((self.host, self.port), None)
self.is_ready.set()
self.server.serve_forever()
except Exception, e:
self.error = e
except Exception, exc: # pylint: disable=W0703
self.error = exc
self.is_ready.set()
lms/djangoapps/bulk_email/tests/test_course_optout.py
......@@ -14,6 +14,11 @@ from student.tests.factories import UserFactory, AdminFactory, CourseEnrollmentF
@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
class TestOptoutCourseEmails(ModuleStoreTestCase):
"""
Test that optouts are referenced in sending course email.
"""
def setUp(self):
self.course = CourseFactory.create()
self.instructor = AdminFactory.create()
......@@ -34,7 +39,7 @@ class TestOptoutCourseEmails(ModuleStoreTestCase):
self.client.login(username=self.instructor.username, password="test")
url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id})
response = self.client.post(url, {'action': 'Send email', 'to': 'all', 'subject': 'test subject for all', 'message': 'test message for all'})
response = self.client.post(url, {'action': 'Send email', 'to_option': 'all', 'subject': 'test subject for all', 'message': 'test message for all'})
self.assertContains(response, "Your email was successfully queued for sending.")
#assert that self.student.email not in mail.to, outbox should be empty
......@@ -52,7 +57,7 @@ class TestOptoutCourseEmails(ModuleStoreTestCase):
self.client.login(username=self.instructor.username, password="test")
url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id})
response = self.client.post(url, {'action': 'Send email', 'to': 'all', 'subject': 'test subject for all', 'message': 'test message for all'})
response = self.client.post(url, {'action': 'Send email', 'to_option': 'all', 'subject': 'test subject for all', 'message': 'test message for all'})
self.assertContains(response, "Your email was successfully queued for sending.")
#assert that self.student.email in mail.to
......
lms/djangoapps/bulk_email/tests/test_email.py
......@@ -18,6 +18,11 @@ STUDENT_COUNT = 10
@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
class TestEmail(ModuleStoreTestCase):
"""
Test that emails send correctly.
"""
def setUp(self):
self.course = CourseFactory.create()
self.instructor = UserFactory.create(username="instructor", email="robot+instructor@edx.org")
......@@ -29,7 +34,7 @@ class TestEmail(ModuleStoreTestCase):
self.staff = [UserFactory() for _ in xrange(STAFF_COUNT)]
staff_group = GroupFactory()
for staff in self.staff:
staff_group.user_set.add(staff)
staff_group.user_set.add(staff) # pylint: disable=E1101
#create students
self.students = [UserFactory() for _ in xrange(STUDENT_COUNT)]
......@@ -43,7 +48,7 @@ class TestEmail(ModuleStoreTestCase):
Make sure email send to myself goes to myself.
"""
url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id})
response = self.client.post(url, {'action': 'Send email', 'to': 'myself', 'subject': 'test subject for myself', 'message': 'test message for myself'})
response = self.client.post(url, {'action': 'Send email', 'to_option': 'myself', 'subject': 'test subject for myself', 'message': 'test message for myself'})
self.assertContains(response, "Your email was successfully queued for sending.")
......@@ -57,7 +62,7 @@ class TestEmail(ModuleStoreTestCase):
Make sure email send to staff and instructors goes there.
"""
url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id})
response = self.client.post(url, {'action': 'Send email', 'to': 'staff', 'subject': 'test subject for staff', 'message': 'test message for subject'})
response = self.client.post(url, {'action': 'Send email', 'to_option': 'staff', 'subject': 'test subject for staff', 'message': 'test message for subject'})
self.assertContains(response, "Your email was successfully queued for sending.")
......@@ -69,7 +74,7 @@ class TestEmail(ModuleStoreTestCase):
Make sure email send to all goes there.
"""
url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id})
response = self.client.post(url, {'action': 'Send email', 'to': 'all', 'subject': 'test subject for all', 'message': 'test message for all'})
response = self.client.post(url, {'action': 'Send email', 'to_option': 'all', 'subject': 'test subject for all', 'message': 'test message for all'})
self.assertContains(response, "Your email was successfully queued for sending.")
......
lms/djangoapps/bulk_email/tests/test_err_handling.py
......@@ -19,6 +19,11 @@ TEST_SMTP_PORT = 1025
@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE, EMAIL_BACKEND='django.core.mail.backends.smtp.EmailBackend', EMAIL_HOST='localhost', EMAIL_PORT=TEST_SMTP_PORT)
class TestEmailErrors(ModuleStoreTestCase):
"""
Test that errors from sending email are handled properly.
"""
def setUp(self):
self.course = CourseFactory.create()
instructor = AdminFactory.create()
......@@ -37,7 +42,7 @@ class TestEmailErrors(ModuleStoreTestCase):
"""
self.smtp_server_thread.server.set_errtype("DATA", "454 Throttling failure: Daily message quota exceeded.")
url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id})
self.client.post(url, {'action': 'Send email', 'to': 'myself', 'subject': 'test subject for myself', 'message': 'test message for myself'})
self.client.post(url, {'action': 'Send email', 'to_option': 'myself', 'subject': 'test subject for myself', 'message': 'test message for myself'})
self.assertTrue(retry.called)
(_, kwargs) = retry.call_args
exc = kwargs['exc']
......@@ -55,7 +60,7 @@ class TestEmailErrors(ModuleStoreTestCase):
CourseEnrollmentFactory.create(user=student, course_id=self.course.id)
url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id})
self.client.post(url, {'action': 'Send email', 'to': 'all', 'subject': 'test subject for all', 'message': 'test message for all'})
self.client.post(url, {'action': 'Send email', 'to_option': 'all', 'subject': 'test subject for all', 'message': 'test message for all'})
self.assertFalse(retry.called)
#test that after the failed email, the rest send successfully
......@@ -70,7 +75,7 @@ class TestEmailErrors(ModuleStoreTestCase):
"""
self.smtp_server_thread.server.set_errtype("DISCONN", "Server disconnected, please try again later.")
url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id})
self.client.post(url, {'action': 'Send email', 'to': 'myself', 'subject': 'test subject for myself', 'message': 'test message for myself'})
self.client.post(url, {'action': 'Send email', 'to_option': 'myself', 'subject': 'test subject for myself', 'message': 'test message for myself'})
self.assertTrue(retry.called)
(_, kwargs) = retry.call_args
exc = kwargs['exc']
......@@ -84,7 +89,7 @@ class TestEmailErrors(ModuleStoreTestCase):
#SMTP reply is already specified in fake SMTP Channel created
self.smtp_server_thread.server.set_errtype("CONN")
url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id})
self.client.post(url, {'action': 'Send email', 'to': 'myself', 'subject': 'test subject for myself', 'message': 'test message for myself'})
self.client.post(url, {'action': 'Send email', 'to_option': 'myself', 'subject': 'test subject for myself', 'message': 'test message for myself'})
self.assertTrue(retry.called)
(_, kwargs) = retry.call_args
exc = kwargs['exc']
......
lms/djangoapps/instructor/views/legacy.py
......@@ -83,7 +83,7 @@ def instructor_dashboard(request, course_id):
forum_admin_access = has_forum_access(request.user, course_id, FORUM_ROLE_ADMINISTRATOR)
msg = ''
to = None
to_option = None
subject = None
html_message = ''
problems = []
......@@ -701,13 +701,13 @@ def instructor_dashboard(request, course_id):
# email
elif action == 'Send email':
to = request.POST.get("to")
to_option = request.POST.get("to_option")
subject = request.POST.get("subject")
html_message = request.POST.get("message")
email = CourseEmail(course_id=course_id,
sender=request.user,
to=to,
to=to_option,
subject=subject,
html_message=html_message,
hash=md5((html_message + subject + datetime.datetime.isoformat(datetime.datetime.now())).encode('utf-8')).hexdigest())
......@@ -716,7 +716,7 @@ def instructor_dashboard(request, course_id):
course_url = request.build_absolute_uri(reverse('course_root', kwargs={'course_id': course_id}))
tasks.delegate_email_batches.delay(email.hash, email.to, course_id, course_url, request.user.id)
if to == "all":
if to_option == "all":
msg = "<font color='green'>Your email was successfully queued for sending. Please note that for large public classe\
s (~10k), it may take 1-2 hours to send all emails.</font>"
else:
......@@ -810,9 +810,9 @@ s (~10k), it may take 1-2 hours to send all emails.</font>"
'course_stats': course_stats,
'msg': msg,
'modeflag': {idash_mode: 'selectedmode'},
'to': to, # email
'subject': subject, # email
'editor': editor, # email
'to_option': to_option, # email
'subject': subject, # email
'editor': editor, # email
'problems': problems, # psychometrics
'plots': plots, # psychometrics
'course_errors': modulestore().get_item_errors(course.location),
......
lms/static/sass/base/_variables.scss
......@@ -44,6 +44,11 @@ $dark-gray: #333;
// used by descriptor css
$lightGrey: #edf1f5;
$darkGrey: #8891a1;
$blue-d1: shade($blue,20%);
$blue-d2: shade($blue,40%);
$blue-d4: shade($blue,80%);
$shadow: rgba($black, 0.2);
$shadow-l1: rgba($black, 0.1);
// edx.org marketing site variables
$m-gray: #8A8C8F;
......
lms/templates/courseware/instructor_dashboard.html
......@@ -445,14 +445,14 @@ function goto( mode)
%if modeflag.get('Email'):
<p>
<label for="id_to">Send to:</label>
<select id="id_to" name="to">
<select id="id_to" name="to_option">
<option value="myself">Myself</option>
%if to == "staff":
%if to_option == "staff":
<option value="staff" selected="selected">Staff and instructors</option>
%else:
<option value="staff">Staff and instructors</option>
%endif
%if to == "all":
%if to_option == "all":
<option value="all" selected="selected">All (students, staff and instructors)</option>
%else:
<option value="all">All (students, staff and instructors)</option>
......
lms/templates/emails/email_footer.html
<%! from django.core.urlresolvers import reverse %>
<br />
----<br />
This email was automatically sent from ${settings.PLATFORM_NAME} to ${name}. <br />
This email was automatically sent from ${settings.PLATFORM_NAME}. <br />
You are receiving this email at address ${ email } because you are enrolled in <a href="${course_url}">${ course_title }</a>.<br />
To stop receiving email like this, update your course email settings <a href="https://${site}${reverse('dashboard')}">here</a>. <br />
To stop receiving email like this, update your course email settings <a href="https://${settings.SITE_NAME}${reverse('dashboard')}">here</a>. <br />
lms/templates/emails/email_footer.txt
<%! from django.core.urlresolvers import reverse %>
----
This email was automatically sent from ${settings.PLATFORM_NAME} to ${name}.
This email was automatically sent from ${settings.PLATFORM_NAME}.
You are receiving this email at address ${ email } because you are enrolled in ${ course_title }
(URL: ${course_url} ).
To stop receiving email like this, update your account settings at https://${site}${reverse('dashboard')}.
To stop receiving email like this, update your account settings at https://${settings.SITE_NAME}${reverse('dashboard')}.
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