From 2c50926785d0b54e19ca87f7e3f9969a650b3e8d Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Mon, 30 Mar 2015 14:41:41 +0100 Subject: [PATCH 1/6] No longer use celery for sending notifications --- wagtail/wagtailadmin/tasks.py | 101 ---------------------------- wagtail/wagtailadmin/tests/tests.py | 2 +- wagtail/wagtailadmin/utils.py | 79 +++++++++++++++++++++- wagtail/wagtailadmin/views/pages.py | 11 +-- wagtail/wagtailforms/models.py | 4 +- 5 files changed, 87 insertions(+), 110 deletions(-) delete mode 100644 wagtail/wagtailadmin/tasks.py diff --git a/wagtail/wagtailadmin/tasks.py b/wagtail/wagtailadmin/tasks.py deleted file mode 100644 index 815971e46cc..00000000000 --- a/wagtail/wagtailadmin/tasks.py +++ /dev/null @@ -1,101 +0,0 @@ -from django.template.loader import render_to_string -from django.core.mail import send_mail -from django.conf import settings -from django.contrib.auth import get_user_model -from django.db.models import Q - -from wagtail.wagtailcore.models import PageRevision, GroupPagePermission -from wagtail.wagtailusers.models import UserProfile - -# The following will check to see if we can import task from celery - -# if not then we definitely haven't installed it -try: - from celery.decorators import task - NO_CELERY = False -except: - NO_CELERY = True - - -# However, we could have installed celery for other projects. So we will also -# check if we have defined the BROKER_URL setting. If not then definitely we -# haven't configured it. -if NO_CELERY or not hasattr(settings, 'BROKER_URL'): - # So if we enter here we will define a different "task" decorator that - # just returns the original function and sets its delay attribute to - # point to the original function: This way, the send_notification - # function will be actually called instead of the the - # send_notification.delay() - def task(f): - f.delay=f - return f - -def users_with_page_permission(page, permission_type, include_superusers=True): - # Get user model - User = get_user_model() - - # Find GroupPagePermission records of the given type that apply to this page or an ancestor - ancestors_and_self = list(page.get_ancestors()) + [page] - perm = GroupPagePermission.objects.filter(permission_type=permission_type, page__in=ancestors_and_self) - q = Q(groups__page_permissions=perm) - - # Include superusers - if include_superusers: - q |= Q(is_superuser=True) - - return User.objects.filter(is_active=True).filter(q).distinct() - - -@task -def send_notification(page_revision_id, notification, excluded_user_id): - # Get revision - revision = PageRevision.objects.get(id=page_revision_id) - - # Get list of recipients - if notification == 'submitted': - # Get list of publishers - recipients = users_with_page_permission(revision.page, 'publish') - elif notification in ['rejected', 'approved']: - # Get submitter - recipients = [revision.user] - else: - return - - # Get list of email addresses - email_addresses = [ - recipient.email for recipient in recipients - if recipient.email and recipient.id != excluded_user_id and getattr(UserProfile.get_for_user(recipient), notification + '_notifications') - ] - - # Return if there are no email addresses - if not email_addresses: - return - - # Get email subject and content - template = 'wagtailadmin/notifications/' + notification + '.html' - rendered_template = render_to_string(template, dict(revision=revision, settings=settings)).split('\n') - email_subject = rendered_template[0] - email_content = '\n'.join(rendered_template[1:]) - - # Get from email - if hasattr(settings, 'WAGTAILADMIN_NOTIFICATION_FROM_EMAIL'): - from_email = settings.WAGTAILADMIN_NOTIFICATION_FROM_EMAIL - elif hasattr(settings, 'DEFAULT_FROM_EMAIL'): - from_email = settings.DEFAULT_FROM_EMAIL - else: - from_email = 'webmaster@localhost' - - # Send email - send_mail(email_subject, email_content, from_email, email_addresses) - - -@task -def send_email_task(email_subject, email_content, email_addresses, from_email=None): - if not from_email: - if hasattr(settings, 'WAGTAILADMIN_NOTIFICATION_FROM_EMAIL'): - from_email = settings.WAGTAILADMIN_NOTIFICATION_FROM_EMAIL - elif hasattr(settings, 'DEFAULT_FROM_EMAIL'): - from_email = settings.DEFAULT_FROM_EMAIL - else: - from_email = 'webmaster@localhost' - - send_mail(email_subject, email_content, from_email, email_addresses) diff --git a/wagtail/wagtailadmin/tests/tests.py b/wagtail/wagtailadmin/tests/tests.py index 834c20de5d3..2adebbfcfcb 100644 --- a/wagtail/wagtailadmin/tests/tests.py +++ b/wagtail/wagtailadmin/tests/tests.py @@ -4,7 +4,7 @@ from wagtail.tests.utils import WagtailTestUtils from wagtail.wagtailcore.models import Page -from wagtail.wagtailadmin.tasks import send_email_task +from wagtail.wagtailadmin.utils import send_email_task class TestHome(TestCase, WagtailTestUtils): diff --git a/wagtail/wagtailadmin/utils.py b/wagtail/wagtailadmin/utils.py index 043b8336d7f..d864507d929 100644 --- a/wagtail/wagtailadmin/utils.py +++ b/wagtail/wagtailadmin/utils.py @@ -1,6 +1,13 @@ +from django.template.loader import render_to_string +from django.core.mail import send_mail +from django.conf import settings +from django.contrib.auth import get_user_model +from django.db.models import Q + from modelcluster.fields import ParentalKey -from wagtail.wagtailcore.models import Page +from wagtail.wagtailcore.models import Page, PageRevision, GroupPagePermission +from wagtail.wagtailusers.models import UserProfile def get_object_usage(obj): @@ -34,3 +41,73 @@ def get_object_usage(obj): ) return pages + + +def users_with_page_permission(page, permission_type, include_superusers=True): + # Get user model + User = get_user_model() + + # Find GroupPagePermission records of the given type that apply to this page or an ancestor + ancestors_and_self = list(page.get_ancestors()) + [page] + perm = GroupPagePermission.objects.filter(permission_type=permission_type, page__in=ancestors_and_self) + q = Q(groups__page_permissions=perm) + + # Include superusers + if include_superusers: + q |= Q(is_superuser=True) + + return User.objects.filter(is_active=True).filter(q).distinct() + + +def send_notification(page_revision_id, notification, excluded_user_id): + # Get revision + revision = PageRevision.objects.get(id=page_revision_id) + + # Get list of recipients + if notification == 'submitted': + # Get list of publishers + recipients = users_with_page_permission(revision.page, 'publish') + elif notification in ['rejected', 'approved']: + # Get submitter + recipients = [revision.user] + else: + return + + # Get list of email addresses + email_addresses = [ + recipient.email for recipient in recipients + if recipient.email and recipient.id != excluded_user_id and getattr(UserProfile.get_for_user(recipient), notification + '_notifications') + ] + + # Return if there are no email addresses + if not email_addresses: + return + + # Get email subject and content + template = 'wagtailadmin/notifications/' + notification + '.html' + rendered_template = render_to_string(template, dict(revision=revision, settings=settings)).split('\n') + email_subject = rendered_template[0] + email_content = '\n'.join(rendered_template[1:]) + + # Get from email + if hasattr(settings, 'WAGTAILADMIN_NOTIFICATION_FROM_EMAIL'): + from_email = settings.WAGTAILADMIN_NOTIFICATION_FROM_EMAIL + elif hasattr(settings, 'DEFAULT_FROM_EMAIL'): + from_email = settings.DEFAULT_FROM_EMAIL + else: + from_email = 'webmaster@localhost' + + # Send email + send_mail(email_subject, email_content, from_email, email_addresses) + + +def send_email_task(email_subject, email_content, email_addresses, from_email=None): + if not from_email: + if hasattr(settings, 'WAGTAILADMIN_NOTIFICATION_FROM_EMAIL'): + from_email = settings.WAGTAILADMIN_NOTIFICATION_FROM_EMAIL + elif hasattr(settings, 'DEFAULT_FROM_EMAIL'): + from_email = settings.DEFAULT_FROM_EMAIL + else: + from_email = 'webmaster@localhost' + + send_mail(email_subject, email_content, from_email, email_addresses) diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py index a707d1df6ab..e1dd99a581f 100644 --- a/wagtail/wagtailadmin/views/pages.py +++ b/wagtail/wagtailadmin/views/pages.py @@ -15,7 +15,8 @@ from wagtail.wagtailadmin.edit_handlers import TabbedInterface, ObjectList from wagtail.wagtailadmin.forms import SearchForm, CopyForm -from wagtail.wagtailadmin import tasks, signals +from wagtail.wagtailadmin.utils import send_notification +from wagtail.wagtailadmin import signals from wagtail.wagtailcore import hooks from wagtail.wagtailcore.models import Page, PageRevision, get_navigation_menu_items @@ -225,7 +226,7 @@ def clean(): messages.success(request, _("Page '{0}' published.").format(page.title)) elif is_submitting: messages.success(request, _("Page '{0}' submitted for moderation.").format(page.title)) - tasks.send_notification.delay(page.get_latest_revision().id, 'submitted', request.user.id) + send_notification(page.get_latest_revision().id, 'submitted', request.user.id) else: messages.success(request, _("Page '{0}' created.").format(page.title)) @@ -361,7 +362,7 @@ def clean(): messages.button(reverse('wagtailadmin_pages_view_draft', args=(page_id,)), _('View draft')), messages.button(reverse('wagtailadmin_pages_edit', args=(page_id,)), _('Edit')) ]) - tasks.send_notification.delay(page.get_latest_revision().id, 'submitted', request.user.id) + send_notification(page.get_latest_revision().id, 'submitted', request.user.id) else: messages.success(request, _("Page '{0}' updated.").format(page.title)) @@ -782,7 +783,7 @@ def approve_moderation(request, revision_id): if request.method == 'POST': revision.approve_moderation() messages.success(request, _("Page '{0}' published.").format(revision.page.title)) - tasks.send_notification.delay(revision.id, 'approved', request.user.id) + send_notification(revision.id, 'approved', request.user.id) return redirect('wagtailadmin_home') @@ -799,7 +800,7 @@ def reject_moderation(request, revision_id): if request.method == 'POST': revision.reject_moderation() messages.success(request, _("Page '{0}' rejected for publication.").format(revision.page.title)) - tasks.send_notification.delay(revision.id, 'rejected', request.user.id) + send_notification(revision.id, 'rejected', request.user.id) return redirect('wagtailadmin_home') diff --git a/wagtail/wagtailforms/models.py b/wagtail/wagtailforms/models.py index 755994dbfee..acbdb08b2c2 100644 --- a/wagtail/wagtailforms/models.py +++ b/wagtail/wagtailforms/models.py @@ -16,7 +16,7 @@ from wagtail.wagtailcore.models import Page, Orderable, UserPagePermissionsProxy, get_page_types from wagtail.wagtailadmin.edit_handlers import FieldPanel -from wagtail.wagtailadmin import tasks +from wagtail.wagtailadmin.utils import send_email_task from .forms import FormBuilder @@ -194,7 +194,7 @@ def process_form_submission(self, form): if self.to_address: content = '\n'.join([x[1].label + ': ' + form.data.get(x[0]) for x in form.fields.items()]) - tasks.send_email_task.delay(self.subject, content, [self.to_address], self.from_address,) + send_email_task(self.subject, content, [self.to_address], self.from_address,) class Meta: From 378b2c546e6a6044afc377f1e48f9b6f1ff5af75 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Mon, 30 Mar 2015 14:54:59 +0100 Subject: [PATCH 2/6] Rename send_email_task => send_mail Also make send_notification use send_mail --- wagtail/wagtailadmin/utils.py | 36 +++++++++++++--------------------- wagtail/wagtailforms/models.py | 4 ++-- 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/wagtail/wagtailadmin/utils.py b/wagtail/wagtailadmin/utils.py index d864507d929..a5cb98a1aa4 100644 --- a/wagtail/wagtailadmin/utils.py +++ b/wagtail/wagtailadmin/utils.py @@ -1,5 +1,5 @@ from django.template.loader import render_to_string -from django.core.mail import send_mail +from django.core.mail import send_mail as django_send_mail from django.conf import settings from django.contrib.auth import get_user_model from django.db.models import Q @@ -59,6 +59,18 @@ def users_with_page_permission(page, permission_type, include_superusers=True): return User.objects.filter(is_active=True).filter(q).distinct() +def send_mail(email_subject, email_content, email_addresses, from_email=None): + if not from_email: + if hasattr(settings, 'WAGTAILADMIN_NOTIFICATION_FROM_EMAIL'): + from_email = settings.WAGTAILADMIN_NOTIFICATION_FROM_EMAIL + elif hasattr(settings, 'DEFAULT_FROM_EMAIL'): + from_email = settings.DEFAULT_FROM_EMAIL + else: + from_email = 'webmaster@localhost' + + django_send_mail(email_subject, email_content, from_email, email_addresses) + + def send_notification(page_revision_id, notification, excluded_user_id): # Get revision revision = PageRevision.objects.get(id=page_revision_id) @@ -89,25 +101,5 @@ def send_notification(page_revision_id, notification, excluded_user_id): email_subject = rendered_template[0] email_content = '\n'.join(rendered_template[1:]) - # Get from email - if hasattr(settings, 'WAGTAILADMIN_NOTIFICATION_FROM_EMAIL'): - from_email = settings.WAGTAILADMIN_NOTIFICATION_FROM_EMAIL - elif hasattr(settings, 'DEFAULT_FROM_EMAIL'): - from_email = settings.DEFAULT_FROM_EMAIL - else: - from_email = 'webmaster@localhost' - # Send email - send_mail(email_subject, email_content, from_email, email_addresses) - - -def send_email_task(email_subject, email_content, email_addresses, from_email=None): - if not from_email: - if hasattr(settings, 'WAGTAILADMIN_NOTIFICATION_FROM_EMAIL'): - from_email = settings.WAGTAILADMIN_NOTIFICATION_FROM_EMAIL - elif hasattr(settings, 'DEFAULT_FROM_EMAIL'): - from_email = settings.DEFAULT_FROM_EMAIL - else: - from_email = 'webmaster@localhost' - - send_mail(email_subject, email_content, from_email, email_addresses) + send_mail(email_subject, email_content, email_addresses) diff --git a/wagtail/wagtailforms/models.py b/wagtail/wagtailforms/models.py index acbdb08b2c2..ff3278167a6 100644 --- a/wagtail/wagtailforms/models.py +++ b/wagtail/wagtailforms/models.py @@ -16,7 +16,7 @@ from wagtail.wagtailcore.models import Page, Orderable, UserPagePermissionsProxy, get_page_types from wagtail.wagtailadmin.edit_handlers import FieldPanel -from wagtail.wagtailadmin.utils import send_email_task +from wagtail.wagtailadmin.utils import send_mail from .forms import FormBuilder @@ -194,7 +194,7 @@ def process_form_submission(self, form): if self.to_address: content = '\n'.join([x[1].label + ': ' + form.data.get(x[0]) for x in form.fields.items()]) - send_email_task(self.subject, content, [self.to_address], self.from_address,) + send_mail(self.subject, content, [self.to_address], self.from_address,) class Meta: From 3d48ba0cd8dba20d4a68dd54218980888c533621 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Mon, 30 Mar 2015 14:55:37 +0100 Subject: [PATCH 3/6] Test for wagtailadmin.utils.send_mail --- wagtail/wagtailadmin/tests/tests.py | 41 ++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/wagtail/wagtailadmin/tests/tests.py b/wagtail/wagtailadmin/tests/tests.py index 2adebbfcfcb..d4af55f361e 100644 --- a/wagtail/wagtailadmin/tests/tests.py +++ b/wagtail/wagtailadmin/tests/tests.py @@ -1,10 +1,10 @@ -from django.test import TestCase +from django.test import TestCase, override_settings from django.core.urlresolvers import reverse from django.core import mail from wagtail.tests.utils import WagtailTestUtils from wagtail.wagtailcore.models import Page -from wagtail.wagtailadmin.utils import send_email_task +from wagtail.wagtailadmin.utils import send_mail class TestHome(TestCase, WagtailTestUtils): @@ -56,15 +56,48 @@ def test_editor_css_and_js_hooks_on_edit(self): self.assertContains(response, '') -class TestSendEmailTask(TestCase): +class TestSendMail(TestCase): def test_send_email(self): - send_email_task("Test subject", "Test content", ["nobody@email.com"], "test@email.com") + send_mail("Test subject", "Test content", ["nobody@email.com"], "test@email.com") # Check that the email was sent self.assertEqual(len(mail.outbox), 1) self.assertEqual(mail.outbox[0].subject, "Test subject") self.assertEqual(mail.outbox[0].body, "Test content") self.assertEqual(mail.outbox[0].to, ["nobody@email.com"]) + self.assertEqual(mail.outbox[0].from_email, "test@email.com") + + @override_settings(WAGTAILADMIN_NOTIFICATION_FROM_EMAIL='anothertest@email.com') + def test_send_fallback_to_wagtailadmin_notification_from_email_setting(self): + send_mail("Test subject", "Test content", ["nobody@email.com"]) + + # Check that the email was sent + self.assertEqual(len(mail.outbox), 1) + self.assertEqual(mail.outbox[0].subject, "Test subject") + self.assertEqual(mail.outbox[0].body, "Test content") + self.assertEqual(mail.outbox[0].to, ["nobody@email.com"]) + self.assertEqual(mail.outbox[0].from_email, "anothertest@email.com") + + @override_settings(DEFAULT_FROM_EMAIL='yetanothertest@email.com') + def test_send_fallback_to_default_from_email_setting(self): + send_mail("Test subject", "Test content", ["nobody@email.com"]) + + # Check that the email was sent + self.assertEqual(len(mail.outbox), 1) + self.assertEqual(mail.outbox[0].subject, "Test subject") + self.assertEqual(mail.outbox[0].body, "Test content") + self.assertEqual(mail.outbox[0].to, ["nobody@email.com"]) + self.assertEqual(mail.outbox[0].from_email, "yetanothertest@email.com") + + def test_send_default_from_email(self): + send_mail("Test subject", "Test content", ["nobody@email.com"]) + + # Check that the email was sent + self.assertEqual(len(mail.outbox), 1) + self.assertEqual(mail.outbox[0].subject, "Test subject") + self.assertEqual(mail.outbox[0].body, "Test content") + self.assertEqual(mail.outbox[0].to, ["nobody@email.com"]) + self.assertEqual(mail.outbox[0].from_email, "webmaster@localhost") class TestExplorerNavView(TestCase, WagtailTestUtils): From c77eafdd8e3e35e52c43af1e3374e9ded0cb2b14 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Tue, 31 Mar 2015 10:25:54 +0100 Subject: [PATCH 4/6] Removed celery from production.py No longer required by Wagtail --- .../project_name/settings/production.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/wagtail/project_template/project_name/settings/production.py b/wagtail/project_template/project_name/settings/production.py index e9e9e1971b5..bf20125a988 100644 --- a/wagtail/project_template/project_name/settings/production.py +++ b/wagtail/project_template/project_name/settings/production.py @@ -13,19 +13,6 @@ COMPRESS_OFFLINE = True -# Send notification emails as a background task using Celery, -# to prevent this from blocking web server threads -# (requires the django-celery package): -# http://celery.readthedocs.org/en/latest/configuration.html - -# import djcelery -# -# djcelery.setup_loader() -# -# CELERY_SEND_TASK_ERROR_EMAILS = True -# BROKER_URL = 'redis://' - - # Use Redis as the cache backend for extra performance # (requires the django-redis-cache package): # http://wagtail.readthedocs.org/en/latest/howto/performance.html#cache From 6f8218f23b6438fd562c2b8898fc29b5742b9cf7 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Tue, 31 Mar 2015 10:29:58 +0100 Subject: [PATCH 5/6] Remove mention of celery from docs --- docs/howto/performance.rst | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/docs/howto/performance.rst b/docs/howto/performance.rst index 924e49d9f3b..0e1b64f687e 100644 --- a/docs/howto/performance.rst +++ b/docs/howto/performance.rst @@ -28,21 +28,6 @@ We recommend `Redis `_ as a fast, persistent cache. Install Re Without a persistent cache, Wagtail will recreate all compressable assets at each server start, e.g. when any files change under ``./manage.py runserver``. -Sending emails in the background using Celery ---------------------------------------------- - -Various actions in the Wagtail admin backend can trigger notification emails - for example, submitting a page for moderation. In Wagtail's default configuration, these are sent as part of the page request/response cycle, which means that web server threads can get tied up for long periods if many emails are being sent. To avoid this, Wagtail can be configured to do this as a background task, using `Celery `_ as a task queue. To install Celery, add ``django-celery`` to your requirements.txt. A sample configuration, using Redis as the queue backend, would look like:: - - import djcelery - - djcelery.setup_loader() - - CELERY_SEND_TASK_ERROR_EMAILS = True - BROKER_URL = 'redis://' - -See the Celery documentation for instructions on running the worker process in development or production. - - Search ------ From f077fb026dfbd4386226ba2550cffa04e801421d Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Tue, 31 Mar 2015 10:30:59 +0100 Subject: [PATCH 6/6] Remove celery from project_template/requirements.txt --- wagtail/project_template/requirements.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/wagtail/project_template/requirements.txt b/wagtail/project_template/requirements.txt index cccb37a3832..a2ad30e9bb5 100644 --- a/wagtail/project_template/requirements.txt +++ b/wagtail/project_template/requirements.txt @@ -8,4 +8,3 @@ wagtail==1.0b1 # Recommended components to improve performance in production: # django-redis==3.8.2 -# django-celery==3.1.10