Merge pull request #18849 from edx/aj/fix-bulk-email

Skip sending email if an email address contains non-ascii char.

lms/djangoapps/bulk_email/tasks.py

@@ -23,6 +23,8 @@ from boto.ses.exceptions import (
     SESLocalAddressCharacterError,
     SESMaxSendingRateExceededError
 )
+from util.string_utils import _has_non_ascii_characters
+
 from celery import current_task, task
 from celery.exceptions import RetryTaskError
 from celery.states import FAILURE, RETRY, SUCCESS
@@ -512,6 +514,19 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
             recipient_num += 1
             current_recipient = to_list[-1]
             email = current_recipient['email']
+            if _has_non_ascii_characters(email):
+                to_list.pop()
+                total_recipients_failed += 1
+                log.info(
+                    "BulkEmail ==> Email address %s contains non-ascii characters. Skipping sending "
+                    "email to %s, EmailId: %s ",
+                    email,
+                    current_recipient['profile__name'],
+                    email_id
+                )
+                subtask_status.increment(failed=1)
+                continue
+
             email_context['email'] = email
             email_context['name'] = current_recipient['profile__name']
             email_context['user_id'] = current_recipient['pk']

lms/djangoapps/bulk_email/tests/test_tasks.py

@@ -278,6 +278,34 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
         # Test that celery handles permanent SMTPDataErrors by failing and not retrying.
         self._test_email_address_failures(SESDomainEndsWithDotError(554, "Email address ends with a dot"))
 
+    def test_bulk_email_skip_with_non_ascii_emails(self):
+        """
+        Tests that bulk email skips the email address containing non-ASCII characters
+        and does not fail.
+        """
+        num_emails = 10
+        emails_with_non_ascii_chars = 3
+        num_of_course_instructors = 1
+
+        students = [self.create_student('robot%d' % i) for i in range(num_emails)]
+        for student in students[:emails_with_non_ascii_chars]:
+            student.email = '{username}@tesá.com'.format(username=student.username)
+            student.save()
+
+        total = num_emails + num_of_course_instructors
+        expected_succeeds = num_emails - emails_with_non_ascii_chars + num_of_course_instructors
+        expected_fails = emails_with_non_ascii_chars
+
+        with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn:
+            get_conn.return_value.send_messages.side_effect = cycle([None])
+            self._test_run_with_task(
+                task_class=send_bulk_course_email,
+                action_name='emailed',
+                total=total,
+                succeeded=expected_succeeds,
+                failed=expected_fails
+            )
+
     def _test_retry_after_limited_retry_error(self, exception):
         """Test that celery handles connection failures by retrying."""
         # If we want the batch to succeed, we need to send fewer emails