Feature/cap 236 guild #96
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…user.-testing still in progress
…e/cap-236-guild
Reviewer's GuideThis PR adds automatic guild linkage during profile creation, tightens safety checks and response handling in the profile deletion flow, streamlines button confirmation views by storing interactions and refining timeouts, and fixes a misnamed import. Sequence diagram for profile creation and automatic guild linkagesequenceDiagram
participant U as actor User
participant D as Discord
participant B as Bot
participant PC as ProfileCog
participant DB as Database
participant G as Guild
U->>D: Initiates profile creation (in guild or DM)
D->>B: Sends interaction
B->>PC: Handles _save_profile()
PC->>DB: Checks for existing user
alt New user
PC->>DB: Adds new user document
PC->>PC: Calls _link_user_to_guilds(interaction)
PC->>PC: _get_target_guild_ids(interaction)
alt In guild
PC->>PC: Returns current guild ID
else In DM
PC->>B: Gets mutual guilds
PC->>PC: Returns all mutual guild IDs
end
loop For each guild
PC->>DB: bulk_update_attr(User, [user_id], "guilds", guild_id)
PC->>PC: _add_user_to_guild(guild_id, user_id)
PC->>DB: Ensure Guild document exists
PC->>DB: Update Guild.users
end
end
PC->>B: Logs success
Sequence diagram for improved profile deletion flow with confirmation and cleanupsequenceDiagram
participant U as actor User
participant D as Discord
participant B as Bot
participant PC as ProfileCog
participant V as ConfirmDeleteView
participant DB as Database
participant E as Event
participant G as Guild
U->>D: Requests profile deletion
D->>B: Sends interaction
B->>PC: Handles delete_profile()
PC->>V: Shows confirmation view
U->>V: Presses Accept/Cancel or times out
V->>PC: Returns value and interaction
alt Accept pressed
PC->>V: Responds with "Deleting your profile..."
PC->>PC: Calls delete_profile_from_events(user)
loop For each event
PC->>E: Removes user from event
E->>DB: Saves event
end
PC->>PC: Calls delete_profile_from_guilds(user)
loop For each guild
PC->>G: Removes user from guild
G->>DB: Saves guild
end
PC->>DB: Deletes user document
PC->>V: Responds with success message
else Cancel pressed
PC->>V: Responds with cancellation message
else Timeout
PC->>V: Responds with timeout message
end
Entity relationship diagram for User and Guild linkageerDiagram
USER {
int id
list guilds
list events
}
GUILD {
int id
list users
}
USER ||--o{ GUILD : "belongs to"
GUILD ||--o{ USER : "has users"
Class diagram for updated ProfileCog and confirmation view classesclassDiagram
class ProfileCog {
+_save_profile(interaction, profile_data_dict)
+_get_target_guild_ids(interaction)
+_ensure_guild_exists(guild_id)
+_add_user_to_guild(guild_id, user_id)
+_link_user_to_guilds(interaction)
+delete_profile(interaction)
}
class ConfirmDeleteView {
+timeout: int
+value: bool | None
+interaction: Interaction | None
+wait()
}
class AcceptCancelView {
+value: bool | None
+interaction: Interaction | None
+accept(interaction, button)
+cancel(interaction, button)
}
ConfirmDeleteView --|> AcceptCancelView
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 there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/capy_app/frontend/cogs/features/profile_cog.py:922` </location>
<code_context>
return
- view = ConfirmDeleteView()
+ view = ConfirmDeleteView(timeout=60)
await interaction.edit_original_response(
content="⚠️ Are you sure you want to delete your profile? This action cannot be undone.",
</code_context>
<issue_to_address>
**suggestion:** Consider making the timeout value configurable.
Hardcoding the timeout to 60 seconds reduces flexibility. Making it configurable would better support different use cases.
Suggested implementation:
```python
async def show_profile_embed(
self,
message_or_interaction: discord.Message | discord.Interaction,
delete_profile_timeout: int = 60,
```
```python
view = ConfirmDeleteView(timeout=delete_profile_timeout)
```
If `show_profile_embed` is called from other places, you may want to pass the `delete_profile_timeout` argument from those call sites, or rely on the default value.
You may also want to document the new parameter in the function's docstring.
</issue_to_address>
### Comment 2
<location> `src/capy_app/frontend/interactions/bases/button_base.py:85` </location>
<code_context>
self.value = True
self._completed = True
- await self._send_status_message("Operation accepted")
self.stop()
@discord.ui.button(label="Cancel", style=ButtonStyle.danger)
</code_context>
<issue_to_address>
**nitpick:** Duplicate call to self.stop() in cancel handler.
Please remove the redundant 'self.stop()' call to simplify the method.
</issue_to_address>
### Comment 3
<location> `src/capy_app/frontend/cogs/features/profile_cog.py:800` </location>
<code_context>
+ # Non-fatal: log and continue
+ self.logger.error(f"Failed to add user {user_id} to Guild.users for {guild_id}: {e}")
+
+ async def _link_user_to_guilds(self, interaction: discord.Interaction) -> None:
+ """Link a user to their guilds after profile creation.
+
</code_context>
<issue_to_address>
**issue (complexity):** Consider merging duplicate loops and consolidating error handling in profile-guild linking and event RSVP removal functions to simplify the code.
Here are two small refactorings that keep all existing behavior but remove a lot of the duplicated loops, logging and try/catch scaffolding:
1) **Merge the two loops in `_link_user_to_guilds` into one**
This folds the “add to User.guilds” and “add to Guild.users” steps into a single pass:
```python
async def _link_user_to_guilds(self, interaction: discord.Interaction) -> None:
user_id = interaction.user.id
guild_ids = self._get_target_guild_ids(interaction)
if not guild_ids:
self.logger.warning(f"No guilds found for user {user_id}")
return
success = 0
for gid in guild_ids:
try:
# 1) update the user side
Database.bulk_update_attr(User, [user_id], "guilds", gid)
# 2) update the guild side
guild = self._ensure_guild_exists(gid)
if user_id not in (guild.users or []):
guild.users = sorted((guild.users or []) + [user_id])
Database.update_document(guild, {"users": guild.users})
success += 1
except Exception as e:
self.logger.error(f"Failed linking user {user_id} ↔ guild {gid}: {e}")
self.logger.info(f"Linked user {user_id} to {success}/{len(guild_ids)} guild(s)")
```
2) **Collapse the three RSVP‐removals inside `delete_profile_from_events`**
Use a small map to drive all three cases and call `save()` once:
```python
async def delete_profile_from_events(user):
if not getattr(user, "events", None):
return
for eid in user.events:
event = Database.get_document(Event, eid)
if not event:
continue
for attr, reaction in (
("yes_users", "yes"),
("no_users", "no"),
("maybe_users", "maybe"),
):
users = getattr(event, attr)
if user.id in users:
users.remove(user.id)
event.details.reactions.modify(reaction, -1)
event.save()
```
These two changes
- Remove duplicate `for …` loops and nested `try/except`s
- Consolidate logging to one place each
- Keep exactly the same side‐effects (DB writes + reaction modifications)
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| return | ||
|
|
||
| view = ConfirmDeleteView() | ||
| view = ConfirmDeleteView(timeout=60) |
There was a problem hiding this comment.
suggestion: Consider making the timeout value configurable.
Hardcoding the timeout to 60 seconds reduces flexibility. Making it configurable would better support different use cases.
Suggested implementation:
async def show_profile_embed(
self,
message_or_interaction: discord.Message | discord.Interaction,
delete_profile_timeout: int = 60, view = ConfirmDeleteView(timeout=delete_profile_timeout)If show_profile_embed is called from other places, you may want to pass the delete_profile_timeout argument from those call sites, or rely on the default value.
You may also want to document the new parameter in the function's docstring.
| self.value = True | ||
| self._completed = True | ||
| await self._send_status_message("Operation accepted") | ||
| self.stop() |
There was a problem hiding this comment.
nitpick: Duplicate call to self.stop() in cancel handler.
Please remove the redundant 'self.stop()' call to simplify the method.
… into feature/cap-236-guild
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Introduce guild linking capability on profile creation and enhance profile deletion workflow with robust confirmation handling and cleanup
New Features:
Enhancements:
Chores: