diff --git a/common/lib/html_to_text.py b/common/lib/html_to_text.py index b4b2295509..daad0c83c7 100644 --- a/common/lib/html_to_text.py +++ b/common/lib/html_to_text.py @@ -1,6 +1,9 @@ """Provides a function to convert html to plaintext.""" +import logging from subprocess import Popen, PIPE +log = logging.getLogger(__name__) + def html_to_text(html_message): """ Converts an html message to plaintext. diff --git a/lms/djangoapps/bulk_email/migrations/0002_change_field_names.py b/lms/djangoapps/bulk_email/migrations/0002_change_field_names.py new file mode 100644 index 0000000000..95c0db339f --- /dev/null +++ b/lms/djangoapps/bulk_email/migrations/0002_change_field_names.py @@ -0,0 +1,94 @@ +# -*- 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): + # Renaming field 'CourseEmail.to' + db.rename_column('bulk_email_courseemail', 'to', 'to_option') + + # Renaming field 'CourseEmail.hash' + db.rename_column('bulk_email_courseemail', 'hash', 'slug') + + # Adding field 'CourseEmail.text_message' + db.add_column('bulk_email_courseemail', 'text_message', + self.gf('django.db.models.fields.TextField')(null=True, blank=True), + keep_default=False) + + + def backwards(self, orm): + # Renaming field 'CourseEmail.to_option' + db.rename_column('bulk_email_courseemail', 'to_option', 'to') + + # Renaming field 'CourseEmail.slug' + db.rename_column('bulk_email_courseemail', 'slug', 'hash') + + # Deleting field 'CourseEmail.text_message' + db.delete_column('bulk_email_courseemail', 'text_message') + + + + + 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.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.optout': { + 'Meta': {'unique_together': "(('email', 'course_id'),)", 'object_name': 'Optout'}, + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'email': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': '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'] diff --git a/lms/djangoapps/bulk_email/models.py b/lms/djangoapps/bulk_email/models.py index ef06e83d13..8fa864b955 100644 --- a/lms/djangoapps/bulk_email/models.py +++ b/lms/djangoapps/bulk_email/models.py @@ -1,5 +1,18 @@ """ 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/ + + +ASSUMPTIONS: modules have unique IDs, even across different module_types + """ from django.db import models from django.contrib.auth.models import User @@ -10,8 +23,7 @@ class Email(models.Model): Abstract base class for common information for an email. """ sender = models.ForeignKey(User, default=1, blank=True, null=True) - # The unique hash for this email. Used to quickly look up an email (see `tasks.py`) - hash = models.CharField(max_length=128, db_index=True) + slug = models.CharField(max_length=128, db_index=True) subject = models.CharField(max_length=128, blank=True) html_message = models.TextField(null=True, blank=True) text_message = models.TextField(null=True, blank=True) @@ -41,7 +53,7 @@ class CourseEmail(Email, models.Model): ('all', 'All') ) course_id = models.CharField(max_length=255, db_index=True) - to = models.CharField(max_length=64, choices=TO_OPTIONS, default='myself') + to_option = models.CharField(max_length=64, choices=TO_OPTIONS, default='myself') def __unicode__(self): return self.subject diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py index b34c3983b6..35c57b9dad 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/lms/djangoapps/bulk_email/tasks.py @@ -24,7 +24,7 @@ log = logging.getLogger(__name__) @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): +def delegate_email_batches(email_id, 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, @@ -37,14 +37,14 @@ def delegate_email_batches(hash_for_msg, to_option, course_id, course_url, user_ try: course = get_course_by_id(course_id) except Http404 as exc: - log.error("get_course_by_id failed: " + exc.args[0]) + log.error("get_course_by_id failed: %s", exc.args[0]) raise Exception("get_course_by_id failed: " + exc.args[0]) try: - CourseEmail.objects.get(hash=hash_for_msg) + CourseEmail.objects.get(id=email_id) 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) + log.warning("Failed to get CourseEmail with id %s, retry %d", email_id, current_task.request.retries) + raise delegate_email_batches.retry(arg=[email_id, 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') @@ -77,12 +77,12 @@ def delegate_email_batches(hash_for_msg, to_option, course_id, course_url, user_ for i in range(num_workers): to_list = recipient_list[i * chunk:i * chunk + chunk] - course_email.delay(hash_for_msg, to_list, course.display_name, course_url, False) + course_email.delay(email_id, to_list, course.display_name, course_url, False) return num_workers @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): +def course_email(email_id, to_list, course_title, course_url, throttle=False): """ Takes a subject and an html formatted email and sends it from sender to all addresses in the to_list, with each recipient @@ -90,7 +90,7 @@ def course_email(hash_for_msg, to_list, course_title, course_url, throttle=False text and html. """ try: - msg = CourseEmail.objects.get(hash=hash_for_msg) + msg = CourseEmail.objects.get(id=email_id) except CourseEmail.DoesNotExist as exc: log.exception(exc.args[0]) raise exc @@ -112,6 +112,7 @@ def course_email(hash_for_msg, to_list, course_title, course_url, throttle=False 'course_title': course_title, 'course_url': course_url } + while to_list: (name, email) = to_list[-1].values() email_context['name'] = name @@ -126,7 +127,6 @@ def course_email(hash_for_msg, to_list, course_title, course_url, throttle=False 'emails/email_footer.txt', email_context ) - email_msg = EmailMultiAlternatives( subject, msg.text_message + plain_footer.encode('utf-8'), @@ -142,7 +142,7 @@ def course_email(hash_for_msg, to_list, course_title, course_url, throttle=False try: connection.send_messages([email_msg]) - log.info('Email with hash ' + hash_for_msg + ' sent to ' + email) + log.info('Email with id %s sent to %s', email_id, email) num_sent += 1 except SMTPDataError as exc: # According to SMTP spec, we'll retry error codes in the 4xx range. 5xx range indicates hard failure @@ -151,7 +151,7 @@ def course_email(hash_for_msg, to_list, course_title, course_url, throttle=False raise exc else: # This will fall through and not retry the message, since it will be popped - log.warning('Email with hash ' + hash_for_msg + ' not delivered to ' + email + ' due to error: ' + exc.smtp_error) + log.warning('Email with id %s not delivered to %s due to error %s', email_id, email, exc.smtp_error) num_error += 1 to_list.pop() @@ -163,7 +163,7 @@ def course_email(hash_for_msg, to_list, course_title, course_url, throttle=False # Error caught here cause the email to be retried. The entire task is actually retried without popping the list raise course_email.retry( arg=[ - hash_for_msg, + email_id, to_list, course_title, course_url, diff --git a/lms/djangoapps/bulk_email/tests/test_email.py b/lms/djangoapps/bulk_email/tests/test_email.py index 6f5c09add9..67f49d1f51 100644 --- a/lms/djangoapps/bulk_email/tests/test_email.py +++ b/lms/djangoapps/bulk_email/tests/test_email.py @@ -146,10 +146,11 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): # 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 = { 'action': 'Send email', 'to_option': 'all', - 'subject': u'téśt śúbjéćt főŕ áĺĺ', + 'subject': uni_subject, 'message': 'test message for all' } response = self.client.post(self.url, test_email) @@ -163,7 +164,7 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): ) self.assertEquals( mail.outbox[0].subject, - '[' + self.course.display_name + ']' + u' téśt śúbjéćt főŕ áĺĺ' + '[' + self.course.display_name + '] ' + uni_subject ) def test_unicode_message_send_to_all(self): @@ -173,11 +174,12 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): # 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 = { 'action': 'Send email', 'to_option': 'all', 'subject': 'test subject for all', - 'message': u'ẗëṡẗ ṁëṡṡäġë ḟöṛ äḷḷ ｲ乇丂ｲ ﾶ乇丂丂ﾑg乇 ｷo尺 ﾑﾚﾚ тэѕт мэѕѕаБэ fоѓ аll' + 'message': uni_message } response = self.client.post(self.url, test_email) @@ -190,7 +192,7 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): ) self.assertIn( - u'ẗëṡẗ ṁëṡṡäġë ḟöṛ äḷḷ ｲ乇丂ｲ ﾶ乇丂丂ﾑg乇 ｷo尺 ﾑﾚﾚ тэѕт мэѕѕаБэ fоѓ аll', + uni_message, mail.outbox[0].body ) @@ -238,4 +240,4 @@ class TestEmailSendExceptions(ModuleStoreTestCase): def test_no_course_email_obj(self): # Make sure course_email handles CourseEmail.DoesNotExist exception. with self.assertRaises(CourseEmail.DoesNotExist): - course_email("dummy hash", [], "_", "_", False) + course_email(101, [], "_", "_", False) diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index 4143639781..3065df2c24 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -57,7 +57,6 @@ from mitxmako.shortcuts import render_to_string from bulk_email.models import CourseEmail from html_to_text import html_to_text import datetime -from hashlib import md5 from bulk_email import tasks log = logging.getLogger(__name__) @@ -89,6 +88,7 @@ def instructor_dashboard(request, course_id): to_option = None subject = None html_message = '' + show_email_tab = False problems = [] plots = [] datatable = {} @@ -711,15 +711,16 @@ def instructor_dashboard(request, course_id): email = CourseEmail(course_id=course_id, sender=request.user, - to=to_option, + to_option=to_option, subject=subject, html_message=html_message, - text_message=text_message, - hash=md5((html_message + subject + datetime.datetime.isoformat(datetime.datetime.now())).encode('utf-8')).hexdigest()) + text_message=text_message) + email.save() 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) + tasks.delegate_email_batches.delay(email.id, email.to_option, course_id, course_url, request.user.id) + if to_option == "all": email_msg = '

Your email was successfully queued for sending. Please note that for large public classes (~10k), it may take 1-2 hours to send all emails.

' @@ -798,11 +799,11 @@ def instructor_dashboard(request, course_id): else: editor = None - # Flag for what backing store this course is (Mongo vs. XML) - if modulestore().get_modulestore_type(course_id) == MONGO_MODULESTORE_TYPE: - is_mongo_modulestore_type = True - else: - is_mongo_modulestore_type = False + # Flag for whether or not we display the email tab (depending upon + # what backing store this course using (Mongo vs. XML)) + if settings.MITX_FEATURES['ENABLE_INSTRUCTOR_EMAIL'] and \ + modulestore().get_modulestore_type(course_id) == MONGO_MODULESTORE_TYPE: + show_email_tab = True # display course stats only if there is no other table to display: course_stats = None @@ -820,10 +821,11 @@ def instructor_dashboard(request, course_id): 'course_stats': course_stats, 'msg': msg, 'modeflag': {idash_mode: 'selectedmode'}, - 'to_option': to_option, # email - 'subject': subject, # email - 'editor': editor, # email - 'email_msg': email_msg, # email + 'to_option': to_option, # email + 'subject': subject, # email + 'editor': editor, # email + 'email_msg': email_msg, # email + 'show_email_tab': show_email_tab, # email 'problems': problems, # psychometrics 'plots': plots, # psychometrics 'course_errors': modulestore().get_item_errors(course.location), @@ -832,7 +834,6 @@ def instructor_dashboard(request, course_id): 'cohorts_ajax_url': reverse('cohorts', kwargs={'course_id': course_id}), 'analytics_results': analytics_results, - 'is_mongo_modulestore_type': is_mongo_modulestore_type, } if settings.MITX_FEATURES.get('ENABLE_INSTRUCTOR_BETA_DASHBOARD'): diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 0d44674741..45738e6d31 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -105,7 +105,7 @@ EMAIL_FILE_PATH = ENV_TOKENS.get('EMAIL_FILE_PATH', None) EMAIL_HOST = ENV_TOKENS.get('EMAIL_HOST', 'localhost') # django default is localhost EMAIL_PORT = ENV_TOKENS.get('EMAIL_PORT', 25) # django default is 25 EMAIL_USE_TLS = ENV_TOKENS.get('EMAIL_USE_TLS', False) # django default is False -EMAILS_PER_TASK = ENV_TOKENS.get('EMAILS_PER_TASK', 10) +EMAILS_PER_TASK = ENV_TOKENS.get('EMAILS_PER_TASK', 100) SITE_NAME = ENV_TOKENS['SITE_NAME'] SESSION_ENGINE = ENV_TOKENS.get('SESSION_ENGINE', SESSION_ENGINE) diff --git a/lms/templates/courseware/instructor_dashboard.html b/lms/templates/courseware/instructor_dashboard.html index 82067d4261..fae76cca93 100644 --- a/lms/templates/courseware/instructor_dashboard.html +++ b/lms/templates/courseware/instructor_dashboard.html @@ -124,7 +124,7 @@ function goto( mode) ${_("Enrollment")} | ${_("DataDump")} | ${_("Manage Groups")} - %if settings.MITX_FEATURES.get('ENABLE_INSTRUCTOR_EMAIL') and is_mongo_modulestore_type: + %if show_email_tab: | Email %endif %if settings.MITX_FEATURES.get('ENABLE_INSTRUCTOR_ANALYTICS'):