From 7aff4acb40c91b9f4b2983548762871fd3a0298a Mon Sep 17 00:00:00 2001
From: Ostap Zherebetskyi <ozherebetskyi@exoft.net>
Date: Thu, 5 Mar 2026 15:06:29 +0200
Subject: [PATCH] Refactor no-login email user filtering to exclude those with
 pending EmailTasks and recent no-login emails

---
 scripts/triggered_mails.py    |  8 +++++++-
 tests/test_triggered_mails.py | 30 +++++++++++++++++++++++-------
 2 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/scripts/triggered_mails.py b/scripts/triggered_mails.py
index 13461c628a9..389a82c257f 100644
--- a/scripts/triggered_mails.py
+++ b/scripts/triggered_mails.py
@@ -79,13 +79,19 @@ def find_inactive_users_without_enqueued_or_sent_no_login():
     ).distinct()
 
     # Exclude users who have already received a no-login email recently
-    return base_q.filter(
+    base_q = 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=F('date_last_login'))
         )
     )
+    # Exlude users who already have a pending/started/retrying EmailTask for no-login
+    base_q = base_q.exclude(
+        emailtask__task_id__startswith=NO_LOGIN_PREFIX,
+        emailtask__status__in=['PENDING', 'STARTED', 'RETRY']
+    )
+    return base_q
 
 
 @celery_app.task(name='scripts.triggered_no_login_email')
diff --git a/tests/test_triggered_mails.py b/tests/test_triggered_mails.py
index 7e0af23e023..33562878f8a 100644
--- a/tests/test_triggered_mails.py
+++ b/tests/test_triggered_mails.py
@@ -3,7 +3,7 @@
 from unittest import mock
 from waffle.testutils import override_switch
 from osf import features
-
+from website import settings
 from django.utils import timezone
 
 from tests.base import OsfTestCase
@@ -22,7 +22,7 @@
 def _inactive_time():
     """Make a timestamp that is definitely 'inactive' regardless of threshold settings."""
     # Very conservative: 12 weeks ago
-    return timezone.now() - timedelta(weeks=52)
+    return timezone.now() - settings.NO_LOGIN_WAIT_TIME
 
 
 def _recent_time():
@@ -119,7 +119,9 @@ def test_finder_excludes_users_with_existing_task(self):
         u2 = UserFactory(fullname='Jason Kelece')
         u1.date_last_login = _inactive_time()
         u2.date_last_login = _inactive_time()
-        u2.no_login_email_last_sent = timezone.now()
+        EmailTask.objects.create(
+            user=u2, task_id=f'{NO_LOGIN_PREFIX}uuid4', status='PENDING',
+        )
         u1.save()
         u2.save()
 
@@ -127,13 +129,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_with_recent_no_login_email(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()
+        u2.date_last_login = _inactive_time()
+        u2.no_login_email_last_sent = timezone.now()
+        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 of recent email
+
     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.date_last_login = _inactive_time()
+        u2.date_last_login = _inactive_time()
+        u2.no_login_email_last_sent = timezone.now() - settings.NO_LOGIN_WAIT_TIME  # old enough to be eligible if not for the login
         u1.save()
         u2.save()
 
@@ -141,7 +157,7 @@ def test_finder_excludes_users_logged_in_before_no_login_email_last_sent(self):
         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.date_last_login = timezone.now() - settings.NO_LOGIN_WAIT_TIME
         u2.save()
 
         users = list(find_inactive_users_without_enqueued_or_sent_no_login())