From bfebe684b3ff3079cdd92b0334ca2d9b58d5617c Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Tue, 3 Sep 2013 16:11:34 -0400 Subject: [PATCH] Cleanup bulk email app --- lms/djangoapps/bulk_email/admin.py | 1 + lms/djangoapps/bulk_email/forms.py | 5 ++++- lms/djangoapps/bulk_email/tasks.py | 4 +++- lms/djangoapps/bulk_email/tests/test_email.py | 6 ------ lms/djangoapps/bulk_email/tests/test_err_handling.py | 3 ++- 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/lms/djangoapps/bulk_email/admin.py b/lms/djangoapps/bulk_email/admin.py index 1505af9ce4..3550e1eb42 100644 --- a/lms/djangoapps/bulk_email/admin.py +++ b/lms/djangoapps/bulk_email/admin.py @@ -18,6 +18,7 @@ class OptoutAdmin(admin.ModelAdmin): class CourseEmailTemplateAdmin(admin.ModelAdmin): + """Admin for course email templates.""" form = CourseEmailTemplateForm fieldsets = ( (None, { diff --git a/lms/djangoapps/bulk_email/forms.py b/lms/djangoapps/bulk_email/forms.py index 2ccdd72d16..fc75d4e4b6 100644 --- a/lms/djangoapps/bulk_email/forms.py +++ b/lms/djangoapps/bulk_email/forms.py @@ -1,3 +1,6 @@ +""" +Defines a form for providing validation of CourseEmail templates. +""" import logging from django import forms @@ -11,7 +14,7 @@ log = logging.getLogger(__name__) class CourseEmailTemplateForm(forms.ModelForm): """Form providing validation of CourseEmail templates.""" - class Meta: + class Meta: # pylint: disable=C0111 model = CourseEmailTemplate def _validate_template(self, template): diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py index effea57a8a..019fb00b2c 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/lms/djangoapps/bulk_email/tasks.py @@ -130,7 +130,7 @@ def course_email(email_id, to_list, course_title, course_url, image_url, throttl optouts = set(optouts) num_optout = len(optouts) - to_list = filter(lambda x: x['email'] not in optouts, to_list) + to_list = [recipient for recipient in to_list if recipient['email'] not in optouts] subject = "[" + course_title + "] " + msg.subject @@ -226,6 +226,8 @@ def course_email(email_id, to_list, course_title, course_url, image_url, throttl log.exception('Email with id %d caused course_email task to fail with uncaught exception. To list: %s', email_id, [i['email'] for i in to_list]) + # Close the connection before we exit + connection.close() raise diff --git a/lms/djangoapps/bulk_email/tests/test_email.py b/lms/djangoapps/bulk_email/tests/test_email.py index ba2633f263..dab7812763 100644 --- a/lms/djangoapps/bulk_email/tests/test_email.py +++ b/lms/djangoapps/bulk_email/tests/test_email.py @@ -288,12 +288,6 @@ class TestEmailSendExceptions(ModuleStoreTestCase): """ Test that exceptions are handled correctly. """ - - def test_get_course_exc(self): - # Make sure delegate_email_batches handles Http404 exception from get_course_by_id. - with self.assertRaises(Exception): - delegate_email_batches("_", "_", "blah/blah/blah", "_", "_") - def test_no_course_email_obj(self): # Make sure course_email handles CourseEmail.DoesNotExist exception. with self.assertRaises(CourseEmail.DoesNotExist): diff --git a/lms/djangoapps/bulk_email/tests/test_err_handling.py b/lms/djangoapps/bulk_email/tests/test_err_handling.py index e0fd222c45..abdbf4dc3b 100644 --- a/lms/djangoapps/bulk_email/tests/test_err_handling.py +++ b/lms/djangoapps/bulk_email/tests/test_err_handling.py @@ -20,6 +20,7 @@ from smtplib import SMTPDataError, SMTPServerDisconnected, SMTPConnectError class EmailTestException(Exception): + """Mock exception for email testing.""" pass @@ -165,7 +166,7 @@ class TestEmailErrors(ModuleStoreTestCase): Tests retries when the email doesn't exist """ delegate_email_batches.delay(-1, self.instructor.id) - ((log_str, email_id, num_retries), _) = mock_log.warning.call_args + ((log_str, email_id, _num_retries), _) = mock_log.warning.call_args self.assertTrue(mock_log.warning.called) self.assertIn('Failed to get CourseEmail with id', log_str) self.assertEqual(email_id, -1)