Skip to content
3 changes: 1 addition & 2 deletions api_tests/notifications/test_notifications_db_transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
AuthUserFactory,
NotificationTypeFactory
)
from datetime import datetime
from osf.models import Notification, NotificationType, NotificationSubscription
from tests.utils import capture_notifications
from django.db import reset_queries, connection
Expand Down Expand Up @@ -54,5 +53,5 @@ def test_emit_frequency_none(self, user_one, test_notification_type):
)
assert Notification.objects.filter(
subscription__notification_type=test_notification_type,
sent=datetime(1000, 1, 1)
fake_sent=True
).exists()
23 changes: 13 additions & 10 deletions notifications/tasks.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import itertools
from calendar import monthrange
from datetime import date, datetime
from datetime import date
from django.contrib.contenttypes.models import ContentType
from django.db import connection
from django.utils import timezone
Expand Down Expand Up @@ -36,7 +36,8 @@ def safe_render_notification(notifications, email_task):
# Mark notifications that failed to render as fake sent
# Use 1000/12/31 to distinguish itself from another type of fake sent 1000/1/1
log_message(f'Error rendering notification, mark as fake sent: [notification_id={notification.id}]')
notification.sent = datetime(1000, 12, 31)
notification.mark_sent()
notification.fake_sent = True
notification.save()
continue

Expand Down Expand Up @@ -102,9 +103,10 @@ def send_user_email_task(self, user_id, notification_ids, **kwargs):
notifications_qs = notifications_qs.exclude(id__in=failed_notifications)

if not rendered_notifications:
email_task.status = 'SUCCESS'
if email_task.error_message:
logger.error(f'Partial success for send_user_email_task for user {user_id}. Task id: {self.request.id}. Errors: {email_task.error_message}')
email_task.status = 'SUCCESS'
email_task.status = 'PARTIAL_SUCCESS'
email_task.save()
return

Expand All @@ -123,10 +125,10 @@ def send_user_email_task(self, user_id, notification_ids, **kwargs):
notifications_qs.update(sent=timezone.now())

email_task.status = 'SUCCESS'
email_task.save()

if email_task.error_message:
logger.error(f'Partial success for send_user_email_task for user {user_id}. Task id: {self.request.id}. Errors: {email_task.error_message}')
email_task.status = 'PARTIAL_SUCCESS'
email_task.save()

except Exception as e:
retry_count = self.request.retries
Expand Down Expand Up @@ -177,9 +179,10 @@ def send_moderator_email_task(self, user_id, notification_ids, provider_content_
notifications_qs = notifications_qs.exclude(id__in=failed_notifications)

if not rendered_notifications:
email_task.status = 'SUCCESS'
if email_task.error_message:
logger.error(f'Partial success for send_moderator_email_task for user {user_id}. Task id: {self.request.id}. Errors: {email_task.error_message}')
email_task.status = 'SUCCESS'
email_task.status = 'PARTIAL_SUCCESS'
email_task.save()
return

Expand Down Expand Up @@ -211,10 +214,10 @@ def send_moderator_email_task(self, user_id, notification_ids, provider_content_
current_admins = provider.get_group('admin')
if current_admins is None or not current_admins.user_set.filter(id=user.id).exists():
log_message(f"User is not a moderator for provider {provider._id} - notifications will be marked as sent.")
email_task.status = 'FAILURE'
email_task.status = 'AUTO_FIXED'
email_task.error_message = f'User is not a moderator for provider {provider._id}'
email_task.save()
notifications_qs.update(sent=datetime(1000, 1, 1))
notifications_qs.update(sent=timezone.now(), fake_sent=True)
return

additional_context = {}
Expand Down Expand Up @@ -274,10 +277,10 @@ def send_moderator_email_task(self, user_id, notification_ids, provider_content_
notifications_qs.update(sent=timezone.now())

email_task.status = 'SUCCESS'
email_task.save()

if email_task.error_message:
logger.error(f'Partial success for send_moderator_email_task for user {user_id}. Task id: {self.request.id}. Errors: {email_task.error_message}')
email_task.status = 'PARTIAL_SUCCESS'
email_task.save()

except Exception as e:
retry_count = self.request.retries
Expand Down
2 changes: 1 addition & 1 deletion osf/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ class EmailTaskAdmin(admin.ModelAdmin):

@admin.register(Notification)
class NotificationAdmin(admin.ModelAdmin):
list_display = ('user', 'notification_type_name', 'sent', 'seen')
list_display = ('user', 'notification_type_name', 'sent', 'fake_sent')
list_filter = ('sent',)
search_fields = ('subscription__notification_type__name', 'subscription__user__username')
list_per_page = 50
Expand Down
34 changes: 34 additions & 0 deletions osf/migrations/0036_notification_fake_sent_and_more.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Generated by Django 4.2.15 on 2026-01-22 10:29

from django.db import migrations, models
import osf.utils.fields


class Migration(migrations.Migration):

dependencies = [
('contenttypes', '0002_remove_content_type_name'),
('osf', '0035_merge_20251215_1451'),
]

operations = [
migrations.AddField(
model_name='notification',
name='fake_sent',
field=models.BooleanField(default=False),
),
migrations.AddField(
model_name='osfuser',
name='no_login_email_last_sent',
field=osf.utils.fields.NonNaiveDateTimeField(blank=True, null=True),
),
migrations.AlterField(
model_name='emailtask',
name='status',
field=models.CharField(choices=[('PENDING', 'Pending'), ('NO_USER_FOUND', 'No User Found'), ('USER_DISABLED', 'User Disabled'), ('STARTED', 'Started'), ('SUCCESS', 'Success'), ('FAILURE', 'Failure'), ('RETRY', 'Retry'), ('PARTIAL_SUCCESS', 'Partial Success'), ('AUTO_FIXED', 'Auto Fixed')], default='PENDING', max_length=20),
),
migrations.AlterUniqueTogether(
name='notificationsubscription',
unique_together={('notification_type', 'user', 'content_type', 'object_id', '_is_digest')},
),
]
2 changes: 2 additions & 0 deletions osf/models/email_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ class EmailTask(models.Model):
('SUCCESS', 'Success'),
('FAILURE', 'Failure'),
('RETRY', 'Retry'),
('PARTIAL_SUCCESS', 'Partial Success'),
('AUTO_FIXED', 'Auto Fixed'),
)

task_id = models.CharField(max_length=255, unique=True)
Expand Down
1 change: 1 addition & 0 deletions osf/models/notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class Notification(models.Model):
sent = models.DateTimeField(null=True, blank=True)
seen = models.DateTimeField(null=True, blank=True)
created = models.DateTimeField(auto_now_add=True)
fake_sent = models.BooleanField(default=False)

def send(
self,
Expand Down
6 changes: 4 additions & 2 deletions osf/models/notification_subscription.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import logging
from datetime import datetime
from django.utils import timezone
from django.db import models
from django.contrib.contenttypes.fields import GenericForeignKey
from django.contrib.contenttypes.models import ContentType
Expand Down Expand Up @@ -59,6 +59,7 @@ class Meta:
verbose_name = 'Notification Subscription'
verbose_name_plural = 'Notification Subscriptions'
db_table = 'osf_notificationsubscription_v2'
unique_together = ('notification_type', 'user', 'content_type', 'object_id', '_is_digest')

def emit(
self,
Expand Down Expand Up @@ -126,7 +127,8 @@ def emit(
Notification.objects.create(
subscription=self,
event_context=event_context,
sent=None if self.message_frequency != 'none' else datetime(1000, 1, 1),
sent=timezone.now() if self.message_frequency == 'none' else None,
fake_sent=True if self.message_frequency == 'none' else False,
)

@property
Expand Down
1 change: 1 addition & 0 deletions osf/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ class OSFUser(DirtyFieldsMixin, GuidMixin, BaseModel, AbstractBaseUser, Permissi
# }

email_last_sent = NonNaiveDateTimeField(null=True, blank=True)
no_login_email_last_sent = NonNaiveDateTimeField(null=True, blank=True)
change_password_last_attempt = NonNaiveDateTimeField(null=True, blank=True)
# Logs number of times user attempted to change their password where their
# old password was invalid
Expand Down
20 changes: 10 additions & 10 deletions scripts/triggered_mails.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,31 +60,29 @@ def find_inactive_users_without_enqueued_or_sent_no_login():
Match your original inactivity rules, but exclude users who already have a no_login EmailTask
either pending, started, retrying, or already sent successfully.
"""
now = timezone.now()

# Subquery: Is there already a not-yet-failed/aborted EmailTask for this user with our prefix?
existing_no_login = EmailTask.objects.filter(
user_id=OuterRef('pk'),
task_id__startswith=NO_LOGIN_PREFIX,
status__in=['PENDING', 'STARTED', 'RETRY', 'SUCCESS'],
)
cutoff_query = Q(date_last_login__gte=settings.NO_LOGIN_EMAIL_CUTOFF - settings.NO_LOGIN_WAIT_TIME) if settings.NO_LOGIN_EMAIL_CUTOFF else Q()
base_q = OSFUser.objects.filter(
cutoff_query,
is_active=True,
).filter(
Q(
date_last_login__lt=timezone.now() - settings.NO_LOGIN_WAIT_TIME,
date_last_login__lt=now - settings.NO_LOGIN_WAIT_TIME,
# NOT tagged osf4m
) & ~Q(tags__name='osf4m')
|
Q(
date_last_login__lt=timezone.now() - settings.NO_LOGIN_OSF4M_WAIT_TIME,
date_last_login__lt=now - settings.NO_LOGIN_OSF4M_WAIT_TIME,
tags__name='osf4m'
)
).distinct()

# Exclude users who already have a task for this email type
return base_q.annotate(_has_task=Exists(existing_no_login)).filter(_has_task=False)
# 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)
)


@celery_app.task(name='scripts.triggered_no_login_email')
Expand Down Expand Up @@ -133,6 +131,8 @@ def send_no_login_email(email_task_id: int):
)
email_task.status = 'SUCCESS'
email_task.save()
user.no_login_email_last_sent = timezone.now()
user.save()

except Exception as exc: # noqa: BLE001
logger.exception(f'EmailTask {email_task.id}: error while sending')
Expand Down
12 changes: 3 additions & 9 deletions tests/test_triggered_mails.py
Original file line number Diff line number Diff line change
Expand Up @@ -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=12)
return timezone.now() - timedelta(weeks=52)


def _recent_time():
Expand Down Expand Up @@ -114,21 +114,15 @@ def test_finder_returns_two_inactive_when_none_queued(self):
assert ids == {u1.id, u2.id}

def test_finder_excludes_users_with_existing_task(self):
"""Inactive users but one already has a no_login EmailTask -> excluded."""
"""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()

# Pretend u2 already had this email flow (SUCCESS qualifies for exclusion)
EmailTask.objects.create(
task_id=f"{NO_LOGIN_PREFIX}existing-success",
user=u2,
status='SUCCESS',
)

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
Expand Down
Loading