diff --git a/notifications/tests.py b/notifications/tests.py index e018fbf..b4131d1 100644 --- a/notifications/tests.py +++ b/notifications/tests.py @@ -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( diff --git a/tasks/models.py b/tasks/models.py index 9bb4b4f..7d45e2b 100644 --- a/tasks/models.py +++ b/tasks/models.py @@ -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 diff --git a/users/models.py b/users/models.py index d69fcb3..24678e5 100644 --- a/users/models.py +++ b/users/models.py @@ -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): """