feat(reminders): per-occurrence reminders for recurring events
Drift-audit Group 8 (final item). _fire_reminders previously gated on the base row (reminder_sent_at IS NULL AND start_dt > now), so a recurring event reminded at most once ever — once the first occurrence passed, no future occurrence qualified. Now recurring events are evaluated every sweep against their next occurrence (rrulestr.after(now)), and reminder_sent_at stores the start of the occurrence last reminded about. Each new occurrence has a distinct marker, so it re-arms and fires exactly once per occurrence. One-shot events keep the classic NULL gate. Also adds the deleted_at filter so trashed events stop reminding. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -16,7 +16,8 @@ from datetime import datetime, timedelta, timezone
|
|||||||
|
|
||||||
from apscheduler.schedulers.background import BackgroundScheduler
|
from apscheduler.schedulers.background import BackgroundScheduler
|
||||||
from apscheduler.triggers.interval import IntervalTrigger
|
from apscheduler.triggers.interval import IntervalTrigger
|
||||||
from sqlalchemy import select
|
from dateutil.rrule import rrulestr
|
||||||
|
from sqlalchemy import and_, or_, select
|
||||||
|
|
||||||
from fabledassistant.models import async_session
|
from fabledassistant.models import async_session
|
||||||
from fabledassistant.models.event import Event
|
from fabledassistant.models.event import Event
|
||||||
@@ -32,7 +33,13 @@ _loop: asyncio.AbstractEventLoop | None = None
|
|||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
async def _fire_reminders() -> None:
|
async def _fire_reminders() -> None:
|
||||||
"""Find events with reminders due in the next 5 minutes and fire push notifications."""
|
"""Fire in-app reminders for events whose reminder time has arrived.
|
||||||
|
|
||||||
|
One-shot events fire once (gated on reminder_sent_at IS NULL). Recurring
|
||||||
|
events fire once PER OCCURRENCE: reminder_sent_at stores the start of the
|
||||||
|
occurrence we last reminded about, so each new occurrence re-arms the
|
||||||
|
reminder instead of the whole series firing only once.
|
||||||
|
"""
|
||||||
now = datetime.now(timezone.utc)
|
now = datetime.now(timezone.utc)
|
||||||
window_end = now + timedelta(minutes=5)
|
window_end = now + timedelta(minutes=5)
|
||||||
|
|
||||||
@@ -40,19 +47,38 @@ async def _fire_reminders() -> None:
|
|||||||
result = await session.execute(
|
result = await session.execute(
|
||||||
select(Event).where(
|
select(Event).where(
|
||||||
Event.reminder_minutes.isnot(None),
|
Event.reminder_minutes.isnot(None),
|
||||||
Event.reminder_sent_at.is_(None),
|
Event.deleted_at.is_(None),
|
||||||
Event.start_dt > now, # event hasn't started yet
|
or_(
|
||||||
# reminder fires when now >= start_dt - reminder_minutes
|
# Recurring events are evaluated every sweep against their
|
||||||
# i.e. start_dt <= now + reminder_minutes (approximated by window_end check)
|
# next occurrence (the base start_dt is long past).
|
||||||
|
Event.recurrence.isnot(None),
|
||||||
|
# One-shot events: classic gate.
|
||||||
|
and_(Event.reminder_sent_at.is_(None), Event.start_dt > now),
|
||||||
|
),
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
candidates = list(result.scalars().all())
|
candidates = list(result.scalars().all())
|
||||||
|
|
||||||
to_notify: list[Event] = []
|
# (event_id, occurrence_start) — occurrence_start is also the dedup marker
|
||||||
|
# written to reminder_sent_at, so a given occurrence reminds exactly once.
|
||||||
|
to_notify: list[tuple[int, datetime]] = []
|
||||||
for event in candidates:
|
for event in candidates:
|
||||||
reminder_dt = event.start_dt - timedelta(minutes=event.reminder_minutes)
|
if event.recurrence:
|
||||||
if reminder_dt <= window_end:
|
try:
|
||||||
to_notify.append(event)
|
rule = rrulestr(event.recurrence, dtstart=event.start_dt, ignoretz=False)
|
||||||
|
occ = rule.after(now, inc=True)
|
||||||
|
except Exception:
|
||||||
|
logger.warning("Failed to expand RRULE for event %d reminder", event.id, exc_info=True)
|
||||||
|
continue
|
||||||
|
if occ is None:
|
||||||
|
continue
|
||||||
|
reminder_dt = occ - timedelta(minutes=event.reminder_minutes)
|
||||||
|
if reminder_dt <= window_end and event.reminder_sent_at != occ:
|
||||||
|
to_notify.append((event.id, occ))
|
||||||
|
else:
|
||||||
|
reminder_dt = event.start_dt - timedelta(minutes=event.reminder_minutes)
|
||||||
|
if reminder_dt <= window_end:
|
||||||
|
to_notify.append((event.id, event.start_dt))
|
||||||
|
|
||||||
if not to_notify:
|
if not to_notify:
|
||||||
return
|
return
|
||||||
@@ -61,22 +87,23 @@ async def _fire_reminders() -> None:
|
|||||||
from fabledassistant.services.notifications import create_in_app_notification
|
from fabledassistant.services.notifications import create_in_app_notification
|
||||||
|
|
||||||
async with async_session() as session:
|
async with async_session() as session:
|
||||||
for event in to_notify:
|
for event_id, occurrence_start in to_notify:
|
||||||
result = await session.execute(
|
ev = (await session.execute(
|
||||||
select(Event).where(Event.id == event.id)
|
select(Event).where(Event.id == event_id)
|
||||||
)
|
)).scalar_one_or_none()
|
||||||
ev = result.scalar_one_or_none()
|
# Skip if this exact occurrence was already reminded (covers a
|
||||||
if ev is None or ev.reminder_sent_at is not None:
|
# concurrent sweep and the one-shot already-sent case).
|
||||||
|
if ev is None or ev.reminder_sent_at == occurrence_start:
|
||||||
continue
|
continue
|
||||||
await create_in_app_notification(ev.user_id, "event_reminder", {
|
await create_in_app_notification(ev.user_id, "event_reminder", {
|
||||||
"event_id": ev.id,
|
"event_id": ev.id,
|
||||||
"title": ev.title,
|
"title": ev.title,
|
||||||
"start_dt": ev.start_dt.isoformat() if ev.start_dt else None,
|
"start_dt": occurrence_start.isoformat(),
|
||||||
"url": "/calendar",
|
"url": "/calendar",
|
||||||
})
|
})
|
||||||
# Stamp only after the notification is created, so a delivery
|
# Stamp the occurrence marker only after the notification is
|
||||||
# failure leaves the reminder eligible to retry next sweep.
|
# created, so a delivery failure leaves it eligible to retry.
|
||||||
ev.reminder_sent_at = datetime.now(timezone.utc)
|
ev.reminder_sent_at = occurrence_start
|
||||||
await session.commit()
|
await session.commit()
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user