diff --git a/cms/templates/emails/activation_email.txt b/cms/templates/emails/activation_email.txt new file mode 100644 index 0000000000..7a54fc6b35 --- /dev/null +++ b/cms/templates/emails/activation_email.txt @@ -0,0 +1,11 @@ +<%! from django.utils.translation import ugettext as _ %> + +${_("Thank you for signing up for {studio_name}! To activate your account, please copy and paste this address into your web browser's address bar:").format(studio_name=settings.STUDIO_NAME)} + +% if is_secure: + https://${ site }/activate/${ key } +% else: + http://${ site }/activate/${ key } +% endif + +${_("If you didn't request this, you don't need to do anything; you won't receive any more email from us. Please do not reply to this e-mail; if you require assistance, check the help section of the {studio_name} web site.").format(studio_name=settings.STUDIO_NAME)} diff --git a/cms/templates/emails/activation_email_subject.txt b/cms/templates/emails/activation_email_subject.txt new file mode 100644 index 0000000000..d638865710 --- /dev/null +++ b/cms/templates/emails/activation_email_subject.txt @@ -0,0 +1,2 @@ +<%! from django.utils.translation import ugettext as _ %> +${_("Your account for {studio_name}").format(studio_name=settings.STUDIO_NAME)} diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index 5ecf3ca871..8115bfc540 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -382,9 +382,7 @@ def generate_activation_email_context(user, registration): 'key': registration.activation_key, 'lms_url': configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL), 'platform_name': configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME), - 'support_url': configuration_helpers.get_value( - 'ACTIVATION_EMAIL_SUPPORT_LINK', settings.ACTIVATION_EMAIL_SUPPORT_LINK - ) or settings.SUPPORT_SITE_LINK, + 'support_url': configuration_helpers.get_value('SUPPORT_SITE_LINK', settings.SUPPORT_SITE_LINK), 'support_email': configuration_helpers.get_value('CONTACT_EMAIL', settings.CONTACT_EMAIL), } diff --git a/common/djangoapps/student/message_types.py b/common/djangoapps/student/message_types.py index 0942e6f469..259bae58e5 100644 --- a/common/djangoapps/student/message_types.py +++ b/common/djangoapps/student/message_types.py @@ -38,10 +38,3 @@ class RecoveryEmailCreate(BaseMessageType): super(RecoveryEmailCreate, self).__init__(*args, **kwargs) self.options['transactional'] = True - - -class AccountActivation(BaseMessageType): - def __init__(self, *args, **kwargs): - super(AccountActivation, self).__init__(*args, **kwargs) - - self.options['transactional'] = True diff --git a/common/djangoapps/student/tasks.py b/common/djangoapps/student/tasks.py index 115fad9073..23b9481c30 100644 --- a/common/djangoapps/student/tasks.py +++ b/common/djangoapps/student/tasks.py @@ -3,40 +3,30 @@ This file contains celery tasks for sending email """ import logging +from boto.exception import NoAuthHandlerFound from celery.exceptions import MaxRetriesExceededError from celery.task import task # pylint: disable=no-name-in-module, import-error from django.conf import settings -from edx_ace import ace -from edx_ace.errors import RecoverableChannelDeliveryError -from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers +from django.core import mail log = logging.getLogger('edx.celery.task') @task(bind=True) -def send_activation_email(self, msg, from_address=None): +def send_activation_email(self, subject, message, from_address, dest_addr): """ Sending an activation email to the user. """ max_retries = settings.RETRY_ACTIVATION_EMAIL_MAX_ATTEMPTS retries = self.request.retries - - if from_address is None: - from_address = configuration_helpers.get_value('ACTIVATION_EMAIL_FROM_ADDRESS') or ( - configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) - ) - msg.options['from_address'] = from_address - - dest_addr = msg.recipient.email_address - try: - ace.send(msg) + mail.send_mail(subject, message, from_address, [dest_addr], fail_silently=False) # Log that the Activation Email has been sent to user without an exception log.info("Activation Email has been sent to User {user_email}".format( user_email=dest_addr )) - except RecoverableChannelDeliveryError: - log.info('Retrying sending email to user {dest_addr}, attempt # {attempt} of {max_attempts}'.format( + except NoAuthHandlerFound: # pylint: disable=broad-except + log.info('Retrying sending email to user {dest_addr}, attempt # {attempt} of {max_attempts}'. format( dest_addr=dest_addr, attempt=retries, max_attempts=max_retries diff --git a/common/djangoapps/student/tests/test_email.py b/common/djangoapps/student/tests/test_email.py index 3c2ea789b8..1baa910361 100644 --- a/common/djangoapps/student/tests/test_email.py +++ b/common/djangoapps/student/tests/test_email.py @@ -26,13 +26,14 @@ from student.models import ( UserProfile, ) from openedx.core.lib.request_utils import safe_get_host -from student.tests.factories import PendingEmailChangeFactory, UserFactory +from student.tests.factories import PendingEmailChangeFactory, RegistrationFactory, UserFactory from student.views import ( SETTING_CHANGE_INITIATED, confirm_email_change, do_email_change_request, validate_new_email ) +from student.views import generate_activation_email_context, send_reactivation_email_for_user from third_party_auth.views import inactive_user_view from util.testing import EventTestMixin @@ -90,9 +91,8 @@ class EmailTestMixin(object): self.addCleanup(settings.ALLOWED_HOSTS.pop) -@ddt.ddt @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') -class ActivationEmailTests(EmailTemplateTagMixin, CacheIsolationTestCase): +class ActivationEmailTests(CacheIsolationTestCase): """ Test sending of the activation email. """ @@ -102,36 +102,22 @@ class ActivationEmailTests(EmailTemplateTagMixin, CacheIsolationTestCase): # Text fragments we expect in the body of an email # sent from an OpenEdX installation. OPENEDX_FRAGMENTS = [ + u"high-quality {platform} courses".format(platform=settings.PLATFORM_NAME), + "http://edx.org/activate/", ( - u"You're almost there! Use the link below to activate your account to access engaging, " - u"high-quality {platform_name} courses. Note that you will not be able to log back into your " - u"account until you have activated it.".format( - platform_name=settings.PLATFORM_NAME - ) - ), - u"{}/activate/".format(settings.LMS_ROOT_URL), - u"If you need help, please use our web form at ", ( - settings.ACTIVATION_EMAIL_SUPPORT_LINK or settings.SUPPORT_SITE_LINK - ), - settings.CONTACT_EMAIL, - u"This email message was automatically sent by ", - settings.LMS_ROOT_URL, - u" because someone attempted to create an account on {platform_name}".format( - platform_name=settings.PLATFORM_NAME - ), - u" using this email address." + u"please use our web form at " + u"{support_url} ".format(support_url=settings.SUPPORT_SITE_LINK) + ) ] - @ddt.data('plain_text', 'html') - def test_activation_email(self, test_body_type): + def test_activation_email(self): self._create_account() - self._assert_activation_email(self.ACTIVATION_SUBJECT, self.OPENEDX_FRAGMENTS, test_body_type) + self._assert_activation_email(self.ACTIVATION_SUBJECT, self.OPENEDX_FRAGMENTS) @with_comprehensive_theme("edx.org") - @ddt.data('plain_text', 'html') - def test_activation_email_edx_domain(self, test_body_type): + def test_activation_email_edx_domain(self): self._create_account() - self._assert_activation_email(self.ACTIVATION_SUBJECT, self.OPENEDX_FRAGMENTS, test_body_type) + self._assert_activation_email(self.ACTIVATION_SUBJECT, self.OPENEDX_FRAGMENTS) def _create_account(self): """ @@ -155,23 +141,15 @@ class ActivationEmailTests(EmailTemplateTagMixin, CacheIsolationTestCase): ) ) - def _assert_activation_email(self, subject, body_fragments, test_body_type): + def _assert_activation_email(self, subject, body_fragments): """ Verify that the activation email was sent. """ self.assertEqual(len(mail.outbox), 1) msg = mail.outbox[0] self.assertEqual(msg.subject, subject) - - body_text = { - 'plain_text': msg.body, - 'html': msg.alternatives[0][0] - } - assert test_body_type in body_text - body_to_be_tested = body_text[test_body_type] - for fragment in body_fragments: - self.assertIn(fragment, body_to_be_tested) + self.assertIn(fragment, msg.body) def test_do_not_send_email_and_do_activate(self): """ @@ -219,6 +197,85 @@ class ActivationEmailTests(EmailTemplateTagMixin, CacheIsolationTestCase): ) +@patch('student.views.management.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True)) +@patch('django.contrib.auth.models.User.email_user') +class ReactivationEmailTests(EmailTestMixin, CacheIsolationTestCase): + """ + Test sending a reactivation email to a user + """ + + def setUp(self): + super(ReactivationEmailTests, self).setUp() + self.user = UserFactory.create() + self.unregisteredUser = UserFactory.create() + self.registration = RegistrationFactory.create(user=self.user) + + def reactivation_email(self, user): + """ + Send the reactivation email to the specified user, + and return the response as json data. + """ + return json.loads(send_reactivation_email_for_user(user).content) + + def assertReactivateEmailSent(self, email_user): + """ + Assert that the correct reactivation email has been sent + """ + context = generate_activation_email_context(self.user, self.registration) + + self.assertEmailUser( + email_user, + 'emails/activation_email_subject.txt', + context, + 'emails/activation_email.txt', + context + ) + + # Thorough tests for safe_get_host are elsewhere; here we just want a quick URL sanity check + request = RequestFactory().post('unused_url') + request.user = self.user + request.META['HTTP_HOST'] = "aGenericValidHostName" + self.append_allowed_hosts("aGenericValidHostName") + + with patch('edxmako.request_context.get_current_request', return_value=request): + body = render_to_string('emails/activation_email.txt', context) + host = safe_get_host(request) + + self.assertIn(host, body) + + def test_reactivation_email_failure(self, email_user): + self.user.email_user.side_effect = Exception + response_data = self.reactivation_email(self.user) + + self.assertReactivateEmailSent(email_user) + self.assertFalse(response_data['success']) + + def test_reactivation_for_unregistered_user(self, email_user): # pylint: disable=unused-argument + """ + Test that trying to send a reactivation email to an unregistered + user fails without throwing a 500 error. + """ + response_data = self.reactivation_email(self.unregisteredUser) + + self.assertFalse(response_data['success']) + + def test_reactivation_for_no_user_profile(self, email_user): # pylint: disable=unused-argument + """ + Test that trying to send a reactivation email to a user without + user profile fails without throwing 500 error. + """ + user = UserFactory.build(username='test_user', email='test_user@test.com') + user.save() + response_data = self.reactivation_email(user) + self.assertFalse(response_data['success']) + + def test_reactivation_email_success(self, email_user): + response_data = self.reactivation_email(self.user) + + self.assertReactivateEmailSent(email_user) + self.assertTrue(response_data['success']) + + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', "Test only valid in LMS") class EmailChangeRequestTests(EventTestMixin, EmailTemplateTagMixin, CacheIsolationTestCase): """ diff --git a/common/djangoapps/student/tests/test_tasks.py b/common/djangoapps/student/tests/test_tasks.py index 460d29ba08..d75af35a2f 100644 --- a/common/djangoapps/student/tests/test_tasks.py +++ b/common/djangoapps/student/tests/test_tasks.py @@ -3,13 +3,12 @@ Tests for the Sending activation email celery tasks """ import mock +from boto.exception import NoAuthHandlerFound from django.conf import settings from django.test import TestCase -from edx_ace.errors import ChannelError, RecoverableChannelDeliveryError + from lms.djangoapps.courseware.tests.factories import UserFactory -from student.models import Registration from student.tasks import send_activation_email -from student.views.management import compose_activation_email class SendActivationEmailTestCase(TestCase): @@ -21,32 +20,18 @@ class SendActivationEmailTestCase(TestCase): super(SendActivationEmailTestCase, self).setUp() self.student = UserFactory() - registration = Registration() - registration.register(self.student) - - self.msg = compose_activation_email("http://www.example.com", self.student, registration) - - def test_ComposeEmail(self): - """ - Tests that attributes of the message are being filled correctly in compose_activation_email - """ - self.assertEquals(self.msg.recipient.username, self.student.username) - self.assertEquals(self.msg.recipient.email_address, self.student.email) - self.assertEquals(self.msg.context['routed_user'], self.student.username) - self.assertEquals(self.msg.context['routed_user_email'], self.student.email) - self.assertEquals(self.msg.context['routed_profile_name'], '') - @mock.patch('time.sleep', mock.Mock(return_value=None)) @mock.patch('student.tasks.log') - @mock.patch('student.tasks.ace.send', mock.Mock(side_effect=RecoverableChannelDeliveryError(None, None))) - def test_RetrySendUntilFail(self, mock_log): + @mock.patch('django.core.mail.send_mail', mock.Mock(side_effect=NoAuthHandlerFound)) + def test_send_email(self, mock_log): """ Tests retries when the activation email doesn't send """ from_address = 'task_testing@example.com' email_max_attempts = settings.RETRY_ACTIVATION_EMAIL_MAX_ATTEMPTS - send_activation_email.delay(self.msg, from_address=from_address) + # pylint: disable=no-member + send_activation_email.delay('Task_test', 'Task_test_message', from_address, self.student.email) # Asserts sending email retry logging. for attempt in range(email_max_attempts): @@ -66,26 +51,3 @@ class SendActivationEmailTestCase(TestCase): exc_info=True ) self.assertEquals(mock_log.error.call_count, 1) - - @mock.patch('student.tasks.log') - @mock.patch('student.tasks.ace.send', mock.Mock(side_effect=ChannelError)) - def test_UnrecoverableSendError(self, mock_log): - """ - Tests that a major failure of the send is logged - """ - from_address = 'task_testing@example.com' - - send_activation_email.delay(self.msg, from_address=from_address) - - # Asserts that the error was logged - mock_log.exception.assert_called_with( - 'Unable to send activation email to user from "%s" to "%s"', - from_address, - self.student.email, - exc_info=True - ) - - # Assert that nothing else was logged - self.assertEquals(mock_log.info.call_count, 0) - self.assertEquals(mock_log.error.call_count, 0) - self.assertEquals(mock_log.exception.call_count, 1) diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index a600885dfc..9286b49ecf 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -67,13 +67,7 @@ from openedx.core.djangolib.markup import HTML, Text from openedx.features.journals.api import get_journals_context from student.forms import AccountCreationForm, PasswordResetFormNoActive, get_registration_extension_form from student.helpers import DISABLE_UNENROLL_CERT_STATES, cert_info, generate_activation_email_context -from student.message_types import ( - AccountActivation, - EmailChange, - EmailChangeConfirmation, - PasswordReset, - RecoveryEmailCreate -) +from student.message_types import EmailChange, EmailChangeConfirmation, PasswordReset, RecoveryEmailCreate from student.models import ( AccountRecovery, CourseEnrollment, @@ -188,40 +182,6 @@ def index(request, extra_context=None, user=AnonymousUser()): return render_to_response('index.html', context) -def compose_activation_email(root_url, user, user_registration=None, route_enabled=False, profile_name=''): - """ - Construct all the required params for the activation email - through celery task - """ - if user_registration is None: - user_registration = Registration.objects.get(user=user) - - message_context = generate_activation_email_context(user, user_registration) - message_context.update({ - 'confirm_activation_link': '{root_url}/activate/{activation_key}'.format( - root_url=root_url, - activation_key=message_context['key'] - ), - 'route_enabled': route_enabled, - 'routed_user': user.username, - 'routed_user_email': user.email, - 'routed_profile_name': profile_name, - }) - - if route_enabled: - dest_addr = settings.FEATURES['REROUTE_ACTIVATION_EMAIL'] - else: - dest_addr = user.email - - msg = AccountActivation().personalize( - recipient=Recipient(user.username, dest_addr), - language=preferences_api.get_user_preference(user, LANGUAGE_KEY), - user_context=message_context, - ) - - return msg - - def compose_and_send_activation_email(user, profile, user_registration=None): """ Construct all the required params and send the activation email @@ -232,12 +192,66 @@ def compose_and_send_activation_email(user, profile, user_registration=None): profile: profile object of the current logged-in user user_registration: registration of the current logged-in user """ - route_enabled = settings.FEATURES.get('REROUTE_ACTIVATION_EMAIL') + dest_addr = user.email + if user_registration is None: + user_registration = Registration.objects.get(user=user) + context = generate_activation_email_context(user, user_registration) + subject = render_to_string('emails/activation_email_subject.txt', context) + # Email subject *must not* contain newlines + subject = ''.join(subject.splitlines()) + message_for_activation = render_to_string('emails/activation_email.txt', context) + from_address = configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) + from_address = configuration_helpers.get_value('ACTIVATION_EMAIL_FROM_ADDRESS', from_address) + if settings.FEATURES.get('REROUTE_ACTIVATION_EMAIL'): + dest_addr = settings.FEATURES['REROUTE_ACTIVATION_EMAIL'] + message_for_activation = ("Activation for %s (%s): %s\n" % (user, user.email, profile.name) + + '-' * 80 + '\n\n' + message_for_activation) + send_activation_email.delay(subject, message_for_activation, from_address, dest_addr) - root_url = configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL) - msg = compose_activation_email(root_url, user, user_registration, route_enabled, profile.name) - send_activation_email.delay(msg) +def send_reactivation_email_for_user(user): + try: + registration = Registration.objects.get(user=user) + except Registration.DoesNotExist: + return JsonResponse({ + "success": False, + "error": _('No inactive user with this e-mail exists'), + }) + + try: + context = generate_activation_email_context(user, registration) + except ObjectDoesNotExist: + log.error( + u'Unable to send reactivation email due to unavailable profile for the user "%s"', + user.username, + exc_info=True + ) + return JsonResponse({ + "success": False, + "error": _('Unable to send reactivation email') + }) + + subject = render_to_string('emails/activation_email_subject.txt', context) + subject = ''.join(subject.splitlines()) + message = render_to_string('emails/activation_email.txt', context) + from_address = configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) + from_address = configuration_helpers.get_value('ACTIVATION_EMAIL_FROM_ADDRESS', from_address) + + try: + user.email_user(subject, message, from_address) + except Exception: # pylint: disable=broad-except + log.error( + u'Unable to send reactivation email from "%s" to "%s"', + from_address, + user.email, + exc_info=True + ) + return JsonResponse({ + "success": False, + "error": _('Unable to send reactivation email') + }) + + return JsonResponse({"success": True}) @login_required @@ -541,9 +555,7 @@ def activate_account(request, key): '{html_start}Your account could not be activated{html_end}' 'Something went wrong, please contact support to resolve this issue.' )).format( - support_url=configuration_helpers.get_value( - 'ACTIVATION_EMAIL_SUPPORT_LINK', settings.ACTIVATION_EMAIL_SUPPORT_LINK - ) or settings.SUPPORT_SITE_LINK, + support_url=configuration_helpers.get_value('SUPPORT_SITE_LINK', settings.SUPPORT_SITE_LINK), html_start=HTML('
'), ), diff --git a/common/templates/student/edx_ace/accountactivation/email/body.html b/common/templates/student/edx_ace/accountactivation/email/body.html deleted file mode 100644 index d4497672da..0000000000 --- a/common/templates/student/edx_ace/accountactivation/email/body.html +++ /dev/null @@ -1,57 +0,0 @@ -{% extends 'ace_common/edx_ace/common/base_body.html' %} - -{% load i18n %} -{% load static %} -{% block content %} -|
-
- {% blocktrans %}This is a routed Account Activation email for {{ routed_user }} ({{ routed_user_email }}): {{ routed_profile_name }}{% endblocktrans %}
- |
-
- - {% trans "Account Activation" %} --
- {% blocktrans %}You're almost there! Use the link below to activate your account to access engaging, high-quality {{ platform_name }} courses. Note that you will not be able to log back into your account until you have activated it.{% endblocktrans %}
- |
-
|
-
- {% blocktrans %}Enjoy learning with {{ platform_name }}.{% endblocktrans %}
- |
-
-
- {% blocktrans %}If you need help, please use our web form at {{ support_url }} or email {{ support_email }}.{% endblocktrans %}
- |
-
|
-
- {% blocktrans %}This email message was automatically sent by {{ lms_url }} because someone attempted to create an account on {{ platform_name }} using this email address.{% endblocktrans %}
- |
-