Merge pull request #855 from edx/sarina/bulkemail-violations

Cleanup bulk email app

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, {

lms/djangoapps/bulk_email/forms.py

+"""
+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):

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
 
 

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):

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)