Merge pull request #5951 from edx/cdodge/treat-empty-email-template-names-as-NULL

Treat an empty string from the admin form to be None/NULL for the Course...

lms/djangoapps/bulk_email/forms.py

@@ -55,7 +55,11 @@ class CourseEmailTemplateForm(forms.ModelForm):  # pylint: disable=R0924
 
     def clean_name(self):
         """Validate the name field. Enforce uniqueness constraint on 'name' field"""
-        name = self.cleaned_data.get("name")
+
+        # Note that we get back a blank string in the Form for an empty 'name' field
+        # we want those to be set to None in Python and NULL in the database
+        name = self.cleaned_data.get("name").strip() or None
+
         # 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:

lms/djangoapps/bulk_email/tests/test_forms.py

@@ -15,8 +15,8 @@ from xmodule.modulestore import ModuleStoreEnum
 
 from mock import patch
 
-from bulk_email.models import CourseAuthorization
-from bulk_email.forms import CourseAuthorizationAdminForm
+from bulk_email.models import CourseAuthorization, CourseEmailTemplate
+from bulk_email.forms import CourseAuthorizationAdminForm, CourseEmailTemplateForm
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 
 
@@ -145,3 +145,153 @@ class CourseAuthorizationXMLFormTest(ModuleStoreTestCase):
 
         with self.assertRaisesRegexp(ValueError, "The CourseAuthorization could not be created because the data didn't validate."):
             form.save()
+
+
+@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
+class CourseEmailTemplateFormTest(ModuleStoreTestCase):
+    """Test the CourseEmailTemplateForm that is used in the Django admin subsystem."""
+
+    def test_missing_message_body_in_html(self):
+        """
+        Asserts that we fail validation if we do not have the {{message_body}} tag
+        in the submitted HTML template
+        """
+        form_data = {
+            'html_template': '',
+            'plain_template': '{{message_body}}',
+            'name': ''
+        }
+        form = CourseEmailTemplateForm(form_data)
+        self.assertFalse(form.is_valid())
+
+    def test_missing_message_body_in_plain(self):
+        """
+        Asserts that we fail validation if we do not have the {{message_body}} tag
+        in the submitted plain template
+        """
+        form_data = {
+            'html_template': '{{message_body}}',
+            'plain_template': '',
+            'name': ''
+        }
+        form = CourseEmailTemplateForm(form_data)
+        self.assertFalse(form.is_valid())
+
+    def test_blank_name_is_null(self):
+        """
+        Asserts that submitting a CourseEmailTemplateForm with a blank name is stored
+        as a NULL in the database
+        """
+        form_data = {
+            'html_template': '{{message_body}}',
+            'plain_template': '{{message_body}}',
+            'name': ''
+        }
+        form = CourseEmailTemplateForm(form_data)
+        self.assertTrue(form.is_valid())
+        form.save()
+
+        # now inspect the database and make sure the blank name was stored as a NULL
+        # Note this will throw an exception if it is not found
+        cet = CourseEmailTemplate.objects.get(name=None)
+        self.assertIsNotNone(cet)
+
+    def test_name_with_only_spaces_is_null(self):
+        """
+        Asserts that submitting a CourseEmailTemplateForm just blank whitespace is stored
+        as a NULL in the database
+        """
+        form_data = {
+            'html_template': '{{message_body}}',
+            'plain_template': '{{message_body}}',
+            'name': '   '
+        }
+        form = CourseEmailTemplateForm(form_data)
+        self.assertTrue(form.is_valid())
+        form.save()
+
+        # now inspect the database and make sure the whitespace only name was stored as a NULL
+        # Note this will throw an exception if it is not found
+        cet = CourseEmailTemplate.objects.get(name=None)
+        self.assertIsNotNone(cet)
+
+    def test_name_with_spaces_is_trimmed(self):
+        """
+        Asserts that submitting a CourseEmailTemplateForm with a name that contains
+        whitespace at the beginning or end of a name is stripped
+        """
+        form_data = {
+            'html_template': '{{message_body}}',
+            'plain_template': '{{message_body}}',
+            'name': ' foo  '
+        }
+        form = CourseEmailTemplateForm(form_data)
+        self.assertTrue(form.is_valid())
+        form.save()
+
+        # now inspect the database and make sure the name is properly
+        # stripped
+        cet = CourseEmailTemplate.objects.get(name='foo')
+        self.assertIsNotNone(cet)
+
+    def test_non_blank_name(self):
+        """
+        Asserts that submitting a CourseEmailTemplateForm with a non-blank name
+        can be found in the database under than name as a look-up key
+        """
+        form_data = {
+            'html_template': '{{message_body}}',
+            'plain_template': '{{message_body}}',
+            'name': 'foo'
+        }
+        form = CourseEmailTemplateForm(form_data)
+        self.assertTrue(form.is_valid())
+        form.save()
+
+        # now inspect the database and make sure the blank name was stored as a NULL
+        # Note this will throw an exception if it is not found
+        cet = CourseEmailTemplate.objects.get(name='foo')
+        self.assertIsNotNone(cet)
+
+    def test_duplicate_name(self):
+        """
+        Assert that we cannot submit a CourseEmailTemplateForm with a name
+        that already exists
+        """
+
+        # first set up one template
+        form_data = {
+            'html_template': '{{message_body}}',
+            'plain_template': '{{message_body}}',
+            'name': 'foo'
+        }
+        form = CourseEmailTemplateForm(form_data)
+        self.assertTrue(form.is_valid())
+        form.save()
+
+        # try to submit form with the same name
+        form = CourseEmailTemplateForm(form_data)
+        self.assertFalse(form.is_valid())
+
+        # try again with a name with extra whitespace
+        # this should fail as we strip the whitespace away
+        form_data = {
+            'html_template': '{{message_body}}',
+            'plain_template': '{{message_body}}',
+            'name': '  foo '
+        }
+        form = CourseEmailTemplateForm(form_data)
+        self.assertFalse(form.is_valid())
+
+        # then try a different name
+        form_data = {
+            'html_template': '{{message_body}}',
+            'plain_template': '{{message_body}}',
+            'name': 'bar'
+        }
+        form = CourseEmailTemplateForm(form_data)
+        self.assertTrue(form.is_valid())
+        form.save()
+
+        form = CourseEmailTemplateForm(form_data)
+        self.assertFalse(form.is_valid())