diff --git a/readthedocs/core/migrations/0012_add_newsletter_setting.py b/readthedocs/core/migrations/0012_add_newsletter_setting.py new file mode 100644 index 00000000000..a38246f622a --- /dev/null +++ b/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, + ), + ] diff --git a/readthedocs/core/models.py b/readthedocs/core/models.py index 364023ef97a..42132529671 100644 --- a/readthedocs/core/models.py +++ b/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): diff --git a/readthedocs/core/signals.py b/readthedocs/core/signals.py index 08c6e312d3c..bf31ea957b7 100644 --- a/readthedocs/core/signals.py +++ b/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. diff --git a/readthedocs/core/tests/test_signup.py b/readthedocs/core/tests/test_signup.py index cadb809f4a3..6849c3c87df 100644 --- a/readthedocs/core/tests/test_signup.py +++ b/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": "test123@gmail.com", - "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": "test123@gmail.com", + "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 diff --git a/readthedocs/forms.py b/readthedocs/forms.py index 499886aeed6..2f4363164c5 100644 --- a/readthedocs/forms.py +++ b/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 diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index 1a9efbdac1b..f10d84b49ef 100644 --- a/readthedocs/settings/base.py +++ b/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 = [