Change mailing list subscription to when the user validates their email (#9384)

* Change mailing list subscription to when the user validates their email

This change also adds support for subscribing the user to an onboarding group.
Users in this group will get custom onboarding message(s),
to help them along on their usage of the platform.

* Update tests

* Update readthedocs/core/signals.py

Co-authored-by: Santos Gallegos <[email protected]>

* Migrate date NULL values to `timezone.now()`

* TODO note to migrate the signal into a Celery task

* Typo

* Darker files

* Update readthedocs/core/signals.py

Co-authored-by: Eric Holscher <[email protected]>

Co-authored-by: Manuel Kaufmann <[email protected]>
Co-authored-by: Santos Gallegos <[email protected]>

Author: 3 people

readthedocs/core/migrations/0012_add_newsletter_setting.py

@@ -0,0 +1,84 @@
+# Generated by Django 3.2.13 on 2022-06-28 21:52
+
+import django.utils.timezone
+import django_extensions.db.fields
+from django.db import migrations, models
+from django.db.models import Q
+
+
+def migrate_null_values(apps, schema_editor):
+    UserProfile = apps.get_model("core", "UserProfile")
+    query = Q(created__isnull=True) | Q(modified__isnull=True)
+    queryset = UserProfile.objects.filter(query)
+    now = django.utils.timezone.now()
+    for profile in queryset.iterator():
+        profile.created = profile.modified = now
+        profile.save()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("core", "0011_alter_historicaluser_first_name"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="historicaluserprofile",
+            name="mailing_list",
+            field=models.BooleanField(
+                default=False,
+                help_text="Subscribe to our mailing list, and get helpful onboarding suggestions.",
+            ),
+        ),
+        migrations.AddField(
+            model_name="userprofile",
+            name="mailing_list",
+            field=models.BooleanField(
+                default=False,
+                help_text="Subscribe to our mailing list, and get helpful onboarding suggestions.",
+            ),
+        ),
+        # Make NULL values for modified/created to be ``timezone.now()``
+        migrations.RunPython(migrate_null_values),
+        migrations.AlterField(
+            model_name="historicaluserprofile",
+            name="created",
+            field=django_extensions.db.fields.CreationDateTimeField(
+                auto_now_add=True,
+                default=django.utils.timezone.now,
+                verbose_name="created",
+            ),
+            preserve_default=False,
+        ),
+        migrations.AlterField(
+            model_name="historicaluserprofile",
+            name="modified",
+            field=django_extensions.db.fields.ModificationDateTimeField(
+                auto_now=True,
+                default=django.utils.timezone.now,
+                verbose_name="modified",
+            ),
+            preserve_default=False,
+        ),
+        migrations.AlterField(
+            model_name="userprofile",
+            name="created",
+            field=django_extensions.db.fields.CreationDateTimeField(
+                auto_now_add=True,
+                default=django.utils.timezone.now,
+                verbose_name="created",
+            ),
+            preserve_default=False,
+        ),
+        migrations.AlterField(
+            model_name="userprofile",
+            name="modified",
+            field=django_extensions.db.fields.ModificationDateTimeField(
+                auto_now=True,
+                default=django.utils.timezone.now,
+                verbose_name="modified",
+            ),
+            preserve_default=False,
+        ),
+    ]

readthedocs/core/models.py

@@ -6,10 +6,6 @@
 from django.urls import reverse
 from django.utils.translation import gettext
 from django.utils.translation import gettext_lazy as _
-from django_extensions.db.fields import (
-    CreationDateTimeField,
-    ModificationDateTimeField,
-)
 from django_extensions.db.models import TimeStampedModel
 from simple_history import register
 
@@ -20,33 +16,34 @@ class UserProfile(TimeStampedModel):
 
     """Additional information about a User."""
 
-    # TODO: Overridden from TimeStampedModel just to allow null values,
-    # remove after deploy.
-    created = CreationDateTimeField(
-        _('created'),
-        null=True,
-        blank=True,
-    )
-    modified = ModificationDateTimeField(
-        _('modified'),
-        null=True,
-        blank=True,
-    )
-
     user = AutoOneToOneField(
         User,
         verbose_name=_('User'),
         related_name='profile',
         on_delete=models.CASCADE,
     )
-    whitelisted = models.BooleanField(_('Whitelisted'), default=False)
-    banned = models.BooleanField(_('Banned'), default=False)
+    # Shown on the users profile
     homepage = models.CharField(_('Homepage'), max_length=100, blank=True)
+
+    # User configuration options
     allow_ads = models.BooleanField(
         _('See paid advertising'),
         help_text=_('If unchecked, you will still see community ads.'),
         default=True,
     )
+
+    mailing_list = models.BooleanField(
+        default=False,
+        help_text=_(
+            "Subscribe to our mailing list, and get helpful onboarding suggestions."
+        ),
+    )
+
+    # Internal tracking
+    whitelisted = models.BooleanField(_("Whitelisted"), default=False)
+    banned = models.BooleanField(_("Banned"), default=False)
+
+    # Model history
     history = ExtraHistoricalRecords()
 
     def __str__(self):

readthedocs/core/signals.py

@@ -1,6 +1,8 @@
 """Signal handling for core app."""
 
+import requests
 import structlog
+from allauth.account.signals import email_confirmed
 from corsheaders import signals
 from django.conf import settings
 from django.db.models.signals import pre_delete
@@ -11,6 +13,7 @@
 
 from readthedocs.analytics.utils import get_client_ip
 from readthedocs.builds.models import Version
+from readthedocs.core.models import UserProfile
 from readthedocs.core.unresolver import unresolve
 from readthedocs.organizations.models import Organization
 from readthedocs.projects.models import Project
@@ -33,6 +36,56 @@
 post_collectstatic = Signal()
 
 
+@receiver(email_confirmed)
+def process_email_confirmed(request, email_address, **kwargs):
+    """
+    Steps to take after a users email address is confirmed.
+
+    We currently:
+
+    * Subscribe the user to the mailing list if they set this in their signup.
+    * Add them to a drip campaign for new users in the new user group.
+    """
+
+    # email_address is an object
+    # https://github.com/pennersr/django-allauth/blob/6315e25/allauth/account/models.py#L15
+    user = email_address.user
+    profile = UserProfile.objects.filter(user=user).first()
+    if profile and profile.mailing_list:
+        # TODO: Unsubscribe users if they unset `mailing_list`.
+        log.bind(
+            email=email_address.email,
+            username=user.username,
+        )
+        log.info("Subscribing user to newsletter and onboarding group.")
+
+        # Try subscribing user to a group, then fallback to normal subscription API
+        url = settings.MAILERLITE_API_ONBOARDING_GROUP_URL
+        if not url:
+            url = settings.MAILERLITE_API_SUBSCRIBERS_URL
+
+        payload = {
+            "email": email_address.email,
+            "resubscribe": True,
+        }
+        headers = {
+            "X-MailerLite-ApiKey": settings.MAILERLITE_API_KEY,
+        }
+        try:
+            # TODO: migrate this signal to a Celery task since it has a `requests.post` on it.
+            resp = requests.post(
+                url,
+                json=payload,
+                headers=headers,
+                timeout=3,  # seconds
+            )
+            resp.raise_for_status()
+        except requests.Timeout:
+            log.warning("Timeout subscribing user to newsletter.")
+        except Exception:  # noqa
+            log.exception("Unknown error subscribing user to newsletter.")
+
+
 def _has_donate_app():
     """
     Check if the current app has the sustainability API.

readthedocs/core/tests/test_signup.py

@@ -1,62 +1,48 @@
-from http import HTTPStatus
 from unittest.mock import patch
 
-import requests
-import requests_mock
-
-from django.test import TestCase
+from allauth.account.models import EmailAddress
+from allauth.account.signals import email_confirmed
 from django.conf import settings
-
-from readthedocs.forms import SignupFormWithNewsletter
+from django.test import TestCase
 
 
 class TestNewsletterSignup(TestCase):
-    form_data = {
-        "email": "[email protected]",
-        "username": "test123",
-        "password1": "123456",
-        "password2": "123456",
-    }
-    form_data_plus_checkbox = form_data.copy()
-    form_data_plus_checkbox["receive_newsletter"] = True
-
-    @patch("readthedocs.forms.requests.post")
+    def setUp(self):
+        self.form_data = {
+            "email": "[email protected]",
+            "username": "test123",
+            "password1": "123456",
+            "password2": "123456",
+        }
+        self.form_data_plus_checkbox = self.form_data.copy()
+        self.form_data_plus_checkbox["receive_newsletter"] = True
+
+    @patch("readthedocs.core.signals.requests.post")
     def test_signup_without_checkbox_does_not_subscribe(self, mock_requests_post):
         response = self.client.post("/accounts/signup/", data=self.form_data)
-
-        assert response.status_code == HTTPStatus.FOUND
+        email_confirmed.send(
+            sender=None,
+            request=None,
+            email_address=EmailAddress.objects.get(email=self.form_data["email"]),
+        )
 
         mock_requests_post.assert_not_called()
 
-
-    @patch("readthedocs.forms.requests.post")
-    def test_signup_with_checkbox_calls_subscribe_api(self, mock_requests_post):
+    @patch("readthedocs.core.signals.requests.post")
+    def test_signup_calls_subscribe_api(self, mock_requests_post):
         response = self.client.post("/accounts/signup/", data=self.form_data_plus_checkbox)
-
-        assert response.status_code == HTTPStatus.FOUND, response
+        email_confirmed.send(
+            sender=None,
+            request=None,
+            email_address=EmailAddress.objects.get(email=self.form_data["email"]),
+        )
 
         mock_requests_post.assert_called_with(
             settings.MAILERLITE_API_SUBSCRIBERS_URL,
             json={
-                "email": self.form_data_plus_checkbox["email"],
+                "email": self.form_data["email"],
                 "resubscribe": True,
             },
             headers={"X-MailerLite-ApiKey": settings.MAILERLITE_API_KEY},
             timeout=3,
         )
-
-    @requests_mock.Mocker(kw='mock_request')
-    def test_signup_with_checkbox_succeeds_if_timeout(self, mock_request):
-        mock_request.post(settings.MAILERLITE_API_SUBSCRIBERS_URL, exc=requests.Timeout)
-
-        response = self.client.post("/accounts/signup/", data=self.form_data_plus_checkbox)
-
-        assert response.status_code == HTTPStatus.FOUND, response
-
-    @requests_mock.Mocker(kw='mock_request')
-    def test_signup_with_checkbox_succeeds_if_bad_response(self, mock_request):
-        mock_request.post(settings.MAILERLITE_API_SUBSCRIBERS_URL, status_code=400)
-
-        response = self.client.post("/accounts/signup/", data=self.form_data_plus_checkbox)
-
-        assert response.status_code == HTTPStatus.FOUND, response

readthedocs/forms.py

@@ -1,13 +1,11 @@
 """Community site-wide form overrides."""
 
 import structlog
-
-import requests
-
 from allauth.account.forms import SignupForm
-from django.conf import settings
 from django import forms
 
+from readthedocs.core.models import UserProfile
+
 log = structlog.get_logger(__name__)  # noqa
 
 
@@ -33,32 +31,9 @@ class SignupFormWithNewsletter(SignupForm):
     def save(self, request):
         user = super().save(request)
 
-        if self.cleaned_data.get("receive_newsletter"):
-            log.bind(
-                user_email=self.cleaned_data["email"],
-                user_username=user.username,
-            )
-            log.info('Subscribing user to newsletter.')
-
-            url = settings.MAILERLITE_API_SUBSCRIBERS_URL
-            payload = {
-                'email': self.cleaned_data["email"],
-                'resubscribe': True,
-            }
-            headers = {
-                'X-MailerLite-ApiKey': settings.MAILERLITE_API_KEY,
-            }
-            try:
-                resp = requests.post(
-                    url,
-                    json=payload,
-                    headers=headers,
-                    timeout=3,  # seconds
-                )
-                resp.raise_for_status()
-            except requests.Timeout:
-                log.warning('Timeout subscribing user to newsletter.')
-            except Exception:  # noqa
-                log.exception('Unknown error subscribing user to newsletter.')
+        receive_newsletter = self.cleaned_data.get("receive_newsletter")
+        profile, _ = UserProfile.objects.get_or_create(user=user)
+        profile.mailing_list = receive_newsletter
+        profile.save()
 
         return user

readthedocs/settings/base.py

@@ -974,6 +974,7 @@ def DOCKER_LIMITS(self):
 
     # MailerLite API for newsletter signups
     MAILERLITE_API_SUBSCRIBERS_URL = 'https://api.mailerlite.com/api/v2/subscribers'
+    MAILERLITE_API_ONBOARDING_GROUP_URL = None
     MAILERLITE_API_KEY = None
 
     RTD_EMBED_API_EXTERNAL_DOMAINS = [