Send individual emails to each notified user (#1429)

Fixes #1428

Previously, we sent a single email with all recipients in the `bcc`
field, and no one in the `to` field. Mailgun does not support this.

This PR changes that behavior to send an email to each user using
Django's `send_messages` (plural) method.

Note: I wasn't able to confirm this in the Mailgun documentation, but I
assume this should not have any cost implications. Sending a "single"
email with 100 `bcc` recipients should cost the same as sending 100
emails each with a single `to` recipient.

Another note: I did not test performance of this with many recipients.
Probably all of our notification emails should be getting sent in a
Celery task... probably a future optimization.

---------

Co-authored-by: Greg Kaleka <gkaleka@energy-solution.com>

news/notifications.py

@@ -1,5 +1,5 @@
 from django.contrib.auth import get_user_model
-from django.core.mail import EmailMessage, send_mail
+from django.core.mail import EmailMessage, get_connection, send_mail
 from django.template import Template, Context
 from django.urls import reverse
 from django.utils.safestring import mark_safe
@@ -113,9 +113,13 @@ def send_email_news_posted(request, entry):
         )
     )
     subject = "Boost.org: News entry posted"
-    return EmailMessage(
-        subject=subject,
-        body=body,
-        from_email=None,
-        bcc=recipient_list,
-    ).send()
+    messages = [
+        EmailMessage(
+            subject=subject,
+            body=body,
+            from_email=None,
+            to=[user_email],
+        )
+        for user_email in recipient_list
+    ]
+    return get_connection().send_messages(messages)

news/tests/test_notifications.py

@@ -146,7 +146,7 @@ def test_send_email_news_posted_no_other_user_allows(
     entry = make_entry(model_class, approved=False)
     request = rf.get("")
 
-    for i in range(3):
+    for _ in range(3):
         u = make_user()
         u.preferences.allow_notification_others_news_posted = []
         u.preferences.save()
@@ -161,35 +161,34 @@ def test_send_email_news_posted_many_users(rf, tp, make_entry, make_user, model_
     entry = make_entry(model_class, approved=False)
     request = rf.get("")
 
-    # user does not allow notifications
-    make_user(email="u1@example.com", allow_notification_others_news_posted=[])
-    # allows nofitications for all news type
-    make_user(
-        email="u2@example.com",
-        allow_notification_others_news_posted=[m.news_type for m in NEWS_MODELS],
-    )
-    # allows only for the same type as entry
-    make_user(email="u3@example.com", allow_notification_others_news_posted=[entry.tag])
-    # allows for any other type except entry's
-    make_user(
-        email="u4@example.com",
-        allow_notification_others_news_posted=[
+    recipients = {
+        # user does not allow notifications
+        "u1@example.com": [],
+        # allows nofitications for all news type
+        "u2@example.com": [m.news_type for m in NEWS_MODELS],
+        # allows only for the same type as entry
+        "u3@example.com": [entry.tag],
+        # allows for any other type except entry's
+        "u4@example.com": [
             m.news_type for m in NEWS_MODELS if m.news_type != entry.tag
         ],
-    )
+    }
+
+    for email, notifications in recipients.items():
+        make_user(email=email, allow_notification_others_news_posted=notifications)
 
     with tp.assertNumQueriesLessThan(2, verbose=True):
         result = send_email_news_posted(request, entry)
 
-    assert result == 1
-    assert len(mail.outbox) == 1
-    msg = mail.outbox[0]
-    assert "news entry posted" in msg.subject.lower()
-    assert entry.title in msg.body
-    assert escape(entry.title) not in msg.body
-    assert entry.author.email not in msg.body  # never disclose author email!
-    assert request.build_absolute_uri(entry.get_absolute_url()) in msg.body
-    assert request.build_absolute_uri(tp.reverse("profile-account")) in msg.body
-    assert msg.to == []  # do not share all emails among all recipients
-    assert msg.bcc == ["u2@example.com", "u3@example.com"]
-    assert msg.recipients() == ["u2@example.com", "u3@example.com"]
+    # We should send two emails - one to u2, one to u3
+    assert result == len(mail.outbox) == 2
+    for usr_num, msg in enumerate(mail.outbox, start=2):
+        assert "news entry posted" in msg.subject.lower()
+        assert entry.title in msg.body
+        assert escape(entry.title) not in msg.body
+        assert entry.author.email not in msg.body  # never disclose author email!
+        assert request.build_absolute_uri(entry.get_absolute_url()) in msg.body
+        assert request.build_absolute_uri(tp.reverse("profile-account")) in msg.body
+        # do not share all emails among all recipients
+        assert msg.to == [f"u{usr_num}@example.com"]
+        assert msg.recipients() == [f"u{usr_num}@example.com"]

news/tests/test_views.py

@@ -556,22 +556,21 @@ def test_news_approve_post(
     entry = make_entry(model_class, approved=False)
     url_params = ("news-approve", entry.slug)
 
-    # user does not allow notifications
-    make_user(email="u1@example.com", allow_notification_others_news_posted=[])
-    # allows nofitications for all news type
-    make_user(
-        email="u2@example.com",
-        allow_notification_others_news_posted=[m.news_type for m in NEWS_MODELS],
-    )
-    # allows only for the same type as entry
-    make_user(email="u3@example.com", allow_notification_others_news_posted=[entry.tag])
-    # allows for any other type except entry's
-    make_user(
-        email="u4@example.com",
-        allow_notification_others_news_posted=[
+    recipients = {
+        # user does not allow notifications
+        "u1@example.com": [],
+        # allows nofitications for all news type
+        "u2@example.com": [m.news_type for m in NEWS_MODELS],
+        # allows only for the same type as entry
+        "u3@example.com": [entry.tag],
+        # allows for any other type except entry's
+        "u4@example.com": [
             m.news_type for m in NEWS_MODELS if m.news_type != entry.tag
         ],
-    )
+    }
+
+    for email, notifications in recipients.items():
+        make_user(email=email, allow_notification_others_news_posted=notifications)
 
     # regular users would still get a 403 on POST
     with tp.login(regular_user):
@@ -591,13 +590,13 @@ def test_news_approve_post(
     assert entry.moderator == moderator_user
     assert before <= entry.approved_at <= after
     assert before <= entry.modified_at <= after
-    # email was sent, one to author, one to other users
-    assert len(mail.outbox) == 2
+    # email was sent, one to author, one to each of the two other matching users
+    assert len(mail.outbox) == 3
     # approval email to author
     actual = mail.outbox[0]
     # render the same email using the notifications' method to assert equality
     assert send_email_news_approved(response.wsgi_request, entry) == 1
-    expected = mail.outbox[2]
+    expected = mail.outbox[3]
     assert actual.subject == expected.subject
     assert actual.body == expected.body
     assert actual.from_email == expected.from_email
@@ -605,8 +604,9 @@ def test_news_approve_post(
     # news posted email to other users
     actual = mail.outbox[1]
     # render the same email using the notifications' method to assert equality
-    assert send_email_news_posted(response.wsgi_request, entry) == 1
-    expected = mail.outbox[3]
+    # here, only two emails are sent - one to each matching user
+    assert send_email_news_posted(response.wsgi_request, entry) == 2
+    expected = mail.outbox[4]
     assert actual.subject == expected.subject
     assert actual.body == expected.body
     assert actual.from_email == expected.from_email