Skip to content

Commit

Permalink
Update from_addr for default from bulk emails (openedx#29001)
Browse files Browse the repository at this point in the history
* fix: update from_addr for default from bulk emails

Co-authored-by: Kevin Valencia <kevin@bitmaker.la>
  • Loading branch information
Kevin Valencia and KevinBitmaker authored Dec 6, 2021
1 parent 87c2a59 commit fa258de
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 13 deletions.
8 changes: 4 additions & 4 deletions lms/djangoapps/bulk_email/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
from common.djangoapps.util.string_utils import _has_non_ascii_characters
from lms.djangoapps.branding.api import get_logo_url_for_email
from lms.djangoapps.bulk_email.api import get_unsubscribed_link
from lms.djangoapps.bulk_email.toggles import is_email_use_default_from_bulk_enabled
from lms.djangoapps.bulk_email.toggles import is_email_use_course_id_from_for_bulk_enabled
from lms.djangoapps.bulk_email.models import CourseEmail, Optout
from lms.djangoapps.courseware.courses import get_course
from lms.djangoapps.instructor_task.models import InstructorTask
Expand Down Expand Up @@ -495,10 +495,10 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
course_title = global_email_context['course_title']
course_language = global_email_context['course_language']

# If EMAIL_USE_DEFAULT_FROM_FOR_BULK is True, use the default email from address.
# If EMAIL_USE_COURSE_ID_FROM_FOR_BULK is False, use the default email from address.
# Otherwise compute a custom from address
if is_email_use_default_from_bulk_enabled():
from_addr = settings.DEFAULT_FROM_EMAIL
if not is_email_use_course_id_from_for_bulk_enabled():
from_addr = settings.BULK_EMAIL_DEFAULT_FROM_EMAIL or settings.DEFAULT_FROM_EMAIL
else:
# use the email from address in the CourseEmail, if it is present, otherwise compute it.
from_addr = course_email.from_addr or _get_source_address(course_email.course_id, course_title, course_language)
Expand Down
11 changes: 6 additions & 5 deletions lms/djangoapps/bulk_email/tests/test_email.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ class LocalizedFromAddressPlatformLangTestCase(SendEmailWithMockedUgettextMixin,
"""
Tests to ensure that the bulk email has the "From" address localized according to LANGUAGE_CODE.
"""
@override_settings(LANGUAGE_CODE='en')
@override_settings(LANGUAGE_CODE='en', EMAIL_USE_COURSE_ID_FROM_FOR_BULK=True)
def test_english_platform(self):
"""
Ensures that the source-code language (English) works well.
Expand All @@ -186,7 +186,7 @@ def test_english_platform(self):
message = self.send_email()
self.assertRegex(message.from_email, '.*Course Staff.*')

@override_settings(LANGUAGE_CODE='eo')
@override_settings(LANGUAGE_CODE='eo', EMAIL_USE_COURSE_ID_FROM_FOR_BULK=True)
def test_esperanto_platform(self):
"""
Tests the fake Esperanto language to ensure proper gettext calls.
Expand Down Expand Up @@ -220,7 +220,7 @@ def setUpClass(cls):
default_store=ModuleStoreEnum.Type.split
)

@override_settings(LANGUAGE_CODE='eo')
@override_settings(LANGUAGE_CODE='eo', EMAIL_USE_COURSE_ID_FROM_FOR_BULK=True)
def test_esperanto_platform_arabic_course(self):
"""
The course language should override the platform's.
Expand Down Expand Up @@ -249,6 +249,7 @@ def test_email_disabled(self):
# We should get back a HttpResponseForbidden (status code 403)
self.assertContains(response, "Email is not enabled for this course.", status_code=403)

@override_settings(EMAIL_USE_COURSE_ID_FROM_FOR_BULK=True)
@patch('lms.djangoapps.bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message', autospec=True)) # lint-amnesty, pylint: disable=line-too-long
def test_send_to_self(self):
"""
Expand Down Expand Up @@ -288,7 +289,7 @@ def test_send_to_staff(self):
assert len(mail.outbox) == (1 + len(self.staff))
assert len([e.to[0] for e in mail.outbox]) == len([self.instructor.email] + [s.email for s in self.staff])

@override_settings(DEFAULT_FROM_EMAIL='test@example.com', EMAIL_USE_DEFAULT_FROM_FOR_BULK=True)
@override_settings(DEFAULT_FROM_EMAIL='test@example.com', BULK_EMAIL_DEFAULT_FROM_EMAIL=None, EMAIL_USE_COURSE_ID_FROM_FOR_BULK=False) # lint-amnesty, pylint: disable=line-too-long
def test_email_from_address(self):
"""
Make sure the from_address should be the DEFAULT_FROM_EMAIL when corresponding flag is enabled.
Expand Down Expand Up @@ -498,7 +499,7 @@ def test_unicode_students_send_to_all(self):
assert len([e.to[0] for e in mail.outbox]) ==\
len([self.instructor.email] + [s.email for s in self.staff] + [s.email for s in self.students])

@override_settings(BULK_EMAIL_DEFAULT_FROM_EMAIL="no-reply@courseupdates.edx.org")
@override_settings(BULK_EMAIL_DEFAULT_FROM_EMAIL="no-reply@courseupdates.edx.org", EMAIL_USE_COURSE_ID_FROM_FOR_BULK=True) # lint-amnesty, pylint: disable=line-too-long
def test_long_course_display_name(self):
"""
This test tests that courses with exorbitantly large display names
Expand Down
8 changes: 4 additions & 4 deletions lms/djangoapps/bulk_email/toggles.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
from edx_toggles.toggles import SettingToggle


# .. toggle_name: bulk_email.EMAIL_USE_DEFAULT_FROM_FOR_BULK
# .. toggle_name: bulk_email.EMAIL_USE_COURSE_ID_FROM_FOR_BULK
# .. toggle_implementation: DjangoSetting
# .. toggle_default: False
# .. toggle_description: If True, use the same DEFAULT_FROM_EMAIL as the from_addr for all bulk email, to avoid issues with spam filtering
# .. toggle_description: If False, use the same BULK_EMAIL_DEFAULT_FROM_EMAIL or DEFAULT_FROM_EMAIL as the from_addr for all bulk email, to avoid issues with spam filtering
# .. toggle_use_cases: open_edx
# .. toggle_creation_date: 2020-10-01
# .. toggle_tickets: OSPR-4957


def is_email_use_default_from_bulk_enabled():
return SettingToggle("EMAIL_USE_DEFAULT_FROM_FOR_BULK", default=False).is_enabled()
def is_email_use_course_id_from_for_bulk_enabled():
return SettingToggle("EMAIL_USE_COURSE_ID_FROM_FOR_BULK", default=False).is_enabled()

0 comments on commit fa258de

Please sign in to comment.