Change mailing list subscription to when the user validates their email

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,55 @@
 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:
+        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 = [