From d5a9e39876ac1ba6a46afb7b667356830d467f57 Mon Sep 17 00:00:00 2001 From: sarayourfriend Date: Wed, 8 May 2024 09:56:21 +1000 Subject: [PATCH] Fix send email query (#4281) * Fix send email query * Fix doc strings --- .../commands/sendapimoveannouncement.py | 25 ++++++++++--------- api/test/factory/models/oauth2.py | 12 +++++++++ .../commands/test_sendapimoveannouncement.py | 11 ++++---- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/api/api/management/commands/sendapimoveannouncement.py b/api/api/management/commands/sendapimoveannouncement.py index 595511a0bd6..c15233a9270 100644 --- a/api/api/management/commands/sendapimoveannouncement.py +++ b/api/api/management/commands/sendapimoveannouncement.py @@ -6,7 +6,7 @@ import django_redis from django_tqdm import BaseCommand -from api.models.oauth import OAuth2Verification +from api.models.oauth import OAuth2Registration, ThrottledApplication message = """ @@ -27,14 +27,12 @@ class Command(BaseCommand): - help = "Resends verification emails for unverified Oauth applications." + help = "Send an email announcing the Openverse API moving to api.openverse.org." """ - This command sends a corrected oauth verification email to users who have - not yet verified their Openverse oauth applications. A previous version sent - an email with an incorrectly formatted link. + This command sends an email announcing the API move to api.openverse.org. It stores a cache of successfully sent emails in Redis, so running it multiple - times (in case of failure) should not be an issue. + times (in case of failure) will not be an issue. """ processed_key = "apimoveannouncement:processed" @@ -57,15 +55,18 @@ def handle(self, *args, **options): email.decode("utf-8") for email in redis.smembers(self.processed_key) ] - # Join from verification because it has a reference to both the application and the email unsent_email_addresses = ( - OAuth2Verification.objects.filter( - # Only send to verified email addresses to avoid bounce rate increase - associated_application__verified=True - ) - .exclude( + OAuth2Registration.objects.exclude( email__in=sent_emails, ) + .filter( + # `name` is unique indexed on OAuth2Registration, so should be safe to query + name__in=( + ThrottledApplication.objects.filter(verified=True).values_list( + "name", flat=True + ) + ) + ) .distinct("email") .values_list("email", flat=True) ) diff --git a/api/test/factory/models/oauth2.py b/api/test/factory/models/oauth2.py index 22b3a697446..d84f912da4a 100644 --- a/api/test/factory/models/oauth2.py +++ b/api/test/factory/models/oauth2.py @@ -33,6 +33,18 @@ class Meta: description = Faker("catch_phrase") email = Faker("email") + @factory.post_generation + def application(obj, create, extracted, **kwargs): + # Only create the application if creating, and either + # `application=True` or some `application__*` kwargs were passed + if not (create and (extracted or kwargs)): + return + + return ThrottledApplicationFactory.create( + name=obj.name, + verified=kwargs.get("verified", False), + ) + class OAuth2VerificationFactory(DjangoModelFactory): class Meta: diff --git a/api/test/unit/management/commands/test_sendapimoveannouncement.py b/api/test/unit/management/commands/test_sendapimoveannouncement.py index 63f027990e7..617259394a1 100644 --- a/api/test/unit/management/commands/test_sendapimoveannouncement.py +++ b/api/test/unit/management/commands/test_sendapimoveannouncement.py @@ -6,7 +6,7 @@ import pytest from test.factory.models.oauth2 import ( - OAuth2VerificationFactory, + OAuth2RegistrationFactory, ) @@ -53,10 +53,11 @@ def call_cmd(**options): def make_emails(count: int, *, verified: bool): - verifications = OAuth2VerificationFactory.create_batch( - count, associated_application__verified=verified + registrations = OAuth2RegistrationFactory.create_batch( + count, + application__verified=verified, ) - return [v.email for v in verifications] + return [r.email for r in registrations] @pytest.mark.django_db @@ -90,7 +91,7 @@ def test_should_not_send_to_unverified_emails(captured_emails): @pytest.mark.django_db def test_should_not_send_to_email_twice(captured_emails): emails = make_emails(10, verified=True) - OAuth2VerificationFactory.create( + OAuth2RegistrationFactory.create( email=emails[0], )