Merge pull request #5531 from edx/release

Merge Release

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

+# -*- 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)