From 6ca739d81175740449d1edac5867fca70f46eb2c Mon Sep 17 00:00:00 2001 From: bmedx Date: Wed, 1 Mar 2017 17:07:42 -0500 Subject: [PATCH] Send emails on completed django-user-tasks - Creates a new app under cms - Sends emails asynchronously on completion of any top-level job - Moves old user task related cms tests to cms_user_tasks --- cms/djangoapps/cms_user_tasks/__init__.py | 0 cms/djangoapps/cms_user_tasks/apps.py | 19 ++++ cms/djangoapps/cms_user_tasks/signals.py | 45 ++++++++ cms/djangoapps/cms_user_tasks/tasks.py | 69 ++++++++++++ .../cms_user_tasks/tests.py} | 105 +++++++++++++++++- cms/envs/common.py | 3 + .../emails/user_task_complete_email.txt | 11 ++ .../user_task_complete_email_subject.txt | 2 + cms/tests/__init__.py | 4 - pavelib/utils/test/suites/nose_suite.py | 4 +- 10 files changed, 254 insertions(+), 8 deletions(-) create mode 100644 cms/djangoapps/cms_user_tasks/__init__.py create mode 100644 cms/djangoapps/cms_user_tasks/apps.py create mode 100644 cms/djangoapps/cms_user_tasks/signals.py create mode 100644 cms/djangoapps/cms_user_tasks/tasks.py rename cms/{tests/test_user_tasks.py => djangoapps/cms_user_tasks/tests.py} (52%) create mode 100644 cms/templates/emails/user_task_complete_email.txt create mode 100644 cms/templates/emails/user_task_complete_email_subject.txt delete mode 100644 cms/tests/__init__.py diff --git a/cms/djangoapps/cms_user_tasks/__init__.py b/cms/djangoapps/cms_user_tasks/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cms/djangoapps/cms_user_tasks/apps.py b/cms/djangoapps/cms_user_tasks/apps.py new file mode 100644 index 0000000000..ae496cb14a --- /dev/null +++ b/cms/djangoapps/cms_user_tasks/apps.py @@ -0,0 +1,19 @@ +""" +CMS user tasks application configuration +Signal handlers are connected here. +""" + +from django.apps import AppConfig + + +class CmsUserTasksConfig(AppConfig): + """ + Application Configuration for cms_user_tasks. + """ + name = u'cms_user_tasks' + + def ready(self): + """ + Connect signal handlers. + """ + from . import signals # pylint: disable=unused-variable diff --git a/cms/djangoapps/cms_user_tasks/signals.py b/cms/djangoapps/cms_user_tasks/signals.py new file mode 100644 index 0000000000..cf943b7a1c --- /dev/null +++ b/cms/djangoapps/cms_user_tasks/signals.py @@ -0,0 +1,45 @@ +""" +Receivers of signals sent from django-user-tasks +""" +from urlparse import urljoin + +from django.core.urlresolvers import reverse +from django.dispatch import receiver +from user_tasks.models import UserTaskArtifact +from user_tasks.signals import user_task_stopped + +from .tasks import send_task_complete_email + + +@receiver(user_task_stopped, dispatch_uid="cms_user_task_stopped") +def user_task_stopped_handler(sender, **kwargs): # pylint: disable=unused-argument + """ + Handles sending notifications when a django-user-tasks completes. + This is a signal receiver for user_task_stopped. Currently it only sends + a generic "task completed" email, and only when a top-level task + completes. Eventually it might make more sense to create specific per-task + handlers. + Arguments: + sender (obj): Currently the UserTaskStatus object class + **kwargs: See below + Keywork Arguments: + status (obj): UserTaskStatus of the completed task + Returns: + None + """ + status = kwargs['status'] + + # Only send email when the entire task is complete, should only send when + # a chain / chord / etc completes, not on sub-tasks. + if status.parent is None: + # `name` and `status` are not unique, first is our best guess + artifact = UserTaskArtifact.objects.filter(status=status, name="BASE_URL").first() + + detail_url = None + if artifact and artifact.url.startswith(('http://', 'https://')): + detail_url = urljoin( + artifact.url, + reverse('usertaskstatus-detail', args=[status.uuid]) + ) + + send_task_complete_email.delay(status.name, status.state_text, status.user.email, detail_url) diff --git a/cms/djangoapps/cms_user_tasks/tasks.py b/cms/djangoapps/cms_user_tasks/tasks.py new file mode 100644 index 0000000000..8233e14ed7 --- /dev/null +++ b/cms/djangoapps/cms_user_tasks/tasks.py @@ -0,0 +1,69 @@ +""" +Celery tasks used by cms_user_tasks +""" + +from celery.task import task +from celery.exceptions import MaxRetriesExceededError +from celery.utils.log import get_task_logger +from boto.exception import NoAuthHandlerFound + +from django.conf import settings +from django.core import mail + +from edxmako.shortcuts import render_to_string +from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers + +LOGGER = get_task_logger(__name__) +TASK_COMPLETE_EMAIL_MAX_RETRIES = 3 +TASK_COMPLETE_EMAIL_TIMEOUT = 60 + + +@task(bind=True) +def send_task_complete_email(self, task_name, task_state_text, dest_addr, detail_url): + """ + Sending an email to the users when an async task completes. + """ + retries = self.request.retries + + context = { + 'task_name': task_name, + 'task_status': task_state_text, + 'detail_url': detail_url + } + + subject = render_to_string('emails/user_task_complete_email_subject.txt', context) + # Eliminate any newlines + subject = ''.join(subject.splitlines()) + message = render_to_string('emails/user_task_complete_email.txt', context) + + from_address = configuration_helpers.get_value( + 'email_from_address', + settings.DEFAULT_FROM_EMAIL + ) + + try: + mail.send_mail(subject, message, from_address, [dest_addr], fail_silently=False) + LOGGER.info("Task complete email has been sent to User %s", dest_addr) + except NoAuthHandlerFound: + LOGGER.info( + 'Retrying sending email to user %s, attempt # %s of %s', + dest_addr, + retries, + TASK_COMPLETE_EMAIL_MAX_RETRIES + ) + try: + self.retry(countdown=TASK_COMPLETE_EMAIL_TIMEOUT, max_retries=TASK_COMPLETE_EMAIL_MAX_RETRIES) + except MaxRetriesExceededError: + LOGGER.error( + 'Unable to send task completion email to user from "%s" to "%s"', + from_address, + dest_addr, + exc_info=True + ) + except Exception: # pylint: disable=broad-except + LOGGER.exception( + 'Unable to send task completion email to user from "%s" to "%s"', + from_address, + dest_addr, + exc_info=True + ) diff --git a/cms/tests/test_user_tasks.py b/cms/djangoapps/cms_user_tasks/tests.py similarity index 52% rename from cms/tests/test_user_tasks.py rename to cms/djangoapps/cms_user_tasks/tests.py index ed924d66fc..3677a27852 100644 --- a/cms/tests/test_user_tasks.py +++ b/cms/djangoapps/cms_user_tasks/tests.py @@ -6,13 +6,19 @@ from __future__ import absolute_import, print_function, unicode_literals from uuid import uuid4 +import mock +from boto.exception import NoAuthHandlerFound +from rest_framework.test import APITestCase + from django.contrib.auth.models import User from django.core.urlresolvers import reverse from django.test import override_settings +from django.conf import settings +from django.core import mail -from rest_framework.test import APITestCase from user_tasks.models import UserTaskArtifact, UserTaskStatus from user_tasks.serializers import ArtifactSerializer, StatusSerializer +from .signals import user_task_stopped # Helper functions for stuff that pylint complains about without disable comments @@ -105,3 +111,100 @@ class TestUserTasks(APITestCase): assert response.status_code == 200 serializer = StatusSerializer([self.status], context=_context(response), many=True) assert _data(response)['results'] == serializer.data + + + +@override_settings(BROKER_URL='memory://localhost/') +class TestUserTaskStopped(APITestCase): + """ + Tests of the django-user-tasks signal handling and email integration. + """ + + @classmethod + def setUpTestData(cls): + cls.user = User.objects.create_user('test_user', 'test@example.com', 'password') + cls.status = UserTaskStatus.objects.create( + user=cls.user, task_id=str(uuid4()), task_class='test_rest_api.sample_task', name='SampleTask 2', + total_steps=5) + + def setUp(self): + super(TestUserTaskStopped, self).setUp() + self.status.refresh_from_db() + self.client.force_authenticate(self.user) # pylint: disable=no-member + + def test_email_sent_with_site(self): + """ + Check the signal receiver and email sending. + """ + UserTaskArtifact.objects.create( + status=self.status, name='BASE_URL', url='https://test.edx.org/' + ) + user_task_stopped.send(sender=UserTaskStatus, status=self.status) + + subject = "{platform_name} {studio_name}: Task Status Update".format( + platform_name=settings.PLATFORM_NAME, studio_name=settings.STUDIO_NAME + ) + body_fragments = [ + "Your {task_name} task has completed with the status".format(task_name=self.status.name), + "https://test.edx.org/", + reverse('usertaskstatus-detail', args=[self.status.uuid]) + ] + + for m in mail.outbox: + print (m.subject) + print (m.body) + + self.assertEqual(len(mail.outbox), 1) + + msg = mail.outbox[0] + + self.assertEqual(msg.subject, subject) + for fragment in body_fragments: + self.assertIn(fragment, msg.body) + + def test_email_not_sent_for_child(self): + """ + No email should be send for child tasks in chords, chains, etc. + """ + child_status = UserTaskStatus.objects.create( + user=self.user, task_id=str(uuid4()), task_class='test_rest_api.sample_task', name='SampleTask 2', + total_steps=5, parent=self.status) + user_task_stopped.send(sender=UserTaskStatus, status=child_status) + self.assertEqual(len(mail.outbox), 0) + + def test_email_sent_without_site(self): + """ + Make sure we send a generic email if the BASE_URL artifact doesn't exist + """ + user_task_stopped.send(sender=UserTaskStatus, status=self.status) + + subject = "{platform_name} {studio_name}: Task Status Update".format( + platform_name=settings.PLATFORM_NAME, studio_name=settings.STUDIO_NAME + ) + fragments = [ + "Your {task_name} task has completed with the status".format(task_name=self.status.name), + "Sign in to view the details of your task or download any files created." + ] + + for m in mail.outbox: + print (m.subject) + print (m.body) + + self.assertEqual(len(mail.outbox), 1) + + msg = mail.outbox[0] + self.assertEqual(msg.subject, subject) + + for fragment in fragments: + self.assertIn(fragment, msg.body) + + def test_email_retries(self): + """ + Make sure we can succeed on retries + """ + with mock.patch('django.core.mail.send_mail') as mock_exception: + mock_exception.side_effect = NoAuthHandlerFound() + + with mock.patch('cms_user_tasks.tasks.send_task_complete_email.retry') as mock_retry: + user_task_stopped.send(sender=UserTaskStatus, status=self.status) + self.assertTrue(mock_retry.called) diff --git a/cms/envs/common.py b/cms/envs/common.py index 31c91cf5cb..40a46e6ab7 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -962,6 +962,9 @@ INSTALLED_APPS = ( # management of user-triggered async tasks (course import/export, etc.) 'user_tasks', + # CMS specific user task handling + 'cms_user_tasks.apps.CmsUserTasksConfig', + # Unusual migrations 'database_fixups', ) diff --git a/cms/templates/emails/user_task_complete_email.txt b/cms/templates/emails/user_task_complete_email.txt new file mode 100644 index 0000000000..c3e9c33b91 --- /dev/null +++ b/cms/templates/emails/user_task_complete_email.txt @@ -0,0 +1,11 @@ +<%! from django.utils.translation import ugettext as _ %> + +% if detail_url: + +${_("Your {task_name} task has completed with the status {task_status}. Use this URL to view task details or download any files created: {detail_url}").format(task_name=task_name, task_status=task_status, detail_url=detail_url)} + +% else: + +${_("Your {task_name} task has completed with the status {task_status}. Sign in to view the details of your task or download any files created.").format(task_name=task_name, task_status=task_status)} + +% endif diff --git a/cms/templates/emails/user_task_complete_email_subject.txt b/cms/templates/emails/user_task_complete_email_subject.txt new file mode 100644 index 0000000000..0876382638 --- /dev/null +++ b/cms/templates/emails/user_task_complete_email_subject.txt @@ -0,0 +1,2 @@ +<%! from django.utils.translation import ugettext as _ %> +${_("{platform_name} {studio_name}: Task Status Update").format(platform_name=settings.PLATFORM_NAME, studio_name=settings.STUDIO_NAME)} diff --git a/cms/tests/__init__.py b/cms/tests/__init__.py deleted file mode 100644 index 7f7be4364e..0000000000 --- a/cms/tests/__init__.py +++ /dev/null @@ -1,4 +0,0 @@ -""" -Module for test in cms folder -All cms/test/* are already included in paver test -""" diff --git a/pavelib/utils/test/suites/nose_suite.py b/pavelib/utils/test/suites/nose_suite.py index 3a1cc4e892..df013b9192 100644 --- a/pavelib/utils/test/suites/nose_suite.py +++ b/pavelib/utils/test/suites/nose_suite.py @@ -152,6 +152,7 @@ class SystemTestSuite(NoseTestSuite): self.extra_args, '--xunitmp-file={}'.format(self.report_dir / "nosetests.xml"), '--with-database-isolation', + #'--stop' ] if self.processes != 0: @@ -191,9 +192,6 @@ class SystemTestSuite(NoseTestSuite): default_test_id += " {system}/tests.py" default_test_id += " openedx/core/djangolib" - if self.root == 'cms': - default_test_id += " {system}/tests/*" - return default_test_id.format(system=self.root)