From b74e964d7c13d0ae532d67bce38433dccdb6b3a7 Mon Sep 17 00:00:00 2001 From: Awais Jibran Date: Tue, 27 Sep 2016 16:19:34 +0500 Subject: [PATCH 1/3] Revert "Fix user registration " --- common/djangoapps/student/tasks.py | 4 +--- common/djangoapps/student/tests/test_tasks.py | 19 ++++++------------- common/djangoapps/student/views.py | 2 +- 3 files changed, 8 insertions(+), 17 deletions(-) diff --git a/common/djangoapps/student/tasks.py b/common/djangoapps/student/tasks.py index dfa350d22c..e745c4ab20 100644 --- a/common/djangoapps/student/tasks.py +++ b/common/djangoapps/student/tasks.py @@ -2,7 +2,6 @@ This file contains celery tasks for sending email """ from django.conf import settings -from django.contrib.auth.models import User from django.core import mail from celery.task import task # pylint: disable=no-name-in-module, import-error @@ -13,11 +12,10 @@ log = get_task_logger(__name__) @task(bind=True) -def send_activation_email(self, user_id, subject, message, from_address): +def send_activation_email(self, user, subject, message, from_address): """ Sending an activation email to the users. """ - user = User.objects.get(id=user_id) max_retries = settings.RETRY_ACTIVATION_EMAIL_MAX_ATTEMPTS retries = self.request.retries + 1 dest_addr = user.email diff --git a/common/djangoapps/student/tests/test_tasks.py b/common/djangoapps/student/tests/test_tasks.py index 6c774a4890..1c568ae416 100644 --- a/common/djangoapps/student/tests/test_tasks.py +++ b/common/djangoapps/student/tests/test_tasks.py @@ -9,36 +9,29 @@ from django.conf import settings from student.tasks import send_activation_email from boto.exception import NoAuthHandlerFound -from lms.djangoapps.courseware.tests.factories import UserFactory - class SendActivationEmailTestCase(TestCase): """ Test for send activation email to user """ - def setUp(self): - """ Setup components used by each test.""" - super(SendActivationEmailTestCase, self).setUp() - self.student = UserFactory() - @mock.patch('time.sleep', mock.Mock(return_value=None)) @mock.patch('student.tasks.log') - @mock.patch('django.contrib.auth.models.User.email_user', mock.Mock(side_effect=NoAuthHandlerFound)) - def test_send_email(self, mock_log): + @mock.patch('django.contrib.auth.models.User') + def test_send_email(self, mock_user, mock_log): """ Tests retries when the activation email doesn't send """ from_address = 'task_testing@edX.com' + mock_user.email_user.side_effect = NoAuthHandlerFound email_max_attempts = settings.RETRY_ACTIVATION_EMAIL_MAX_ATTEMPTS + 1 - # pylint: disable=no-member - send_activation_email.delay(self.student.id, 'Task_test', 'Task_test_message', from_address) + send_activation_email.delay(mock_user, 'Task_test', 'Task_test_message', from_address) # Asserts sending email retry logging. for attempt in xrange(1, email_max_attempts): mock_log.info.assert_any_call( 'Retrying sending email to user {dest_addr}, attempt # {attempt} of {max_attempts}'.format( - dest_addr=self.student.email, + dest_addr=mock_user.email, attempt=attempt, max_attempts=email_max_attempts )) @@ -48,7 +41,7 @@ class SendActivationEmailTestCase(TestCase): mock_log.error.assert_called_with( 'Unable to send activation email to user from "%s" to "%s"', from_address, - self.student.email, + mock_user.email, exc_info=True ) self.assertEquals(mock_log.error.call_count, 1) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 4e6ab2491b..29519860f3 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -1790,7 +1790,7 @@ def create_account_with_params(request, params): 'email_from_address', settings.DEFAULT_FROM_EMAIL ) - send_activation_email.delay(user.id, subject, message, from_address) + send_activation_email.delay(user, subject, message, from_address) else: registration.activate() _enroll_user_in_pending_courses(user) # Enroll student in any pending courses From 648bc7efb408f445ad64b74044abd580f7dbf12e Mon Sep 17 00:00:00 2001 From: attiyaishaque Date: Tue, 27 Sep 2016 17:20:57 +0500 Subject: [PATCH 2/3] Revert "Adding celery task for sending activation email." This reverts commit fe136122dbae9ea9623a2bf571f609a82429548a. --- cms/envs/test.py | 1 - common/djangoapps/student/tasks.py | 55 ------------------- common/djangoapps/student/tests/test_tasks.py | 47 ---------------- common/djangoapps/student/views.py | 30 +++++++++- lms/envs/common.py | 1 - 5 files changed, 28 insertions(+), 106 deletions(-) delete mode 100644 common/djangoapps/student/tasks.py delete mode 100644 common/djangoapps/student/tests/test_tasks.py diff --git a/cms/envs/test.py b/cms/envs/test.py index 3b08d4f61d..0a7b75f772 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -338,4 +338,3 @@ OAUTH2_PROVIDER_APPLICATION_MODEL = 'oauth2_provider.Application' # Used with Email sending RETRY_ACTIVATION_EMAIL_MAX_ATTEMPTS = 5 -RETRY_ACTIVATION_EMAIL_TIMEOUT = 0.5 diff --git a/common/djangoapps/student/tasks.py b/common/djangoapps/student/tasks.py deleted file mode 100644 index e745c4ab20..0000000000 --- a/common/djangoapps/student/tasks.py +++ /dev/null @@ -1,55 +0,0 @@ -""" -This file contains celery tasks for sending email -""" -from django.conf import settings -from django.core import mail - -from celery.task import task # pylint: disable=no-name-in-module, import-error -from celery.utils.log import get_task_logger # pylint: disable=no-name-in-module, import-error -from celery.exceptions import MaxRetriesExceededError -from boto.exception import NoAuthHandlerFound -log = get_task_logger(__name__) - - -@task(bind=True) -def send_activation_email(self, user, subject, message, from_address): - """ - Sending an activation email to the users. - """ - max_retries = settings.RETRY_ACTIVATION_EMAIL_MAX_ATTEMPTS - retries = self.request.retries + 1 - dest_addr = user.email - try: - if settings.FEATURES.get('REROUTE_ACTIVATION_EMAIL'): - dest_addr = settings.FEATURES['REROUTE_ACTIVATION_EMAIL'] - message = ("Activation for %s (%s): %s\n" % (user, user.email, user.profile.name) + - '-' * 80 + '\n\n' + message) - mail.send_mail(subject, message, from_address, [dest_addr], fail_silently=False) - else: - user.email_user(subject, message, from_address) - # Log that the Activation Email has been sent to user without an exception - log.info("Activataion Email has been sent to User {user_email}".format( - user_email=dest_addr - )) - 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 + 1 - )) - try: - self.retry(countdown=settings.RETRY_ACTIVATION_EMAIL_TIMEOUT, max_retries=max_retries) - except MaxRetriesExceededError: - log.error( - 'Unable to send activation email to user from "%s" to "%s"', - from_address, - dest_addr, - exc_info=True - ) - except: # pylint: disable=bare-except - log.exception( - 'Unable to send activation email to user from "%s" to "%s"', - from_address, - dest_addr, - exc_info=True - ) diff --git a/common/djangoapps/student/tests/test_tasks.py b/common/djangoapps/student/tests/test_tasks.py deleted file mode 100644 index 1c568ae416..0000000000 --- a/common/djangoapps/student/tests/test_tasks.py +++ /dev/null @@ -1,47 +0,0 @@ -""" -Tests for the Sending activation email celery tasks -""" - -import mock - -from django.test import TestCase -from django.conf import settings -from student.tasks import send_activation_email -from boto.exception import NoAuthHandlerFound - - -class SendActivationEmailTestCase(TestCase): - """ - Test for send activation email to user - """ - @mock.patch('time.sleep', mock.Mock(return_value=None)) - @mock.patch('student.tasks.log') - @mock.patch('django.contrib.auth.models.User') - def test_send_email(self, mock_user, mock_log): - """ - Tests retries when the activation email doesn't send - """ - from_address = 'task_testing@edX.com' - mock_user.email_user.side_effect = NoAuthHandlerFound - email_max_attempts = settings.RETRY_ACTIVATION_EMAIL_MAX_ATTEMPTS + 1 - - send_activation_email.delay(mock_user, 'Task_test', 'Task_test_message', from_address) - - # Asserts sending email retry logging. - for attempt in xrange(1, email_max_attempts): - mock_log.info.assert_any_call( - 'Retrying sending email to user {dest_addr}, attempt # {attempt} of {max_attempts}'.format( - dest_addr=mock_user.email, - attempt=attempt, - max_attempts=email_max_attempts - )) - self.assertEquals(mock_log.info.call_count, 6) - - # Asserts that the error was logged on crossing max retry attempts. - mock_log.error.assert_called_with( - 'Unable to send activation email to user from "%s" to "%s"', - from_address, - mock_user.email, - exc_info=True - ) - self.assertEquals(mock_log.error.call_count, 1) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 29519860f3..b63d293999 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -56,7 +56,6 @@ from student.models import ( DashboardConfiguration, LinkedInAddToProfileConfiguration, ManualEnrollmentAudit, ALLOWEDTOENROLL_TO_ENROLLED, LogoutViewConfiguration) from student.forms import AccountCreationForm, PasswordResetFormNoActive, get_registration_extension_form -from student.tasks import send_activation_email from lms.djangoapps.commerce.utils import EcommerceService # pylint: disable=import-error from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification # pylint: disable=import-error from bulk_email.models import Optout, BulkEmailFlag # pylint: disable=import-error @@ -1776,10 +1775,12 @@ def create_account_with_params(request, params): ) ) if send_email: + dest_addr = user.email context = { 'name': profile.name, 'key': registration.activation_key, } + # composes activation email subject = render_to_string('emails/activation_email_subject.txt', context) # Email subject *must not* contain newlines @@ -1790,7 +1791,32 @@ def create_account_with_params(request, params): 'email_from_address', settings.DEFAULT_FROM_EMAIL ) - send_activation_email.delay(user, subject, message, from_address) + retries = settings.RETRY_ACTIVATION_EMAIL_MAX_ATTEMPTS + while retries: + try: + if settings.FEATURES.get('REROUTE_ACTIVATION_EMAIL'): + dest_addr = settings.FEATURES['REROUTE_ACTIVATION_EMAIL'] + message = ("Activation for %s (%s): %s\n" % (user, user.email, profile.name) + + '-' * 80 + '\n\n' + message) + mail.send_mail(subject, message, from_address, [dest_addr], fail_silently=False) + else: + user.email_user(subject, message, from_address) + # Log that the Activation Email has been sent to user without an exception + log.info("Activataion Email has been sent to User {user_email}".format( + user_email=dest_addr + )) + break + except Exception: # pylint: disable=broad-except + retries -= 1 + if retries <= 0: + break + log.error( + u'Unable to send activation email to user from "%s" to "%s"', + from_address, + dest_addr, + exc_info=True + ) + else: registration.activate() _enroll_user_in_pending_courses(user) # Enroll student in any pending courses diff --git a/lms/envs/common.py b/lms/envs/common.py index 47adc782f9..5bce4db235 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -372,7 +372,6 @@ XQUEUE_WAITTIME_BETWEEN_REQUESTS = 5 # seconds # Used with Email sending RETRY_ACTIVATION_EMAIL_MAX_ATTEMPTS = 5 -RETRY_ACTIVATION_EMAIL_TIMEOUT = 0.5 ############################# SET PATH INFORMATION ############################# From e8f109a3286d0ae900105aae5f4c141f832853ff Mon Sep 17 00:00:00 2001 From: attiyaishaque Date: Tue, 27 Sep 2016 17:21:24 +0500 Subject: [PATCH 3/3] Revert "ECOM-5281 Add retires for email sending while registering." This reverts commit 4bbeaf32aeb343b323b7cada2b959987a9fa990c. --- cms/envs/test.py | 3 --- common/djangoapps/student/views.py | 41 +++++++++++------------------- lms/envs/common.py | 3 --- 3 files changed, 15 insertions(+), 32 deletions(-) diff --git a/cms/envs/test.py b/cms/envs/test.py index 0a7b75f772..5074c48d98 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -335,6 +335,3 @@ INSTALLED_APPS += ('openedx.core.djangoapps.api_admin',) # Set the default Oauth2 Provider Model so that migrations can run in # verbose mode OAUTH2_PROVIDER_APPLICATION_MODEL = 'oauth2_provider.Application' - -# Used with Email sending -RETRY_ACTIVATION_EMAIL_MAX_ATTEMPTS = 5 diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index b63d293999..45c8b75541 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -1791,32 +1791,21 @@ def create_account_with_params(request, params): 'email_from_address', settings.DEFAULT_FROM_EMAIL ) - retries = settings.RETRY_ACTIVATION_EMAIL_MAX_ATTEMPTS - while retries: - try: - if settings.FEATURES.get('REROUTE_ACTIVATION_EMAIL'): - dest_addr = settings.FEATURES['REROUTE_ACTIVATION_EMAIL'] - message = ("Activation for %s (%s): %s\n" % (user, user.email, profile.name) + - '-' * 80 + '\n\n' + message) - mail.send_mail(subject, message, from_address, [dest_addr], fail_silently=False) - else: - user.email_user(subject, message, from_address) - # Log that the Activation Email has been sent to user without an exception - log.info("Activataion Email has been sent to User {user_email}".format( - user_email=dest_addr - )) - break - except Exception: # pylint: disable=broad-except - retries -= 1 - if retries <= 0: - break - log.error( - u'Unable to send activation email to user from "%s" to "%s"', - from_address, - dest_addr, - exc_info=True - ) - + try: + if settings.FEATURES.get('REROUTE_ACTIVATION_EMAIL'): + dest_addr = settings.FEATURES['REROUTE_ACTIVATION_EMAIL'] + message = ("Activation for %s (%s): %s\n" % (user, user.email, profile.name) + + '-' * 80 + '\n\n' + message) + mail.send_mail(subject, message, from_address, [dest_addr], fail_silently=False) + else: + user.email_user(subject, message, from_address) + except Exception: # pylint: disable=broad-except + log.error( + u'Unable to send activation email to user from "%s" to "%s"', + from_address, + dest_addr, + exc_info=True + ) else: registration.activate() _enroll_user_in_pending_courses(user) # Enroll student in any pending courses diff --git a/lms/envs/common.py b/lms/envs/common.py index 5bce4db235..0dc4b47b0a 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -370,9 +370,6 @@ GENERATE_PROFILE_SCORES = False # Used with XQueue XQUEUE_WAITTIME_BETWEEN_REQUESTS = 5 # seconds -# Used with Email sending -RETRY_ACTIVATION_EMAIL_MAX_ATTEMPTS = 5 - ############################# SET PATH INFORMATION ############################# PROJECT_ROOT = path(__file__).abspath().dirname().dirname() # /edx-platform/lms