From 5b329d0afc627a356cbd7a0b94757545e8e8bfe1 Mon Sep 17 00:00:00 2001 From: leo Date: Thu, 5 Mar 2026 17:54:33 +0800 Subject: [PATCH] fix: correct interval reminder scheduling and done action behavior - Replace IntervalTrigger with OrTrigger of CronTriggers for interval reminders, ensuring triggers align to window start each day without drift - Refactor executor to extract _send_reminder_message helper, removing the runtime window check (now handled at scheduling layer) - Change "done" button to only acknowledge the notification without deactivating the reminder Co-Authored-By: Claude Sonnet 4.6 --- bot/handlers/callback.py | 6 +-- bot/scheduler/executor.py | 80 ++++++++++++++++-------------------- bot/scheduler/job_manager.py | 27 ++++++++++-- 3 files changed, 61 insertions(+), 52 deletions(-) diff --git a/bot/handlers/callback.py b/bot/handlers/callback.py index ed11cf8..5f1920c 100644 --- a/bot/handlers/callback.py +++ b/bot/handlers/callback.py @@ -31,10 +31,8 @@ async def handle_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> return if action == "done": - reminder.is_active = False - session.commit() - remove_reminder_job(reminder_id) - await query.edit_message_text(f"✅ 已完成提醒：{reminder.title}") + # Just acknowledge, don't stop the reminder + await query.edit_message_text(f"✅ 已确认提醒：{reminder.title}") elif action == "snooze": # Ask user for snooze minutes diff --git a/bot/scheduler/executor.py b/bot/scheduler/executor.py index 9681de7..acc9818 100644 --- a/bot/scheduler/executor.py +++ b/bot/scheduler/executor.py @@ -1,7 +1,7 @@ from __future__ import annotations import logging -from datetime import datetime, timedelta, timezone +from datetime import datetime, timezone import pytz from telegram import Bot @@ -31,41 +31,8 @@ async def execute_reminder(reminder_id: int, bot: Bot) -> None: logger.info("Skipping reminder %d on holiday", reminder_id) return - # For interval reminders, check time window - if reminder.reminder_type == "interval": - if not _in_window(now_local, reminder.interval_start_time, reminder.interval_end_time): - logger.debug("Reminder %d outside time window, skipping", reminder_id) - return - - # Build message - user = reminder.logs # just to load relationship lazily – not needed here - text = _build_message(reminder) - keyboard = reminder_action_keyboard(reminder.id) - - # Fetch the telegram_id from users table - from bot.models.user import User # avoid circular import at module level - user_obj = session.get(User, reminder.user_id) - if user_obj is None: - return - - await bot.send_message( - chat_id=user_obj.telegram_id, - text=text, - parse_mode="Markdown", - reply_markup=keyboard, - ) - - # Update reminder timestamps - reminder.last_sent_at = now_utc - log = ReminderLog(reminder_id=reminder.id, sent_at=now_utc, status="sent") - session.add(log) - - # Deactivate one-time reminders after firing - if reminder.reminder_type == "once": - reminder.is_active = False - - session.commit() - logger.info("Sent reminder %d to user %d", reminder_id, user_obj.telegram_id) + # Send the reminder + await _send_reminder_message(reminder, bot, session, now_utc, now_local) except Exception: logger.exception("Error executing reminder %d", reminder_id) @@ -74,15 +41,38 @@ async def execute_reminder(reminder_id: int, bot: Bot) -> None: Session.remove() -def _in_window(now_local: datetime, start: str | None, end: str | None) -> bool: - if not start or not end: - return True - sh, sm = map(int, start.split(":")) - eh, em = map(int, end.split(":")) - current_minutes = now_local.hour * 60 + now_local.minute - start_minutes = sh * 60 + sm - end_minutes = eh * 60 + em - return start_minutes <= current_minutes <= end_minutes +async def _send_reminder_message( + reminder: Reminder, bot: Bot, session, now_utc: datetime, now_local: datetime +) -> None: + """Send a single reminder message.""" + # Build message + text = _build_message(reminder) + keyboard = reminder_action_keyboard(reminder.id) + + # Fetch the telegram_id from users table + from bot.models.user import User # avoid circular import at module level + user_obj = session.get(User, reminder.user_id) + if user_obj is None: + return + + await bot.send_message( + chat_id=user_obj.telegram_id, + text=text, + parse_mode="Markdown", + reply_markup=keyboard, + ) + + # Update reminder timestamps + reminder.last_sent_at = now_utc + log = ReminderLog(reminder_id=reminder.id, sent_at=now_utc, status="sent") + session.add(log) + + # Deactivate one-time reminders after firing + if reminder.reminder_type == "once": + reminder.is_active = False + + session.commit() + logger.info("Sent reminder %d to user %d", reminder.id, user_obj.telegram_id) def _build_message(reminder: Reminder) -> str: diff --git a/bot/scheduler/job_manager.py b/bot/scheduler/job_manager.py index ed80960..e28c873 100644 --- a/bot/scheduler/job_manager.py +++ b/bot/scheduler/job_manager.py @@ -6,9 +6,9 @@ from typing import Optional import pytz from apscheduler.schedulers.asyncio import AsyncIOScheduler +from apscheduler.triggers.combining import OrTrigger from apscheduler.triggers.cron import CronTrigger from apscheduler.triggers.date import DateTrigger -from apscheduler.triggers.interval import IntervalTrigger from telegram import Bot from bot.models.database import Session @@ -121,8 +121,29 @@ def _build_trigger(reminder: Reminder): return CronTrigger(day_of_week=dow, hour=h, minute=m, timezone=SHANGHAI_TZ) if rtype == "interval": - if not reminder.interval_minutes: + if not reminder.interval_minutes or not reminder.interval_start_time or not reminder.interval_end_time: return None - return IntervalTrigger(minutes=reminder.interval_minutes, timezone=SHANGHAI_TZ) + # Calculate all trigger times within the window + sh, sm = map(int, reminder.interval_start_time.split(":")) + eh, em = map(int, reminder.interval_end_time.split(":")) + + window_start_minutes = sh * 60 + sm + window_end_minutes = eh * 60 + em + + # Generate all trigger times + triggers = [] + current_minutes = window_start_minutes + while current_minutes <= window_end_minutes: + h = current_minutes // 60 + m = current_minutes % 60 + if h < 24: + triggers.append(CronTrigger(hour=h, minute=m, timezone=SHANGHAI_TZ)) + current_minutes += reminder.interval_minutes + + if not triggers: + return None + if len(triggers) == 1: + return triggers[0] + return OrTrigger(triggers) return None