diff --git a/scripts/triggered_mails.py b/scripts/triggered_mails.py index 81da30645d3..13461c628a9 100644 --- a/scripts/triggered_mails.py +++ b/scripts/triggered_mails.py @@ -2,7 +2,7 @@ import uuid from django.db import transaction -from django.db.models import Q, Exists, OuterRef +from django.db.models import Q, F from django.utils import timezone from framework.celery_tasks import app as celery_app @@ -81,7 +81,10 @@ def find_inactive_users_without_enqueued_or_sent_no_login(): # Exclude users who have already received a no-login email recently return base_q.filter( Q(no_login_email_last_sent__isnull=True) | - Q(no_login_email_last_sent__lt=now - settings.NO_LOGIN_WAIT_TIME) + ( + Q(no_login_email_last_sent__lt=now - settings.NO_LOGIN_WAIT_TIME) & + Q(no_login_email_last_sent__lt=F('date_last_login')) + ) ) diff --git a/tests/test_triggered_mails.py b/tests/test_triggered_mails.py index 30f6159ed8c..7e0af23e023 100644 --- a/tests/test_triggered_mails.py +++ b/tests/test_triggered_mails.py @@ -127,6 +127,27 @@ def test_finder_excludes_users_with_existing_task(self): ids = {u.id for u in users} assert ids == {u1.id} # u2 excluded because of existing task + def test_finder_excludes_users_logged_in_before_no_login_email_last_sent(self): + """Inactive users but one already has a no_login_email_last_sent -> excluded.""" + u1 = UserFactory(fullname='Jalen Hurts') + u2 = UserFactory(fullname='Jason Kelece') + u1.date_last_login = _inactive_time() - timedelta(weeks=2) + u2.date_last_login = _inactive_time() - timedelta(weeks=2) # old login, but still inactive + u2.no_login_email_last_sent = timezone.now() - timedelta(weeks=53) # old enough to be eligible if not for the login + u1.save() + u2.save() + + users = list(find_inactive_users_without_enqueued_or_sent_no_login()) + ids = {u.id for u in users} + assert ids == {u1.id} # u2 excluded because last login was before email was sent + + u2.date_last_login = timezone.now() - timedelta(weeks=53) + u2.save() + + users = list(find_inactive_users_without_enqueued_or_sent_no_login()) + ids = {u.id for u in users} + assert ids == {u1.id, u2.id} # u2 included again because they logged in after the email was sent + @override_switch(features.ENABLE_NO_LOGIN_EMAILS, active=False) def test_disable_task(self): u1 = UserFactory(fullname='Jalen Hurts')