Skip to content

E
edx-platform
Commit 4b122f31 by Nimisha Asthagiri
Options

Merge pull request #5523 from edx/hotfix/2014-10-08 

Hotfix for 2014-10-08
parents 2690f784 7f669015
Showing with 256 additions and 17 deletions
common/test/db_cache/bok_choy_data.json
This source diff could not be displayed because it is too large. You can view the blob instead.
lms/djangoapps/bulk_email/admin.py
......@@ -23,7 +23,7 @@ class CourseEmailTemplateAdmin(admin.ModelAdmin):
fieldsets = (
(None, {
# make the HTML template display above the plain template:
'fields': ('html_template', 'plain_template'),
'fields': ('html_template', 'plain_template', 'name'),
'description': '''
Enter template to be used by course staff when sending emails to enrolled students.
......@@ -49,11 +49,11 @@ unsupported tags will cause email sending to fail.
actions = None
def has_add_permission(self, request):
"""Disables the ability to add new templates, as we want to maintain a Singleton."""
return False
"""Enable the ability to add new templates, as we want to be able to define multiple templates."""
return True
def has_delete_permission(self, request, obj=None):
"""Disables the ability to remove existing templates, as we want to maintain a Singleton."""
"""Disables the ability to remove existing templates, as we'd like to make sure we don't have dangling references."""
return False
......
lms/djangoapps/bulk_email/forms.py
......@@ -20,8 +20,10 @@ log = logging.getLogger(__name__)
class CourseEmailTemplateForm(forms.ModelForm): # pylint: disable=R0924
"""Form providing validation of CourseEmail templates."""
name = forms.CharField(required=False)
class Meta: # pylint: disable=C0111
model = CourseEmailTemplate
fields = ('html_template', 'plain_template', 'name')
def _validate_template(self, template):
"""Check the template for required tags."""
......@@ -50,6 +52,21 @@ class CourseEmailTemplateForm(forms.ModelForm): # pylint: disable=R0924
self._validate_template(template)
return template
def clean_name(self):
"""Validate the name field. Enforce uniqueness constraint on 'name' field"""
name = self.cleaned_data.get("name")
# if we are creating a new CourseEmailTemplate, then we need to
# enforce the uniquess constraint as part of the Form validation
if not self.instance.pk:
try:
CourseEmailTemplate.get_template(name)
# already exists, this is no good
raise ValidationError('Name of "{}" already exists, this must be unique.'.format(name))
except CourseEmailTemplate.DoesNotExist:
# this is actually the successful validation
pass
return name
class CourseAuthorizationAdminForm(forms.ModelForm): # pylint: disable=R0924
"""Input form for email enabling, allowing us to verify data."""
......
lms/djangoapps/bulk_email/migrations/0010_auto__chg_field_optout_course_id__add_field_courseemail_template_name_.py 0 → 100644
# -*- 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):
# Changing field 'Optout.course_id'
db.alter_column('bulk_email_optout', 'course_id', self.gf('xmodule_django.models.CourseKeyField')(max_length=255))
# Adding field 'CourseEmail.template_name'
db.add_column('bulk_email_courseemail', 'template_name',
self.gf('django.db.models.fields.CharField')(max_length=255, null=True),
keep_default=False)
# Adding field 'CourseEmail.from_addr'
db.add_column('bulk_email_courseemail', 'from_addr',
self.gf('django.db.models.fields.CharField')(max_length=255, null=True),
keep_default=False)
# Changing field 'CourseEmail.course_id'
db.alter_column('bulk_email_courseemail', 'course_id', self.gf('xmodule_django.models.CourseKeyField')(max_length=255))
# Adding field 'CourseEmailTemplate.name'
db.add_column('bulk_email_courseemailtemplate', 'name',
self.gf('django.db.models.fields.CharField')(max_length=255, unique=True, null=True),
keep_default=False)
# Changing field 'CourseAuthorization.course_id'
db.alter_column('bulk_email_courseauthorization', 'course_id', self.gf('xmodule_django.models.CourseKeyField')(unique=True, max_length=255))
def backwards(self, orm):
# Changing field 'Optout.course_id'
db.alter_column('bulk_email_optout', 'course_id', self.gf('django.db.models.fields.CharField')(max_length=255))
# Deleting field 'CourseEmail.template_name'
db.delete_column('bulk_email_courseemail', 'template_name')
# Deleting field 'CourseEmail.from_addr'
db.delete_column('bulk_email_courseemail', 'from_addr')
# Changing field 'CourseEmail.course_id'
db.alter_column('bulk_email_courseemail', 'course_id', self.gf('django.db.models.fields.CharField')(max_length=255))
# Deleting field 'CourseEmailTemplate.name'
db.delete_column('bulk_email_courseemailtemplate', 'name')
# Changing field 'CourseAuthorization.course_id'
db.alter_column('bulk_email_courseauthorization', 'course_id', self.gf('django.db.models.fields.CharField')(max_length=255, unique=True))
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': ('xmodule_django.models.CourseKeyField', [], {'unique': 'True', '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': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}),
'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
'from_addr': ('django.db.models.fields.CharField', [], {'max_length': '255', 'null': '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'}),
'template_name': ('django.db.models.fields.CharField', [], {'max_length': '255', 'null': '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'}),
'name': ('django.db.models.fields.CharField', [], {'max_length': '255', 'unique': 'True', 'null': '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': ('xmodule_django.models.CourseKeyField', [], {'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
......@@ -67,12 +67,14 @@ class CourseEmail(Email):
)
course_id = CourseKeyField(max_length=255, db_index=True)
to_option = models.CharField(max_length=64, choices=TO_OPTION_CHOICES, default=SEND_TO_MYSELF)
template_name = models.CharField(null=True, max_length=255)
from_addr = models.CharField(null=True, max_length=255)
def __unicode__(self):
return self.subject
@classmethod
def create(cls, course_id, sender, to_option, subject, html_message, text_message=None):
def create(cls, course_id, sender, to_option, subject, html_message, text_message=None, template_name=None, from_addr=None):
"""
Create an instance of CourseEmail.
......@@ -101,6 +103,8 @@ class CourseEmail(Email):
subject=subject,
html_message=html_message,
text_message=text_message,
template_name=template_name,
from_addr=from_addr,
)
course_email.save_now()
......@@ -120,6 +124,11 @@ class CourseEmail(Email):
"""
self.save()
def get_template(self):
"""
Returns the corresponding CourseEmailTemplate for this CourseEmail.
"""
return CourseEmailTemplate.get_template(name=self.template_name)
class Optout(models.Model):
"""
......@@ -151,16 +160,17 @@ class CourseEmailTemplate(models.Model):
"""
html_template = models.TextField(null=True, blank=True)
plain_template = models.TextField(null=True, blank=True)
name = models.CharField(null=True, max_length=255, unique=True, blank=True)
@staticmethod
def get_template():
def get_template(name=None):
"""
Fetch the current template
If one isn't stored, an exception is thrown.
"""
try:
return CourseEmailTemplate.objects.get()
return CourseEmailTemplate.objects.get(name=name)
except CourseEmailTemplate.DoesNotExist:
log.exception("Attempting to fetch a non-existent course email template")
raise
......
lms/djangoapps/bulk_email/tasks.py
......@@ -438,9 +438,13 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
course_title = global_email_context['course_title']
subject = "[" + course_title + "] " + course_email.subject
from_addr = _get_source_address(course_email.course_id, course_title)
course_email_template = CourseEmailTemplate.get_template()
# use the email from address in the CourseEmail, if it is present, otherwise compute it
from_addr = course_email.from_addr if course_email.from_addr else \
_get_source_address(course_email.course_id, course_title)
# use the CourseEmailTemplate that was associated with the CourseEmail
course_email_template = course_email.get_template()
try:
connection = get_connection()
connection.open()
......
lms/djangoapps/bulk_email/tests/test_models.py
......@@ -29,6 +29,23 @@ class CourseEmailTest(TestCase):
self.assertEquals(email.html_message, html_message)
self.assertEquals(email.sender, sender)
def test_creation_with_optional_attributes(self):
course_id = SlashSeparatedCourseKey('abc', '123', 'doremi')
sender = UserFactory.create()
to_option = SEND_TO_STAFF
subject = "dummy subject"
html_message = "<html>dummy message</html>"
template_name = "branded_template"
from_addr = "branded@branding.com"
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.to_option, 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)
def test_bad_to_option(self):
course_id = SlashSeparatedCourseKey('abc', '123', 'doremi')
sender = UserFactory.create()
......@@ -72,10 +89,19 @@ class CourseEmailTemplateTest(TestCase):
return context
def test_get_template(self):
# Get the default template, which has name=None
template = CourseEmailTemplate.get_template()
self.assertIsNotNone(template.html_template)
self.assertIsNotNone(template.plain_template)
def test_get_branded_template(self):
# Get a branded (non default) template and make sure we get what we expect
template = CourseEmailTemplate.get_template(name="branded.template")
self.assertIsNotNone(template.html_template)
self.assertIsNotNone(template.plain_template)
self.assertIn(u"THIS IS A BRANDED HTML TEMPLATE", template.html_template)
self.assertIn(u"THIS IS A BRANDED TEXT TEMPLATE", template.plain_template)
def test_render_html_without_context(self):
template = CourseEmailTemplate.get_template()
base_context = self._get_sample_html_context()
......
lms/djangoapps/instructor/views/api.py
......@@ -1462,10 +1462,25 @@ def send_email(request, course_id):
subject = request.POST.get("subject")
message = request.POST.get("message")
# allow two branding points to come from Microsites: which CourseEmailTemplate should be used
# and what the 'from' field in the email should be
#
# If these are None (because we are not in a Microsite or they are undefined in Microsite config) than
# the system will use normal system defaults
template_name = microsite.get_value('course_email_template_name')
from_addr = microsite.get_value('course_email_from_addr')
# Create the CourseEmail object. This is saved immediately, so that
# any transaction that has been pending up to this point will also be
# committed.
email = CourseEmail.create(course_id, request.user, send_to, subject, message)
email = CourseEmail.create(
course_id,
request.user,
send_to,
subject,message,
template_name=template_name,
from_addr=from_addr
)
# Submit the task, so that the correct InstructorTask object gets created (for monitoring purposes)
instructor_task.api.submit_bulk_course_email(request, course_id, email.id) # pylint: disable=E1101
......
lms/djangoapps/instructor_task/models.py
......@@ -209,6 +209,15 @@ class ReportStore(object):
elif storage_type.lower() == "localfs":
return LocalFSReportStore.from_config()
def _get_utf8_encoded_rows(self, rows):
"""
Given a list of `rows` containing unicode strings, return a
new list of rows with those strings encoded as utf-8 for CSV
compatibility.
"""
for row in rows:
yield [unicode(item).encode('utf-8') for item in row]
class S3ReportStore(ReportStore):
"""
......@@ -306,7 +315,8 @@ class S3ReportStore(ReportStore):
"""
output_buffer = StringIO()
gzip_file = GzipFile(fileobj=output_buffer, mode="wb")
csv.writer(gzip_file).writerows(rows)
csvwriter = csv.writer(gzip_file)
csvwriter.writerows(self._get_utf8_encoded_rows(rows))
gzip_file.close()
self.store(course_id, filename, output_buffer)
......@@ -384,7 +394,9 @@ class LocalFSReportStore(ReportStore):
write this data out.
"""
output_buffer = StringIO()
csv.writer(output_buffer).writerows(rows)
csvwriter = csv.writer(output_buffer)
csvwriter.writerows(self._get_utf8_encoded_rows(rows))
self.store(course_id, filename, output_buffer)
def links_for(self, course_id):
......
lms/djangoapps/instructor_task/tasks_helper.py
......@@ -575,7 +575,7 @@ def push_grades_to_s3(_xmodule_instance_args, _entry_id, course_id, _task_input,
# possible for a student to have a 0.0 show up in their row but
# still have 100% for the course.
row_percents = [percents.get(label, 0.0) for label in header]
rows.append([student.id, student.email.encode('utf-8'), student.username, gradeset['percent']] + row_percents)
rows.append([student.id, student.email, student.username, gradeset['percent']] + row_percents)
else:
# An empty gradeset means we failed to grade a student.
num_failed += 1
......
lms/djangoapps/instructor_task/tests/test_tasks_helper.py
......@@ -33,16 +33,16 @@ class TestReport(ModuleStoreTestCase):
if os.path.exists(settings.GRADES_DOWNLOAD['ROOT_PATH']):
shutil.rmtree(settings.GRADES_DOWNLOAD['ROOT_PATH'])
def create_student(self, username, email):
student = UserFactory.create(username=username, email=email)
CourseEnrollmentFactory.create(user=student, course_id=self.course.id)
@ddt.ddt
class TestInstructorGradeReport(TestReport):
"""
Tests that CSV grade report generation works.
"""
def create_student(self, username, email):
student = UserFactory.create(username=username, email=email)
CourseEnrollmentFactory.create(user=student, course_id=self.course.id)
@ddt.data([u'student@example.com', u'ni\xf1o@example.com'])
def test_unicode_emails(self, emails):
"""
......@@ -60,6 +60,7 @@ class TestInstructorGradeReport(TestReport):
self.assertEquals(result['succeeded'], result['attempted'])
@ddt.ddt
class TestStudentReport(TestReport):
"""
Tests that CSV student profile report generation works.
......@@ -73,3 +74,27 @@ class TestStudentReport(TestReport):
self.assertEquals(len(links), 1)
self.assertEquals(result, UPDATE_STATUS_SUCCEEDED)
@ddt.data([u'student', u'student\xec'])
def test_unicode_usernames(self, students):
"""
Test that students with unicode characters in their usernames
are handled.
"""
for i, student in enumerate(students):
self.create_student(username=student, email='student{0}@example.com'.format(i))
self.current_task = Mock()
self.current_task.update_state = Mock()
task_input = {
'features': [
'id', 'username', 'name', 'email', 'language', 'location',
'year_of_birth', 'gender', 'level_of_education', 'mailing_address',
'goals'
]
}
with patch('instructor_task.tasks_helper._get_current_task') as mock_current_task:
mock_current_task.return_value = self.current_task
result = push_students_csv_to_s3(None, None, self.course.id, task_input, 'calculated')
#This assertion simply confirms that the generation completed with no errors
self.assertEquals(result, UPDATE_STATUS_SUCCEEDED)
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