Skip to content

Commit

Permalink
Fix send email query (#4281)
Browse files Browse the repository at this point in the history
* Fix send email query

* Fix doc strings
  • Loading branch information
sarayourfriend authored May 7, 2024
1 parent 0a7d31b commit d5a9e39
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 17 deletions.
25 changes: 13 additions & 12 deletions api/api/management/commands/sendapimoveannouncement.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = """
Expand All @@ -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"
Expand All @@ -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)
)
Expand Down
12 changes: 12 additions & 0 deletions api/test/factory/models/oauth2.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import pytest

from test.factory.models.oauth2 import (
OAuth2VerificationFactory,
OAuth2RegistrationFactory,
)


Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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],
)

Expand Down

0 comments on commit d5a9e39

Please sign in to comment.