From 73af914cfc8c82fa7bd0fc8e5fd0c1eb9b81494c Mon Sep 17 00:00:00 2001 From: Omar Al-Ithawi <i@omardo.com> Date: Wed, 18 Apr 2018 22:56:03 +0300 Subject: [PATCH] Use edx-ace for password reset email --- cms/envs/devstack.py | 3 +- common/djangoapps/student/forms.py | 67 +++++++++-------- common/djangoapps/student/message_types.py | 17 +++++ .../tests/test_configuration_overrides.py | 1 + .../student/tests/test_reset_password.py | 72 ++++++++++++------- .../edx_ace/passwordreset/email/body.html | 40 +++++++++++ .../edx_ace/passwordreset/email/body.txt | 7 +- .../edx_ace/passwordreset/email/from_name.txt | 1 + .../edx_ace/passwordreset/email/head.html | 1 + .../edx_ace/passwordreset/email/subject.txt | 4 ++ .../student_account/test/test_views.py | 17 +++-- lms/djangoapps/student_account/views.py | 38 ++++++---- lms/envs/devstack.py | 3 +- .../emails/password_reset_subject.txt | 4 -- openedx/core/djangoapps/ace_common/apps.py | 2 +- .../djangoapps/ace_common/settings/aws.py | 7 ++ .../djangoapps/ace_common/settings/common.py | 4 +- .../ace_common/settings/devstack.py | 19 +++++ .../ace_common/edx_ace/common/base_body.html | 24 +++---- .../commands/tests/send_email_base.py | 20 ++++-- .../core/djangoapps/user_api/accounts/api.py | 4 +- .../user_api/accounts/tests/test_api.py | 17 ++++- requirements/edx/base.in | 2 +- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/testing.txt | 2 +- .../ace_common/edx_ace/common/base_body.html | 24 +++---- 27 files changed, 270 insertions(+), 134 deletions(-) create mode 100644 common/djangoapps/student/message_types.py create mode 100644 common/templates/student/edx_ace/passwordreset/email/body.html rename lms/templates/registration/password_reset_email.html => common/templates/student/edx_ace/passwordreset/email/body.txt (85%) create mode 100644 common/templates/student/edx_ace/passwordreset/email/from_name.txt create mode 100644 common/templates/student/edx_ace/passwordreset/email/head.html create mode 100644 common/templates/student/edx_ace/passwordreset/email/subject.txt delete mode 100644 lms/templates/emails/password_reset_subject.txt create mode 100644 openedx/core/djangoapps/ace_common/settings/devstack.py diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index 8f675cdcf6b..47aa91d9298 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -27,7 +27,8 @@ for pkg_name in ['track.contexts', 'track.middleware', 'dd.dogapi']: ################################ EMAIL ######################################## -EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend' +EMAIL_BACKEND = 'django.core.mail.backends.filebased.EmailBackend' +EMAIL_FILE_PATH = '/edx/src/ace_messages/' ################################# LMS INTEGRATION ############################# diff --git a/common/djangoapps/student/forms.py b/common/djangoapps/student/forms.py index c844dc6899e..a5c9054dcc4 100644 --- a/common/djangoapps/student/forms.py +++ b/common/djangoapps/student/forms.py @@ -10,15 +10,22 @@ from django.contrib.auth.forms import PasswordResetForm from django.contrib.auth.hashers import UNUSABLE_PASSWORD_PREFIX from django.contrib.auth.models import User from django.contrib.auth.tokens import default_token_generator +from django.contrib.sites.models import Site from django.core.exceptions import ValidationError +from django.core.urlresolvers import reverse +from django.core.validators import RegexValidator, slug_re from django.forms import widgets -from django.template import loader from django.utils.http import int_to_base36 from django.utils.translation import ugettext_lazy as _ -from django.core.validators import RegexValidator, slug_re +from edx_ace import ace +from edx_ace.recipient import Recipient +from openedx.core.djangoapps.ace_common.template_context import get_base_template_context +from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.user_api import accounts as accounts_settings +from openedx.core.djangoapps.user_api.preferences.api import get_user_preference +from student.message_types import PasswordReset from student.models import CourseEnrollmentAllowed, email_exists_or_retired from util.password_policy_validators import password_max_length, password_min_length, validate_password @@ -47,41 +54,39 @@ class PasswordResetFormNoActive(PasswordResetForm): raise forms.ValidationError(self.error_messages['unusable']) return email - def save( - self, - subject_template_name='emails/password_reset_subject.txt', - email_template_name='registration/password_reset_email.html', - use_https=False, - token_generator=default_token_generator, - from_email=configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL), - request=None - ): + def save(self, # pylint: disable=arguments-differ + use_https=False, + token_generator=default_token_generator, + request=None, + **_kwargs): """ Generates a one-use only link for resetting password and sends to the user. """ - # This import is here because we are copying and modifying the .save from Django 1.4.5's - # django.contrib.auth.forms.PasswordResetForm directly, which has this import in this place. - from django.core.mail import send_mail for user in self.users_cache: - site_name = configuration_helpers.get_value( - 'SITE_NAME', - settings.SITE_NAME + site = Site.objects.get_current() + message_context = get_base_template_context(site) + + message_context.update({ + 'request': request, # Used by google_analytics_tracking_pixel + # TODO: This overrides `platform_name` from `get_base_template_context` to make the tests passes + 'platform_name': configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME), + 'reset_link': '{protocol}://{site}{link}'.format( + protocol='https' if use_https else 'http', + site=configuration_helpers.get_value('SITE_NAME', settings.SITE_NAME), + link=reverse('password_reset_confirm', kwargs={ + 'uidb36': int_to_base36(user.id), + 'token': token_generator.make_token(user), + }), + ) + }) + + msg = PasswordReset().personalize( + recipient=Recipient(user.username, user.email), + language=get_user_preference(user, LANGUAGE_KEY), + user_context=message_context, ) - context = { - 'email': user.email, - 'site_name': site_name, - 'uid': int_to_base36(user.id), - 'user': user, - 'token': token_generator.make_token(user), - 'protocol': 'https' if use_https else 'http', - 'platform_name': configuration_helpers.get_value('platform_name', settings.PLATFORM_NAME) - } - subject = loader.render_to_string(subject_template_name, context) - # Email subject *must not* contain newlines - subject = subject.replace('\n', '') - email = loader.render_to_string(email_template_name, context) - send_mail(subject, email, from_email, [user.email]) + ace.send(msg) class TrueCheckbox(widgets.CheckboxInput): diff --git a/common/djangoapps/student/message_types.py b/common/djangoapps/student/message_types.py new file mode 100644 index 00000000000..0aac833376f --- /dev/null +++ b/common/djangoapps/student/message_types.py @@ -0,0 +1,17 @@ +""" +ACE message types for the student module. +""" +from django.conf import settings + +from edx_ace.message import MessageType +from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers + + +class PasswordReset(MessageType): + def __init__(self, *args, **kwargs): + super(PasswordReset, self).__init__(*args, **kwargs) + + self.options['transactional'] = True + self.options['from_address'] = configuration_helpers.get_value( + 'email_from_address', settings.DEFAULT_FROM_EMAIL + ) diff --git a/common/djangoapps/student/tests/test_configuration_overrides.py b/common/djangoapps/student/tests/test_configuration_overrides.py index 4b14a6c3407..01fefe3a09c 100644 --- a/common/djangoapps/student/tests/test_configuration_overrides.py +++ b/common/djangoapps/student/tests/test_configuration_overrides.py @@ -15,6 +15,7 @@ FAKE_SITE = { "university": "fakeuniversity", "course_org_filter": "fakeorg", "platform_name": "Fake University", + "PLATFORM_NAME": "Fake University", "email_from_address": "no-reply@fakeuniversity.com", "REGISTRATION_EXTRA_FIELDS": { "address1": "required", diff --git a/common/djangoapps/student/tests/test_reset_password.py b/common/djangoapps/student/tests/test_reset_password.py index 0a6217c8920..1c80cd6a78a 100644 --- a/common/djangoapps/student/tests/test_reset_password.py +++ b/common/djangoapps/student/tests/test_reset_password.py @@ -11,6 +11,7 @@ from django.contrib.auth.hashers import UNUSABLE_PASSWORD_PREFIX from django.contrib.auth.models import User from django.contrib.auth.tokens import default_token_generator from django.core.cache import cache +from django.core import mail from django.core.urlresolvers import reverse from django.test.client import RequestFactory from django.test.utils import override_settings @@ -118,15 +119,12 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): cache.clear() @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', "Test only valid in LMS") - @patch('django.core.mail.send_mail') - @patch('student.views.management.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True)) - def test_reset_password_email(self, send_email): - """ - Tests contents of reset password email, and that user is not active - """ - + @ddt.data('plain_text', 'html') + def test_reset_password_email(self, body_type): + """Tests contents of reset password email, and that user is not active""" good_req = self.request_factory.post('/password_reset/', {'email': self.user.email}) good_req.user = self.user + good_req.site = Mock(domain='example.com') dop_client = ClientFactory() dop_access_token = AccessTokenFactory(user=self.user, client=dop_client) RefreshTokenFactory(user=self.user, client=dop_client, access_token=dop_access_token) @@ -140,26 +138,35 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): self.assertFalse(dot_models.AccessToken.objects.filter(user=self.user).exists()) self.assertFalse(dot_models.RefreshToken.objects.filter(user=self.user).exists()) obj = json.loads(good_resp.content) - self.assertEquals(obj, { - 'success': True, - 'value': "('registration/password_reset_done.html', [])", - }) + self.assertTrue(obj['success']) + self.assertIn('e-mailed you instructions for setting your password', obj['value']) - (subject, msg, from_addr, to_addrs) = send_email.call_args[0] - self.assertIn("Password reset", subject) - self.assertIn("You're receiving this e-mail because you requested a password reset", msg) - self.assertEquals(from_addr, configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL)) - self.assertEquals(len(to_addrs), 1) - self.assertIn(self.user.email, to_addrs) + from_email = configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) + sent_message = mail.outbox[0] + + bodies = { + 'plain_text': sent_message.body, + 'html': sent_message.alternatives[0][0], + } + + body = bodies[body_type] + + self.assertIn("Password reset", sent_message.subject) + self.assertIn("You're receiving this e-mail because you requested a password reset", body) + self.assertEquals(sent_message.from_email, from_email) + self.assertEquals(len(sent_message.to), 1) + self.assertIn(self.user.email, sent_message.to) self.assert_event_emitted( SETTING_CHANGE_INITIATED, user_id=self.user.id, setting=u'password', old=None, new=None, ) - #test that the user is not active + # Test that the user is not active self.user = User.objects.get(pk=self.user.pk) self.assertFalse(self.user.is_active) - re.search(r'password_reset_confirm/(?P<uidb36>[0-9A-Za-z]+)-(?P<token>.+)/', msg).groupdict() + + self.assertIn('password_reset_confirm/', body) + re.search(r'password_reset_confirm/(?P<uidb36>[0-9A-Za-z]+)-(?P<token>.+)/', body).groupdict() @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', "Test only valid in LMS") @patch('django.core.mail.send_mail') @@ -172,6 +179,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): req = self.request_factory.post( '/password_reset/', {'email': self.user.email} ) + req.site = Mock(domain='example.com') req.is_secure = Mock(return_value=is_secure) req.user = self.user password_reset(req) @@ -199,6 +207,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): '/password_reset/', {'email': self.user.email} ) req.user = self.user + req.site = Mock(domain='example.com') password_reset(req) _, msg, _, _ = send_email.call_args[0] @@ -216,8 +225,8 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', "Test only valid in LMS") @patch("openedx.core.djangoapps.site_configuration.helpers.get_value", fake_get_value) - @patch('django.core.mail.send_mail') - def test_reset_password_email_configuration_override(self, send_email): + @ddt.data('plain_text', 'html') + def test_reset_password_email_configuration_override(self, body_type): """ Tests that the right url domain and platform name is included in the reset password email @@ -226,18 +235,28 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): '/password_reset/', {'email': self.user.email} ) req.get_host = Mock(return_value=None) + req.site = Mock(domain='example.com') req.user = self.user - password_reset(req) - _, msg, from_addr, _ = send_email.call_args[0] - reset_msg = "you requested a password reset for your user account at {}".format(fake_get_value('platform_name')) + with patch('crum.get_current_request', return_value=req): + password_reset(req) + + sent_message = mail.outbox[0] + bodies = { + 'plain_text': sent_message.body, + 'html': sent_message.alternatives[0][0], + } + + body = bodies[body_type] + + reset_msg = "you requested a password reset for your user account at {}".format(fake_get_value('PLATFORM_NAME')) - self.assertIn(reset_msg, msg) + self.assertIn(reset_msg, body) self.assert_event_emitted( SETTING_CHANGE_INITIATED, user_id=self.user.id, setting=u'password', old=None, new=None ) - self.assertEqual(from_addr, "no-reply@fakeuniversity.com") + self.assertEqual(sent_message.from_email, "no-reply@fakeuniversity.com") @ddt.data( ('invalidUid', 'invalid_token'), @@ -391,6 +410,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): '/password_reset/', {'email': self.user.email} ) req.user = self.user + req.site = Mock(domain='example.com') password_reset(req) subj, _, _, _ = send_email.call_args[0] diff --git a/common/templates/student/edx_ace/passwordreset/email/body.html b/common/templates/student/edx_ace/passwordreset/email/body.html new file mode 100644 index 00000000000..1b67cf1c63b --- /dev/null +++ b/common/templates/student/edx_ace/passwordreset/email/body.html @@ -0,0 +1,40 @@ +{% extends 'ace_common/edx_ace/common/base_body.html' %} + +{% load i18n %} +{% load static %} +{% block content %} +<table width="100%" align="left" border="0" cellpadding="0" cellspacing="0" role="presentation"> + <tr> + <td> + <h1> + {% trans "Password Reset" %} + </h1> + <p style="color: rgba(0,0,0,.75);"> + {% blocktrans %}You're receiving this e-mail because you requested a password reset for your user account at {{ platform_name }}.{% endblocktrans %} + <br /> + </p> + + {% if failed %} + <p style="color: rgba(0,0,0,.75);"> + {% blocktrans %}However, there is currently no user account associated with your email address: {{ email_address }}.{% endblocktrans %} + <br /> + </p> + + <p style="color: rgba(0,0,0,.75);"> + {% trans "If you did not request this change, you can ignore this email." %} + <br /> + </p> + {% else %} + <p style="color: rgba(0,0,0,.75);"> + {% trans "If you didn't request this change, you can disregard this email - we have not yet reset your password." %} + <br /> + </p> + + {% trans "Change my Password" as course_cta_text %} + + {% include "ace_common/edx_ace/common/return_to_course_cta.html" with course_cta_text=course_cta_text course_cta_url=reset_link %} + {% endif %} + </td> + </tr> +</table> +{% endblock %} diff --git a/lms/templates/registration/password_reset_email.html b/common/templates/student/edx_ace/passwordreset/email/body.txt similarity index 85% rename from lms/templates/registration/password_reset_email.html rename to common/templates/student/edx_ace/passwordreset/email/body.txt index 611863b5b19..2b316582f26 100644 --- a/lms/templates/registration/password_reset_email.html +++ b/common/templates/student/edx_ace/passwordreset/email/body.txt @@ -7,15 +7,12 @@ {% trans "If you did not request this change, you can ignore this email." %} {% else %} {% trans "Please go to the following page and choose a new password:" %} -{% block reset_link %} -{{ protocol }}://{{ site_name }}{% url 'password_reset_confirm' uidb36=uid token=token %} -{% endblock %} + +{{ reset_link }} {% trans "If you didn't request this change, you can disregard this email - we have not yet reset your password." %} {% trans "Thanks for using our site!" %} {% endif %} - {% blocktrans %}The {{ platform_name }} Team{% endblocktrans %} - {% endautoescape %} diff --git a/common/templates/student/edx_ace/passwordreset/email/from_name.txt b/common/templates/student/edx_ace/passwordreset/email/from_name.txt new file mode 100644 index 00000000000..dcbc23c0048 --- /dev/null +++ b/common/templates/student/edx_ace/passwordreset/email/from_name.txt @@ -0,0 +1 @@ +{{ platform_name }} diff --git a/common/templates/student/edx_ace/passwordreset/email/head.html b/common/templates/student/edx_ace/passwordreset/email/head.html new file mode 100644 index 00000000000..366ada7ad92 --- /dev/null +++ b/common/templates/student/edx_ace/passwordreset/email/head.html @@ -0,0 +1 @@ +{% extends 'ace_common/edx_ace/common/base_head.html' %} diff --git a/common/templates/student/edx_ace/passwordreset/email/subject.txt b/common/templates/student/edx_ace/passwordreset/email/subject.txt new file mode 100644 index 00000000000..a568d46de64 --- /dev/null +++ b/common/templates/student/edx_ace/passwordreset/email/subject.txt @@ -0,0 +1,4 @@ +{% load i18n %} +{% autoescape off %} +{% blocktrans trimmed %}Password reset on {{ platform_name }}{% endblocktrans %} +{% endautoescape %} diff --git a/lms/djangoapps/student_account/test/test_views.py b/lms/djangoapps/student_account/test/test_views.py index 778f7cc520f..e5f2b207ec7 100644 --- a/lms/djangoapps/student_account/test/test_views.py +++ b/lms/djangoapps/student_account/test/test_views.py @@ -164,13 +164,18 @@ class StudentAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin): self.assertEqual(len(mail.outbox), 1) # Verify that the body contains the failed password reset message - email_body = mail.outbox[0].body - self.assertIn( - 'However, there is currently no user account associated with your email address: {email}'.format( + sent_message = mail.outbox[0] + text_body = sent_message.body + html_body = sent_message.alternatives[0][0] + + for email_body in [text_body, html_body]: + msg = 'However, there is currently no user account associated with your email address: {email}'.format( email=bad_email - ), - email_body, - ) + ) + + assert u'reset for your user account at {}'.format(settings.PLATFORM_NAME) in email_body + assert 'password_reset_confirm' not in email_body, 'The link should not be added if user was not found' + assert msg in email_body @ddt.data(True, False) def test_password_change_logged_out(self, send_email): diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index 3b99866d5e3..1a9d0e9dfa0 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -9,19 +9,22 @@ from django.conf import settings from django.contrib import messages from django.contrib.auth import get_user_model from django.contrib.auth.decorators import login_required -from django.core.mail import send_mail -from django.core.urlresolvers import resolve, reverse +from django.contrib.sites.models import Site +from django.core.urlresolvers import reverse from django.http import HttpRequest, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden from django.shortcuts import redirect -from django.template import loader from django.utils.translation import ugettext as _ from django.views.decorators.csrf import ensure_csrf_cookie from django.views.decorators.http import require_http_methods from django_countries import countries import third_party_auth + +from edx_ace import ace +from edx_ace.recipient import Recipient from edxmako.shortcuts import render_to_response from lms.djangoapps.commerce.models import CommerceConfiguration from lms.djangoapps.commerce.utils import EcommerceService +from openedx.core.djangoapps.ace_common.template_context import get_base_template_context from openedx.core.djangoapps.commerce.utils import ecommerce_api_client from openedx.core.djangoapps.external_auth.login_and_register import login as external_auth_login from openedx.core.djangoapps.external_auth.login_and_register import register as external_auth_register @@ -48,6 +51,7 @@ from openedx.features.enterprise_support.utils import ( update_account_settings_context_for_enterprise, ) from student.helpers import destroy_oauth_tokens, get_next_url_for_login_page +from student.message_types import PasswordReset from student.models import UserProfile from student.views import register_user as old_register_view, signin_user as old_login_view from third_party_auth import pipeline @@ -55,6 +59,7 @@ from third_party_auth.decorators import xframe_allow_whitelisted from util.bad_request_rate_limiter import BadRequestRateLimiter from util.date_utils import strftime_localized + AUDIT_LOG = logging.getLogger("audit") log = logging.getLogger(__name__) User = get_user_model() # pylint:disable=invalid-name @@ -222,20 +227,23 @@ def password_change_request_handler(request): # no user associated with the email if configuration_helpers.get_value('ENABLE_PASSWORD_RESET_FAILURE_EMAIL', settings.FEATURES['ENABLE_PASSWORD_RESET_FAILURE_EMAIL']): - context = { + + site = Site.objects.get_current() + message_context = get_base_template_context(site) + + message_context.update({ 'failed': True, + 'request': request, # Used by google_analytics_tracking_pixel 'email_address': email, - 'platform_name': configuration_helpers.get_value('platform_name', settings.PLATFORM_NAME), - - } - subject = loader.render_to_string('emails/password_reset_subject.txt', context) - subject = ''.join(subject.splitlines()) - message = loader.render_to_string('registration/password_reset_email.html', context) - from_email = configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) - try: - send_mail(subject, message, from_email, [email]) - except Exception: # pylint: disable=broad-except - log.exception(u'Unable to send password reset failure email notification from "%s"', from_email) + }) + + msg = PasswordReset().personalize( + recipient=Recipient(username='', email_address=email), + language=settings.LANGUAGE_CODE, + user_context=message_context, + ) + + ace.send(msg) except UserAPIInternalError as err: log.exception('Error occured during password change for user {email}: {error}' .format(email=email, error=err)) diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index e4da2000400..7e674a5facd 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -39,7 +39,8 @@ for log_name, log_level in LOG_OVERRIDES: ################################ EMAIL ######################################## -EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend' +EMAIL_BACKEND = 'django.core.mail.backends.filebased.EmailBackend' +EMAIL_FILE_PATH = '/edx/src/ace_messages/' ############################ PYFS XBLOCKS SERVICE ############################# # Set configuration for Django pyfilesystem diff --git a/lms/templates/emails/password_reset_subject.txt b/lms/templates/emails/password_reset_subject.txt deleted file mode 100644 index cf927ce629d..00000000000 --- a/lms/templates/emails/password_reset_subject.txt +++ /dev/null @@ -1,4 +0,0 @@ -{% load i18n %} -{% autoescape off %} -{% blocktrans %}Password reset on {{ platform_name }}{% endblocktrans %} -{% endautoescape %} diff --git a/openedx/core/djangoapps/ace_common/apps.py b/openedx/core/djangoapps/ace_common/apps.py index 68d49deee8d..3c671cc0702 100644 --- a/openedx/core/djangoapps/ace_common/apps.py +++ b/openedx/core/djangoapps/ace_common/apps.py @@ -19,7 +19,7 @@ class AceCommonConfig(AppConfig): ProjectType.LMS: { SettingsType.AWS: {PluginSettings.RELATIVE_PATH: u'settings.aws'}, SettingsType.COMMON: {PluginSettings.RELATIVE_PATH: u'settings.common'}, - SettingsType.DEVSTACK: {PluginSettings.RELATIVE_PATH: u'settings.common'}, + SettingsType.DEVSTACK: {PluginSettings.RELATIVE_PATH: u'settings.devstack'}, } } } diff --git a/openedx/core/djangoapps/ace_common/settings/aws.py b/openedx/core/djangoapps/ace_common/settings/aws.py index 56f149f247c..bd8c5899cef 100644 --- a/openedx/core/djangoapps/ace_common/settings/aws.py +++ b/openedx/core/djangoapps/ace_common/settings/aws.py @@ -14,3 +14,10 @@ def plugin_settings(settings): 'ACE_CHANNEL_SAILTHRU_API_SECRET', settings.ACE_CHANNEL_SAILTHRU_API_SECRET, ) settings.ACE_ROUTING_KEY = settings.ENV_TOKENS.get('ACE_ROUTING_KEY', settings.ACE_ROUTING_KEY) + + settings.ACE_CHANNEL_DEFAULT_EMAIL = settings.ENV_TOKENS.get( + 'ACE_CHANNEL_DEFAULT_EMAIL', settings.ACE_CHANNEL_DEFAULT_EMAIL + ) + settings.ACE_CHANNEL_TRANSACTIONAL_EMAIL = settings.ENV_TOKENS.get( + 'ACE_CHANNEL_TRANSACTIONAL_EMAIL', settings.ACE_CHANNEL_TRANSACTIONAL_EMAIL + ) diff --git a/openedx/core/djangoapps/ace_common/settings/common.py b/openedx/core/djangoapps/ace_common/settings/common.py index c825505c2ae..994034f7b9e 100644 --- a/openedx/core/djangoapps/ace_common/settings/common.py +++ b/openedx/core/djangoapps/ace_common/settings/common.py @@ -1,6 +1,6 @@ def plugin_settings(settings): settings.ACE_ENABLED_CHANNELS = [ - 'file_email' + 'django_email' ] settings.ACE_ENABLED_POLICIES = [ 'bulk_email_optout' @@ -9,6 +9,8 @@ def plugin_settings(settings): settings.ACE_CHANNEL_SAILTHRU_TEMPLATE_NAME = 'Automated Communication Engine Email' settings.ACE_CHANNEL_SAILTHRU_API_KEY = None settings.ACE_CHANNEL_SAILTHRU_API_SECRET = None + settings.ACE_CHANNEL_DEFAULT_EMAIL = 'django_email' + settings.ACE_CHANNEL_TRANSACTIONAL_EMAIL = 'django_email' settings.ACE_ROUTING_KEY = 'edx.core.low' diff --git a/openedx/core/djangoapps/ace_common/settings/devstack.py b/openedx/core/djangoapps/ace_common/settings/devstack.py new file mode 100644 index 00000000000..b94f7f41a57 --- /dev/null +++ b/openedx/core/djangoapps/ace_common/settings/devstack.py @@ -0,0 +1,19 @@ +""" +Settings for edX ACE on devstack. +""" + +from openedx.core.djangoapps.ace_common.settings import common + + +def plugin_settings(settings): + """ + Override common settings and use `file_email` for better debugging. + """ + common.plugin_settings(settings) + + settings.ACE_ENABLED_CHANNELS = [ + 'file_email' + ] + + settings.ACE_CHANNEL_DEFAULT_EMAIL = 'file_email' + settings.ACE_CHANNEL_TRANSACTIONAL_EMAIL = 'file_email' diff --git a/openedx/core/djangoapps/ace_common/templates/ace_common/edx_ace/common/base_body.html b/openedx/core/djangoapps/ace_common/templates/ace_common/edx_ace/common/base_body.html index db47420c9bb..2c5d3efc249 100644 --- a/openedx/core/djangoapps/ace_common/templates/ace_common/edx_ace/common/base_body.html +++ b/openedx/core/djangoapps/ace_common/templates/ace_common/edx_ace/common/base_body.html @@ -20,9 +20,9 @@ {% block preview_text %}{% endblock %} </div> -{# Note {beacon_src} is not a template variable that is evaluated by the Django template engine. It is evaluated by #} -{# Sailthru when the email is sent. Other email providers would need to replace this variable in the HTML as well. #} -<img src="{beacon_src}" alt="" role="presentation" aria-hidden="true" /> +{% for image_src in channel.tracker_image_sources %} + <img src="{image_src}" alt="" role="presentation" aria-hidden="true" /> +{% endfor %} {% google_analytics_tracking_pixel %} @@ -157,17 +157,13 @@ <tr> <!-- Actions --> <td style="padding-bottom: 20px;"> - {# Note that these variables are evaluated by Sailthru, not the Django template engine #} - <p> - <a href="{view_url}" style="color: #005686"> - <font color="#005686"><b>{% trans "View on Web" %}</b></font> - </a> - </p> - <p> - <a href="{optout_confirm_url}" style="color: #005686"> - <font color="#005686"><b>{% trans "Unsubscribe from this list" %}</b></font> - </a> - </p> + {% for action_link_url, action_link_text in channel.action_links %} + <p> + <a href="{{ action_link_url }}" style="color: #005686"> + <font color="#005686"><b>{{ action_link_text }}</b></font> + </a> + </p> + {% endfor %} </td> </tr> <tr> diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py b/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py index b682f149dda..8418bc01f37 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py @@ -7,8 +7,8 @@ import attr import ddt import pytz from django.conf import settings -from edx_ace.channel import ChannelType -from edx_ace.test_utils import StubPolicy, patch_channels, patch_policies +from edx_ace.channel import ChannelMap, ChannelType +from edx_ace.test_utils import StubPolicy, patch_policies from edx_ace.utils.date import serialize from freezegun import freeze_time from mock import Mock, patch @@ -383,11 +383,16 @@ class ScheduleSendEmailTestMixin(FilteredQueryCountMixin): ) patch_policies(self, [StubPolicy([ChannelType.PUSH])]) + mock_channel = Mock( - name='test_channel', - channel_type=ChannelType.EMAIL + channel_type=ChannelType.EMAIL, + action_links=[], + tracker_image_sources=[], ) - patch_channels(self, [mock_channel]) + + channel_map = ChannelMap([ + ['sailthru', mock_channel], + ]) sent_messages = [] with self.settings(TEMPLATES=self._get_template_overrides()): @@ -411,8 +416,9 @@ class ScheduleSendEmailTestMixin(FilteredQueryCountMixin): with self.assertNumQueries(NUM_QUERIES_PER_MESSAGE_DELIVERY): with patch('analytics.track') as mock_analytics_track: - self.deliver_task(*sent_messages[0]) - self.assertEqual(mock_analytics_track.call_count, 1) + with patch('edx_ace.channel.channels', return_value=channel_map): + self.deliver_task(*sent_messages[0]) + self.assertEqual(mock_analytics_track.call_count, 1) self.assertEqual(mock_channel.deliver.call_count, 1) for (_name, (_msg, email), _kwargs) in mock_channel.deliver.mock_calls: diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 33f26d0b42a..ef9ce561773 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -11,6 +11,7 @@ from django.core.exceptions import ObjectDoesNotExist from django.conf import settings from django.core.validators import validate_email, ValidationError from django.http import HttpResponseForbidden +from openedx.core.djangoapps.theming.helpers import get_current_request from six import text_type from student.models import User, UserProfile, Registration, email_exists_or_retired @@ -432,7 +433,8 @@ def request_password_change(email, is_secure): # and email it to the user. form.save( from_email=configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL), - use_https=is_secure + use_https=is_secure, + request=get_current_request(), ) else: # No user with the provided email address exists. diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py index 65f4e68b7b7..6e3c08bf719 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py @@ -19,6 +19,7 @@ from nose.plugins.attrib import attr from nose.tools import raises from six import iteritems +from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory from openedx.core.djangoapps.user_api.accounts import PRIVATE_VISIBILITY, USERNAME_MAX_LENGTH from openedx.core.djangoapps.user_api.accounts.api import ( activate_account, @@ -431,8 +432,13 @@ class AccountCreationActivationAndPasswordChangeTest(TestCase): activation_key = create_account(self.USERNAME, self.PASSWORD, self.EMAIL) activate_account(activation_key) - # Request a password change - request_password_change(self.EMAIL, self.IS_SECURE) + request = RequestFactory().post('/password') + request.user = Mock() + request.site = SiteFactory() + + with patch('crum.get_current_request', return_value=request): + # Request a password change + request_password_change(self.EMAIL, self.IS_SECURE) # Verify that one email message has been sent self.assertEqual(len(mail.outbox), 1) @@ -456,7 +462,12 @@ class AccountCreationActivationAndPasswordChangeTest(TestCase): # Create an account, but do not activate it create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - request_password_change(self.EMAIL, self.IS_SECURE) + request = RequestFactory().post('/password') + request.user = Mock() + request.site = SiteFactory() + + with patch('crum.get_current_request', return_value=request): + request_password_change(self.EMAIL, self.IS_SECURE) # Verify that the activation email was still sent self.assertEqual(len(mail.outbox), 1) diff --git a/requirements/edx/base.in b/requirements/edx/base.in index f2f1573d343..27ed86bf2d8 100644 --- a/requirements/edx/base.in +++ b/requirements/edx/base.in @@ -64,7 +64,7 @@ django-waffle==0.12.0 django-webpack-loader==0.4.1 djangorestframework-jwt dogapi==1.2.1 # Python bindings to Datadog's API, for metrics gathering -edx-ace==0.1.6 +edx-ace==0.1.7 edx-analytics-data-api-client edx-ccx-keys edx-celeryutils diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index b7f03cde807..2e548fd74e9 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -109,7 +109,7 @@ dm.xmlsec.binding==1.3.3 # via python-saml docopt==0.6.2 docutils==0.14 # via botocore dogapi==1.2.1 -edx-ace==0.1.6 +edx-ace==0.1.7 edx-analytics-data-api-client==0.14.4 edx-ccx-keys==0.2.1 edx-celeryutils==0.2.7 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 3c8dff13599..1faaf1364df 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -128,7 +128,7 @@ dm.xmlsec.binding==1.3.3 docopt==0.6.2 docutils==0.14 dogapi==1.2.1 -edx-ace==0.1.6 +edx-ace==0.1.7 edx-analytics-data-api-client==0.14.4 edx-ccx-keys==0.2.1 edx-celeryutils==0.2.7 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index e5adb74a905..45aa613a373 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -123,7 +123,7 @@ dm.xmlsec.binding==1.3.3 docopt==0.6.2 docutils==0.14 dogapi==1.2.1 -edx-ace==0.1.6 +edx-ace==0.1.7 edx-analytics-data-api-client==0.14.4 edx-ccx-keys==0.2.1 edx-celeryutils==0.2.7 diff --git a/themes/red-theme/lms/templates/ace_common/edx_ace/common/base_body.html b/themes/red-theme/lms/templates/ace_common/edx_ace/common/base_body.html index f89bf4aad6e..548fa6dc878 100644 --- a/themes/red-theme/lms/templates/ace_common/edx_ace/common/base_body.html +++ b/themes/red-theme/lms/templates/ace_common/edx_ace/common/base_body.html @@ -22,9 +22,9 @@ <!-- TEST RED THEME MARKER: Do not remove this comment, it is used by the tests to tell if this theme was used --> -{# Note {beacon_src} is not a template variable that is evaluated by the Django template engine. It is evaluated by #} -{# Sailthru when the email is sent. Other email providers would need to replace this variable in the HTML as well. #} -<img src="{beacon_src}" alt="" role="presentation" aria-hidden="true" /> +{% for image_src in channel.tracker_image_sources %} + <img src="{image_src}" alt="" role="presentation" aria-hidden="true" /> +{% endfor %} {% google_analytics_tracking_pixel %} @@ -159,17 +159,13 @@ <tr> <!-- Actions --> <td style="padding-bottom: 20px;"> - {# Note that these variables are evaluated by Sailthru, not the Django template engine #} - <p> - <a href="{view_url}" style="color: #960909"> - <font color="#960909"><b>{% trans "View on Web" %}</b></font> - </a> - </p> - <p> - <a href="{optout_confirm_url}" style="color: #960909"> - <font color="#960909"><b>{% trans "Unsubscribe from this list" %}</b></font> - </a> - </p> + {% for action_link_url, action_link_text in channel.action_links %} + <p> + <a href="{{ action_link_url }}" style="color: #960909"> + <font color="#960909"><b>{{ action_link_text }}</b></font> + </a> + </p> + {% endfor %} </td> </tr> <tr> -- GitLab