[ENG-10535] Update the no_login mail send conditions

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')

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,29 +119,45 @@ 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()
 
         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 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()
 
         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.date_last_login = timezone.now() - settings.NO_LOGIN_WAIT_TIME
         u2.save()
 
         users = list(find_inactive_users_without_enqueued_or_sent_no_login())