Feature/capr 23 scaffold event cog from deprecated repo - #70
Conversation
…eature/cap-253-event-creation-modal
…o feature/capr-23-scaffold-event_cog-from-deprecated-repo
…Better timezone handling.
…o feature/capr-23-scaffold-event_cog-from-deprecated-repo
…o feature/capr-23-scaffold-event_cog-from-deprecated-repo
Reviewer's GuideIntroduces a new Discord "Event" cog with full event lifecycle management (create, edit, list, show, delete, announce, and "my events") backed by a temporary in-memory store, plus a Pydantic EventSchema for modal-driven form validation and parsing of event date/time text inputs. Sequence diagram for /event command lifecycle handlingsequenceDiagram
actor User
participant DiscordClient
participant EventCog
participant ModelModal
participant EventDropdownView
participant ConfirmDeleteView
User->>DiscordClient: /event action=create
DiscordClient->>EventCog: event(interaction, action=create)
EventCog->>EventCog: handle_create_action(interaction)
EventCog->>ModelModal: create ModelModal(EventSchema, _handle_event_submit)
EventCog->>DiscordClient: interaction.response.send_modal(modal)
User->>DiscordClient: submits event modal
DiscordClient->>ModelModal: validate fields via EventSchema
ModelModal->>EventCog: _handle_event_submit(interaction, event)
EventCog->>EventCog: events[guild_id].append(event)
EventCog->>DiscordClient: interaction.response.send_message(embeds=[success, event_embed])
User->>DiscordClient: /event action=edit
DiscordClient->>EventCog: event(interaction, action=edit)
EventCog->>EventCog: handle_edit_action(interaction)
EventCog->>EventCog: events = self.events[guild_id]
EventCog->>DiscordClient: interaction.response.defer(ephemeral=True)
EventCog->>EventDropdownView: create EventDropdownView(events, cog, placeholder, _on_edit_select)
EventCog->>DiscordClient: followup.send(view=EventDropdownView)
User->>EventDropdownView: select event
EventDropdownView->>EventCog: _on_edit_select(interaction, selected_event)
EventCog->>ModelModal: create ModelModal(EventSchema, _handle_event_update)
EventCog->>DiscordClient: interaction.response.send_modal(modal)
User->>DiscordClient: submits updated modal
DiscordClient->>ModelModal: validate via EventSchema
ModelModal->>EventCog: _handle_event_update(interaction, updated_event, original_event)
EventCog->>EventCog: replace original_event with updated_event in events[guild_id]
EventCog->>DiscordClient: interaction.response.send_message(embeds=[success, event_embed])
User->>DiscordClient: /event action=delete
DiscordClient->>EventCog: event(interaction, action=delete)
EventCog->>EventCog: handle_delete_action(interaction)
EventCog->>EventDropdownView: create EventDropdownView(events, cog, placeholder, _on_delete_select)
EventCog->>DiscordClient: followup.send(view=EventDropdownView)
User->>EventDropdownView: select event
EventDropdownView->>EventCog: _on_delete_select(interaction, selected_event)
EventCog->>ConfirmDeleteView: create ConfirmDeleteView()
EventCog->>DiscordClient: interaction.response.send_message(embed=confirm, view=ConfirmDeleteView)
User->>ConfirmDeleteView: clicks Delete
ConfirmDeleteView->>ConfirmDeleteView: value = True, disable_all_items()
ConfirmDeleteView->>DiscordClient: interaction.response.edit_message(view=self)
EventCog->>EventCog: remove selected_event from events[guild_id]
EventCog->>DiscordClient: interaction.followup.send(success_embed)
Sequence diagram for event announcement and RSVP-based myevents lookupsequenceDiagram
actor User
participant DiscordClient
participant EventCog
participant EventDropdownView
participant AnnouncementChannel
User->>DiscordClient: /event action=announce
DiscordClient->>EventCog: event(interaction, action=announce)
EventCog->>EventCog: handle_announce_action(interaction)
EventCog->>EventCog: events = self.events[guild_id]
EventCog->>DiscordClient: interaction.response.defer(ephemeral=True)
EventCog->>EventDropdownView: create EventDropdownView(events, cog, placeholder, _on_announce_select)
EventCog->>DiscordClient: followup.send(view=EventDropdownView)
User->>EventDropdownView: select event
EventDropdownView->>EventCog: _on_announce_select(interaction, selected_event)
EventCog->>EventCog: find announcement_channel in guild.text_channels
EventCog->>EventCog: announce_embed = _create_announcement_embed(selected_event)
EventCog->>AnnouncementChannel: send(embed=announce_embed)
AnnouncementChannel-->>EventCog: message
EventCog->>AnnouncementChannel: message.add_reaction("✅")
EventCog->>AnnouncementChannel: message.add_reaction("❌")
EventCog->>EventCog: event_announcements[guild_id][event_name] = message.id
EventCog->>DiscordClient: interaction.response.send_message(success_embed, ephemeral=True)
User->>AnnouncementChannel: reacts with ✅ on announcement
User->>DiscordClient: /event action=myevents
DiscordClient->>EventCog: event(interaction, action=myevents)
EventCog->>EventCog: handle_myevents_action(interaction)
EventCog->>EventCog: events = self.events[guild_id]
EventCog->>DiscordClient: interaction.response.defer(ephemeral=True)
loop for each upcoming event
EventCog->>EventCog: _is_user_registered(event, guild, user)
EventCog->>EventCog: message_id = event_announcements[guild.id][event.event_name]
EventCog->>AnnouncementChannel: fetch_message(message_id)
AnnouncementChannel-->>EventCog: message
EventCog->>EventCog: inspect message.reactions for emoji ✅ containing user
EventCog-->>EventCog: True/False
end
EventCog->>DiscordClient: interaction.followup.send(embed=registered_events_embed, ephemeral=True)
Class diagram for new Event cog and schemasclassDiagram
class EventSchema {
<<pydantic.BaseModel>>
+str event_name
+date event_date
+time event_time
+str location
+str description
+_parse_event_date(value)
+_parse_event_time(value)
}
class EventDropdownSelect {
<<discord.ui.Select>>
+EventDropdownView view_ref
+__init__(options, view, placeholder)
+callback(interaction)
}
class EventDropdownView {
<<BaseView>>
+list~EventSchema~ event_list
+Event cog
+on_select(interaction, selected_event)
+__init__(events, cog, placeholder, on_select_callback)
}
class ConfirmDeleteView {
<<BaseView>>
+bool value
+__init__()
+confirm(interaction, _button)
+cancel(interaction, _button)
}
class Event {
<<commands.Cog>>
+commands.Bot bot
+Logger log
+dict~int, list~EventSchema~~ events
+dict~int, dict~str, int~~ event_announcements
+__init__(bot)
+event(interaction, action)
+handle_create_action(interaction)
+handle_edit_action(interaction)
+handle_show_action(interaction)
+handle_delete_action(interaction)
+handle_list_action(interaction)
+handle_announce_action(interaction)
+handle_myevents_action(interaction)
+_is_user_registered(event, guild, user)
+_on_edit_select(interaction, selected_event)
+_on_announce_select(interaction, selected_event)
+_create_announcement_embed(event)
+_handle_event_submit(interaction, event)
+_create_event_embed(event)
+_handle_event_update(interaction, updated_event, original_event)
+_on_show_select(interaction, selected_event)
+_on_delete_select(interaction, selected_event)
}
class BaseView {
}
class commands_Cog {
}
class pydantic_BaseModel {
}
EventDropdownSelect --> EventDropdownView : references view_ref
EventDropdownView --> EventSchema : uses EventSchema as event_list
EventDropdownView --> Event : holds cog reference
ConfirmDeleteView --> BaseView : inherits
EventDropdownView --> BaseView : inherits
Event --> commands_Cog : inherits
EventSchema --> pydantic_BaseModel : inherits
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
_is_user_registered, the list comprehensionusers = [user async for user in reaction.users()]both shadows theuserparameter and loads all users into memory; consider iterating the async iterator directly and comparing IDs (e.g.,async for u in reaction.users(): if u.id == user.id: ...). - The logic to combine
event_dateandevent_timeand normalize timezone is duplicated in several places (handle_list_action,handle_myevents_action,_create_announcement_embed,_create_event_embed); extracting a small helper (e.g.,_event_datetime(event: EventSchema) -> datetime) would reduce repetition and the risk of inconsistencies. - For
self.event_announcements, usingevent.event_nameas the key can lead to collisions (e.g., events with the same name or after renames); consider using a more stable identifier (such as an index, UUID, or a dedicated event ID) to track announcement messages.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_is_user_registered`, the list comprehension `users = [user async for user in reaction.users()]` both shadows the `user` parameter and loads all users into memory; consider iterating the async iterator directly and comparing IDs (e.g., `async for u in reaction.users(): if u.id == user.id: ...`).
- The logic to combine `event_date` and `event_time` and normalize timezone is duplicated in several places (`handle_list_action`, `handle_myevents_action`, `_create_announcement_embed`, `_create_event_embed`); extracting a small helper (e.g., `_event_datetime(event: EventSchema) -> datetime`) would reduce repetition and the risk of inconsistencies.
- For `self.event_announcements`, using `event.event_name` as the key can lead to collisions (e.g., events with the same name or after renames); consider using a more stable identifier (such as an index, UUID, or a dedicated event ID) to track announcement messages.
## Individual Comments
### Comment 1
<location> `capy_discord/exts/event/event.py:419-421` </location>
<code_context>
+ return False
+
+ # Try to find the announcement message and check reactions
+ announcement_channel: discord.TextChannel | None = None
+ for channel in guild.text_channels:
+ if "announce" in channel.name.lower():
+ announcement_channel = channel
+ break
</code_context>
<issue_to_address>
**suggestion:** Factor out the repeated logic for locating the announcement channel.
This "first text channel with 'announce' in the name" lookup is duplicated in `_is_user_registered` and `_on_announce_select`. Please extract it into a shared helper like `_get_announcement_channel(guild)` so the selection rules remain consistent and easier to maintain.
Suggested implementation:
```python
# Get announcement messages for this guild
guild_announcements = self.event_announcements.get(guild.id, {})
message_id = guild_announcements.get(event.event_name)
if not message_id:
return False
def _get_announcement_channel(guild: discord.Guild) -> discord.TextChannel | None:
"""Return the first text channel whose name contains 'announce'."""
for channel in guild.text_channels:
if "announce" in channel.name.lower():
return channel
return None
# Try to find the announcement message and check reactions
announcement_channel = _get_announcement_channel(guild)
if not announcement_channel:
return False
```
To fully apply the review comment and avoid duplicating the lookup logic:
1. Extract `_get_announcement_channel(guild)` to a shared helper (e.g. a private method on the cog class or a module-level function) instead of a local function, so both `_is_user_registered` and `_on_announce_select` can use it.
2. In `_on_announce_select`, replace the duplicated loop that finds the "announce" text channel with a call to the shared `_get_announcement_channel(guild)` helper.
3. Remove any now-unused local implementations of the announcement channel lookup to ensure there is a single source of truth for the selection rules.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
Adds a new /event Discord cog to scaffold event creation/editing/listing/announcement flows, including a Pydantic-backed modal schema and supporting UI views.
Changes:
- Introduces an
Eventcog with slash-command actions (create/edit/show/delete/list/announce/myevents). - Adds
EventSchema(Pydantic) for modal-driven event data entry and parsing. - Adds the
capy_discord.exts.eventpackage initializer.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
capy_discord/exts/event/event.py |
New Event cog implementation with dropdown selection views, announcement posting, and RSVP lookup logic. |
capy_discord/exts/event/_schemas.py |
New Pydantic schema for event fields, including custom parsing validators for date/time string inputs. |
capy_discord/exts/event/__init__.py |
Adds module docstring for the new extension package. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @field_validator("event_date", mode="before") | ||
| @classmethod | ||
| def _parse_event_date(cls, value: object) -> date | object: | ||
| if isinstance(value, str): | ||
| value = value.strip() | ||
| return datetime.strptime(f"{value} +0000", "%m-%d-%Y %z").date() | ||
| return value | ||
|
|
||
| @field_validator("event_time", mode="before") | ||
| @classmethod | ||
| def _parse_event_time(cls, value: object) -> time | object: | ||
| if isinstance(value, str): | ||
| value = value.strip() | ||
| if " " in value: | ||
| parsed = datetime.strptime(f"{value} +0000", "%I:%M %p %z") | ||
| else: | ||
| parsed = datetime.strptime(f"{value} +0000", "%H:%M %z") | ||
| return parsed.timetz().replace(tzinfo=None) |
There was a problem hiding this comment.
New date/time parsing and validation logic in EventSchema is user-facing (it drives the modal input format) but currently has no unit tests. Adding tests for accepted formats (e.g., MM-DD-YYYY, HH:MM, HH:MM AM/PM) and failure/blank-input behavior would help prevent regressions, similar to how other cogs are covered under tests/capy_discord/exts/.
| now = datetime.now().astimezone() | ||
| upcoming_events: list[EventSchema] = [] | ||
| past_events: list[EventSchema] = [] | ||
|
|
||
| for event in events: | ||
| event_time = datetime.combine(event.event_date, event.event_time) | ||
| if event_time.tzinfo is None: | ||
| local_tz = datetime.now().astimezone().tzinfo or ZoneInfo("UTC") | ||
| event_time = event_time.replace(tzinfo=local_tz) | ||
|
|
||
| if event_time >= now: | ||
| upcoming_events.append(event) | ||
| else: | ||
| past_events.append(event) | ||
|
|
||
| # Sort events | ||
| upcoming_events.sort(key=lambda e: datetime.combine(e.event_date, e.event_time)) | ||
| past_events.sort(key=lambda e: datetime.combine(e.event_date, e.event_time), reverse=True) | ||
|
|
There was a problem hiding this comment.
handle_list_action classifies events using timezone-normalized datetimes, but then sorts with datetime.combine(...) again without normalizing tzinfo. If some EventSchema.event_time values are tz-aware (e.g., from the default_factory) and others are naive (e.g., parsed input strips tzinfo), Python will raise TypeError: can't compare offset-naive and offset-aware datetimes during sorting or produce inconsistent ordering. Consider centralizing event datetime construction in a helper that always returns an aware datetime (or always naive) and use it consistently for both classification and sorting.
| # Check if user has registered for this event | ||
| if await self._is_user_registered(event, guild, interaction.user): | ||
| registered_events.append(event) | ||
|
|
||
| registered_events.sort(key=lambda e: datetime.combine(e.event_date, e.event_time)) | ||
|
|
There was a problem hiding this comment.
registered_events.sort(key=lambda e: datetime.combine(e.event_date, e.event_time)) has the same naive/aware datetime mixing risk as the list command. If any events have tz-aware event_time while others are naive, sorting will throw a TypeError. Reuse the same timezone-normalizing helper used for comparisons so sorting is stable and error-free.
| # Check if bot has permission to post in the channel | ||
| if not announcement_channel.permissions_for(guild.me).send_messages: | ||
| embed = error_embed( | ||
| "No Permission", | ||
| "I don't have permission to send messages in the announcement channel.", | ||
| ) | ||
| await interaction.response.send_message(embed=embed, ephemeral=True) | ||
| return |
There was a problem hiding this comment.
announcement_channel.permissions_for(guild.me) can raise if guild.me is None (it’s a cached member and may be missing depending on intents/cache state). Use guild.membership/guild.get_member(self.bot.user.id)/interaction.guild.me with a fallback, or announcement_channel.permissions_for(guild.self_member) (if available in your discord.py version) to avoid an AttributeError and ensure permissions are checked against the bot member.
| users = [user async for user in reaction.users()] | ||
| if user in users: | ||
| return True |
There was a problem hiding this comment.
users = [user async for user in reaction.users()] materializes all users for the reaction just to check membership, which can be slow and memory-heavy on large reactions. Prefer iterating the async iterator and short-circuiting on the first matching user.id (and avoid fetching users for unrelated emojis).
| users = [user async for user in reaction.users()] | |
| if user in users: | |
| return True | |
| async for reacted_user in reaction.users(): | |
| if reacted_user.id == user.id: | |
| return True |
| if original_event in guild_events: | ||
| idx = guild_events.index(original_event) | ||
| guild_events[idx] = updated_event | ||
|
|
There was a problem hiding this comment.
event_announcements is keyed by event.event_name. If an event is edited and its name changes, RSVP tracking for that event will break because _is_user_registered will look up the new name while the announcement map still uses the old one. Consider adding a stable event_id field to EventSchema (hidden from the modal via json_schema_extra) and key event_announcements by that ID, or update the mapping when the name changes during _handle_event_update.
| # Keep any announcement / RSVP mapping in sync if the event name changed. | |
| if original_event.event_name != updated_event.event_name: | |
| announcements = getattr(self, "event_announcements", None) | |
| if isinstance(announcements, dict): | |
| guild_announcements = announcements.get(guild_id) | |
| if isinstance(guild_announcements, dict) and original_event.event_name in guild_announcements: | |
| guild_announcements[updated_event.event_name] = guild_announcements.pop( | |
| original_event.event_name | |
| ) |
| event_date: date = Field( | ||
| title="Event Date", | ||
| description="Date of the event (MM-DD-YYYY)", | ||
| default_factory=date.today, | ||
| ) | ||
| event_time: time = Field( | ||
| title="Event Time", | ||
| description="Time of the event (HH:MM, 24-hour) or (HH:MM AM/PM)", | ||
| default_factory=lambda: datetime.now().astimezone().time(), | ||
| ) | ||
| location: str = Field(title="Location", description="Location of the event", max_length=200, default="") | ||
| description: str = Field( | ||
| title="Description", description="Detailed description of the event", max_length=1000, default="" | ||
| ) | ||
|
|
||
| @field_validator("event_date", mode="before") | ||
| @classmethod | ||
| def _parse_event_date(cls, value: object) -> date | object: | ||
| if isinstance(value, str): | ||
| value = value.strip() | ||
| return datetime.strptime(f"{value} +0000", "%m-%d-%Y %z").date() | ||
| return value | ||
|
|
||
| @field_validator("event_time", mode="before") | ||
| @classmethod | ||
| def _parse_event_time(cls, value: object) -> time | object: | ||
| if isinstance(value, str): | ||
| value = value.strip() | ||
| if " " in value: | ||
| parsed = datetime.strptime(f"{value} +0000", "%I:%M %p %z") | ||
| else: | ||
| parsed = datetime.strptime(f"{value} +0000", "%H:%M %z") | ||
| return parsed.timetz().replace(tzinfo=None) |
There was a problem hiding this comment.
EventSchema.event_date / event_time have defaults, and ModelModal marks them as not-required, but the modal will still pass empty strings ("") when users leave the inputs blank. The validators currently attempt to parse "" and will raise a validation error instead of falling back to the default. Treat empty/whitespace-only strings as missing (e.g., return PydanticUndefined/None and let defaults apply) or make these fields explicitly required in the UI to match the validation behavior.
| """ | ||
| super().__init__(timeout=60) | ||
| self.event_list = events | ||
| self.cog = cog |
There was a problem hiding this comment.
EventDropdownView.__init__ stores self.cog = cog, but that attribute is never referenced anywhere in this module. If it’s not needed, remove it to avoid dead state; if it is intended for future use, consider using it in on_select (or adding a comment) so it’s clear why it’s retained.
| self.cog = cog |
|
Few things:
|
…nsolidated the timezone logic into helper methods
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 16 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| description=f"Are you sure you want to delete **{selected_event.event_name}**?", | ||
| color=discord.Color.red(), | ||
| ) | ||
| await interaction.response.send_message(embed=embed, view=view, ephemeral=True) |
There was a problem hiding this comment.
ConfirmDeleteView is a BaseView but it’s sent via interaction.response.send_message(...) without setting view.message, so BaseView.on_timeout() can’t disable/edit the message on timeout. Prefer view.reply(...) or store view.message = await interaction.original_response() after sending so the timeout handler can update the UI.
| await interaction.response.send_message(embed=embed, view=view, ephemeral=True) | |
| await view.reply(interaction, embed=embed, ephemeral=True) |
| """ | ||
| super().__init__(timeout=60) | ||
| self.event_list = events | ||
| self.cog = cog |
There was a problem hiding this comment.
EventDropdownView.__init__ stores cog on self.cog but the attribute is never used. Removing the parameter/attribute (or using it for something concrete like permission checks) will reduce dead state and make the view easier to reuse.
| self.cog = cog |
| event_date: date = Field( | ||
| title="Event Date", | ||
| description="Date of the event (MM-DD-YYYY)", | ||
| default_factory=date.today, | ||
| ) | ||
| event_time: time = Field( | ||
| title="Event Time", | ||
| description="Time of the event (HH:MM, 24-hour) or (HH:MM AM/PM)", | ||
| default_factory=lambda: datetime.now().astimezone().time(), | ||
| ) |
There was a problem hiding this comment.
event_date/event_time use default_factory, which makes them non-required in Pydantic, so ModelModal will render them as optional with no default text. If the user leaves them blank, Discord submits an empty string and the validators will raise a parsing error instead of using the default. Either (1) make these fields required (remove defaults) or (2) treat blank strings as “missing” in the validators and substitute the intended default, and/or pass initial_data in the create modal to prefill current date/time.
| # Add upcoming events | ||
| for event in upcoming_events: | ||
| timestamp = int(self._event_datetime(event).timestamp()) | ||
| embed.add_field( | ||
| name=event.event_name, | ||
| value=f"**When:** <t:{timestamp}:F>\n**Where:** {event.location or 'TBD'}", | ||
| inline=False, | ||
| ) | ||
|
|
||
| # Add past events with [OLD] prefix | ||
| for event in past_events: | ||
| timestamp = int(self._event_datetime(event).timestamp()) | ||
| embed.add_field( | ||
| name=f"[OLD] {event.event_name}", | ||
| value=f"**When:** <t:{timestamp}:F>\n**Where:** {event.location or 'TBD'}", | ||
| inline=False, | ||
| ) |
There was a problem hiding this comment.
The list output builds a single embed field per event. Discord embeds are limited to 25 fields; if a guild has more than 25 events this will raise an HTTPException and the command will fail. Paginate the results (multiple embeds / multiple messages) or truncate with a clear indicator when the limit is reached.
| # Add upcoming events | |
| for event in upcoming_events: | |
| timestamp = int(self._event_datetime(event).timestamp()) | |
| embed.add_field( | |
| name=event.event_name, | |
| value=f"**When:** <t:{timestamp}:F>\n**Where:** {event.location or 'TBD'}", | |
| inline=False, | |
| ) | |
| # Add past events with [OLD] prefix | |
| for event in past_events: | |
| timestamp = int(self._event_datetime(event).timestamp()) | |
| embed.add_field( | |
| name=f"[OLD] {event.event_name}", | |
| value=f"**When:** <t:{timestamp}:F>\n**Where:** {event.location or 'TBD'}", | |
| inline=False, | |
| ) | |
| # Prepare all event fields (upcoming first, then past) | |
| event_fields: list[tuple[str, str]] = [] | |
| # Upcoming events | |
| for event in upcoming_events: | |
| timestamp = int(self._event_datetime(event).timestamp()) | |
| name = event.event_name | |
| value = f"**When:** <t:{timestamp}:F>\n**Where:** {event.location or 'TBD'}" | |
| event_fields.append((name, value)) | |
| # Past events with [OLD] prefix | |
| for event in past_events: | |
| timestamp = int(self._event_datetime(event).timestamp()) | |
| name = f"[OLD] {event.event_name}" | |
| value = f"**When:** <t:{timestamp}:F>\n**Where:** {event.location or 'TBD'}" | |
| event_fields.append((name, value)) | |
| # Discord embeds can have at most 25 fields; truncate with an indicator if needed | |
| max_fields = 25 | |
| if len(event_fields) > max_fields: | |
| # Show as many events as possible while reserving one field for the truncation notice | |
| visible_fields = event_fields[: max_fields - 1] | |
| remaining = len(event_fields) - len(visible_fields) | |
| for name, value in visible_fields: | |
| embed.add_field(name=name, value=value, inline=False) | |
| embed.add_field( | |
| name="More events not shown", | |
| value=f"{remaining} additional event(s) could not be displayed due to Discord's embed field limit.", | |
| inline=False, | |
| ) | |
| else: | |
| for name, value in event_fields: | |
| embed.add_field(name=name, value=value, inline=False) |
| users = [user async for user in reaction.users()] | ||
| if user in users: | ||
| return True |
There was a problem hiding this comment.
_is_user_registered materializes the entire reaction user list (users = [u async for u in reaction.users()]) just to check membership. For large announcements this can be slow and memory-heavy. Iterate the async generator and short-circuit on a matching user id instead of building a list.
| users = [user async for user in reaction.users()] | |
| if user in users: | |
| return True | |
| async for reaction_user in reaction.users(): | |
| if reaction_user.id == user.id: | |
| return True |
| # Check if bot has permission to post in the channel | ||
| if not announcement_channel.permissions_for(guild.me).send_messages: | ||
| embed = error_embed( | ||
| "No Permission", | ||
| "I don't have permission to send messages in the announcement channel.", | ||
| ) | ||
| await interaction.response.send_message(embed=embed, ephemeral=True) | ||
| return |
There was a problem hiding this comment.
announcement_channel.permissions_for(guild.me) can raise if guild.me is None (bot member not cached). Resolve the bot’s member safely (e.g., guild.get_member(bot.user.id) or fetch) and handle the None case before calling permissions_for.
| # [DB CALL]: Store announcement message ID for RSVP tracking | ||
| if guild.id not in self.event_announcements: | ||
| self.event_announcements[guild.id] = {} | ||
| self.event_announcements[guild.id][selected_event.event_name] = message.id |
There was a problem hiding this comment.
Announcement tracking uses event_name as the key (event_announcements[guild.id][selected_event.event_name] = message.id). This breaks RSVP tracking if an event is renamed via edit, and collides if two events share the same name. Use a stable unique identifier for events (e.g., generated id/uuid stored in the schema) and key announcements by that id; also ensure edit/delete update/remove the associated announcement mapping.
| self.event_announcements[guild.id][selected_event.event_name] = message.id | |
| self.event_announcements[guild.id][selected_event.id] = message.id |
| upcoming_events.sort(key=lambda e: self._event_datetime(e)) | ||
| past_events.sort(key=lambda e: self._event_datetime(e), reverse=True) |
There was a problem hiding this comment.
This 'lambda' is just a simple wrapper around a callable object. Use that object directly.
| upcoming_events.sort(key=lambda e: self._event_datetime(e)) | |
| past_events.sort(key=lambda e: self._event_datetime(e), reverse=True) | |
| upcoming_events.sort(key=self._event_datetime) | |
| past_events.sort(key=self._event_datetime, reverse=True) |
| upcoming_events.sort(key=lambda e: self._event_datetime(e)) | ||
| past_events.sort(key=lambda e: self._event_datetime(e), reverse=True) |
There was a problem hiding this comment.
This 'lambda' is just a simple wrapper around a callable object. Use that object directly.
| upcoming_events.sort(key=lambda e: self._event_datetime(e)) | |
| past_events.sort(key=lambda e: self._event_datetime(e), reverse=True) | |
| upcoming_events.sort(key=self._event_datetime) | |
| past_events.sort(key=self._event_datetime, reverse=True) |
| if await self._is_user_registered(event, guild, interaction.user): | ||
| registered_events.append(event) | ||
|
|
||
| registered_events.sort(key=lambda e: self._event_datetime(e)) |
There was a problem hiding this comment.
This 'lambda' is just a simple wrapper around a callable object. Use that object directly.
| registered_events.sort(key=lambda e: self._event_datetime(e)) | |
| registered_events.sort(key=self._event_datetime) |
… time handling from system timestamp to EST time.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 11 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Event System Configuration | ||
| announcement_channel_name: str = "test-announcements" | ||
|
|
There was a problem hiding this comment.
A new setting is introduced but example.env doesn’t document it. Consider adding ANNOUNCEMENT_CHANNEL_NAME= (or whatever env var name is expected by pydantic-settings) to example.env/README so deployments know how to configure the announcement channel.
| if not events: | ||
| return | ||
|
|
||
| options = [discord.SelectOption(label=event.event_name[:100], value=str(i)) for i, event in enumerate(events)] |
There was a problem hiding this comment.
Discord select menus support a maximum of 25 options. options = [...] for i, event in enumerate(events) will raise when a guild has >25 events. Consider limiting to 25 (as done in exts/tools/hotswap.py) and/or implementing pagination.
| options = [discord.SelectOption(label=event.event_name[:100], value=str(i)) for i, event in enumerate(events)] | |
| max_options = 25 | |
| if len(events) > max_options: | |
| logging.warning( | |
| "EventDropdownView received %d events, truncating to %d options for Discord select menu.", | |
| len(events), | |
| max_options, | |
| ) | |
| options = [ | |
| discord.SelectOption(label=event.event_name[:100], value=str(i)) | |
| for i, event in enumerate(events[:max_options]) | |
| ] |
| await interaction.response.defer(ephemeral=True) | ||
|
|
||
| view = EventDropdownView(events, self, "Select an event to delete", self._on_delete_select) | ||
| await interaction.followup.send(content="Select an event to delete:", view=view, ephemeral=True) |
There was a problem hiding this comment.
Same timeout/message-tracking issue: the delete dropdown view is sent with followup.send without setting view.message, so BaseView.on_timeout can’t disable the menu. Consider capturing the sent message and storing it on the view (or use view.reply).
| await interaction.followup.send(content="Select an event to delete:", view=view, ephemeral=True) | |
| message = await interaction.followup.send(content="Select an event to delete:", view=view, ephemeral=True) | |
| view.message = message |
| @app_commands.command(name="event", description="Manage events") | ||
| @app_commands.describe(action="The action to perform with events") | ||
| @app_commands.choices( | ||
| action=[ | ||
| app_commands.Choice(name="create", value="create"), | ||
| app_commands.Choice(name="edit", value="edit"), | ||
| app_commands.Choice(name="show", value="show"), | ||
| app_commands.Choice(name="delete", value="delete"), | ||
| app_commands.Choice(name="list", value="list"), | ||
| app_commands.Choice(name="announce", value="announce"), | ||
| app_commands.Choice(name="myevents", value="myevents"), | ||
| ] | ||
| ) | ||
| async def event(self, interaction: discord.Interaction, action: app_commands.Choice[str]) -> None: | ||
| """Manage events based on the action specified.""" | ||
| match action.value: | ||
| case "create": | ||
| await self.handle_create_action(interaction) | ||
| case "edit": | ||
| await self.handle_edit_action(interaction) | ||
| case "show": | ||
| await self.handle_show_action(interaction) | ||
| case "delete": | ||
| await self.handle_delete_action(interaction) | ||
| case "list": | ||
| await self.handle_list_action(interaction) | ||
| case "announce": | ||
| await self.handle_announce_action(interaction) | ||
| case "myevents": | ||
| await self.handle_myevents_action(interaction) | ||
|
|
There was a problem hiding this comment.
No automated tests are added for the new /event command flows. There are existing pytest-based cog tests under tests/capy_discord/exts/; adding tests for at least the core command routing and event CRUD handlers would help prevent regressions.
| event_date: date = Field( | ||
| title="Event Date", | ||
| description="Date of the event (MM-DD-YYYY)", | ||
| default_factory=date.today, | ||
| ) | ||
| event_time: time = Field( | ||
| title="Event Time", | ||
| description="Time of the event (HH:MM, 24-hour) or (HH:MM AM/PM)", | ||
| default_factory=lambda: datetime.now().astimezone().time(), | ||
| ) | ||
| location: str = Field(title="Location", description="Location of the event", max_length=200, default="") | ||
| description: str = Field( | ||
| title="Description", description="Detailed description of the event", max_length=1000, default="" | ||
| ) | ||
|
|
||
| @field_validator("event_date", mode="before") | ||
| @classmethod | ||
| def _parse_event_date(cls, value: object) -> date | object: | ||
| if isinstance(value, str): | ||
| value = value.strip() | ||
| return datetime.strptime(f"{value} +0000", "%m-%d-%Y %z").date() | ||
| return value |
There was a problem hiding this comment.
event_date/event_time have default_factory, so ModelModal will mark them as not required. However, if the user leaves the inputs blank, Discord sends an empty string and the validators will attempt to strptime(''), causing validation errors instead of using defaults. Consider either making these fields required (no defaults) or updating the validators to treat empty/whitespace-only strings as “missing” and return the default value.
| # Build embed | ||
| embed = discord.Embed( | ||
| title="Your Registered Events", | ||
| description="Events you have registered for by reacting with ✅", | ||
| color=discord.Color.purple(), | ||
| ) | ||
|
|
||
| if not registered_events: | ||
| embed.description = ( | ||
| "You haven't registered for any upcoming events.\nReact to event announcements with ✅ to register!" | ||
| ) | ||
| await interaction.followup.send(embed=embed, ephemeral=True) | ||
| return | ||
|
|
||
| # Add registered events | ||
| for event in registered_events: | ||
| timestamp = int(self._event_datetime(event).timestamp()) | ||
| embed.add_field( | ||
| name=event.event_name, | ||
| value=f"**When:** <t:{timestamp}:F>\n**Where:** {event.location or 'TBD'}", | ||
| inline=False, | ||
| ) | ||
|
|
||
| await interaction.followup.send(embed=embed, ephemeral=True) | ||
|
|
There was a problem hiding this comment.
Embeds have a 25-field limit; myevents adds one field per registered event and can fail for users registered for many events. Consider pagination/splitting into multiple embeds or limiting output.
| # Build embed | |
| embed = discord.Embed( | |
| title="Your Registered Events", | |
| description="Events you have registered for by reacting with ✅", | |
| color=discord.Color.purple(), | |
| ) | |
| if not registered_events: | |
| embed.description = ( | |
| "You haven't registered for any upcoming events.\nReact to event announcements with ✅ to register!" | |
| ) | |
| await interaction.followup.send(embed=embed, ephemeral=True) | |
| return | |
| # Add registered events | |
| for event in registered_events: | |
| timestamp = int(self._event_datetime(event).timestamp()) | |
| embed.add_field( | |
| name=event.event_name, | |
| value=f"**When:** <t:{timestamp}:F>\n**Where:** {event.location or 'TBD'}", | |
| inline=False, | |
| ) | |
| await interaction.followup.send(embed=embed, ephemeral=True) | |
| # Build embed(s) | |
| base_title = "Your Registered Events" | |
| base_description = "Events you have registered for by reacting with ✅" | |
| if not registered_events: | |
| embed = discord.Embed( | |
| title=base_title, | |
| description=( | |
| "You haven't registered for any upcoming events.\n" | |
| "React to event announcements with ✅ to register!" | |
| ), | |
| color=discord.Color.purple(), | |
| ) | |
| await interaction.followup.send(embed=embed, ephemeral=True) | |
| return | |
| # Discord embeds support up to 25 fields. Paginate events in chunks of 25. | |
| total_pages = (len(registered_events) + 24) // 25 | |
| for page_index, start in enumerate(range(0, len(registered_events), 25)): | |
| end = start + 25 | |
| page_events = registered_events[start:end] | |
| embed = discord.Embed( | |
| title=base_title, | |
| description=base_description, | |
| color=discord.Color.purple(), | |
| ) | |
| for event in page_events: | |
| timestamp = int(self._event_datetime(event).timestamp()) | |
| embed.add_field( | |
| name=event.event_name, | |
| value=( | |
| f"**When:** <t:{timestamp}:F>\n" | |
| f"**Where:** {event.location or 'TBD'}" | |
| ), | |
| inline=False, | |
| ) | |
| if total_pages > 1: | |
| embed.set_footer(text=f"Page {page_index + 1}/{total_pages}") | |
| await interaction.followup.send(embed=embed, ephemeral=True) |
| # Check if user has registered for this event | ||
| if await self._is_user_registered(event, guild, interaction.user): | ||
| registered_events.append(event) |
There was a problem hiding this comment.
/myevents calls _is_user_registered for every upcoming event, and _is_user_registered performs an HTTP fetch_message each time. This becomes N API calls per command and can hit rate limits in guilds with many events. Consider persisting RSVP data when reactions are added/removed (reaction listeners), caching announcement messages/reactions, or fetching messages in batches where possible.
| self.log.info("Deleted event '%s' from guild %s", selected_event.event_name, guild_id) | ||
|
|
||
| success = success_embed("Event Deleted", "The event has been deleted successfully!") | ||
| await interaction.followup.send(embed=success, ephemeral=True) |
There was a problem hiding this comment.
If the user clicks Cancel or the confirmation view times out (view.value is False/None), this handler sends no follow-up message. Consider explicitly handling those cases (e.g., info embed for cancel and a timeout notice) so the user gets feedback and the flow completes cleanly.
| await interaction.followup.send(embed=success, ephemeral=True) | |
| await interaction.followup.send(embed=success, ephemeral=True) | |
| elif view.value is False: | |
| cancel_embed = discord.Embed( | |
| title="Deletion Cancelled", | |
| description="The event was not deleted.", | |
| color=discord.Color.orange(), | |
| ) | |
| await interaction.followup.send(embed=cancel_embed, ephemeral=True) | |
| else: | |
| timeout_embed = discord.Embed( | |
| title="Timed Out", | |
| description="Event deletion was not confirmed in time, so no changes were made.", | |
| color=discord.Color.orange(), | |
| ) | |
| await interaction.followup.send(embed=timeout_embed, ephemeral=True) |
| self.event_announcements[guild.id][selected_event.event_name] = message.id | ||
|
|
There was a problem hiding this comment.
RSVP tracking keys announcements by event.event_name. If two events share a name, or if an event is renamed via edit, the mapping will collide/break and registrations may be attributed incorrectly. Consider storing a stable identifier for events (e.g., generated event_id) and key the announcement mapping by that instead; also consider cleaning up mappings on delete/edit.
| self.event_announcements[guild.id][selected_event.event_name] = message.id | |
| # Prefer a stable event identifier (e.g., database id) over the mutable name | |
| event_key = ( | |
| getattr(selected_event, "id", None) | |
| or getattr(selected_event, "event_id", None) | |
| or selected_event.event_name | |
| ) | |
| self.event_announcements[guild.id][event_key] = message.id |
| await interaction.response.defer(ephemeral=True) | ||
|
|
||
| view = EventDropdownView(events, self, "Select an event to announce", self._on_announce_select) | ||
| await interaction.followup.send(content="Select an event to announce:", view=view, ephemeral=True) |
There was a problem hiding this comment.
Same timeout/message-tracking issue for the announce dropdown: since the view is sent via followup.send without storing the message on the view, timeouts won’t disable the menu.
| await interaction.followup.send(content="Select an event to announce:", view=view, ephemeral=True) | |
| message = await interaction.followup.send(content="Select an event to announce:", view=view, ephemeral=True) | |
| view.message = message |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Check if bot has permission to post in the channel | ||
| if not announcement_channel.permissions_for(guild.me).send_messages: |
There was a problem hiding this comment.
guild.me can be None (e.g., member cache not available), which would cause permissions_for(guild.me) to raise at runtime. Resolve the bot member safely (e.g., guild.me or guild.get_member(self.bot.user.id) / fetch as needed) and handle the None case before checking permissions.
| # Check if bot has permission to post in the channel | |
| if not announcement_channel.permissions_for(guild.me).send_messages: | |
| # Safely resolve the bot member before checking permissions | |
| bot_member: discord.Member | None = guild.me | |
| bot_user = getattr(interaction.client, "user", None) | |
| if bot_member is None and bot_user is not None: | |
| bot_member = guild.get_member(bot_user.id) | |
| if bot_member is None and bot_user is not None: | |
| try: | |
| bot_member = await guild.fetch_member(bot_user.id) | |
| except (discord.HTTPException, discord.Forbidden, discord.NotFound): | |
| bot_member = None | |
| if bot_member is None: | |
| embed = error_embed( | |
| "Permission Check Failed", | |
| "I couldn't determine my permissions in the announcement channel. " | |
| "Please check my role and try again.", | |
| ) | |
| await interaction.response.send_message(embed=embed, ephemeral=True) | |
| return | |
| # Check if bot has permission to post in the channel | |
| if not announcement_channel.permissions_for(bot_member).send_messages: |
| await interaction.response.defer(ephemeral=True) | ||
|
|
||
| view = EventDropdownView(events, self, f"Select an event to {action_name}", callback) | ||
| await interaction.followup.send(content=f"Select an event to {action_name}:", view=view, ephemeral=True) |
There was a problem hiding this comment.
This view is sent via interaction.followup.send(...), but BaseView's timeout handling relies on self.message being set to edit/disable components. As written, EventDropdownView never records the message, so on-timeout it won't update the UI. Capture the sent message (e.g., use wait=True and assign to view.message, or add a helper on BaseView for followup sends).
| await interaction.followup.send(content=f"Select an event to {action_name}:", view=view, ephemeral=True) | |
| message = await interaction.followup.send( | |
| content=f"Select an event to {action_name}:", | |
| view=view, | |
| ephemeral=True, | |
| wait=True, | |
| ) | |
| view.message = message |
| message_id = guild_announcements.get(event.event_name) | ||
|
|
There was a problem hiding this comment.
RSVP tracking keys announcement messages by event.event_name, which is not stable (duplicates across events, and editing/renaming an event will break lookups). Use a stable identifier (e.g., add an event_id field) for event_announcements keys, or update the mapping when an event is renamed.
| message_id = guild_announcements.get(event.event_name) | |
| # Prefer a stable identifier (e.g., event.id or event.event_id) for lookups, | |
| # but fall back to event_name to preserve existing behavior. | |
| candidate_keys: list[Any] = [] | |
| if hasattr(event, "id"): | |
| candidate_keys.append(getattr(event, "id")) | |
| if hasattr(event, "event_id"): | |
| candidate_keys.append(getattr(event, "event_id")) | |
| candidate_keys.append(event.event_name) | |
| message_id = None | |
| for key in candidate_keys: | |
| if key is None: | |
| continue | |
| message_id = guild_announcements.get(key) | |
| if message_id: | |
| break |
| if not guild_id: | ||
| embed = error_embed("No Server", f"Events must be {action_name}ed in a server.") |
There was a problem hiding this comment.
The error message builds past tense by appending "ed" to action_name, which produces incorrect strings like "deleteed" and "announceed". Use an explicit mapping for the user-facing verb/past tense (or pass the fully formatted verb into this helper) so messages read correctly for all actions.
| if not guild_id: | |
| embed = error_embed("No Server", f"Events must be {action_name}ed in a server.") | |
| # Map action names to correctly formatted past tense/adjectival forms for messages. | |
| action_past_map = { | |
| "edit": "edited", | |
| "view": "viewed", | |
| "delete": "deleted", | |
| "announce": "announced", | |
| } | |
| action_past = action_past_map.get(action_name, f"{action_name}ed") | |
| if not guild_id: | |
| embed = error_embed("No Server", f"Events must be {action_past} in a server.") |
| description=f"Are you sure you want to delete **{selected_event.event_name}**?", | ||
| color=discord.Color.red(), | ||
| ) | ||
| await interaction.response.send_message(embed=embed, view=view, ephemeral=True) |
There was a problem hiding this comment.
ConfirmDeleteView is sent with interaction.response.send_message(...) but the view never sets BaseView.message, so on-timeout it can't edit the message to disable components / show timed-out state. Consider sending via view.reply(...) or setting view.message = await interaction.original_response() after sending.
| await interaction.response.send_message(embed=embed, view=view, ephemeral=True) | |
| await interaction.response.send_message(embed=embed, view=view, ephemeral=True) | |
| view.message = await interaction.original_response() |
| """Format an event's date/time in EST for user-facing display.""" | ||
| event_dt_est = self._event_datetime(event).astimezone(ZoneInfo("America/New_York")) | ||
| return event_dt_est.strftime("%B %d, %Y at %I:%M %p EST") |
There was a problem hiding this comment.
This formatter hardcodes the timezone label as "EST", but America/New_York observes DST, so in summer the correct label is "EDT". Use %Z in the format string (or a neutral label like "ET") so the displayed timezone matches the actual offset.
| """Format an event's date/time in EST for user-facing display.""" | |
| event_dt_est = self._event_datetime(event).astimezone(ZoneInfo("America/New_York")) | |
| return event_dt_est.strftime("%B %d, %Y at %I:%M %p EST") | |
| """Format an event's date/time in Eastern Time for user-facing display.""" | |
| event_dt_est = self._event_datetime(event).astimezone(ZoneInfo("America/New_York")) | |
| return event_dt_est.strftime("%B %d, %Y at %I:%M %p %Z") |
| users = [user async for user in reaction.users()] | ||
| if user in users: | ||
| return True |
There was a problem hiding this comment.
This builds a full list of all users who reacted, which can be slow and memory-heavy for large events. Iterate the async users iterator and short-circuit on matching user.id (or otherwise avoid materializing the whole list).
| users = [user async for user in reaction.users()] | |
| if user in users: | |
| return True | |
| async for reacting_user in reaction.users(): | |
| if reacting_user.id == user.id: | |
| return True |
| event_date: date = Field( | ||
| title="Event Date", | ||
| description="Date of the event (MM-DD-YYYY)", | ||
| default_factory=date.today, | ||
| ) |
There was a problem hiding this comment.
event_date/event_time use default_factory, so ModelModal will mark these inputs as not required. If the user leaves an optional TextInput blank, Discord submits an empty string, and the current validators will try strptime on "", causing a validation error instead of falling back to the default. Consider making these fields required (remove defaults) and/or treating blank strings as missing in the validators so defaults apply.
Summary by Sourcery
Add a new Discord cog for event management, including schema validation and interaction flows for creating, viewing, editing, deleting, listing, announcing, and tracking user registrations for events.
New Features: