From adbe133f5cf7c5b54829ec6c796e7567026d916a Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 13 Nov 2014 12:53:44 -0500 Subject: [PATCH] Treat an empty string from the admin form to be None/NULL for the CourseEmailTemplateForm --- lms/djangoapps/bulk_email/forms.py | 6 +- lms/djangoapps/bulk_email/tests/test_forms.py | 154 +++++++++++++++++- 2 files changed, 157 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/bulk_email/forms.py b/lms/djangoapps/bulk_email/forms.py index 65872f18e0..815df5d3a6 100644 --- a/lms/djangoapps/bulk_email/forms.py +++ b/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: diff --git a/lms/djangoapps/bulk_email/tests/test_forms.py b/lms/djangoapps/bulk_email/tests/test_forms.py index c15c957009..9e311882c8 100644 --- a/lms/djangoapps/bulk_email/tests/test_forms.py +++ b/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())