Skip to content

Commit

Permalink
Sent emails when new user fills out profile and their profile matches…
Browse files Browse the repository at this point in the history
… query (#2782)

* Send emails when new user fills out their profile

* Combine excepts

* Make failed_recipient_emails

* Lint

* Set query

* Move send_automatic_emails to task

* Add except clause for IntegrityError

* Make factories

* Use side_effect

* Use different mock

* Use fake sender_name
  • Loading branch information
George Schneeloch authored Mar 15, 2017
1 parent f47555d commit 9cfc3ab
Show file tree
Hide file tree
Showing 19 changed files with 661 additions and 94 deletions.
45 changes: 20 additions & 25 deletions dashboard/signals_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,64 +2,59 @@
Tests for signals
"""

from unittest.mock import patch

from django.db.models.signals import post_save
from django.test import (
override_settings,
TestCase,
)
from factory.django import mute_signals

from courses.factories import ProgramFactory, CourseFactory

from dashboard.models import ProgramEnrollment
from profiles.factories import ProfileFactory
from search.base import MockedESTestCase


# Make sure that any unmocked ES activity results in an error
@override_settings(ELASTICSEARCH_URL="fake")
class IndexingTests(TestCase):
class ProgramEnrollmentTests(MockedESTestCase):
"""
Test class for signals that index certain objects in Elasticsearch
Test indexing on program enrollment
"""

@classmethod
def setUpTestData(cls):
super(IndexingTests, cls).setUpTestData()
super().setUpTestData()

with mute_signals(post_save):
cls.user = ProfileFactory.create().user
cls.program = ProgramFactory.create()
cls.course = CourseFactory.create(program=cls.program)

def setUp(self):
super().setUp()

for mock in self.patcher_mocks:
if mock.name == "_remove_program_enrolled_user":
self.remove_program_enrolled_user_mock = mock
elif mock.name == "_index_program_enrolled_users":
self.index_program_enrolled_users_mock = mock

class ProgramEnrollmentTests(IndexingTests):
"""
Test indexing on program enrollment
"""
def test_create(self):
"""
Tests that the database is reindexed when a ProgramEnrollment is created
"""
with patch('search.tasks._index_program_enrolled_users', autospec=True) as mocked:
enrollment = ProgramEnrollment.objects.create(user=self.user, program=self.program)
mocked.assert_called_once_with([enrollment])
enrollment = ProgramEnrollment.objects.create(user=self.user, program=self.program)
self.index_program_enrolled_users_mock.assert_called_once_with([enrollment])

def test_update(self):
"""
Tests that the database is reindexed when a ProgramEnrollment is created
"""
with mute_signals(post_save):
enrollment = ProgramEnrollment.objects.create(user=self.user, program=self.program)
with patch('search.tasks._index_program_enrolled_users', autospec=True) as mocked:
enrollment.save()
mocked.assert_called_once_with([enrollment])
enrollment.save()
self.index_program_enrolled_users_mock.assert_called_once_with([enrollment])

def test_delete(self):
"""
Tests that if a CachedEnrollment is updated with data=None, the enrollment in the program is not deleted.
"""
with mute_signals(post_save):
enrollment = ProgramEnrollment.objects.create(user=self.user, program=self.program)
with patch('search.tasks._remove_program_enrolled_user', autospec=True) as mocked:
enrollment.delete()
mocked.assert_called_once_with(enrollment)
enrollment.delete()
self.remove_program_enrolled_user_mock.assert_called_once_with(enrollment)
89 changes: 88 additions & 1 deletion mail/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,23 @@
import requests

from django.conf import settings
from django.contrib.auth.models import User
from django.core.exceptions import ImproperlyConfigured
from django.db import transaction
from django.db.utils import IntegrityError
from rest_framework import status

from mail.exceptions import SendBatchException
from mail.models import FinancialAidEmailAudit
from mail.models import (
AutomaticEmail,
FinancialAidEmailAudit,
SentAutomaticEmail,
)
from search.api import (
adjust_search_for_percolator,
search_percolate_queries,
)
from search.models import PercolateQuery


log = logging.getLogger(__name__)
Expand Down Expand Up @@ -248,3 +260,78 @@ def send_course_team_email( # pylint: disable=too-many-arguments
raise_for_status=raise_for_status,
)
return response


def send_automatic_emails(program_enrollment):
"""
Send all automatic emails which match the search criteria for a program enrollment
Args:
program_enrollment (ProgramEnrollment): A ProgramEnrollment
"""
percolate_queries = search_percolate_queries(program_enrollment.id)
automatic_emails = AutomaticEmail.objects.filter(
query__in=percolate_queries,
enabled=True,
).exclude(sentautomaticemail__user__programenrollment=program_enrollment)
user = program_enrollment.user
for automatic_email in automatic_emails:
try:
MailgunClient.send_individual_email(
automatic_email.email_subject,
automatic_email.email_body,
user.email,
sender_name=automatic_email.sender_name,
)
SentAutomaticEmail.objects.create(
user=user,
automatic_email=automatic_email,
)
except IntegrityError:
log.exception("IntegrityError: SentAutomaticEmail was likely already created")
except: # pylint: disable=bare-except
log.exception("Error sending mailgun mail for automatic email %s", automatic_email)


def add_automatic_email(original_search, email_subject, email_body, sender_name):
"""
Add an automatic email entry
Args:
original_search (Search):
The original search, which contains all back end filtering but no filtering specific to mail
or for percolated queries.
email_subject (str): Subject for the email
email_body (str): Body for the email
sender_name (str): The name of the sender of the email
"""
updated_search = adjust_search_for_percolator(original_search)
with transaction.atomic():
percolate_query = PercolateQuery.objects.create(
original_query=original_search.to_dict(),
query=updated_search.to_dict(),
)
return AutomaticEmail.objects.create(
query=percolate_query,
enabled=True,
email_subject=email_subject,
email_body=email_body,
sender_name=sender_name,
)


@transaction.atomic
def mark_emails_as_sent(automatic_email, emails):
"""
Mark users who have the given emails as sent
Args:
automatic_email (AutomaticEmail): An instance of AutomaticEmail
emails (iterable): An iterable of emails
"""
users = User.objects.filter(email__in=emails)
for user in users:
SentAutomaticEmail.objects.get_or_create(
user=user,
automatic_email=automatic_email,
)
119 changes: 117 additions & 2 deletions mail/api_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from django.core.exceptions import ImproperlyConfigured
from django.db.models.signals import post_save
from django.test import override_settings
from elasticsearch_dsl import Search
from factory.django import mute_signals
from requests import Response
from requests.exceptions import HTTPError
Expand All @@ -20,14 +21,26 @@
)

from dashboard.models import ProgramEnrollment
from dashboard.factories import ProgramEnrollmentFactory
from courses.factories import CourseFactory
from ecommerce.factories import CoursePriceFactory
from financialaid.factories import FinancialAidFactory
from mail.api import MailgunClient
from mail.exceptions import SendBatchException
from mail.models import FinancialAidEmailAudit
from mail.api import (
MailgunClient,
add_automatic_email,
mark_emails_as_sent,
send_automatic_emails,
)
from mail.models import (
AutomaticEmail,
FinancialAidEmailAudit,
SentAutomaticEmail,
)
from mail.factories import AutomaticEmailFactory
from mail.views_test import mocked_json
from profiles.factories import ProfileFactory
from search.api import adjust_search_for_percolator
from search.base import MockedESTestCase


Expand Down Expand Up @@ -487,3 +500,105 @@ def test_send_to_course_team_error(self, raise_for_status, mock_post):
assert response.raise_for_status.called is raise_for_status
assert response.status_code == HTTP_400_BAD_REQUEST
assert response.json() == {}


class AutomaticEmailTests(MockedESTestCase):
"""Tests regarding automatic emails"""

@classmethod
def setUpTestData(cls):
super().setUpTestData()

cls.program_enrollment_unsent = ProgramEnrollmentFactory.create()
cls.program_enrollment_sent = ProgramEnrollmentFactory.create()
cls.automatic_email = AutomaticEmailFactory.create(enabled=True)
cls.percolate_query = cls.automatic_email.query
cls.automatic_email_disabled = AutomaticEmailFactory.create(enabled=False)
cls.percolate_query_disabled = cls.automatic_email_disabled.query
SentAutomaticEmail.objects.create(
automatic_email=cls.automatic_email,
user=cls.program_enrollment_sent.user,
)

def test_send_automatic_emails(self):
"""send_automatic_emails should send emails to users which fit criteria and mark them so we don't send twice"""
with patch(
'mail.api.search_percolate_queries', autospec=True, return_value=[self.percolate_query],
) as mock_search_queries, patch('mail.api.MailgunClient') as mock_mailgun:
send_automatic_emails(self.program_enrollment_unsent)

mock_search_queries.assert_called_with(self.program_enrollment_unsent.id)
mock_mailgun.send_individual_email.assert_called_with(
self.automatic_email.email_subject,
self.automatic_email.email_body,
self.program_enrollment_unsent.user.email,
sender_name=self.automatic_email.sender_name,
)

def test_no_matching_query(self):
"""If there are no queries matching percolate we should do nothing"""
with patch(
'mail.api.search_percolate_queries', autospec=True, return_value=[],
) as mock_search_queries, patch('mail.api.MailgunClient') as mock_mailgun:
send_automatic_emails(self.program_enrollment_unsent)

mock_search_queries.assert_called_with(self.program_enrollment_unsent.id)
assert mock_mailgun.send_individual_email.called is False

def test_not_enabled(self):
"""If the automatic email is not enabled we should do nothing"""
with patch(
'mail.api.search_percolate_queries', autospec=True, return_value=[self.percolate_query_disabled],
) as mock_search_queries, patch('mail.api.MailgunClient') as mock_mailgun:
send_automatic_emails(self.program_enrollment_unsent)

mock_search_queries.assert_called_with(self.program_enrollment_unsent.id)
assert mock_mailgun.send_individual_email.called is False

def test_already_sent(self):
"""If a user was already sent email we should not send it again"""
with patch(
'mail.api.search_percolate_queries', autospec=True, return_value=[self.percolate_query],
) as mock_search_queries, patch('mail.api.MailgunClient') as mock_mailgun:
send_automatic_emails(self.program_enrollment_sent)

mock_search_queries.assert_called_with(self.program_enrollment_sent.id)
assert mock_mailgun.send_individual_email.called is False

def test_failed_send(self):
"""If we fail to send email to the first user we should still send it to the second"""

new_automatic = AutomaticEmailFactory.create(enabled=True)

with patch('mail.api.search_percolate_queries', autospec=True, return_value=[
self.percolate_query, new_automatic.query
]) as mock_search_queries, patch(
'mail.api.MailgunClient', send_individual_email=Mock(side_effect=[KeyError(), None])
) as mock_mailgun:
send_automatic_emails(self.program_enrollment_unsent)

mock_search_queries.assert_called_with(self.program_enrollment_unsent.id)
assert mock_mailgun.send_individual_email.call_count == 2

def test_add_automatic_email(self):
"""Add an AutomaticEmail entry with associated PercolateQuery"""
assert AutomaticEmail.objects.count() == 2
search_obj = Search.from_dict({"query": {"match": {}}})

new_automatic = add_automatic_email(search_obj, 'subject', 'body', 'sender')
assert AutomaticEmail.objects.count() == 3
assert new_automatic.sender_name == 'sender'
assert new_automatic.email_subject == 'subject'
assert new_automatic.email_body == 'body'
assert new_automatic.query.query == adjust_search_for_percolator(search_obj).to_dict()

def test_mark_emails_as_sent(self):
"""Mark emails as sent"""
emails = [
self.program_enrollment_unsent.user.email,
self.program_enrollment_sent.user.email,
]
mark_emails_as_sent(self.automatic_email, emails)
assert sorted(self.automatic_email.sentautomaticemail_set.values_list('user__email', flat=True)) == sorted(
emails
)
8 changes: 8 additions & 0 deletions mail/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,11 @@ def __init__(self, exception_pairs):
"""
super().__init__(exception_pairs)
self.exception_pairs = exception_pairs

@property
def failed_recipient_emails(self):
"""
Yields a list of recipient emails that we failed to send to
"""
for recipients, _ in self.exception_pairs:
yield from recipients
22 changes: 22 additions & 0 deletions mail/factories.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
"""Factories for mail models"""
from factory import (
Faker,
SubFactory,
)
from factory.django import DjangoModelFactory
from factory.fuzzy import FuzzyText

from mail.models import AutomaticEmail
from search.factories import PercolateQueryFactory


class AutomaticEmailFactory(DjangoModelFactory):
"""Factory for AutomaticEmail"""
query = SubFactory(PercolateQueryFactory)
enabled = Faker('boolean')
email_subject = FuzzyText()
email_body = FuzzyText()
sender_name = Faker('name')

class Meta:
model = AutomaticEmail
32 changes: 32 additions & 0 deletions mail/migrations/0005_sentautomaticemail.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.10.5 on 2017-03-06 15:34
from __future__ import unicode_literals

from django.conf import settings
from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

dependencies = [
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
('mail', '0004_automaticemail'),
]

operations = [
migrations.CreateModel(
name='SentAutomaticEmail',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('created_on', models.DateTimeField(auto_now_add=True)),
('updated_on', models.DateTimeField(auto_now=True)),
('automatic_email', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='mail.AutomaticEmail')),
('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)),
],
),
migrations.AlterUniqueTogether(
name='sentautomaticemail',
unique_together=set([('user', 'automatic_email')]),
),
]
Loading

0 comments on commit 9cfc3ab

Please sign in to comment.