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 <noreply@anthropic.com>
This commit is contained in:
parent
9ac7e88ac6
commit
5b329d0afc
@ -31,10 +31,8 @@ async def handle_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) ->
|
|||||||
return
|
return
|
||||||
|
|
||||||
if action == "done":
|
if action == "done":
|
||||||
reminder.is_active = False
|
# Just acknowledge, don't stop the reminder
|
||||||
session.commit()
|
await query.edit_message_text(f"✅ 已确认提醒:{reminder.title}")
|
||||||
remove_reminder_job(reminder_id)
|
|
||||||
await query.edit_message_text(f"✅ 已完成提醒:{reminder.title}")
|
|
||||||
|
|
||||||
elif action == "snooze":
|
elif action == "snooze":
|
||||||
# Ask user for snooze minutes
|
# Ask user for snooze minutes
|
||||||
|
|||||||
@ -1,7 +1,7 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import logging
|
import logging
|
||||||
from datetime import datetime, timedelta, timezone
|
from datetime import datetime, timezone
|
||||||
|
|
||||||
import pytz
|
import pytz
|
||||||
from telegram import Bot
|
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)
|
logger.info("Skipping reminder %d on holiday", reminder_id)
|
||||||
return
|
return
|
||||||
|
|
||||||
# For interval reminders, check time window
|
# Send the reminder
|
||||||
if reminder.reminder_type == "interval":
|
await _send_reminder_message(reminder, bot, session, now_utc, now_local)
|
||||||
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)
|
|
||||||
|
|
||||||
except Exception:
|
except Exception:
|
||||||
logger.exception("Error executing reminder %d", reminder_id)
|
logger.exception("Error executing reminder %d", reminder_id)
|
||||||
@ -74,15 +41,38 @@ async def execute_reminder(reminder_id: int, bot: Bot) -> None:
|
|||||||
Session.remove()
|
Session.remove()
|
||||||
|
|
||||||
|
|
||||||
def _in_window(now_local: datetime, start: str | None, end: str | None) -> bool:
|
async def _send_reminder_message(
|
||||||
if not start or not end:
|
reminder: Reminder, bot: Bot, session, now_utc: datetime, now_local: datetime
|
||||||
return True
|
) -> None:
|
||||||
sh, sm = map(int, start.split(":"))
|
"""Send a single reminder message."""
|
||||||
eh, em = map(int, end.split(":"))
|
# Build message
|
||||||
current_minutes = now_local.hour * 60 + now_local.minute
|
text = _build_message(reminder)
|
||||||
start_minutes = sh * 60 + sm
|
keyboard = reminder_action_keyboard(reminder.id)
|
||||||
end_minutes = eh * 60 + em
|
|
||||||
return start_minutes <= current_minutes <= end_minutes
|
# 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:
|
def _build_message(reminder: Reminder) -> str:
|
||||||
|
|||||||
@ -6,9 +6,9 @@ from typing import Optional
|
|||||||
|
|
||||||
import pytz
|
import pytz
|
||||||
from apscheduler.schedulers.asyncio import AsyncIOScheduler
|
from apscheduler.schedulers.asyncio import AsyncIOScheduler
|
||||||
|
from apscheduler.triggers.combining import OrTrigger
|
||||||
from apscheduler.triggers.cron import CronTrigger
|
from apscheduler.triggers.cron import CronTrigger
|
||||||
from apscheduler.triggers.date import DateTrigger
|
from apscheduler.triggers.date import DateTrigger
|
||||||
from apscheduler.triggers.interval import IntervalTrigger
|
|
||||||
from telegram import Bot
|
from telegram import Bot
|
||||||
|
|
||||||
from bot.models.database import Session
|
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)
|
return CronTrigger(day_of_week=dow, hour=h, minute=m, timezone=SHANGHAI_TZ)
|
||||||
|
|
||||||
if rtype == "interval":
|
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 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
|
return None
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user