Refactor reminder modification modals to support date, cron, and interval triggers

discord_reminder_bot/main.py

@@ -9,6 +9,11 @@ from functools import partial
 from pathlib import Path
 from typing import TYPE_CHECKING, Any
 
+import apscheduler
+import apscheduler.triggers
+import apscheduler.triggers.cron
+import apscheduler.triggers.date
+import apscheduler.triggers.interval
 import discord
 import sentry_sdk
 from apscheduler import events
@@ -20,7 +25,7 @@ from discord_webhook import DiscordWebhook
 from loguru import logger
 
 from discord_reminder_bot.helpers import calculate, generate_markdown_state, generate_state, get_human_readable_time, parse_time
-from discord_reminder_bot.modals import ReminderModifyModal
+from discord_reminder_bot.modals import CronReminderModifyModal, DateReminderModifyModal, IntervalReminderModifyModal
 from discord_reminder_bot.settings import scheduler
 
 if TYPE_CHECKING:
@@ -303,7 +308,24 @@ class ReminderListView(discord.ui.View):
             await interaction.response.send_message(f"Failed to get job for '{job_id}'", ephemeral=True)
             return
 
-        await interaction.response.send_modal(ReminderModifyModal(job))
+        # Check if the job is a date-based job
+        if isinstance(job.trigger, apscheduler.triggers.date.DateTrigger):
+            await interaction.response.send_modal(DateReminderModifyModal(job))
+            return
+        if isinstance(job.trigger, apscheduler.triggers.cron.CronTrigger):
+            await interaction.response.send_modal(CronReminderModifyModal(job))
+            return
+        if isinstance(job.trigger, apscheduler.triggers.interval.IntervalTrigger):
+            await interaction.response.send_modal(IntervalReminderModifyModal(job))
+            return
+
+        logger.error(f"Job {job_id} is not a date-based job, cron job, or interval job. Cannot modify.")
+        await interaction.response.send_message(
+            f"Job is not a date-based job, cron job, or interval job. Cannot modify.\n"
+            f"Job ID: `{escape_markdown(job_id)}`\n"
+            f"Job Trigger: `{job.trigger}`",
+            ephemeral=True,
+        )
 
     async def handle_pause_unpause(self, interaction: discord.Interaction, job_id: str) -> None:
         """Handle pausing or unpausing a reminder job.

discord_reminder_bot/modals.py

@@ -1,13 +1,9 @@
 from __future__ import annotations
 
-import re
 import traceback
 from typing import TYPE_CHECKING
 
 import discord
-from apscheduler.triggers.cron import CronTrigger
-from apscheduler.triggers.date import DateTrigger
-from apscheduler.triggers.interval import IntervalTrigger
 from discord.utils import escape_markdown
 from loguru import logger
 
@@ -20,19 +16,18 @@ if TYPE_CHECKING:
     from apscheduler.job import Job
 
 
-class ReminderModifyModal(discord.ui.Modal, title="Modify reminder"):
+class DateReminderModifyModal(discord.ui.Modal, title="Modify reminder"):
-    """Modal for modifying a APScheduler job."""
+    """Modal for modifying a date-based APScheduler job (one-time reminder)."""
 
     def __init__(self, job: Job) -> None:
-        """Initialize the modal for modifying a reminder.
+        """Initialize the modal for modifying a date-based reminder.
 
         Args:
-            job (Job): The APScheduler job to modify.
+            job (Job): The APScheduler job to modify. Must be a date-based job.
         """
         super().__init__(title="Modify Reminder")
         self.job = job
         self.job_id = job.id
-        self.trigger_type = self._get_trigger_type(job.trigger)
 
         self.message_input = discord.ui.TextInput(
             label="Reminder message",
@@ -41,31 +36,7 @@ class ReminderModifyModal(discord.ui.Modal, title="Modify reminder"):
             max_length=200,
         )
 
-        # Different input fields based on trigger type
+        # Only allow editing the date/time for date-based reminders
-        if self.trigger_type == "date":
-            self.time_input = discord.ui.TextInput(
-                label="New time",
-                placeholder="e.g. tomorrow at 3 PM",
-                required=True,
-            )
-        elif self.trigger_type == "interval":
-            interval_text = self._format_interval_from_trigger(job.trigger)
-            self.time_input = discord.ui.TextInput(
-                label="New interval",
-                placeholder="e.g. 1d 2h 30m (days, hours, minutes)",
-                required=True,
-                default=interval_text,
-            )
-        elif self.trigger_type == "cron":
-            cron_text = self._format_cron_from_trigger(job.trigger)
-            self.time_input = discord.ui.TextInput(
-                label="New cron expression",
-                placeholder="e.g. 0 9 * * 1-5 (min hour day month day_of_week)",
-                required=True,
-                default=cron_text,
-            )
-        else:
-            # Fallback to date input for unknown trigger types
         self.time_input = discord.ui.TextInput(
             label="New time",
             placeholder="e.g. tomorrow at 3 PM",
@@ -75,133 +46,6 @@ class ReminderModifyModal(discord.ui.Modal, title="Modify reminder"):
         self.add_item(self.message_input)
         self.add_item(self.time_input)
 
-    def _get_trigger_type(self, trigger: DateTrigger | IntervalTrigger | CronTrigger) -> str:
-        """Determine the type of trigger.
-
-        Args:
-            trigger: The APScheduler trigger.
-
-        Returns:
-            str: The type of trigger ("date", "interval", "cron", or "unknown").
-        """
-        if isinstance(trigger, DateTrigger):
-            return "date"
-        if isinstance(trigger, IntervalTrigger):
-            return "interval"
-        if isinstance(trigger, CronTrigger):
-            return "cron"
-        return "unknown"
-
-    def _format_interval_from_trigger(self, trigger: IntervalTrigger) -> str:
-        """Format an interval trigger into a human-readable string.
-
-        Args:
-            trigger (IntervalTrigger): The interval trigger.
-
-        Returns:
-            str: Formatted interval string (e.g., "1d 2h 30m").
-        """
-        parts = []
-
-        # Get interval values from the trigger.__getstate__() dictionary
-        trigger_state = trigger.__getstate__()
-
-        if trigger_state.get("weeks", 0):
-            parts.append(f"{trigger_state['weeks']}w")
-        if trigger_state.get("days", 0):
-            parts.append(f"{trigger_state['days']}d")
-        if trigger_state.get("hours", 0):
-            parts.append(f"{trigger_state['hours']}h")
-        if trigger_state.get("minutes", 0):
-            parts.append(f"{trigger_state['minutes']}m")
-
-        seconds = trigger_state.get("seconds", 0)
-        if seconds and seconds % 60 != 0:  # Only show seconds if not even minutes
-            parts.append(f"{seconds % 60}s")
-
-        return " ".join(parts) if parts else "0m"
-
-    def _format_cron_from_trigger(self, trigger: CronTrigger) -> str:
-        """Format a cron trigger into a string representation.
-
-        Args:
-            trigger (CronTrigger): The cron trigger.
-
-        Returns:
-            str: Formatted cron string.
-        """
-        fields = []
-
-        # Get the fields in standard cron order
-        for field in ["second", "minute", "hour", "day", "month", "day_of_week", "year"]:
-            if hasattr(trigger, field) and getattr(trigger, field) is not None:
-                expr = getattr(trigger, field).expression
-                fields.append(expr if expr != "*" else "*")
-
-        # Return only the standard 5 cron fields by default
-        return " ".join(fields[:5])
-
-    def _parse_interval_string(self, interval_str: str) -> dict[str, int]:
-        """Parse an interval string into component parts.
-
-        Args:
-            interval_str (str): String like "1w 2d 3h 4m 5s"
-
-        Returns:
-            dict[str, int]: Dictionary with interval components.
-        """
-        interval_dict = {"weeks": 0, "days": 0, "hours": 0, "minutes": 0, "seconds": 0}
-
-        # Define regex patterns for each time unit
-        patterns = {r"(\d+)w": "weeks", r"(\d+)d": "days", r"(\d+)h": "hours", r"(\d+)m": "minutes", r"(\d+)s": "seconds"}
-
-        # Extract values for each unit
-        for pattern, key in patterns.items():
-            match = re.search(pattern, interval_str)
-            if match:
-                interval_dict[key] = int(match.group(1))
-
-        # Ensure at least 30 seconds total interval
-        total_seconds = (
-            interval_dict["weeks"] * 604800
-            + interval_dict["days"] * 86400
-            + interval_dict["hours"] * 3600
-            + interval_dict["minutes"] * 60
-            + interval_dict["seconds"]
-        )
-
-        if total_seconds < 30:
-            interval_dict["seconds"] = 30
-
-        return interval_dict
-
-    def _parse_cron_string(self, cron_str: str) -> dict[str, str]:
-        """Parse a cron string into its components.
-
-        Args:
-            cron_str (str): Cron string like "0 9 * * 1-5"
-
-        Returns:
-            dict[str, str]: Dictionary with cron components.
-        """
-        parts = cron_str.strip().split()
-        cron_dict = {}
-
-        # Map position to field name
-        field_names = ["second", "minute", "hour", "day", "month", "day_of_week", "year"]
-
-        # Handle standard 5-part cron string (minute hour day month day_of_week)
-        if len(parts) == 5:
-            # Add a 0 for seconds (as APScheduler expects it)
-            parts.insert(0, "0")
-
-        # Map parts to field names
-        for i, part in enumerate(parts):
-            if i < len(field_names) and part != "*":  # Only add non-default values
-                cron_dict[field_names[i]] = part
-
-        return cron_dict
-
     def _process_date_trigger(self, new_time_str: str, old_time: datetime.datetime | None) -> tuple[bool, str, Job | None]:
         """Process date trigger modification.
 
@@ -228,46 +72,6 @@ class ReminderModifyModal(discord.ui.Modal, title="Modify reminder"):
         else:
             return True, "", rescheduled_job
 
-    def _process_interval_trigger(self, new_time_str: str) -> tuple[bool, str, Job | None]:
-        """Process interval trigger modification.
-
-        Args:
-            new_time_str (str): The new interval string to parse.
-
-        Returns:
-            tuple[bool, str, Job | None]: Success flag, error message, and rescheduled job.
-        """
-        try:
-            interval_dict = self._parse_interval_string(new_time_str)
-            logger.info(f"Rescheduling interval job {self.job_id} with {interval_dict}")
-            rescheduled_job = scheduler.reschedule_job(self.job_id, trigger="interval", **interval_dict)
-        except (ValueError, TypeError, AttributeError) as e:
-            error_msg = f"Invalid interval format: `{new_time_str}`"
-            logger.exception(f"Failed to parse interval: {e}")
-            return False, error_msg, None
-        else:
-            return True, "", rescheduled_job
-
-    def _process_cron_trigger(self, new_time_str: str) -> tuple[bool, str, Job | None]:
-        """Process cron trigger modification.
-
-        Args:
-            new_time_str (str): The new cron string to parse.
-
-        Returns:
-            tuple[bool, str, Job | None]: Success flag, error message, and rescheduled job.
-        """
-        try:
-            cron_dict = self._parse_cron_string(new_time_str)
-            logger.info(f"Rescheduling cron job {self.job_id} with {cron_dict}")
-            rescheduled_job = scheduler.reschedule_job(self.job_id, trigger="cron", **cron_dict)
-        except (ValueError, TypeError, AttributeError) as e:
-            error_msg = f"Invalid cron format: `{new_time_str}`"
-            logger.exception(f"Failed to parse cron: {e}")
-            return False, error_msg, None
-        else:
-            return True, "", rescheduled_job
-
     async def _update_message(self, old_message: str, new_message: str) -> bool:
         """Update the message of a job.
 
@@ -281,7 +85,7 @@ class ReminderModifyModal(discord.ui.Modal, title="Modify reminder"):
         if new_message == old_message:
             return False
 
-        job = scheduler.get_job(self.job_id)
+        job: Job | None = scheduler.get_job(self.job_id)
         if not job:
             return False
 
@@ -299,7 +103,7 @@ class ReminderModifyModal(discord.ui.Modal, title="Modify reminder"):
         return True
 
     async def on_submit(self, interaction: discord.Interaction) -> None:
-        """Called when the modal is submitted.
+        """Called when the modal is submitted for a date-based reminder.
 
         Args:
             interaction (discord.Interaction): The Discord interaction where this modal was triggered from.
@@ -323,15 +127,8 @@ class ReminderModifyModal(discord.ui.Modal, title="Modify reminder"):
         # Defer early for long operations
         await interaction.response.defer(ephemeral=True)
 
-        # Process time/schedule changes based on trigger type
+        # Process date trigger
-        success, error_msg, rescheduled_job = False, "", None
-
-        if self.trigger_type == "date":
         success, error_msg, rescheduled_job = self._process_date_trigger(new_time_str, old_time)
-        elif self.trigger_type == "interval":
-            success, error_msg, rescheduled_job = self._process_interval_trigger(new_time_str)
-        elif self.trigger_type == "cron":
-            success, error_msg, rescheduled_job = self._process_cron_trigger(new_time_str)
 
         # If time input is invalid, send error message
         if not success and error_msg:
@@ -354,7 +151,7 @@ class ReminderModifyModal(discord.ui.Modal, title="Modify reminder"):
             changes_made = True
 
         # Update message if changed
-        message_changed = await self._update_message(old_message, new_message)
+        message_changed: bool = await self._update_message(old_message, new_message)
         if message_changed:
             msg += f"Old message: `{escape_markdown(old_message)}`\n"
             msg += f"New message: `{escape_markdown(new_message)}`.\n"
@@ -384,3 +181,31 @@ class ReminderModifyModal(discord.ui.Modal, title="Modify reminder"):
 
         logger.exception(f"Error in ReminderModifyModal: {error}")
         traceback.print_exception(type(error), error, error.__traceback__)
+
+
+class CronReminderModifyModal(discord.ui.Modal, title="Modify reminder"):
+    """A modal for modifying a cron-based reminder."""
+
+    def __init__(self, job: Job) -> None:
+        """Initialize the modal for modifying a date-based reminder.
+
+        Args:
+            job (Job): The APScheduler job to modify. Must be a date-based job.
+        """
+        super().__init__(title="Modify Reminder")
+        self.job = job
+        self.job_id = job.id
+
+
+class IntervalReminderModifyModal(discord.ui.Modal, title="Modify reminder"):
+    """A modal for modifying an interval-based reminder."""
+
+    def __init__(self, job: Job) -> None:
+        """Initialize the modal for modifying a date-based reminder.
+
+        Args:
+            job (Job): The APScheduler job to modify. Must be a date-based job.
+        """
+        super().__init__(title="Modify Reminder")
+        self.job = job
+        self.job_id = job.id