Skip to content

Commit

Permalink
revert: add brand_color variable for the email templates (#33421)"
Browse files Browse the repository at this point in the history
This reverts commit 4ec70eb.

This commit introduced a new setting (`brand_color`) that does not appear to be set and is causing issues with account deletion and other parts of the courseware.

Reverting until we can understand the change better.
  • Loading branch information
justinhynes committed Jan 24, 2024
1 parent 75daee1 commit 426ee16
Show file tree
Hide file tree
Showing 16 changed files with 95 additions and 113 deletions.
19 changes: 11 additions & 8 deletions common/djangoapps/student/tests/test_email.py
Original file line number Diff line number Diff line change
Expand Up @@ -406,9 +406,10 @@ def test_email_success(self):
subject='Request to change édX account e-mail',
body_fragments=[
'We received a request to change the e-mail associated with',
'your édX account from',
old_email,
new_email,
'your édX account from {old_email} to {new_email}.'.format(
old_email=old_email,
new_email=new_email,
),
'If this is correct, please confirm your new e-mail address by visiting:',
f'http://edx.org/email_confirm/{registration_key}',
'Please do not reply to this e-mail; if you require assistance,',
Expand Down Expand Up @@ -466,11 +467,13 @@ def setUp(self):

# Text fragments we expect in the body of the confirmation email
self.email_fragments = [
"This is to confirm that you changed the e-mail associated with ",
str(settings.PLATFORM_NAME),
"If you did not make this request, please contact us immediately. Contact information is listed at:",
self.user.email,
PendingEmailChange.objects.get(activation_key=self.key).new_email,
"This is to confirm that you changed the e-mail associated with {platform_name}"
" from {old_email} to {new_email}. If you did not make this request, please contact us immediately."
" Contact information is listed at:".format(
platform_name=settings.PLATFORM_NAME,
old_email=self.user.email,
new_email=PendingEmailChange.objects.get(activation_key=self.key).new_email
),
"We keep a log of old e-mails, so if this request was unintentional, we can investigate."
]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ <h1>
{% blocktrans trimmed asvar assist_msg %}
If you need help, please use our web form at {start_anchor_web}{{ support_url }}{end_anchor} or email {start_anchor_email}{{ support_email }}{end_anchor}.
{% endblocktrans %}
{% interpolate_html assist_msg start_anchor_web='<a style="color: '|add:brand_color|add:';" href="'|add:support_url|add:'">'|safe start_anchor_email='<a style="color: '|add:brand_color|add:';" href="mailto:'|add:support_email|add:'">'|safe end_anchor='</a>'|safe %}
{% interpolate_html assist_msg start_anchor_web='<a href="'|add:support_url|add:'">'|safe start_anchor_email='<a href="mailto:'|add:support_email|add:'">'|safe end_anchor='</a>'|safe %}
<br />
</p>
</td>
Expand Down
2 changes: 0 additions & 2 deletions common/templates/student/edx_ace/emailchange/email/body.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
{% load i18n %}
{% load static %}
{% block content %}
{% with old_email='<a href="mailto:'|add:old_email|add:'" style="color: '|add:brand_color|add:';">'|add:old_email|add:'</a>'|safe new_email='<a href="mailto:'|add:new_email|add:'" style="color: '|add:brand_color|add:';">'|add:new_email|add:'</a>'|safe %}
<table width="100%" align="left" border="0" cellpadding="0" cellspacing="0" role="presentation">
<tr>
<td>
Expand Down Expand Up @@ -32,5 +31,4 @@ <h1>
</td>
</tr>
</table>
{% endwith %}
{% endblock %}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
{% load i18n %}
{% load static %}
{% block content %}
{% with old_email='<a href="mailto:'|add:old_email|add:'" style="color: '|add:brand_color|add:';">'|add:old_email|add:'</a>'|safe new_email='<a href="mailto:'|add:new_email|add:'" style="color: '|add:brand_color|add:';">'|add:new_email|add:'</a>'|safe %}
<table width="100%" align="left" border="0" cellpadding="0" cellspacing="0" role="presentation">
<tr>
<td>
Expand Down Expand Up @@ -31,5 +30,4 @@ <h1>
</td>
</tr>
</table>
{% endwith %}
{% endblock %}
139 changes: 70 additions & 69 deletions lms/djangoapps/instructor/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -1228,17 +1228,14 @@ def test_enroll_with_email(self, protocol):
assert text_body.startswith('Dear NotEnrolled Student\n\n')

for body in [text_body, html_body]:
assert f'You have been enrolled in {self.course.display_name} at ' in body
assert self.site_name in body
assert ' by a member of the course staff.' in body
assert 'This course will now appear on your ' in body
assert f'You have been enrolled in {self.course.display_name} at edx.org by a member of the course staff.'\
in body

assert 'This course will now appear on your edx.org dashboard.' in body
assert f'{protocol}://{self.site_name}{self.course_path}' in body

assert 'To start accessing course materials, please visit' in text_body
assert (
f'This email was automatically sent from {self.site_name} to {self.notenrolled_student.first_name} '
f'{self.notenrolled_student.last_name}\n\n'
) in text_body
assert 'This email was automatically sent from edx.org to NotEnrolled Student\n\n' in text_body

@ddt.data('http', 'https')
def test_enroll_with_email_not_registered(self, protocol):
Expand Down Expand Up @@ -1267,21 +1264,22 @@ def test_enroll_with_email_not_registered(self, protocol):
assert register_url in html_body

for body in [text_body, html_body]:
assert 'You have been invited to join {course} at '.format(
assert 'You have been invited to join {course} at edx.org by a member of the course staff.'.format(
course=self.course.display_name
) in body
assert self.site_name in body
assert ' by a member of the course staff.' in body
assert 'fill out the registration form making sure to use ' in body
assert self.notregistered_email in body
assert ' in the Email field' in body

assert ('fill out the registration form making sure to use '
'robot-not-an-email-yet@robot.org in the Email field') in body

assert 'Once you have registered and activated your account,' in body

assert '{proto}://{site}{about_path}'.format(
proto=protocol,
site=self.site_name,
about_path=self.about_path
) in body
assert 'This email was automatically sent from ' in body

assert 'This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org' in body

@ddt.data('http', 'https')
@patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True})
Expand All @@ -1304,22 +1302,23 @@ def test_enroll_email_not_registered_mktgsite(self, protocol):
assert 'Please finish your registration and fill' in html_body

for body in [text_body, html_body]:
assert 'You have been invited to join {display_name} at '.format(
assert 'You have been invited to join {display_name} at edx.org by a member of the course staff.'.format(
display_name=self.course.display_name
) in body
assert self.site_name in body
assert 'by a member of the course staff.' in body

assert '{proto}://{site}/register'.format(
proto=protocol,
site=self.site_name
) in body
assert 'fill out the registration form making sure to use ' in body
assert self.notregistered_email in body
assert ' in the Email field' in body

assert ('fill out the registration form making sure to use '
'robot-not-an-email-yet@robot.org in the Email field') in body

assert 'You can then enroll in {display_name}.'.format(
display_name=self.course.display_name
) in body
assert 'This email was automatically sent from ' in body

assert 'This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org' in body

@ddt.data('http', 'https')
def test_enroll_with_email_not_registered_autoenroll(self, protocol):
Expand Down Expand Up @@ -1354,19 +1353,20 @@ def test_enroll_with_email_not_registered_autoenroll(self, protocol):
assert register_url in html_body

for body in [text_body, html_body]:
assert 'You have been invited to join {display_name} at '.format(
assert 'You have been invited to join {display_name} at edx.org by a member of the course staff.'.format(
display_name=self.course.display_name
) in body
assert self.site_name in body
assert 'by a member of the course staff.' in body
assert ' and fill out the registration form making sure to use ' in body
assert self.notregistered_email in body
assert 'in the Email field' in body

assert (' and fill '
'out the registration form making sure to use robot-not-an-email-yet@robot.org '
'in the Email field') in body

assert ('Once you have registered and activated your account, '
'you will see {display_name} listed on your dashboard.').format(
display_name=self.course.display_name
) in body
assert 'This email was automatically sent from ' in body

assert 'This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org' in body

def test_unenroll_without_email(self):
url = reverse('students_update_enrollment', kwargs={'course_id': str(self.course.id)})
Expand Down Expand Up @@ -1461,15 +1461,13 @@ def test_unenroll_with_email(self):
assert text_body.startswith('Dear Enrolled Student')

for body in [text_body, html_body]:
assert 'You have been unenrolled from {display_name} at '.format(
assert 'You have been unenrolled from {display_name} at edx.org by a member of the course staff.'.format(
display_name=self.course.display_name,
) in body
assert self.site_name in body
assert ' by a member of the course staff.' in body
assert 'This course will no longer appear on your ' in body

assert 'This course will no longer appear on your edx.org dashboard.' in body
assert 'Your other courses have not been affected.' in body
assert 'This email was automatically sent from ' in body
assert f'to {self.enrolled_student.first_name} {self.enrolled_student.last_name}' in body
assert 'This email was automatically sent from edx.org to Enrolled Student' in body

def test_unenroll_with_email_allowed_student(self):
url = reverse('students_update_enrollment', kwargs={'course_id': str(self.course.id)})
Expand Down Expand Up @@ -1521,9 +1519,7 @@ def test_unenroll_with_email_allowed_student(self):
) in body

assert 'Please disregard the invitation previously sent.' in body
assert 'This email was automatically sent from ' in body
assert self.site_name in body
assert self.allowed_email in body
assert 'This email was automatically sent from edx.org to robot-allowed@robot.org' in body

@ddt.data('http', 'https')
@patch('lms.djangoapps.instructor.enrollment.uses_shib')
Expand Down Expand Up @@ -1555,13 +1551,11 @@ def test_enroll_with_email_not_registered_with_shib(self, protocol, mock_uses_sh
assert course_url in html_body

for body in [text_body, html_body]:
assert 'You have been invited to join {display_name} at '.format(
assert 'You have been invited to join {display_name} at edx.org by a member of the course staff.'.format(
display_name=self.course.display_name,
) in body
assert self.site_name in body
assert ' by a member of the course staff.' in body
assert 'This email was automatically sent from ' in body
assert self.notregistered_email in body

assert 'This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org' in body

@patch('lms.djangoapps.instructor.enrollment.uses_shib')
@patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True})
Expand All @@ -1582,13 +1576,11 @@ def test_enroll_email_not_registered_shib_mktgsite(self, mock_uses_shib):
assert text_body.startswith('Dear student,')

for body in [text_body, html_body]:
assert 'You have been invited to join {display_name} at '.format(
assert 'You have been invited to join {display_name} at edx.org by a member of the course staff.'.format(
display_name=self.course.display_name,
) in body
assert self.site_name in body
assert ' by a member of the course staff.' in body
assert 'This email was automatically sent from ' in body
assert self.notregistered_email in body

assert 'This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org' in body

@ddt.data('http', 'https')
@patch('lms.djangoapps.instructor.enrollment.uses_shib')
Expand Down Expand Up @@ -1619,13 +1611,11 @@ def test_enroll_with_email_not_registered_with_shib_autoenroll(self, protocol, m
assert 'To access this course click on the button below and login:' in html_body

for body in [text_body, html_body]:
assert 'You have been invited to join {display_name} at '.format(
assert 'You have been invited to join {display_name} at edx.org by a member of the course staff.'.format(
display_name=self.course.display_name,
) in body
assert ' by a member of the course staff.' in body
assert 'This email was automatically sent from ' in body
assert self.site_name in body
assert self.notregistered_email in body

assert 'This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org' in body

def test_enroll_already_enrolled_student(self):
"""
Expand Down Expand Up @@ -2008,19 +1998,22 @@ def test_add_notenrolled_with_email(self, protocol):
assert f'Visit {self.course.display_name}' in html_body

for body in [text_body, html_body]:
assert 'You have been invited to be a beta tester for {display_name} at '.format(
assert 'You have been invited to be a beta tester for {display_name} at edx.org'.format(
display_name=self.course.display_name,
) in body
assert self.site_name in body

assert 'by a member of the course staff.' in body
assert 'enroll in this course and begin the beta test' in body

assert '{proto}://{site}{about_path}'.format(
proto=protocol,
site=self.site_name,
about_path=self.about_path,
) in body
assert 'This email was automatically sent from ' in body
assert self.notenrolled_student.email in body

assert 'This email was automatically sent from edx.org to {student_email}'.format(
student_email=self.notenrolled_student.email,
) in body

@ddt.data('http', 'https')
def test_add_notenrolled_with_email_autoenroll(self, protocol):
Expand Down Expand Up @@ -2057,19 +2050,22 @@ def test_add_notenrolled_with_email_autoenroll(self, protocol):
assert text_body.startswith(f'Dear {student_name}')

for body in [text_body, html_body]:
assert 'You have been invited to be a beta tester for {display_name} at '.format(
assert 'You have been invited to be a beta tester for {display_name} at edx.org'.format(
display_name=self.course.display_name,
) in body
assert self.site_name in body

assert 'by a member of the course staff' in body

assert 'To start accessing course materials, please visit' in body
assert '{proto}://{site}{course_path}'.format(
proto=protocol,
site=self.site_name,
course_path=self.course_path
)
assert 'This email was automatically sent from ' in body
assert self.notenrolled_student.email in body

assert 'This email was automatically sent from edx.org to {student_email}'.format(
student_email=self.notenrolled_student.email,
) in body

@patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True})
def test_add_notenrolled_email_mktgsite(self):
Expand All @@ -2085,14 +2081,16 @@ def test_add_notenrolled_email_mktgsite(self):
assert text_body.startswith(f'Dear {student_name}')

for body in [text_body, html_body]:
assert 'You have been invited to be a beta tester for {display_name} at '.format(
assert 'You have been invited to be a beta tester for {display_name} at edx.org'.format(
display_name=self.course.display_name,
) in body
assert self.site_name in body

assert 'by a member of the course staff.' in body
assert 'Visit edx.org' in body
assert 'enroll in this course and begin the beta test' in body
assert 'This email was automatically sent from ' in body
assert self.notenrolled_student.email in body
assert 'This email was automatically sent from edx.org to {student_email}'.format(
student_email=self.notenrolled_student.email,
) in body

def test_enroll_with_email_not_registered(self):
# User doesn't exist
Expand Down Expand Up @@ -2186,15 +2184,18 @@ def test_remove_with_email(self):
assert text_body.startswith(f'Dear {self.beta_tester.profile.name}')

for body in [text_body, html_body]:
assert 'You have been removed as a beta tester for {display_name} at '.format(
assert 'You have been removed as a beta tester for {display_name} at edx.org'.format(
display_name=self.course.display_name,
) in body
assert self.site_name in body

assert ('This course will remain on your dashboard, but you will no longer be '
'part of the beta testing group.') in body

assert 'Your other courses have not been affected.' in body
assert 'This email was automatically sent from ' in body
assert self.beta_tester.email in body

assert 'This email was automatically sent from edx.org to {email_address}'.format(
email_address=self.beta_tester.email,
) in body


class TestInstructorAPILevelsAccess(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
{% load i18n %}
{% load static %}
{% block content %}
{% with site_name='<a href="'|add:site_name|add:'" style="color: '|add:brand_color|add:';">'|safe|add:site_name|add:'</a>'|safe email_address='<a href="mailto:'|add:email_address|add:'" style="color: '|add:brand_color|add:';">'|safe|add:email_address|add:'</a>'|safe %}
<table width="100%" align="left" border="0" cellpadding="0" cellspacing="0" role="presentation">
<tr>
<td>
Expand Down Expand Up @@ -68,5 +67,4 @@ <h1>
</td>
</tr>
</table>
{% endwith %}
{% endblock %}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
{% load i18n %}
{% load static %}
{% block content %}
{% with site_name='<a href="'|add:site_name|add:'" style="color: '|add:brand_color|add:';">'|safe|add:site_name|add:'</a>'|safe email_address='<a href="mailto:'|add:email_address|add:'" style="color: '|add:brand_color|add:';">'|safe|add:email_address|add:'</a>'|safe %}
<table width="100%" align="left" border="0" cellpadding="0" cellspacing="0" role="presentation">
<tr>
<td>
Expand Down Expand Up @@ -82,5 +81,4 @@ <h1>
</td>
</tr>
</table>
{% endwith %}
{% endblock %}
Loading

0 comments on commit 426ee16

Please sign in to comment.