Internal
Public Access
Delay overdue notification an hour, retroactively apply reminder setting
Two refinements from live testing of the reminder feature: - Due-time tasks scheduled "due now" and "overdue" at the exact same moment. Task._due_and_overdue_moments() now delays "overdue" by an hour so they don't arrive together. The overdue badge/styling elsewhere is unaffected - only this notification's timing changes. - Task.reschedule_reminders() only captures user.default_reminder_minutes at the moment a task's own due_date/due_time is set, so changing the profile setting didn't reach tasks whose due date was already set. User.save() now detects a change to that setting and calls the new User.reschedule_reminder_notifications(), which recomputes the "before due" reminder on active due tasks that don't have their own explicit reminder_at override. 8 new/updated tests cover the overdue delay and the retroactive rescheduling (including that unrelated profile saves and tasks with an explicit reminder_at are left untouched). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
a27fcc4c69
commit
f92ce7a3d4
+88
-3
@@ -41,7 +41,7 @@ class RescheduleRemindersTests(TestCase):
|
||||
|
||||
expected_due = django_timezone.make_aware(datetime(2026, 1, 15, 17, 0, 0))
|
||||
self.assertEqual(due_soon.remind_at, expected_due)
|
||||
self.assertEqual(overdue.remind_at, expected_due)
|
||||
self.assertEqual(overdue.remind_at, expected_due + timedelta(hours=1))
|
||||
self.assertEqual(before_due.remind_at, expected_due - timedelta(minutes=30))
|
||||
|
||||
def test_date_only_task_only_gets_overdue_reminder_next_day(self):
|
||||
@@ -107,6 +107,72 @@ class RescheduleRemindersTests(TestCase):
|
||||
self.assertEqual(ScheduledReminder.objects.filter(task=task, is_sent=False).count(), 3)
|
||||
|
||||
|
||||
class DefaultReminderMinutesChangeTests(TestCase):
|
||||
"""
|
||||
Task.reschedule_reminders() only captures user.default_reminder_minutes
|
||||
at the moment a task's own due_date/due_time is set - changing the
|
||||
profile setting afterward doesn't reach existing tasks on its own.
|
||||
User.reschedule_reminder_notifications(), triggered from User.save()
|
||||
when the setting changes, is what makes that retroactive.
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
self.user = User.objects.create_user(
|
||||
username='reminderuser2', email='reminderuser2@example.com', password='testpass123',
|
||||
default_reminder_minutes=30,
|
||||
)
|
||||
|
||||
def test_changing_default_minutes_reschedules_before_due_reminder(self):
|
||||
task = Task.objects.create(
|
||||
user=self.user, title='Task', due_date=date(2026, 1, 15), due_time=dt_time(17, 0, 0),
|
||||
)
|
||||
due_moment = django_timezone.make_aware(datetime(2026, 1, 15, 17, 0, 0))
|
||||
before_due = ScheduledReminder.objects.get(task=task, reminder_type='reminder')
|
||||
self.assertEqual(before_due.remind_at, due_moment - timedelta(minutes=30))
|
||||
|
||||
self.user.default_reminder_minutes = 15
|
||||
self.user.save()
|
||||
|
||||
before_due = ScheduledReminder.objects.get(task=task, reminder_type='reminder')
|
||||
self.assertEqual(before_due.remind_at, due_moment - timedelta(minutes=15))
|
||||
|
||||
def test_task_with_explicit_reminder_at_is_left_alone(self):
|
||||
custom_reminder = django_timezone.make_aware(datetime(2026, 1, 15, 9, 0, 0))
|
||||
task = Task.objects.create(
|
||||
user=self.user, title='Task', due_date=date(2026, 1, 15), due_time=dt_time(17, 0, 0),
|
||||
reminder_at=custom_reminder,
|
||||
)
|
||||
|
||||
self.user.default_reminder_minutes = 15
|
||||
self.user.save()
|
||||
|
||||
before_due = ScheduledReminder.objects.get(task=task, reminder_type='reminder')
|
||||
self.assertEqual(before_due.remind_at, custom_reminder)
|
||||
|
||||
def test_completed_task_is_not_touched(self):
|
||||
task = Task.objects.create(
|
||||
user=self.user, title='Task', due_date=date(2026, 1, 15), due_time=dt_time(17, 0, 0), status='completed',
|
||||
)
|
||||
self.assertFalse(ScheduledReminder.objects.filter(task=task, is_sent=False).exists())
|
||||
|
||||
self.user.default_reminder_minutes = 15
|
||||
self.user.save()
|
||||
|
||||
self.assertFalse(ScheduledReminder.objects.filter(task=task, is_sent=False).exists())
|
||||
|
||||
def test_unrelated_profile_field_save_does_not_reschedule(self):
|
||||
task = Task.objects.create(
|
||||
user=self.user, title='Task', due_date=date(2026, 1, 15), due_time=dt_time(17, 0, 0),
|
||||
)
|
||||
original_id = ScheduledReminder.objects.get(task=task, reminder_type='reminder').id
|
||||
|
||||
self.user.first_name = 'Changed'
|
||||
self.user.save()
|
||||
|
||||
# Same row (not deleted and recreated by reschedule_reminders()).
|
||||
self.assertEqual(ScheduledReminder.objects.get(task=task, reminder_type='reminder').id, original_id)
|
||||
|
||||
|
||||
class SendScheduledRemindersTests(TestCase):
|
||||
"""Tests for the send_scheduled_reminders Celery task."""
|
||||
|
||||
@@ -131,11 +197,30 @@ class SendScheduledRemindersTests(TestCase):
|
||||
mock_now.return_value = reminder.remind_at + timedelta(seconds=1)
|
||||
sent_count = send_scheduled_reminders()
|
||||
|
||||
self.assertEqual(sent_count, 3) # reminder, due_soon, overdue all past remind_at by then
|
||||
self.assertEqual(len(mail.outbox), 3)
|
||||
# "reminder" (30 min before) and "due_soon" have passed; "overdue"
|
||||
# is delayed an hour past due_soon so it hasn't fired yet.
|
||||
self.assertEqual(sent_count, 2)
|
||||
self.assertEqual(len(mail.outbox), 2)
|
||||
reminder.refresh_from_db()
|
||||
self.assertTrue(reminder.is_sent)
|
||||
self.assertIsNotNone(reminder.sent_at)
|
||||
self.assertFalse(ScheduledReminder.objects.get(task=task, reminder_type='overdue').is_sent)
|
||||
|
||||
def test_overdue_reminder_fires_an_hour_after_due(self):
|
||||
task = Task.objects.create(
|
||||
user=self.user, title='Water the plants', due_date=date(2026, 1, 15), due_time=dt_time(17, 0, 0),
|
||||
)
|
||||
overdue = ScheduledReminder.objects.get(task=task, reminder_type='overdue')
|
||||
due_soon = ScheduledReminder.objects.get(task=task, reminder_type='due_soon')
|
||||
self.assertEqual(overdue.remind_at, due_soon.remind_at + timedelta(hours=1))
|
||||
|
||||
with patch('django.utils.timezone.now') as mock_now:
|
||||
mock_now.return_value = overdue.remind_at + timedelta(seconds=1)
|
||||
send_scheduled_reminders()
|
||||
|
||||
overdue.refresh_from_db()
|
||||
self.assertTrue(overdue.is_sent)
|
||||
self.assertIn(f'Overdue: {task.title}', [m.subject for m in mail.outbox])
|
||||
|
||||
def test_future_reminder_not_sent_yet(self):
|
||||
task = Task.objects.create(
|
||||
|
||||
+3
-1
@@ -188,7 +188,9 @@ class Task(models.Model):
|
||||
|
||||
if self.due_time:
|
||||
due_moment = datetime.combine(self.due_date, self.due_time, tzinfo=user_tz)
|
||||
return due_moment, due_moment
|
||||
# Give "overdue" some breathing room after "due now" instead of
|
||||
# firing both notifications at the exact same moment.
|
||||
return due_moment, due_moment + timedelta(hours=1)
|
||||
|
||||
overdue_moment = datetime.combine(self.due_date + timedelta(days=1), dt_time.min, tzinfo=user_tz)
|
||||
return None, overdue_moment
|
||||
|
||||
@@ -57,6 +57,39 @@ class User(AbstractUser):
|
||||
def __str__(self):
|
||||
return self.email
|
||||
|
||||
@classmethod
|
||||
def from_db(cls, db, field_names, values):
|
||||
instance = super().from_db(db, field_names, values)
|
||||
instance._loaded_default_reminder_minutes = dict(zip(field_names, values)).get('default_reminder_minutes')
|
||||
return instance
|
||||
|
||||
def save(self, *args, **kwargs):
|
||||
loaded_reminder_minutes = getattr(self, '_loaded_default_reminder_minutes', None)
|
||||
reminder_minutes_changed = (
|
||||
loaded_reminder_minutes is not None and loaded_reminder_minutes != self.default_reminder_minutes
|
||||
)
|
||||
|
||||
super().save(*args, **kwargs)
|
||||
self._loaded_default_reminder_minutes = self.default_reminder_minutes
|
||||
|
||||
if reminder_minutes_changed:
|
||||
self.reschedule_reminder_notifications()
|
||||
|
||||
def reschedule_reminder_notifications(self):
|
||||
"""
|
||||
Recompute "before due" reminders for active tasks using the current
|
||||
default_reminder_minutes. Task.reschedule_reminders() only captures
|
||||
this setting at the moment a task's own due_date/due_time is set,
|
||||
so it doesn't apply retroactively on its own - this is what makes a
|
||||
profile-level change to the setting reach existing tasks. Tasks
|
||||
with their own explicit reminder_at override are left alone.
|
||||
"""
|
||||
tasks = self.tasks.filter(
|
||||
due_date__isnull=False, is_deleted=False, reminder_at__isnull=True,
|
||||
).exclude(status__in=('completed', 'cancelled'))
|
||||
for task in tasks:
|
||||
task.reschedule_reminders()
|
||||
|
||||
|
||||
class DeviceToken(models.Model):
|
||||
"""
|
||||
|
||||
Reference in New Issue
Block a user