Migrate all discord-related apis from runtime to dapi. #12
Conversation
📝 WalkthroughWalkthroughAdds a broad set of new API request handlers, types, and module exports across automoderation, channels, guilds, messages, reactions, invites, interactions, commands, and webhooks. Each handler is a serde-serializable struct implementing ApiReq that performs validation/permission checks and delegates work to the provider controller. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @Miya25. * #12 (comment) The following files were modified: * `crates/dapi/src/api/automoderation/create_auto_moderation_rule.rs` * `crates/dapi/src/api/automoderation/delete_auto_moderation_rule.rs` * `crates/dapi/src/api/automoderation/edit_auto_moderation_rule.rs` * `crates/dapi/src/api/channels/create_channel_invite.rs` * `crates/dapi/src/api/channels/delete_channel.rs` * `crates/dapi/src/api/channels/delete_channel_permission.rs` * `crates/dapi/src/api/channels/edit_channel_permissions.rs` * `crates/dapi/src/api/channels/get_channel.rs` * `crates/dapi/src/api/commands/create_guild_command.rs` * `crates/dapi/src/api/commands/create_guild_commands.rs` * `crates/dapi/src/api/commands/get_guild_command.rs` * `crates/dapi/src/api/commands/get_guild_commands.rs` * `crates/dapi/src/api/guilds/add_guild_member_role.rs` * `crates/dapi/src/api/guilds/create_guild_ban.rs` * `crates/dapi/src/api/guilds/create_guild_channel.rs` * `crates/dapi/src/api/guilds/create_guild_role.rs` * `crates/dapi/src/api/guilds/delete_guild_role.rs` * `crates/dapi/src/api/guilds/get_guild.rs` * `crates/dapi/src/api/guilds/get_guild_ban.rs` * `crates/dapi/src/api/guilds/get_guild_bans.rs` * `crates/dapi/src/api/guilds/get_guild_channels.rs` * `crates/dapi/src/api/guilds/get_guild_member.rs` * `crates/dapi/src/api/guilds/get_guild_preview.rs` * `crates/dapi/src/api/guilds/get_guild_role.rs` * `crates/dapi/src/api/guilds/get_guild_roles.rs` * `crates/dapi/src/api/guilds/list_active_guild_threads.rs` * `crates/dapi/src/api/guilds/list_guild_members.rs` * `crates/dapi/src/api/guilds/modify_guild_channel_positions.rs` * `crates/dapi/src/api/guilds/modify_guild_member.rs` * `crates/dapi/src/api/guilds/modify_guild_role.rs` * `crates/dapi/src/api/guilds/modify_guild_role_positions.rs` * `crates/dapi/src/api/guilds/remove_guild_ban.rs` * `crates/dapi/src/api/guilds/remove_guild_member.rs` * `crates/dapi/src/api/guilds/remove_guild_member_role.rs` * `crates/dapi/src/api/guilds/search_guild_members.rs` * `crates/dapi/src/api/interactions/create_followup_message.rs` * `crates/dapi/src/api/interactions/create_interaction_response.rs` * `crates/dapi/src/api/interactions/delete_followup_message.rs` * `crates/dapi/src/api/interactions/delete_original_interaction_response.rs` * `crates/dapi/src/api/interactions/edit_followup_message.rs` * `crates/dapi/src/api/interactions/edit_original_interaction_response.rs` * `crates/dapi/src/api/interactions/get_followup_message.rs` * `crates/dapi/src/api/interactions/get_original_interaction_response.rs` * `crates/dapi/src/api/invites/delete_invite.rs` * `crates/dapi/src/api/invites/get_invite.rs` * `crates/dapi/src/api/messages/bulk_delete_messages.rs` * `crates/dapi/src/api/messages/create_message.rs` * `crates/dapi/src/api/messages/crosspost_message.rs` * `crates/dapi/src/api/messages/delete_message.rs` * `crates/dapi/src/api/messages/edit_message.rs` * `crates/dapi/src/api/messages/get_channel_message.rs` * `crates/dapi/src/api/messages/get_channel_messages.rs` * `crates/dapi/src/api/reactions/create_reaction.rs` * `crates/dapi/src/api/reactions/delete_all_reactions.rs` * `crates/dapi/src/api/reactions/delete_all_reactions_for_emoji.rs` * `crates/dapi/src/api/reactions/delete_own_reaction.rs` * `crates/dapi/src/api/reactions/delete_user_reaction.rs` * `crates/dapi/src/api/reactions/get_reactions.rs` * `crates/dapi/src/api/webhooks/create_webhook.rs` * `crates/dapi/src/api/webhooks/delete_webhook.rs` * `crates/dapi/src/api/webhooks/execute_webhook.rs` * `crates/dapi/src/api/webhooks/get_channel_webhooks.rs` * `crates/dapi/src/api/webhooks/get_guild_webhooks.rs` * `crates/dapi/src/api/webhooks/get_webhook.rs` * `crates/dapi/src/api/webhooks/modify_webhook.rs` * `crates/dapi/src/apilist.rs`
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (14)
crates/dapi/src/api/automoderation/delete_auto_moderation_rule.rs (1)
4-8: Consider removingDefaultderive.The
Defaulttrait on a delete request struct could lead to accidental misuse, asDeleteAutoModerationRule::default()would produce a meaninglessrule_id(likely 0). IfDefaultis required for some specific deserialization scenario, consider documenting why.crates/dapi/src/api/messages/crosspost_message.rs (1)
13-26: Consider validating that the channel is an announcement channel.Crossposting only works for announcement channels (ChannelType::News). Currently, the controller call will fail with a Discord API error if used on a non-announcement channel. Adding an upfront check would provide a clearer error message.
Additionally, the permission requirement could be refined: crossposting your own message only requires
SEND_MESSAGES, while crossposting others' messages requiresMANAGE_MESSAGES. The current approach requiring both is safe but more restrictive than necessary.crates/dapi/src/api/guilds/get_guild_ban.rs (1)
12-25: LGTM!The implementation correctly checks
BAN_MEMBERSpermission before fetching ban information, which aligns with Discord's API requirements and the pattern used increate_guild_ban.rs.Minor nit: There's trailing whitespace on line 15.
crates/dapi/src/api/guilds/create_guild_role.rs (1)
32-37: Consider breaking early from the feature detection loop.The loop continues iterating after finding
ROLE_ICONS. Adding abreakwould be slightly more efficient.🔎 Proposed fix
let mut guild_has_role_icons = false; for feature in guild.features.iter() { if feature.as_str() == "ROLE_ICONS" { - guild_has_role_icons = true + guild_has_role_icons = true; + break; } }Alternatively, using iterator methods:
let guild_has_role_icons = guild.features.iter().any(|f| f.as_str() == "ROLE_ICONS");crates/dapi/src/api/webhooks/modify_webhook.rs (1)
48-54: Misleading error message on guild mismatch.Line 53 returns the same error message as line 49, but the context is different—here the webhook has a guild_id but it doesn't match the current guild. A more precise message would help with debugging.
🔎 Proposed fix
if guild_id != &this.guild_id().to_string() { - return Err("Webhook does not belong to a guild".into()); + return Err("Webhook does not belong to this guild".into()); }crates/dapi/src/api/guilds/delete_guild_role.rs (1)
36-38: Error message mentions "modify" but operation is "delete".The error message refers to modifying a role, but this endpoint deletes roles. This could cause confusion during debugging.
🔎 Proposed fix
if bot_highest_role <= mod_role { - return Err("The bot must have a role that is higher than the role it is trying to modify".into()); + return Err("The bot must have a role that is higher than the role it is trying to delete".into()); }crates/dapi/src/api/webhooks/delete_webhook.rs (1)
35-37: Misleading error message on guild mismatch.Same issue as in
modify_webhook.rs— the webhook has a guild_id, but it doesn't match the current context. A distinct error message would improve debuggability.🔎 Proposed fix
if guild_id != &this.guild_id().to_string() { - return Err("Webhook does not belong to a guild".into()); + return Err("Webhook does not belong to this guild".into()); }crates/dapi/src/api/guilds/modify_guild_role.rs (2)
2-2: Unused importmember_permissions.The
member_permissionsfunction is imported fromserenity_backportsbut never used in this file.🔎 Proposed fix
-use crate::{ApiReq, context::DiscordContext, controller::DiscordProvider, types::EditRole, get_format_from_image_data, serenity_backports::{member_permissions, highest_role}}; +use crate::{ApiReq, context::DiscordContext, controller::DiscordProvider, types::EditRole, get_format_from_image_data, serenity_backports::highest_role};
43-48: Consider using iterator method for feature detection.The loop can be simplified using
.any()for a more idiomatic Rust pattern.🔎 Proposed fix
- let mut guild_has_role_icons = false; - for feature in guild.features.iter() { - if feature.as_str() == "ROLE_ICONS" { - guild_has_role_icons = true - } - } + let guild_has_role_icons = guild.features.iter().any(|f| f.as_str() == "ROLE_ICONS");crates/dapi/src/api/channels/delete_channel.rs (1)
4-8: Consider removingDefaultderive or validating channel_id.The
Defaultderive on a struct containingGenericChannelIdwill produce a zero/invalid channel ID. If this struct can be default-constructed and used without proper initialization, it could lead to confusing API errors when Discord rejects the invalid ID.🔎 Suggested fix
-#[derive(serde::Serialize, Default, serde::Deserialize)] +#[derive(serde::Serialize, serde::Deserialize)] pub struct DeleteChannel { pub channel_id: serenity::all::GenericChannelId, pub reason: String, }crates/dapi/src/api/channels/edit_channel_permissions.rs (1)
10-11: Consider validating thekindfield.The
kindfield is a rawu8, but Discord's API only accepts0(role) or1(member) as valid values. Invalid values will cause a Discord API error.🔎 Suggested validation
async fn execute<T: DiscordProvider>(self, this: &DiscordContext<T>) -> Result<Self::Resp, crate::Error> { this.check_reason(&self.reason)?; + + if self.kind > 1 { + return Err("Invalid permission overwrite type: must be 0 (role) or 1 (member)".into()); + } let Some(bot_user) = this.current_user() else {crates/dapi/src/api/messages/get_channel_messages.rs (1)
22-25: Consider including Stage channels in the CONNECT permission check.The check only handles
ChannelType::Voice, but Stage channels (ChannelType::Stage) also require theCONNECTpermission to view messages.🔎 Suggested fix
- if guild_channel.base.kind == serenity::all::ChannelType::Voice - && !perms.connect() { + if matches!(guild_channel.base.kind, serenity::all::ChannelType::Voice | serenity::all::ChannelType::Stage) + && !perms.connect() { return Err("Bot does not have permission to connect to the given voice channel".into()); }crates/dapi/src/api/automoderation/edit_auto_moderation_rule.rs (1)
4-9: Consider removingDefaultderive.Similar to
DeleteChannel, theDefaultderive creates a zero/invalidRuleId. If this can lead to confusing behavior, consider removing it.🔎 Suggested fix
-#[derive(serde::Serialize, Default, serde::Deserialize)] +#[derive(serde::Serialize, serde::Deserialize)] pub struct EditAutoModerationRule { pub rule_id: serenity::all::RuleId, pub reason: String, pub data: EditAutoModRule, }crates/dapi/src/api/guilds/modify_guild_member.rs (1)
63-71: Document the purpose of the 10-second buffer or reconsider its necessity.The 10-second buffer added to the 28-day timeout validation (in both
crates/dapi/src/api/guilds/modify_guild_member.rs:65andcrates/runtime/src/plugins/antiraid/discord/mod.rs:1109) lacks any explanation. No inline comments, Discord API documentation references, or tests justify this choice. If it's intentional (e.g., for clock skew tolerance), add a comment explaining why. Otherwise, remove it to match the documented 28-day Discord limit exactly.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (76)
crates/dapi/src/api/automoderation/create_auto_moderation_rule.rscrates/dapi/src/api/automoderation/delete_auto_moderation_rule.rscrates/dapi/src/api/automoderation/edit_auto_moderation_rule.rscrates/dapi/src/api/automoderation/mod.rscrates/dapi/src/api/channels/create_channel_invite.rscrates/dapi/src/api/channels/delete_channel.rscrates/dapi/src/api/channels/delete_channel_permission.rscrates/dapi/src/api/channels/edit_channel_permissions.rscrates/dapi/src/api/channels/get_channel.rscrates/dapi/src/api/channels/mod.rscrates/dapi/src/api/commands/create_guild_command.rscrates/dapi/src/api/commands/create_guild_commands.rscrates/dapi/src/api/commands/get_guild_command.rscrates/dapi/src/api/commands/get_guild_commands.rscrates/dapi/src/api/commands/mod.rscrates/dapi/src/api/guilds/add_guild_member_role.rscrates/dapi/src/api/guilds/create_guild_ban.rscrates/dapi/src/api/guilds/create_guild_channel.rscrates/dapi/src/api/guilds/create_guild_role.rscrates/dapi/src/api/guilds/delete_guild_role.rscrates/dapi/src/api/guilds/get_guild.rscrates/dapi/src/api/guilds/get_guild_ban.rscrates/dapi/src/api/guilds/get_guild_bans.rscrates/dapi/src/api/guilds/get_guild_channels.rscrates/dapi/src/api/guilds/get_guild_member.rscrates/dapi/src/api/guilds/get_guild_preview.rscrates/dapi/src/api/guilds/get_guild_role.rscrates/dapi/src/api/guilds/get_guild_roles.rscrates/dapi/src/api/guilds/list_active_guild_threads.rscrates/dapi/src/api/guilds/list_guild_members.rscrates/dapi/src/api/guilds/mod.rscrates/dapi/src/api/guilds/modify_guild_channel_positions.rscrates/dapi/src/api/guilds/modify_guild_member.rscrates/dapi/src/api/guilds/modify_guild_role.rscrates/dapi/src/api/guilds/modify_guild_role_positions.rscrates/dapi/src/api/guilds/remove_guild_ban.rscrates/dapi/src/api/guilds/remove_guild_member.rscrates/dapi/src/api/guilds/remove_guild_member_role.rscrates/dapi/src/api/guilds/search_guild_members.rscrates/dapi/src/api/interactions/create_followup_message.rscrates/dapi/src/api/interactions/create_interaction_response.rscrates/dapi/src/api/interactions/delete_followup_message.rscrates/dapi/src/api/interactions/delete_original_interaction_response.rscrates/dapi/src/api/interactions/edit_followup_message.rscrates/dapi/src/api/interactions/edit_original_interaction_response.rscrates/dapi/src/api/interactions/get_followup_message.rscrates/dapi/src/api/interactions/get_original_interaction_response.rscrates/dapi/src/api/interactions/mod.rscrates/dapi/src/api/invites/delete_invite.rscrates/dapi/src/api/invites/get_invite.rscrates/dapi/src/api/invites/mod.rscrates/dapi/src/api/messages/bulk_delete_messages.rscrates/dapi/src/api/messages/create_message.rscrates/dapi/src/api/messages/crosspost_message.rscrates/dapi/src/api/messages/delete_message.rscrates/dapi/src/api/messages/edit_message.rscrates/dapi/src/api/messages/get_channel_message.rscrates/dapi/src/api/messages/get_channel_messages.rscrates/dapi/src/api/messages/mod.rscrates/dapi/src/api/mod.rscrates/dapi/src/api/reactions/create_reaction.rscrates/dapi/src/api/reactions/delete_all_reactions.rscrates/dapi/src/api/reactions/delete_all_reactions_for_emoji.rscrates/dapi/src/api/reactions/delete_own_reaction.rscrates/dapi/src/api/reactions/delete_user_reaction.rscrates/dapi/src/api/reactions/get_reactions.rscrates/dapi/src/api/reactions/mod.rscrates/dapi/src/api/webhooks/create_webhook.rscrates/dapi/src/api/webhooks/delete_webhook.rscrates/dapi/src/api/webhooks/execute_webhook.rscrates/dapi/src/api/webhooks/get_channel_webhooks.rscrates/dapi/src/api/webhooks/get_guild_webhooks.rscrates/dapi/src/api/webhooks/get_webhook.rscrates/dapi/src/api/webhooks/mod.rscrates/dapi/src/api/webhooks/modify_webhook.rscrates/dapi/src/apilist.rs
🧰 Additional context used
🧬 Code graph analysis (55)
crates/dapi/src/api/webhooks/execute_webhook.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/commands/create_guild_command.rs (2)
execute(11-19)to_apilist(21-23)
crates/dapi/src/api/guilds/get_guild_channels.rs (9)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/guilds/add_guild_member_role.rs (4)
serde_json(27-27)serde_json(30-30)execute(14-55)to_apilist(57-59)crates/dapi/src/api/guilds/remove_guild_member_role.rs (2)
serde_json(27-27)serde_json(30-30)crates/dapi/src/api/channels/delete_channel.rs (2)
execute(13-48)to_apilist(50-52)crates/dapi/src/api/channels/get_channel.rs (2)
execute(11-18)to_apilist(20-22)crates/dapi/src/api/commands/get_guild_command.rs (2)
execute(11-17)to_apilist(19-21)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(9-15)to_apilist(17-19)crates/dapi/src/api/guilds/create_guild_channel.rs (2)
execute(13-76)to_apilist(78-80)crates/dapi/src/api/guilds/create_guild_role.rs (2)
execute(13-62)to_apilist(64-66)
crates/dapi/src/api/webhooks/get_webhook.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/channels/get_channel.rs (1)
execute(11-18)
crates/dapi/src/api/invites/mod.rs (1)
crates/dapi/src/controller.rs (2)
get_invite(532-543)delete_invite(545-554)
crates/dapi/src/api/interactions/edit_original_interaction_response.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/channels/get_channel.rs (2)
execute(11-18)to_apilist(20-22)
crates/dapi/src/api/invites/get_invite.rs (4)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/guilds/add_guild_member_role.rs (4)
serde_json(27-27)serde_json(30-30)execute(14-55)to_apilist(57-59)crates/dapi/src/api/channels/get_channel.rs (2)
execute(11-18)to_apilist(20-22)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(9-15)to_apilist(17-19)
crates/dapi/src/api/channels/delete_channel_permission.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/channels/delete_channel.rs (1)
execute(13-48)
crates/dapi/src/api/interactions/create_followup_message.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/channels/create_channel_invite.rs (2)
execute(14-30)to_apilist(32-34)
crates/dapi/src/api/commands/mod.rs (1)
crates/dapi/src/controller.rs (4)
get_guild_command(934-942)get_guild_commands(927-932)create_guild_command(944-952)create_guild_commands(954-962)
crates/dapi/src/api/interactions/get_original_interaction_response.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(9-15)to_apilist(17-19)
crates/dapi/src/api/interactions/delete_original_interaction_response.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(9-15)to_apilist(17-19)
crates/dapi/src/api/guilds/get_guild_preview.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(9-15)to_apilist(17-19)
crates/dapi/src/api/commands/create_guild_command.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(9-15)to_apilist(17-19)
crates/dapi/src/api/guilds/delete_guild_role.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/runtime/src/utils/serenity_utils.rs (1)
highest_role(2-20)
crates/dapi/src/api/commands/get_guild_command.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(9-15)to_apilist(17-19)
crates/dapi/src/api/guilds/modify_guild_role.rs (2)
crates/runtime/src/utils/serenity_utils.rs (1)
highest_role(2-20)crates/dapi/src/api/guilds/create_guild_role.rs (1)
execute(13-62)
crates/dapi/src/api/guilds/remove_guild_member_role.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/runtime/src/utils/serenity_utils.rs (1)
highest_role(2-20)crates/dapi/src/api/guilds/add_guild_member_role.rs (1)
execute(14-55)
crates/dapi/src/api/guilds/remove_guild_ban.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/guilds/create_guild_ban.rs (1)
execute(14-49)
crates/dapi/src/api/guilds/list_active_guild_threads.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(9-15)to_apilist(17-19)
crates/dapi/src/api/guilds/get_guild_member.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/channels/get_channel.rs (2)
execute(11-18)to_apilist(20-22)crates/dapi/src/api/commands/get_guild_command.rs (2)
execute(11-17)to_apilist(19-21)
crates/dapi/src/api/webhooks/modify_webhook.rs (5)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/automoderation/create_auto_moderation_rule.rs (1)
execute(13-31)crates/dapi/src/api/channels/create_channel_invite.rs (1)
execute(14-30)crates/dapi/src/api/channels/delete_channel.rs (1)
execute(13-48)crates/dapi/src/api/guilds/create_guild_role.rs (1)
execute(13-62)
crates/dapi/src/api/interactions/mod.rs (1)
crates/dapi/src/controller.rs (8)
create_interaction_response(747-758)get_original_interaction_response(760-768)edit_original_interaction_response(771-781)delete_original_interaction_response(783-791)create_followup_message(804-814)get_followup_message(793-802)edit_followup_message(816-827)delete_followup_message(829-838)
crates/dapi/src/api/reactions/mod.rs (1)
crates/dapi/src/controller.rs (6)
create_reaction(604-614)delete_own_reaction(616-626)delete_user_reaction(628-639)get_reactions(641-683)delete_all_reactions(685-694)delete_all_reactions_for_emoji(696-706)
crates/dapi/src/api/automoderation/delete_auto_moderation_rule.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(9-15)to_apilist(17-19)
crates/dapi/src/api/reactions/create_reaction.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/channels/delete_channel_permission.rs (2)
execute(14-30)to_apilist(32-34)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(9-15)to_apilist(17-19)
crates/dapi/src/api/channels/get_channel.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/commands/get_guild_command.rs (2)
execute(11-17)to_apilist(19-21)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(9-15)to_apilist(17-19)
crates/dapi/src/api/interactions/get_followup_message.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(9-15)to_apilist(17-19)
crates/dapi/src/api/messages/get_channel_message.rs (3)
crates/dapi/src/api/channels/delete_channel.rs (1)
execute(13-48)crates/dapi/src/api/channels/edit_channel_permissions.rs (1)
execute(18-59)crates/dapi/src/api/channels/get_channel.rs (1)
execute(11-18)
crates/dapi/src/api/guilds/list_guild_members.rs (4)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/guilds/add_guild_member_role.rs (4)
serde_json(27-27)serde_json(30-30)execute(14-55)to_apilist(57-59)crates/dapi/src/api/channels/get_channel.rs (2)
execute(11-18)to_apilist(20-22)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(9-15)to_apilist(17-19)
crates/dapi/src/api/webhooks/get_guild_webhooks.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/channels/get_channel.rs (2)
execute(11-18)to_apilist(20-22)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(9-15)to_apilist(17-19)
crates/dapi/src/api/messages/crosspost_message.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/channels/delete_channel.rs (1)
execute(13-48)
crates/dapi/src/api/reactions/delete_user_reaction.rs (5)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/automoderation/delete_auto_moderation_rule.rs (2)
execute(13-29)to_apilist(31-33)crates/dapi/src/api/channels/get_channel.rs (2)
execute(11-18)to_apilist(20-22)crates/dapi/src/api/commands/get_guild_command.rs (2)
execute(11-17)to_apilist(19-21)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(9-15)to_apilist(17-19)
crates/dapi/src/api/channels/edit_channel_permissions.rs (4)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/channels/delete_channel_permission.rs (1)
execute(14-30)crates/dapi/src/api/channels/get_channel.rs (1)
execute(11-18)crates/dapi/src/api/guilds/add_guild_member_role.rs (3)
execute(14-55)serde_json(27-27)serde_json(30-30)
crates/dapi/src/api/automoderation/create_auto_moderation_rule.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/automoderation/delete_auto_moderation_rule.rs (1)
execute(13-29)crates/dapi/src/api/automoderation/edit_auto_moderation_rule.rs (1)
execute(14-32)
crates/dapi/src/api/guilds/modify_guild_role_positions.rs (3)
crates/runtime/src/utils/serenity_utils.rs (1)
highest_role(2-20)crates/dapi/src/api/guilds/add_guild_member_role.rs (3)
serde_json(27-27)serde_json(30-30)execute(14-55)crates/dapi/src/api/guilds/create_guild_role.rs (1)
execute(13-62)
crates/dapi/src/api/messages/bulk_delete_messages.rs (7)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/channels/create_channel_invite.rs (2)
execute(14-30)to_apilist(32-34)crates/dapi/src/api/channels/delete_channel.rs (2)
execute(13-48)to_apilist(50-52)crates/dapi/src/api/channels/edit_channel_permissions.rs (2)
execute(18-59)to_apilist(61-63)crates/dapi/src/api/channels/get_channel.rs (2)
execute(11-18)to_apilist(20-22)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(9-15)to_apilist(17-19)crates/dapi/src/api/guilds/create_guild_channel.rs (2)
execute(13-76)to_apilist(78-80)
crates/dapi/src/api/guilds/get_guild_ban.rs (4)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/channels/get_channel.rs (2)
execute(11-18)to_apilist(20-22)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(9-15)to_apilist(17-19)crates/dapi/src/api/guilds/create_guild_ban.rs (2)
execute(14-49)to_apilist(51-53)
crates/dapi/src/api/guilds/add_guild_member_role.rs (4)
crates/dapi/src/context.rs (1)
controller(46-48)crates/runtime/src/utils/serenity_utils.rs (1)
highest_role(2-20)crates/dapi/src/api/guilds/create_guild_role.rs (1)
execute(13-62)crates/dapi/src/api/guilds/delete_guild_role.rs (1)
execute(13-45)
crates/dapi/src/api/interactions/edit_followup_message.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/channels/get_channel.rs (2)
execute(11-18)to_apilist(20-22)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(9-15)to_apilist(17-19)
crates/dapi/src/api/webhooks/mod.rs (1)
crates/dapi/src/controller.rs (7)
create_webhook(841-851)get_channel_webhooks(853-861)get_guild_webhooks(863-870)get_webhook(872-880)modify_webhook(884-894)delete_webhook(896-905)execute_webhook(909-921)
crates/dapi/src/api/guilds/modify_guild_channel_positions.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/channels/get_channel.rs (2)
execute(11-18)to_apilist(20-22)crates/dapi/src/api/guilds/create_guild_channel.rs (2)
execute(13-76)to_apilist(78-80)
crates/dapi/src/api/guilds/get_guild_roles.rs (4)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/channels/get_channel.rs (2)
execute(11-18)to_apilist(20-22)crates/dapi/src/api/commands/get_guild_command.rs (2)
execute(11-17)to_apilist(19-21)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(9-15)to_apilist(17-19)
crates/dapi/src/api/commands/create_guild_commands.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/commands/create_guild_command.rs (2)
execute(11-19)to_apilist(21-23)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(9-15)to_apilist(17-19)
crates/dapi/src/api/channels/create_channel_invite.rs (4)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/automoderation/delete_auto_moderation_rule.rs (2)
execute(13-29)to_apilist(31-33)crates/dapi/src/api/channels/delete_channel.rs (2)
execute(13-48)to_apilist(50-52)crates/dapi/src/api/channels/get_channel.rs (2)
execute(11-18)to_apilist(20-22)
crates/dapi/src/api/commands/get_guild_commands.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/commands/get_guild_command.rs (2)
execute(11-17)to_apilist(19-21)
crates/dapi/src/api/automoderation/edit_auto_moderation_rule.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/automoderation/create_auto_moderation_rule.rs (2)
execute(13-31)to_apilist(33-35)crates/dapi/src/api/automoderation/delete_auto_moderation_rule.rs (2)
execute(13-29)to_apilist(31-33)
crates/dapi/src/api/guilds/modify_guild_member.rs (2)
crates/runtime/src/utils/serenity_utils.rs (1)
highest_role(2-20)crates/dapi/src/api/guilds/add_guild_member_role.rs (3)
serde_json(27-27)serde_json(30-30)execute(14-55)
crates/dapi/src/api/guilds/get_guild_bans.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/guilds/add_guild_member_role.rs (3)
serde_json(27-27)serde_json(30-30)execute(14-55)crates/dapi/src/api/guilds/create_guild_ban.rs (1)
execute(14-49)
crates/dapi/src/api/messages/get_channel_messages.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/channels/delete_channel.rs (1)
execute(13-48)crates/dapi/src/api/channels/get_channel.rs (1)
execute(11-18)
crates/dapi/src/api/automoderation/mod.rs (1)
crates/dapi/src/controller.rs (4)
list_auto_moderation_rules(40-47)create_auto_moderation_rule(59-68)edit_auto_moderation_rule(70-80)delete_auto_moderation_rule(82-91)
crates/dapi/src/api/webhooks/get_channel_webhooks.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/automoderation/delete_auto_moderation_rule.rs (2)
execute(13-29)to_apilist(31-33)crates/dapi/src/api/channels/get_channel.rs (2)
execute(11-18)to_apilist(20-22)
crates/dapi/src/api/reactions/delete_all_reactions.rs (6)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/automoderation/delete_auto_moderation_rule.rs (1)
execute(13-29)crates/dapi/src/api/channels/delete_channel.rs (1)
execute(13-48)crates/dapi/src/api/channels/get_channel.rs (1)
execute(11-18)crates/dapi/src/api/commands/get_guild_command.rs (1)
execute(11-17)crates/dapi/src/api/commands/get_guild_commands.rs (1)
execute(9-15)
crates/dapi/src/api/webhooks/create_webhook.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/channels/create_channel_invite.rs (1)
execute(14-30)
crates/dapi/src/api/guilds/mod.rs (2)
crates/dapi/src/controller.rs (21)
modify_guild(239-248)get_guild_preview(231-236)get_guild_channels(253-259)create_guild_channel(262-271)modify_guild_channel_positions(274-282)list_active_guild_threads(285-290)get_guild_member(295-314)list_guild_members(317-326)search_guild_members(329-338)modify_guild_member(344-354)add_guild_member_role(358-368)remove_guild_member_role(370-380)remove_guild_member(382-391)get_guild_bans(393-402)get_guild_ban(404-429)create_guild_ban(431-448)remove_guild_ban(450-459)get_guild_roles(464-472)get_guild_role(474-482)create_guild_role(484-493)modify_guild_role_positions(495-504)crates/cli/src/provider/mod.rs (1)
get_guild(507-515)
crates/dapi/src/apilist.rs (1)
crates/dapi/src/controller.rs (63)
create_auto_moderation_rule(59-68)edit_auto_moderation_rule(70-80)delete_auto_moderation_rule(82-91)get_channel_invites(158-166)create_channel_invite(168-178)delete_channel_permission(180-190)follow_announcement_channel(192-216)get_guild_preview(231-236)get_guild_channels(253-259)create_guild_channel(262-271)modify_guild_channel_positions(274-282)list_active_guild_threads(285-290)get_guild_member(295-314)list_guild_members(317-326)search_guild_members(329-338)modify_guild_member(344-354)add_guild_member_role(358-368)remove_guild_member_role(370-380)remove_guild_member(382-391)get_guild_bans(393-402)get_guild_ban(404-429)create_guild_ban(431-448)remove_guild_ban(450-459)get_guild_roles(464-472)get_guild_role(474-482)create_guild_role(484-493)modify_guild_role_positions(495-504)modify_guild_role(506-516)delete_guild_role(518-527)get_invite(532-543)delete_invite(545-554)get_channel_messages(558-568)get_channel_message(570-579)create_message(581-591)crosspost_message(593-602)edit_message(708-719)delete_message(721-731)bulk_delete_messages(733-743)create_reaction(604-614)delete_own_reaction(616-626)delete_user_reaction(628-639)get_reactions(641-683)delete_all_reactions(685-694)delete_all_reactions_for_emoji(696-706)create_interaction_response(747-758)get_original_interaction_response(760-768)edit_original_interaction_response(771-781)delete_original_interaction_response(783-791)create_followup_message(804-814)get_followup_message(793-802)edit_followup_message(816-827)delete_followup_message(829-838)get_guild_command(934-942)get_guild_commands(927-932)create_guild_command(944-952)create_guild_commands(954-962)create_webhook(841-851)get_channel_webhooks(853-861)get_guild_webhooks(863-870)get_webhook(872-880)modify_webhook(884-894)delete_webhook(896-905)execute_webhook(909-921)
| this.check_permissions(bot_user.id, Permissions::MANAGE_ROLES) | ||
| .await?; |
There was a problem hiding this comment.
Use channel-specific permission check instead of guild-level check.
The code uses check_permissions, which appears to perform a guild-level permission check. Channel permission overwrites require verifying that the bot has MANAGE_ROLES permission in the specific channel context, not just at the guild level. Without channel-specific validation, the bot could attempt to delete permission overwrites in channels where it lacks the necessary permissions.
🔎 Proposed fix using check_channel_permissions
- this.check_permissions(bot_user.id, Permissions::MANAGE_ROLES)
- .await?;
+ this.check_channel_permissions(bot_user.id, self.channel_id, Permissions::MANAGE_ROLES)
+ .await?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.check_permissions(bot_user.id, Permissions::MANAGE_ROLES) | |
| .await?; | |
| this.check_channel_permissions(bot_user.id, self.channel_id, Permissions::MANAGE_ROLES) | |
| .await?; |
🤖 Prompt for AI Agents
In crates/dapi/src/api/channels/delete_channel_permission.rs around lines 21-22,
the code calls this.check_permissions(bot_user.id,
Permissions::MANAGE_ROLES).await? which does a guild-level permission check;
replace it with a channel-specific check (e.g.,
this.check_channel_permissions(channel_id, bot_user.id,
Permissions::MANAGE_ROLES).await?) so the bot verifies it has MANAGE_ROLES in
the target channel context before attempting to delete permission overwrites,
propagating any error as before.
| if let Some(limit) = self.limit { | ||
| if limit.get() > 1000 { | ||
| return Err("Limit must be less than 1000".into()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix misleading error message.
The check limit.get() > 1000 correctly allows 1000, but the error message states "less than 1000" which is inaccurate and would confuse users who set limit=1000.
🔎 Suggested fix
if let Some(limit) = self.limit {
if limit.get() > 1000 {
- return Err("Limit must be less than 1000".into());
+ return Err("Limit must be at most 1000".into());
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if let Some(limit) = self.limit { | |
| if limit.get() > 1000 { | |
| return Err("Limit must be less than 1000".into()); | |
| } | |
| } | |
| if let Some(limit) = self.limit { | |
| if limit.get() > 1000 { | |
| return Err("Limit must be at most 1000".into()); | |
| } | |
| } |
🤖 Prompt for AI Agents
In crates/dapi/src/api/guilds/get_guild_bans.rs around lines 29 to 33, the
runtime check uses `limit.get() > 1000` (which permits 1000) but the error
message says "Limit must be less than 1000"; update the error text to accurately
reflect the check (e.g., "Limit must be less than or equal to 1000") so the
message matches the condition, or alternatively change the condition to `>=
1000` if you intend to disallow 1000.
| if let Some(limit) = self.limit { | ||
| if limit.get() > 1000 { | ||
| return Err("The maximum `limit` for get_guild_members is 1000".into()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix incorrect error message referencing wrong endpoint.
The error message mentions get_guild_members but this is the search_guild_members endpoint.
🔎 Suggested fix
if let Some(limit) = self.limit {
if limit.get() > 1000 {
- return Err("The maximum `limit` for get_guild_members is 1000".into());
+ return Err("The maximum `limit` for search_guild_members is 1000".into());
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if let Some(limit) = self.limit { | |
| if limit.get() > 1000 { | |
| return Err("The maximum `limit` for get_guild_members is 1000".into()); | |
| } | |
| } | |
| if let Some(limit) = self.limit { | |
| if limit.get() > 1000 { | |
| return Err("The maximum `limit` for search_guild_members is 1000".into()); | |
| } | |
| } |
🤖 Prompt for AI Agents
In crates/dapi/src/api/guilds/search_guild_members.rs around lines 13 to 17, the
error string incorrectly references `get_guild_members`; change the message to
reference `search_guild_members` (e.g. "The maximum `limit` for
search_guild_members is 1000") so the error accurately reflects this endpoint
while keeping the same error return shape.
| let files = if let Some(ref attachments) = self.data.attachments { | ||
| attachments.take_files()? | ||
| } else { | ||
| Vec::new() | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find take_files method implementation
rg "fn take_files" -A 10Repository: Anti-Raid/khronos
Length of output: 1852
🏁 Script executed:
# Also check the type definition of attachments
rg "attachments:" -B 5 -A 2 crates/dapi/src/api/interactions/edit_original_interaction_response.rsRepository: Anti-Raid/khronos
Length of output: 43
🏁 Script executed:
# Look for the struct that has take_files method
rg "struct.*Attachment" -A 10Repository: Anti-Raid/khronos
Length of output: 2707
🏁 Script executed:
# Check the full implementation of take_files to see what it actually does
rg "pub fn take_files" -A 30 crates/dapi/src/types/attachment.rsRepository: Anti-Raid/khronos
Length of output: 1490
Rename take_files() to better reflect its read-only behavior.
The method name take_files() semantically implies mutation (like Vec::drain() or Option::take()), but the implementation performs a read-only transformation that iterates over &self.new_and_existing_attachments, clones data, and returns a new Vec. Consider renaming to clone_files(), get_files(), or to_files() to accurately reflect that the original attachments are not consumed or modified.
🤖 Prompt for AI Agents
In crates/dapi/src/api/interactions/edit_original_interaction_response.rs around
lines 13 to 17, the call to attachments.take_files() uses a method name that
implies mutation even though it only reads/clones data; rename the method to a
non-mutating name (e.g., clone_files, to_files, or get_files) in the attachments
type, update this call to the new name, and adjust any other call sites, docs,
and tests accordingly so behavior remains identical but the API name reflects
its read-only behavior.
| if let Some(guild) = invite.guild { | ||
| if guild.id != this.guild_id() { | ||
| return Err("Invite does not belong to the current guild".into()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Handle the case where invite.guild is None.
If invite.guild is None, the guild ownership check is silently skipped, potentially allowing deletion of invites that don't belong to the current guild. Consider explicitly requiring guild information or returning an error when it's absent.
🔎 Proposed fix
- if let Some(guild) = invite.guild {
- if guild.id != this.guild_id() {
- return Err("Invite does not belong to the current guild".into());
- }
- }
+ let Some(guild) = invite.guild else {
+ return Err("Invite does not have guild information".into());
+ };
+
+ if guild.id != this.guild_id() {
+ return Err("Invite does not belong to the current guild".into());
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if let Some(guild) = invite.guild { | |
| if guild.id != this.guild_id() { | |
| return Err("Invite does not belong to the current guild".into()); | |
| } | |
| } | |
| let Some(guild) = invite.guild else { | |
| return Err("Invite does not have guild information".into()); | |
| }; | |
| if guild.id != this.guild_id() { | |
| return Err("Invite does not belong to the current guild".into()); | |
| } |
🤖 Prompt for AI Agents
In crates/dapi/src/api/invites/delete_invite.rs around lines 27 to 31, the code
currently only checks guild ownership when invite.guild is Some, silently
skipping the check when invite.guild is None; change this to explicitly handle
the None case by returning an error (e.g., Err("Invite missing guild
information".into())) when guild is absent, and otherwise perform the existing
id equality check and error return if it doesn't match the current guild; ensure
the error message clearly states the missing guild info and use the same error
type as the surrounding function.
| if guild_channel.base.kind == serenity::all::ChannelType::Voice | ||
| && !perms.connect() { | ||
| return Err("Bot does not have permission to connect to the given voice channel".into()); | ||
| } |
There was a problem hiding this comment.
Voice channel CONNECT check appears incorrect.
Voice channels in Discord don't typically contain text messages—text messages are sent in text channels. The CONNECT permission is for joining voice channels, not viewing messages. This check may incorrectly reject valid message retrieval requests or be unnecessary.
Consider removing this check or verifying that voice channels can contain messages in your use case.
Verification script to check voice channel message handling
#!/bin/bash
# Search for other voice channel message handling patterns in the codebase
rg -n -C3 'ChannelType::Voice' --type rust🤖 Prompt for AI Agents
crates/dapi/src/api/messages/get_channel_message.rs around lines 21 to 24: the
code currently rejects requests if the channel is a Voice channel and the bot
lacks the CONNECT permission, which is inappropriate for message retrieval;
remove this voice/CONNECT check and instead ensure the channel type supports
messages (e.g., Text, News, or Thread) and validate message-related permissions
such as VIEW_CHANNEL and READ_MESSAGE_HISTORY (or simply omit the type check if
your use case allows messages in that channel) so message fetching isn't
incorrectly blocked.
| this.check_channel_permissions(bot_user.id, self.channel_id, Permissions::READ_MESSAGE_HISTORY | Permissions::ADD_REACTIONS) | ||
| .await?; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Discord API delete own reaction permissions requirements endpoint documentation
💡 Result:
Endpoint: DELETE /channels/{channel.id}/messages/{message.id}/reactions/{emoji}/@me — deletes the current user's reaction and returns 204 on success.
Permissions: requires READ_MESSAGE_HISTORY (and implicitly the ability to view the channel). [1][2]
Sources:
[1] Discordeno RestManager docs (deleteOwnReaction requires READ_MESSAGE_HISTORY).
[2] Message resource docs / mirrors showing the DELETE .../@me endpoint and behaviour.
Remove ADD_REACTIONS permission check for deleting own reaction.
The code requires both READ_MESSAGE_HISTORY and ADD_REACTIONS permissions, but according to Discord API documentation, deleting the bot's own reaction requires only READ_MESSAGE_HISTORY. The ADD_REACTIONS requirement is overly restrictive and prevents the bot from cleaning up its own reactions in channels where it no longer has that permission.
- this.check_channel_permissions(bot_user.id, self.channel_id, Permissions::READ_MESSAGE_HISTORY | Permissions::ADD_REACTIONS)
+ this.check_channel_permissions(bot_user.id, self.channel_id, Permissions::READ_MESSAGE_HISTORY)
.await?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.check_channel_permissions(bot_user.id, self.channel_id, Permissions::READ_MESSAGE_HISTORY | Permissions::ADD_REACTIONS) | |
| .await?; | |
| this.check_channel_permissions(bot_user.id, self.channel_id, Permissions::READ_MESSAGE_HISTORY) | |
| .await?; |
🤖 Prompt for AI Agents
In crates/dapi/src/api/reactions/delete_own_reaction.rs around lines 19-20, the
permission check currently requires both READ_MESSAGE_HISTORY and ADD_REACTIONS;
update the call to check only READ_MESSAGE_HISTORY since deleting the bot's own
reaction does not require ADD_REACTIONS per Discord API. Replace the permission
mask passed to this.check_channel_permissions with just
Permissions::READ_MESSAGE_HISTORY (leave the await and error handling
unchanged).
| if guild_id != &this.guild_id().to_string() { | ||
| return Err("Webhook does not belong to a guild".into()); | ||
| } |
There was a problem hiding this comment.
Clarify error message for guild mismatch.
The error message "Webhook does not belong to a guild" on line 27 is misleading when the actual issue is that the webhook belongs to a different guild than the current context. This makes debugging harder for API consumers.
🔎 Proposed fix for clearer error message
if guild_id != &this.guild_id().to_string() {
- return Err("Webhook does not belong to a guild".into());
+ return Err("Webhook does not belong to this guild".into());
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if guild_id != &this.guild_id().to_string() { | |
| return Err("Webhook does not belong to a guild".into()); | |
| } | |
| if guild_id != &this.guild_id().to_string() { | |
| return Err("Webhook does not belong to this guild".into()); | |
| } |
🤖 Prompt for AI Agents
In crates/dapi/src/api/webhooks/execute_webhook.rs around lines 26 to 28, the
current error "Webhook does not belong to a guild" is misleading when the
webhook belongs to a different guild; change the error to clearly state the
mismatch and include both the expected guild_id and the webhook's guild id in
the message (e.g., "Webhook belongs to a different guild: expected {expected},
found {actual}") so callers can see both IDs; construct the formatted string and
return it with .into() to preserve the existing error type.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/dapi/src/types/channels.rs (1)
233-243: Consider adding#[must_use]attribute for consistency.All other similar structs in this file (e.g.,
CreateChannel,EditChannel,CreateInvite) have the#[must_use]attribute. Adding it toModifyChannelPositionwould maintain consistency and help prevent accidental non-usage of the struct.🔎 Proposed fix
/// [Discord docs](https://discord.com/developers/docs/resources/guild#modify-guild-channel-positions) +#[must_use] #[derive(Clone, Debug, Serialize, Deserialize)] pub struct ModifyChannelPosition {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
crates/dapi/src/api/channels/mod.rscrates/dapi/src/api/guilds/mod.rscrates/dapi/src/api/webhooks/create_webhook.rscrates/dapi/src/apilist.rscrates/dapi/src/serenity_backports.rscrates/dapi/src/types/channels.rscrates/dapi/src/types/guilds.rscrates/dapi/src/types/webhook.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/dapi/src/api/channels/mod.rs
🧰 Additional context used
🧬 Code graph analysis (2)
crates/dapi/src/api/webhooks/create_webhook.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/channels/create_channel_invite.rs (1)
execute(14-30)
crates/dapi/src/apilist.rs (1)
crates/dapi/src/controller.rs (61)
create_auto_moderation_rule(59-68)edit_auto_moderation_rule(70-80)delete_auto_moderation_rule(82-91)create_channel_invite(168-178)delete_channel_permission(180-190)get_guild_preview(231-236)get_guild_channels(253-259)create_guild_channel(262-271)modify_guild_channel_positions(274-282)list_active_guild_threads(285-290)get_guild_member(295-314)list_guild_members(317-326)search_guild_members(329-338)modify_guild_member(344-354)add_guild_member_role(358-368)remove_guild_member_role(370-380)remove_guild_member(382-391)get_guild_bans(393-402)get_guild_ban(404-429)create_guild_ban(431-448)remove_guild_ban(450-459)get_guild_roles(464-472)get_guild_role(474-482)create_guild_role(484-493)modify_guild_role_positions(495-504)modify_guild_role(506-516)delete_guild_role(518-527)get_invite(532-543)delete_invite(545-554)get_channel_messages(558-568)get_channel_message(570-579)create_message(581-591)crosspost_message(593-602)edit_message(708-719)delete_message(721-731)bulk_delete_messages(733-743)create_reaction(604-614)delete_own_reaction(616-626)delete_user_reaction(628-639)get_reactions(641-683)delete_all_reactions(685-694)delete_all_reactions_for_emoji(696-706)create_interaction_response(747-758)get_original_interaction_response(760-768)edit_original_interaction_response(771-781)delete_original_interaction_response(783-791)create_followup_message(804-814)get_followup_message(793-802)edit_followup_message(816-827)delete_followup_message(829-838)get_guild_command(934-942)get_guild_commands(927-932)create_guild_command(944-952)create_guild_commands(954-962)create_webhook(841-851)get_channel_webhooks(853-861)get_guild_webhooks(863-870)get_webhook(872-880)modify_webhook(884-894)delete_webhook(896-905)execute_webhook(909-921)
🔇 Additional comments (7)
crates/dapi/src/serenity_backports.rs (1)
78-99: LGTM!The
highest_rolefunction correctly implements Discord's role hierarchy logic:
- Compares by position first
- Uses smaller role ID as tie-breaker (matching Discord's behavior)
- Falls back to @everyone when member has no other roles
crates/dapi/src/types/webhook.rs (1)
18-26: LGTM!Good use of
MultiOption<String>for theavatarfield. This correctly supports the tri-state semantics needed for PATCH operations where:
- Not serialized = don't change
null= remove the avatar"value"= set new avatarcrates/dapi/src/types/guilds.rs (1)
113-135: LGTM!The
ModifyRolePositionstruct and its trait implementations are well-designed:
PartialEqcorrectly checks both ID and position equalityPartialOrdusesunwrap_or(other.position)to treatNoneas "keep current position"- The reversed ID comparison (
other.id.partial_cmp(&self.id)) correctly implements Discord's tie-breaker where smaller IDs have higher precedencecrates/dapi/src/api/webhooks/create_webhook.rs (1)
1-35: LGTM!The naming collision issue from the previous review has been resolved. The struct is now correctly named
CreateWebhookRequestto distinguish it from the importedtypes::CreateWebhookpayload type. The implementation follows the established pattern from similar endpoints (e.g.,CreateChannelInvite).crates/dapi/src/api/guilds/mod.rs (1)
1-24: LGTM!The guild API surface expansion is well-organized with consistent naming that maps directly to Discord API endpoints. The one-module-per-endpoint pattern aids maintainability and aligns with the structure in other API modules (automoderation, channels, webhooks, etc.).
crates/dapi/src/apilist.rs (2)
3-102: LGTM!The API enum expansion is consistent and well-structured:
- All new variants follow the established naming convention
- Request types with potential naming conflicts correctly use the
*Requestsuffix (e.g.,CreateMessageRequest,CreateWebhookRequest,ExecuteWebhookRequest)- Logical grouping by API domain (Auto Moderation, Channels, Guilds, etc.) aids readability
119-200: LGTM!The
executeimplementation correctly dispatches all new API variants through theop!macro, maintaining consistency with existing patterns. The commented-out match arms align with the commented-out enum variants.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
crates/dapi/src/api/messages/edit_message.rs (1)
42-43: Permission check still overly restrictive for self-edits.As noted in the previous review, this always requires
MANAGE_MESSAGESeven when editing the bot's own messages. Discord allows bots to edit their own messages without this permission. The suggested fix is to fetch the message first and only check permissions ifmessage.author.id != bot_user.id.crates/dapi/src/api/guilds/get_guild_bans.rs (1)
52-56: Fix misleading error message (duplicate concern).The validation check
limit.get() > 1000correctly allows 1000, but the error message states "less than 1000" which would confuse users settinglimit=1000.🔎 Suggested fix
if let Some(limit) = self.limit { if limit.get() > 1000 { - return Err("Limit must be less than 1000".into()); + return Err("Limit must be at most 1000".into()); } }crates/dapi/src/api/reactions/delete_own_reaction.rs (1)
37-38: Remove ADD_REACTIONS permission check (duplicate concern).The permission check requires both
READ_MESSAGE_HISTORYandADD_REACTIONS, but per Discord API documentation, deleting the bot's own reaction requires onlyREAD_MESSAGE_HISTORY. TheADD_REACTIONSrequirement is overly restrictive and blocks the bot from cleaning up its own reactions in channels where it no longer has that permission.🔎 Suggested fix
- this.check_channel_permissions(bot_user.id, self.channel_id, Permissions::READ_MESSAGE_HISTORY | Permissions::ADD_REACTIONS) + this.check_channel_permissions(bot_user.id, self.channel_id, Permissions::READ_MESSAGE_HISTORY) .await?;crates/dapi/src/api/messages/get_channel_message.rs (1)
43-46: Voice channel permission check flagged in previous review.This voice channel CONNECT permission check has already been flagged as potentially incorrect in a previous review, as voice channels typically don't contain text messages that can be retrieved.
crates/dapi/src/api/messages/delete_message.rs (1)
35-38: Missing reason validation flagged in previous review.A previous review identified that this endpoint is missing
this.check_reason(&self.reason)?validation, which is present in similar endpoints for consistency.crates/dapi/src/api/guilds/modify_guild_role_positions.rs (1)
61-70: Type mismatch in role position comparison persists.The comparison
modify_role_position >= bot_highest_roleon line 67 compares a&ModifyRolePositionto a&Role, which is semantically incorrect. The current role hierarchy check (role >= bot_highest_role) is valid, but the second condition comparing the position modification struct to a role object doesn't properly validate whether the target position would exceed the bot's authority.If the intent is to prevent moving a role to a position above the bot's highest role, you need to compare the target position value against
bot_highest_role.position, not the struct itself.🔎 Suggested fix
- if role >= bot_highest_role || modify_role_position >= bot_highest_role { + if role >= bot_highest_role { return Err(format!("Bot does not have permission to modify the requested role ({}, ``{}``)", role.id, role.name.replace("`", "\\`")).into()); } + + // Optionally, if you need to validate target position: + // if modify_role_position.position >= bot_highest_role.position { + // return Err("Cannot move role to a position at or above bot's highest role".into()); + // }
🧹 Nitpick comments (6)
crates/dapi/src/api/guilds/remove_guild_member_role.rs (1)
89-127: Clean up documentation formatting.The documentation for
to_apilistcontains numerous blank lines with trailing annotation markers. While this doesn't affect functionality, cleaning up the formatting would improve readability.🔎 Suggested cleanup
Remove the extra blank lines and consolidate the documentation:
- /// Convert the request into the corresponding `apilist::API` variant. - - /// - - /// # Returns - - /// - - /// An `apilist::API` value that wraps this `RemoveGuildMemberRole` request. - - /// - - /// # Examples - - /// - - /// ``` - - /// use crates::dapi::api::guilds::remove_guild_member_role::RemoveGuildMemberRole; - - /// use serenity::all::{UserId, RoleId}; - - /// - - /// let req = RemoveGuildMemberRole { - - /// user_id: UserId(1), - - /// role_id: RoleId(2), - - /// reason: "cleanup".into(), - - /// }; - - /// let api = req.to_apilist(); - - /// // `api` is `apilist::API::RemoveGuildMemberRole(req)` - - /// ``` + /// Convert the request into the corresponding `apilist::API` variant. + /// + /// # Returns + /// + /// An `apilist::API` value that wraps this `RemoveGuildMemberRole` request. + /// + /// # Examples + /// + /// ``` + /// use crates::dapi::api::guilds::remove_guild_member_role::RemoveGuildMemberRole; + /// use serenity::all::{UserId, RoleId}; + /// + /// let req = RemoveGuildMemberRole { + /// user_id: UserId(1), + /// role_id: RoleId(2), + /// reason: "cleanup".into(), + /// }; + /// let api = req.to_apilist(); + /// // `api` is `apilist::API::RemoveGuildMemberRole(req)` + /// ```crates/dapi/src/api/automoderation/delete_auto_moderation_rule.rs (1)
4-8: Consider removing Default derive.The
Defaultderive on a delete request may not be semantically meaningful, since deleting a rule requires a specificrule_id. While theexecutemethod performs validation, having a default instance could be confusing.🔎 Optional refactor
-#[derive(serde::Serialize, Default, serde::Deserialize)] +#[derive(serde::Serialize, serde::Deserialize)] pub struct DeleteAutoModerationRule { pub rule_id: serenity::all::RuleId, pub reason: String, }crates/dapi/src/api/guilds/create_guild_role.rs (1)
57-62: Consider using iterator methods for feature detection.The feature check could be more concise using iterator methods.
🔎 Proposed refactor
- let mut guild_has_role_icons = false; - for feature in guild.features.iter() { - if feature.as_str() == "ROLE_ICONS" { - guild_has_role_icons = true - } - } + let guild_has_role_icons = guild.features.iter() + .any(|f| f.as_str() == "ROLE_ICONS");crates/dapi/src/api/guilds/modify_guild_role.rs (1)
65-70: Consider extracting feature check to reduce duplication.This feature detection pattern is duplicated in
create_guild_role.rs. Consider extracting to a helper function or using iterator methods for a more concise implementation.🔎 Proposed refactor
- let mut guild_has_role_icons = false; - for feature in guild.features.iter() { - if feature.as_str() == "ROLE_ICONS" { - guild_has_role_icons = true - } - } + let guild_has_role_icons = guild.features.iter() + .any(|f| f.as_str() == "ROLE_ICONS");Alternatively, extract to a helper method in
DiscordContextor a utility module:fn has_guild_feature(guild: &PartialGuild, feature: &str) -> bool { guild.features.iter().any(|f| f.as_str() == feature) }crates/dapi/src/api/guilds/modify_guild_member.rs (1)
63-73: Permission check may be incomplete for unmuting/undeafening.The current logic only requires
MUTE_MEMBERSwhenmuteistrueandDEAFEN_MEMBERSwhendeafistrue. However, toggling these states off (unmuting or undeafening a member) also requires the same permissions.Consider updating to check permissions regardless of the boolean value:
🔎 Suggested fix
- if let Some(mute) = self.data.mute { - if mute { - needed_perms |= Permissions::MUTE_MEMBERS; - } + if self.data.mute.is_some() { + needed_perms |= Permissions::MUTE_MEMBERS; } - if let Some(deaf) = self.data.deaf { - if deaf { - needed_perms |= Permissions::DEAFEN_MEMBERS; - } + if self.data.deaf.is_some() { + needed_perms |= Permissions::DEAFEN_MEMBERS; }crates/dapi/src/apilist.rs (1)
26-29: Consider removing or implementing commented-out API variants.
GetChannelInvitesandFollowAnnouncementChannelare commented out in both the enum definition and match dispatch. If these are planned for future implementation, consider adding a TODO issue to track them. If they're not needed, removing the comments would reduce noise.Also applies to: 151-154
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (66)
crates/dapi/src/api/automoderation/create_auto_moderation_rule.rscrates/dapi/src/api/automoderation/delete_auto_moderation_rule.rscrates/dapi/src/api/automoderation/edit_auto_moderation_rule.rscrates/dapi/src/api/channels/create_channel_invite.rscrates/dapi/src/api/channels/delete_channel.rscrates/dapi/src/api/channels/delete_channel_permission.rscrates/dapi/src/api/channels/edit_channel_permissions.rscrates/dapi/src/api/channels/get_channel.rscrates/dapi/src/api/commands/create_guild_command.rscrates/dapi/src/api/commands/create_guild_commands.rscrates/dapi/src/api/commands/get_guild_command.rscrates/dapi/src/api/commands/get_guild_commands.rscrates/dapi/src/api/guilds/add_guild_member_role.rscrates/dapi/src/api/guilds/create_guild_ban.rscrates/dapi/src/api/guilds/create_guild_channel.rscrates/dapi/src/api/guilds/create_guild_role.rscrates/dapi/src/api/guilds/delete_guild_role.rscrates/dapi/src/api/guilds/get_guild.rscrates/dapi/src/api/guilds/get_guild_ban.rscrates/dapi/src/api/guilds/get_guild_bans.rscrates/dapi/src/api/guilds/get_guild_channels.rscrates/dapi/src/api/guilds/get_guild_member.rscrates/dapi/src/api/guilds/get_guild_preview.rscrates/dapi/src/api/guilds/get_guild_role.rscrates/dapi/src/api/guilds/get_guild_roles.rscrates/dapi/src/api/guilds/list_active_guild_threads.rscrates/dapi/src/api/guilds/list_guild_members.rscrates/dapi/src/api/guilds/modify_guild_channel_positions.rscrates/dapi/src/api/guilds/modify_guild_member.rscrates/dapi/src/api/guilds/modify_guild_role.rscrates/dapi/src/api/guilds/modify_guild_role_positions.rscrates/dapi/src/api/guilds/remove_guild_ban.rscrates/dapi/src/api/guilds/remove_guild_member.rscrates/dapi/src/api/guilds/remove_guild_member_role.rscrates/dapi/src/api/guilds/search_guild_members.rscrates/dapi/src/api/interactions/create_followup_message.rscrates/dapi/src/api/interactions/create_interaction_response.rscrates/dapi/src/api/interactions/delete_followup_message.rscrates/dapi/src/api/interactions/delete_original_interaction_response.rscrates/dapi/src/api/interactions/edit_followup_message.rscrates/dapi/src/api/interactions/edit_original_interaction_response.rscrates/dapi/src/api/interactions/get_followup_message.rscrates/dapi/src/api/interactions/get_original_interaction_response.rscrates/dapi/src/api/invites/delete_invite.rscrates/dapi/src/api/invites/get_invite.rscrates/dapi/src/api/messages/bulk_delete_messages.rscrates/dapi/src/api/messages/create_message.rscrates/dapi/src/api/messages/crosspost_message.rscrates/dapi/src/api/messages/delete_message.rscrates/dapi/src/api/messages/edit_message.rscrates/dapi/src/api/messages/get_channel_message.rscrates/dapi/src/api/messages/get_channel_messages.rscrates/dapi/src/api/reactions/create_reaction.rscrates/dapi/src/api/reactions/delete_all_reactions.rscrates/dapi/src/api/reactions/delete_all_reactions_for_emoji.rscrates/dapi/src/api/reactions/delete_own_reaction.rscrates/dapi/src/api/reactions/delete_user_reaction.rscrates/dapi/src/api/reactions/get_reactions.rscrates/dapi/src/api/webhooks/create_webhook.rscrates/dapi/src/api/webhooks/delete_webhook.rscrates/dapi/src/api/webhooks/execute_webhook.rscrates/dapi/src/api/webhooks/get_channel_webhooks.rscrates/dapi/src/api/webhooks/get_guild_webhooks.rscrates/dapi/src/api/webhooks/get_webhook.rscrates/dapi/src/api/webhooks/modify_webhook.rscrates/dapi/src/apilist.rs
🚧 Files skipped from review as they are similar to previous changes (26)
- crates/dapi/src/api/reactions/delete_all_reactions_for_emoji.rs
- crates/dapi/src/api/guilds/search_guild_members.rs
- crates/dapi/src/api/guilds/get_guild_roles.rs
- crates/dapi/src/api/interactions/delete_followup_message.rs
- crates/dapi/src/api/interactions/get_followup_message.rs
- crates/dapi/src/api/guilds/modify_guild_channel_positions.rs
- crates/dapi/src/api/guilds/list_active_guild_threads.rs
- crates/dapi/src/api/guilds/get_guild_ban.rs
- crates/dapi/src/api/channels/create_channel_invite.rs
- crates/dapi/src/api/guilds/get_guild_preview.rs
- crates/dapi/src/api/channels/delete_channel_permission.rs
- crates/dapi/src/api/commands/get_guild_command.rs
- crates/dapi/src/api/guilds/create_guild_ban.rs
- crates/dapi/src/api/invites/delete_invite.rs
- crates/dapi/src/api/automoderation/create_auto_moderation_rule.rs
- crates/dapi/src/api/guilds/remove_guild_ban.rs
- crates/dapi/src/api/commands/get_guild_commands.rs
- crates/dapi/src/api/webhooks/get_webhook.rs
- crates/dapi/src/api/channels/get_channel.rs
- crates/dapi/src/api/webhooks/create_webhook.rs
- crates/dapi/src/api/messages/crosspost_message.rs
- crates/dapi/src/api/invites/get_invite.rs
- crates/dapi/src/api/channels/delete_channel.rs
- crates/dapi/src/api/guilds/list_guild_members.rs
- crates/dapi/src/api/reactions/create_reaction.rs
- crates/dapi/src/api/webhooks/modify_webhook.rs
🧰 Additional context used
🧬 Code graph analysis (25)
crates/dapi/src/api/guilds/get_guild.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/apilist.rs (1)
execute(135-214)crates/dapi/src/api/channels/get_channel.rs (1)
execute(28-35)
crates/dapi/src/api/webhooks/execute_webhook.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/webhooks/create_webhook.rs (1)
execute(39-55)
crates/dapi/src/api/interactions/create_followup_message.rs (1)
crates/dapi/src/context.rs (1)
controller(46-48)
crates/dapi/src/api/guilds/get_guild_member.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/apilist.rs (2)
execute(135-214)to_apilist(216-218)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(24-30)to_apilist(45-47)
crates/dapi/src/api/interactions/get_original_interaction_response.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/apilist.rs (1)
execute(135-214)
crates/dapi/src/api/guilds/remove_guild_member.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/apilist.rs (1)
execute(135-214)crates/dapi/src/api/guilds/add_guild_member_role.rs (1)
execute(36-77)
crates/dapi/src/api/automoderation/delete_auto_moderation_rule.rs (7)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/apilist.rs (2)
execute(135-214)to_apilist(216-218)crates/dapi/src/api/automoderation/create_auto_moderation_rule.rs (2)
execute(32-50)to_apilist(64-66)crates/dapi/src/api/automoderation/edit_auto_moderation_rule.rs (2)
execute(37-55)to_apilist(74-76)crates/dapi/src/api/channels/get_channel.rs (2)
execute(28-35)to_apilist(52-54)crates/dapi/src/api/commands/get_guild_command.rs (2)
execute(27-33)to_apilist(52-54)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(24-30)to_apilist(45-47)
crates/dapi/src/api/webhooks/get_guild_webhooks.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/webhooks/create_webhook.rs (1)
execute(39-55)crates/dapi/src/apilist.rs (1)
execute(135-214)
crates/dapi/src/api/webhooks/delete_webhook.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/webhooks/create_webhook.rs (1)
to_apilist(74-76)crates/dapi/src/apilist.rs (1)
to_apilist(216-218)
crates/dapi/src/api/interactions/edit_original_interaction_response.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/apilist.rs (1)
execute(135-214)
crates/dapi/src/api/webhooks/get_channel_webhooks.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/webhooks/create_webhook.rs (1)
execute(39-55)crates/dapi/src/apilist.rs (1)
execute(135-214)
crates/dapi/src/api/guilds/create_guild_channel.rs (7)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/guilds/add_guild_member_role.rs (3)
serde_json(49-49)serde_json(52-52)execute(36-77)crates/dapi/src/api/guilds/remove_guild_member_role.rs (2)
serde_json(59-59)serde_json(62-62)crates/dapi/src/apilist.rs (1)
execute(135-214)crates/dapi/src/api/channels/create_channel_invite.rs (1)
execute(49-65)crates/dapi/src/api/channels/delete_channel.rs (1)
execute(34-69)crates/dapi/src/api/guilds/create_guild_ban.rs (1)
execute(25-60)
crates/dapi/src/api/automoderation/edit_auto_moderation_rule.rs (4)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/apilist.rs (1)
execute(135-214)crates/dapi/src/api/automoderation/create_auto_moderation_rule.rs (1)
execute(32-50)crates/dapi/src/api/automoderation/delete_auto_moderation_rule.rs (1)
execute(28-44)
crates/dapi/src/api/messages/get_channel_messages.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/apilist.rs (2)
execute(135-214)to_apilist(216-218)
crates/dapi/src/api/reactions/get_reactions.rs (4)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/apilist.rs (2)
execute(135-214)to_apilist(216-218)crates/dapi/src/api/channels/get_channel.rs (2)
execute(28-35)to_apilist(52-54)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(24-30)to_apilist(45-47)
crates/dapi/src/api/messages/get_channel_message.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/apilist.rs (1)
execute(135-214)crates/dapi/src/api/channels/get_channel.rs (1)
execute(28-35)
crates/dapi/src/api/channels/edit_channel_permissions.rs (4)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/apilist.rs (1)
execute(135-214)crates/dapi/src/api/channels/delete_channel.rs (1)
execute(34-69)crates/dapi/src/api/channels/delete_channel_permission.rs (1)
execute(37-53)
crates/dapi/src/api/interactions/edit_followup_message.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/apilist.rs (2)
execute(135-214)to_apilist(216-218)
crates/dapi/src/api/guilds/modify_guild_role.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/guilds/add_guild_member_role.rs (3)
serde_json(49-49)serde_json(52-52)execute(36-77)crates/dapi/src/apilist.rs (1)
execute(135-214)
crates/dapi/src/api/messages/create_message.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/apilist.rs (1)
execute(135-214)crates/dapi/src/api/channels/create_channel_invite.rs (1)
execute(49-65)
crates/dapi/src/api/guilds/create_guild_role.rs (4)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/guilds/add_guild_member_role.rs (3)
serde_json(49-49)serde_json(52-52)execute(36-77)crates/dapi/src/apilist.rs (1)
execute(135-214)crates/dapi/src/api/guilds/create_guild_ban.rs (1)
execute(25-60)
crates/dapi/src/api/reactions/delete_all_reactions.rs (4)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/channels/delete_channel.rs (2)
execute(34-69)to_apilist(79-81)crates/dapi/src/api/channels/get_channel.rs (2)
execute(28-35)to_apilist(52-54)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(24-30)to_apilist(45-47)
crates/dapi/src/api/guilds/get_guild_channels.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/apilist.rs (2)
execute(135-214)to_apilist(216-218)crates/dapi/src/api/commands/get_guild_commands.rs (2)
execute(24-30)to_apilist(45-47)
crates/dapi/src/api/commands/create_guild_commands.rs (3)
crates/dapi/src/apilist.rs (1)
execute(135-214)crates/dapi/src/api/commands/create_guild_command.rs (1)
execute(27-35)crates/dapi/src/api/commands/get_guild_commands.rs (1)
execute(24-30)
crates/dapi/src/api/guilds/get_guild_bans.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/apilist.rs (1)
execute(135-214)crates/dapi/src/api/guilds/create_guild_ban.rs (1)
execute(25-60)
🔇 Additional comments (34)
crates/dapi/src/api/guilds/remove_guild_member_role.rs (1)
46-87: LGTM!The permission validation and role hierarchy checks are correctly implemented. The error message properly escapes backticks and provides clear context.
crates/dapi/src/api/interactions/edit_original_interaction_response.rs (1)
27-39: LGTM!The implementation correctly handles optional attachments and delegates to the controller for the edit operation.
crates/dapi/src/api/webhooks/get_channel_webhooks.rs (1)
28-45: LGTM!The permission check and controller delegation follow the established pattern correctly.
crates/dapi/src/api/commands/create_guild_commands.rs (1)
29-39: LGTM!The batch validation ensures all commands are valid before attempting creation, following the same pattern as the single-command variant.
crates/dapi/src/api/webhooks/execute_webhook.rs (1)
32-59: LGTM!The implementation correctly validates the webhook payload, verifies guild ownership, and handles optional attachments before executing the webhook.
crates/dapi/src/api/guilds/remove_guild_member.rs (1)
35-54: LGTM!The permission and hierarchy validation correctly uses the helper method before delegating to the controller.
crates/dapi/src/api/messages/bulk_delete_messages.rs (1)
42-43: LGTM!The permission check for
MANAGE_MESSAGESis correctly implemented.crates/dapi/src/api/guilds/get_guild_member.rs (1)
31-37: LGTM!The implementation correctly delegates to the controller for a straightforward read operation.
crates/dapi/src/api/reactions/delete_user_reaction.rs (1)
35-48: LGTM!The permission check for
MANAGE_MESSAGESis appropriate for removing another user's reaction, and the controller delegation is correct.crates/dapi/src/api/messages/create_message.rs (1)
33-54: LGTM!The implementation correctly validates input, checks
SEND_MESSAGESpermission, handles attachments, and delegates to the controller.crates/dapi/src/api/guilds/create_guild_channel.rs (1)
46-109: LGTM!The permission checks and input validation are thorough and correct:
- Validates topic, rate limits, and tag lengths against Discord limits
- Ensures the bot has
MANAGE_ROLESbefore setting permission overwrites- Verifies the bot possesses all permissions it's attempting to grant or deny
crates/dapi/src/api/interactions/delete_original_interaction_response.rs (1)
28-34: LGTM!Interaction tokens are self-authenticating, so no additional permission checks are needed. The implementation correctly delegates to the controller.
crates/dapi/src/api/guilds/get_guild.rs (1)
4-4: GetGuild struct design is correct as-is.The controller's
get_guild()method takes no parameters—it retrieves the guild from the controller's implicit context viaself.guild_id(). This matches the design pattern used by other guild-level operations likeGetGuildRoles,GetGuildChannels, andGetGuildPreview, which are also empty structs. Fields are only needed when the controller method requires identifying which specific resource to fetch (e.g.,GetChannelhaschannel_id,GetGuildRolehasrole_id).crates/dapi/src/api/webhooks/get_guild_webhooks.rs (1)
37-41: The permission check is correct.The code appropriately uses
check_permissionsfor a guild-level operation. Thecheck_permissionsmethod validates guild-level permissions by retrieving the guild, fetching the member, and checking their permissions in the guild context. There is nocheck_guild_permissionsmethod in the codebase; the distinction is betweencheck_permissions(for guild-level operations) andcheck_channel_permissions(for channel-level operations). Sinceget_guild_webhooksretrieves webhooks for the entire guild rather than a specific channel, usingcheck_permissionswithMANAGE_WEBHOOKSis the correct approach, consistent with other guild-level webhook operations likemodify_webhookanddelete_webhook.Likely an incorrect or invalid review comment.
crates/dapi/src/api/messages/get_channel_messages.rs (1)
51-54: Voice channel message retrieval logic lacks clear justification and may be incorrect.The
CONNECTpermission check for voice channels in message retrieval endpoints (lines 51-54 here and identical code inget_channel_message.rs) is inconsistent with the rest of the codebase. This check only appears in message retrieval endpoints, not in message creation or deletion, and it contradicts Discord's design where voice channels don't store messages.Since
CONNECTpermission is for voice connections rather than message reading, this requirement appears incorrect. Consider:
- Explicitly rejecting voice channels at the API level with a clear error message (better than delegating to Discord API)
- Removing the check entirely and letting the controller handle invalid channels
- Verifying whether the Discord API actually allows message retrieval from voice channels and if
CONNECTpermission is truly relevantcrates/dapi/src/api/commands/create_guild_command.rs (1)
1-53: LGTM! Clean API wrapper implementation.The implementation correctly follows the pattern of validation → controller invocation → response. The permission model delegates appropriately to the controller layer.
crates/dapi/src/api/interactions/create_followup_message.rs (1)
1-56: LGTM! Proper attachment handling.The implementation correctly extracts files from attachments when present and falls back to an empty vector otherwise. The controller delegation is clean and appropriate.
crates/dapi/src/api/automoderation/delete_auto_moderation_rule.rs (1)
28-44: LGTM! Proper permission validation.The implementation correctly validates the reason, ensures the bot user is available, checks
MANAGE_GUILDpermissions, and delegates to the controller. This follows the established pattern for automoderation endpoints.crates/dapi/src/api/guilds/add_guild_member_role.rs (1)
36-77: LGTM! Comprehensive role hierarchy validation.The implementation correctly validates all requirements for adding a role to a member:
- Reason validation and bot user verification
- Bot membership confirmation in the guild
MANAGE_ROLESpermission check- Role hierarchy enforcement (bot's highest role must outrank the target role)
- Clear error messages with proper escaping
This follows Discord's role management rules precisely.
crates/dapi/src/api/interactions/get_original_interaction_response.rs (1)
1-49: LGTM! Appropriate for token-based interaction endpoints.The implementation correctly relies on the interaction token for authentication and directly delegates to the controller. No additional validation is needed.
crates/dapi/src/api/guilds/get_guild_bans.rs (1)
37-50: LGTM! Proper permission checks and pagination logic.The implementation correctly verifies
BAN_MEMBERSpermission and handles pagination with appropriate precedence (before takes priority over after).crates/dapi/src/api/reactions/delete_own_reaction.rs (1)
32-45: Otherwise, proper validation flow.The bot user check and controller delegation (with reaction type conversion) are implemented correctly.
crates/dapi/src/api/guilds/get_guild_role.rs (1)
25-31: No action needed. The Discord API endpoint for fetching a guild role (GET) does not require special permissions—only a valid bot token with access to the guild. The implementation correctly omits permission checks and delegates to the controller, following the API specification.Likely an incorrect or invalid review comment.
crates/dapi/src/api/interactions/create_interaction_response.rs (1)
1-66: LGTM!The implementation correctly extracts attachment files from the interaction response data and delegates to the controller. The pattern follows established conventions in the codebase.
crates/dapi/src/api/interactions/edit_followup_message.rs (1)
1-62: LGTM!The implementation correctly handles optional attachments and follows the established pattern for webhook message editing.
crates/dapi/src/api/guilds/delete_guild_role.rs (1)
43-67: LGTM!The implementation includes appropriate safeguards: protecting the @everyone role, validating reason, checking permissions, and ensuring the bot's role hierarchy is respected before deletion.
crates/dapi/src/api/reactions/get_reactions.rs (1)
50-69: Execution flow looks correct.The implementation properly validates the bot user, checks channel permissions, and delegates to the controller. The error handling and async flow follow the established patterns in this codebase.
crates/dapi/src/api/automoderation/edit_auto_moderation_rule.rs (1)
37-55: LGTM!The execution flow correctly validates the reason, ensures bot user exists, checks
MANAGE_GUILDpermission, validates the rule data, and delegates to the controller. This is consistent with the patterns increate_auto_moderation_rule.rsanddelete_auto_moderation_rule.rs.crates/dapi/src/api/channels/edit_channel_permissions.rs (1)
46-87: LGTM!The implementation correctly:
- Validates the reason and bot user presence
- Checks
MANAGE_ROLESpermission on the channel- Ensures the bot can only allow/deny permissions it possesses itself (lines 56-70)
- Constructs the JSON payload with the correct field mapping
The permission validation loop prevents privilege escalation, which is good security practice.
crates/dapi/src/api/reactions/delete_all_reactions.rs (1)
26-39: LGTM!The implementation correctly requires
MANAGE_MESSAGESpermission before deleting all reactions, which aligns with Discord's permission requirements for this operation. The execution flow is clean and follows established patterns.crates/dapi/src/api/guilds/modify_guild_member.rs (2)
55-61: Empty roles array validation may be too restrictive.Rejecting an empty
rolesarray prevents legitimately removing all roles from a member (leaving only@everyone). If the intent is to disallow no-op requests, consider allowing empty arrays or clarifying the expected behavior in documentation.
95-106: Flags permission check and sanitization look correct.The permission check on lines 96-97 properly requires elevated permissions to modify member flags, and lines 100-105 correctly sanitize the flags to only allow
BYPASSES_VERIFICATION, preventing arbitrary flag manipulation.crates/dapi/src/api/guilds/get_guild_channels.rs (1)
26-33: LGTM!Simple and clean implementation that delegates to the controller. The absence of explicit permission checks is acceptable since guild channel visibility is implicit for bots in the guild, consistent with similar endpoints like
GetGuildCommands.crates/dapi/src/apilist.rs (1)
135-213: Dispatch routing is comprehensive and correct.All ~70 new API variants are properly wired in the match statement, each delegating through the
op!macro. The organization by category (Auto Moderation, Channels, Guilds, etc.) maintains good readability despite the large surface area.
| this.check_channel_permissions(bot_user.id, self.channel_id, serenity::all::Permissions::empty()) | ||
| .await?; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Discord API get reactions endpoint permissions required
💡 Result:
Short answer — To GET reactions (GET /channels/{channel.id}/messages/{message.id}/reactions/{emoji}) your bot/user needs to be able to view the channel and read its message history. In practice that means VIEW_CHANNEL (so the channel is visible) and READ_MESSAGE_HISTORY; higher-level reaction operations require extra perms: ADD_REACTIONS to add, MANAGE_MESSAGES to delete others’ reactions or clear reactions. [1][2]
Sources:
- API docs summary (reactions endpoints & required perms). [1]
- Discord library/docs noting READ_MESSAGE_HISTORY requirement for message/reaction operations and MANAGE_MESSAGES for clearing/removing others’ reactions. [2]
🏁 Script executed:
rg "fn check_channel_permissions" -A 10 crates/dapi/src/Repository: Anti-Raid/khronos
Length of output: 747
🏁 Script executed:
rg "fn check_channel_permissions" -A 30 crates/dapi/src/context.rsRepository: Anti-Raid/khronos
Length of output: 1125
🏁 Script executed:
rg "fn check_channel_permissions" -A 70 crates/dapi/src/context.rsRepository: Anti-Raid/khronos
Length of output: 2783
Require READ_MESSAGE_HISTORY permission for fetching reactions.
The permission check uses Permissions::empty(), which does not verify any specific permissions. Discord's get reactions endpoint requires READ_MESSAGE_HISTORY to read message history and fetch reactions. Update the call to pass serenity::all::Permissions::READ_MESSAGE_HISTORY instead.
🤖 Prompt for AI Agents
In crates/dapi/src/api/reactions/get_reactions.rs around lines 55 to 56, the
permission check currently passes Permissions::empty() which doesn't enforce any
permissions; replace that with serenity::all::Permissions::READ_MESSAGE_HISTORY
so the call requires the READ_MESSAGE_HISTORY permission when fetching
reactions. Update the this.check_channel_permissions(...) invocation to pass
serenity::all::Permissions::READ_MESSAGE_HISTORY and keep the await/? flow
unchanged.
| if guild_id != &this.guild_id().to_string() { | ||
| return Err("Webhook does not belong to a guild".into()); | ||
| } |
There was a problem hiding this comment.
Error message is misleading.
The error message states "Webhook does not belong to a guild" when the actual issue is that the webhook belongs to a different guild than the current one. This could confuse users debugging failed requests.
🔎 Proposed fix
if guild_id != &this.guild_id().to_string() {
- return Err("Webhook does not belong to a guild".into());
+ return Err("Webhook does not belong to this guild".into());
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if guild_id != &this.guild_id().to_string() { | |
| return Err("Webhook does not belong to a guild".into()); | |
| } | |
| if guild_id != &this.guild_id().to_string() { | |
| return Err("Webhook does not belong to this guild".into()); | |
| } |
🤖 Prompt for AI Agents
In crates/dapi/src/api/webhooks/delete_webhook.rs around lines 54 to 56, the
error returned when the webhook's guild ID doesn't match the current guild is
misleading; change the message to indicate the webhook belongs to a different
guild (e.g., "Webhook belongs to a different guild") and preserve returning an
Err with that updated message so callers can accurately diagnose cross-guild
mismatch failures.
| #[derive(serde::Serialize, serde::Deserialize)] | ||
| pub struct GetGuildWebhooks; |
There was a problem hiding this comment.
Critical: Missing guild_id field.
The GetGuildWebhooks struct has no fields, but the controller method get_guild_webhooks() (line 44) is called without a guild ID parameter. Guild webhooks are scoped to a specific guild, so this endpoint cannot function without a guild_id field.
🔎 Proposed fix
#[derive(serde::Serialize, serde::Deserialize)]
pub struct GetGuildWebhooks {
+ pub guild_id: serenity::all::GuildId,
}Then pass it to the controller:
let webhooks = this.controller()
- .get_guild_webhooks()
+ .get_guild_webhooks(self.guild_id)
.await?;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In crates/dapi/src/api/webhooks/get_guild_webhooks.rs around lines 4-5, the
GetGuildWebhooks struct is empty but must carry the guild identifier; add a
guild_id field (using the same ID type your project uses for guilds, e.g. u64 or
your Snowflake/Id type) and derive Deserialize/Serialize as already done, then
update the handler that constructs/calls the controller at line 44 to pass this
guild_id into get_guild_webhooks(guild_id) so the controller receives the guild
context.
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (7)
crates/dapi/src/types/app_commands.rs (1)
26-30: Consider adding documentation or implementing validation logic.The
validatemethod currently performs no actual validation and always returnsOk(()). While this follows the same pattern as other types in the codebase (message.rs,webhook.rs), it would be beneficial to either:
- Add documentation explaining that this is a placeholder for future validation, or
- Implement actual validation for Discord API requirements (e.g., command name length 1-32 characters, description length 1-100 characters for ChatInput commands)
💡 Example: Adding a TODO comment
impl CreateCommand { pub fn validate(&self) -> Result<(), crate::Error> { + // TODO: Implement validation for Discord API requirements + // - Command name must be 1-32 characters + // - Description must be 1-100 characters for ChatInput commands + // - Validate nested options Ok(()) } }Would you like me to help implement the validation logic based on Discord's API requirements?
crates/dapi/src/api/guilds/delete_guild_role.rs (1)
14-14: Prefer direct ID comparison.The
to_string()calls are unnecessary overhead. BothRoleIdandGuildIdimplementPartialEqand can be compared directly.🔎 Proposed fix
- if self.role_id.to_string() == this.guild_id().to_string() { + if self.role_id.get() == this.guild_id().get() {crates/dapi/src/api/guilds/add_guild_member_role.rs (2)
1-1: Remove commented-out dead code.The commented-out import serves no purpose and should be removed to keep the codebase clean.
🔎 Proposed fix
-// use serenity::all::Permissions; use crate::{ApiReq, context::DiscordContext, controller::DiscordProvider, serenity_backports::{member_permissions, highest_role}};
38-40: Clarify the error message for bot role hierarchy.The error message "Bot does not have a role" is slightly misleading. When
highest_rolereturnsNone, it means the bot has no explicit roles beyond@everyone, not that it has no role at all. Consider a more precise message like "Bot has insufficient role hierarchy to add roles" or "Bot does not have any assigned roles beyond @everyone".🔎 Proposed fix
let Some(bot_highest_role) = highest_role(&guild, &bot_member) else { - return Err("Bot does not have a role".into()); + return Err("Bot has insufficient role hierarchy to add roles".into()); };crates/dapi/src/types/webhook.rs (2)
64-68: Consider adding Discord API constraint validation.Similar to
ExecuteWebhook::validate, this method currently returnsOk(())without validation. Discord's webhook modification API has constraints such as:
namelength (1-80 characters when present)avatarmust be a valid data URI format if providedAdding these checks would improve error handling by catching invalid inputs before API calls.
58-62: Consider adding Discord API constraint validation toExecuteWebhook::validate.The method currently returns
Ok(())without checks. Discord's webhook execution API enforces specific constraints:
content: maximum 2000 charactersembeds: maximum 10 per messageusername: maximum 80 characters- Individual embed fields: up to 25 per embed; field names ≤ 256 chars, values ≤ 1024 chars; title ≤ 256, description ≤ 4096, footer ≤ 2048
While this placeholder pattern is consistent with other types in the codebase, adding validation would catch invalid payloads before they reach the Discord API and improve reliability.
crates/dapi/src/types/message.rs (1)
145-149: Empty validation methods - consider documenting intent.The
validate()methods currently returnOk(())without performing any checks. This follows the pattern in other types (e.g.,webhook.rs,app_commands.rs), but consider adding a brief comment indicating whether validation will be added later or if these types intentionally skip validation.Also applies to: 169-173
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
crates/dapi/src/api/guilds/add_guild_member_role.rscrates/dapi/src/api/guilds/delete_guild_role.rscrates/dapi/src/api/guilds/modify_guild_member.rscrates/dapi/src/api/guilds/modify_guild_role.rscrates/dapi/src/api/guilds/modify_guild_role_positions.rscrates/dapi/src/api/guilds/remove_guild_member_role.rscrates/dapi/src/api/messages/create_reaction.rscrates/dapi/src/api/messages/delete_all_reactions.rscrates/dapi/src/api/messages/delete_all_reactions_for_emoji.rscrates/dapi/src/api/messages/delete_own_reaction.rscrates/dapi/src/api/messages/delete_user_reaction.rscrates/dapi/src/api/messages/get_channel_messages.rscrates/dapi/src/api/messages/get_reactions.rscrates/dapi/src/api/messages/mod.rscrates/dapi/src/types/app_commands.rscrates/dapi/src/types/guilds.rscrates/dapi/src/types/message.rscrates/dapi/src/types/webhook.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/dapi/src/api/guilds/remove_guild_member_role.rs
- crates/dapi/src/api/guilds/modify_guild_role.rs
🧰 Additional context used
🧬 Code graph analysis (11)
crates/dapi/src/api/messages/delete_all_reactions.rs (7)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/messages/create_reaction.rs (2)
execute(14-27)to_apilist(29-31)crates/dapi/src/api/messages/delete_all_reactions_for_emoji.rs (2)
execute(14-27)to_apilist(29-31)crates/dapi/src/api/messages/delete_own_reaction.rs (2)
execute(14-27)to_apilist(29-31)crates/dapi/src/api/messages/delete_user_reaction.rs (2)
execute(15-28)to_apilist(30-32)crates/dapi/src/api/messages/get_channel_messages.rs (2)
execute(14-32)to_apilist(34-36)crates/dapi/src/api/messages/get_reactions.rs (2)
execute(16-36)to_apilist(38-40)
crates/dapi/src/api/guilds/delete_guild_role.rs (4)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/guilds/modify_guild_role.rs (1)
execute(14-73)crates/dapi/src/api/guilds/modify_guild_role_positions.rs (1)
execute(13-43)crates/dapi/src/api/messages/delete_all_reactions.rs (1)
execute(13-26)
crates/dapi/src/types/app_commands.rs (2)
crates/dapi/src/types/message.rs (2)
validate(146-148)validate(170-172)crates/dapi/src/types/webhook.rs (2)
validate(59-61)validate(65-67)
crates/dapi/src/api/messages/create_reaction.rs (4)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/messages/delete_all_reactions.rs (1)
execute(13-26)crates/dapi/src/api/messages/get_channel_messages.rs (1)
execute(14-32)crates/dapi/src/api/messages/get_reactions.rs (1)
execute(16-36)
crates/dapi/src/api/guilds/modify_guild_role_positions.rs (1)
crates/dapi/src/api/guilds/modify_guild_role.rs (1)
to_apilist(75-77)
crates/dapi/src/types/message.rs (3)
crates/dapi/src/types/app_commands.rs (1)
validate(27-29)crates/dapi/src/types/webhook.rs (2)
validate(59-61)validate(65-67)crates/dapi/src/types/auto_moderation.rs (2)
validate(25-39)validate(82-96)
crates/dapi/src/api/messages/get_channel_messages.rs (4)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/messages/create_reaction.rs (1)
execute(14-27)crates/dapi/src/api/messages/delete_all_reactions.rs (1)
execute(13-26)crates/dapi/src/api/messages/get_reactions.rs (1)
execute(16-36)
crates/dapi/src/api/messages/delete_own_reaction.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/messages/delete_all_reactions.rs (2)
execute(13-26)to_apilist(28-30)
crates/dapi/src/api/messages/get_reactions.rs (3)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/messages/create_reaction.rs (2)
execute(14-27)to_apilist(29-31)crates/dapi/src/api/messages/delete_all_reactions.rs (2)
execute(13-26)to_apilist(28-30)
crates/dapi/src/api/messages/mod.rs (1)
crates/dapi/src/controller.rs (13)
get_channel_messages(558-568)get_channel_message(570-579)create_message(581-591)crosspost_message(593-602)edit_message(708-719)delete_message(721-731)bulk_delete_messages(733-743)create_reaction(604-614)delete_own_reaction(616-626)delete_user_reaction(628-639)get_reactions(641-683)delete_all_reactions(685-694)delete_all_reactions_for_emoji(696-706)
crates/dapi/src/types/webhook.rs (3)
crates/dapi/src/types/app_commands.rs (1)
validate(27-29)crates/dapi/src/types/message.rs (2)
validate(146-148)validate(170-172)crates/dapi/src/types/auto_moderation.rs (2)
validate(25-39)validate(82-96)
🔇 Additional comments (23)
crates/dapi/src/api/guilds/delete_guild_role.rs (6)
1-2: LGTM!Imports are appropriate for the DeleteGuildRole implementation.
4-8: LGTM!Struct definition follows the established pattern for API request handlers with appropriate fields.
18-22: LGTM!Reason validation and bot user retrieval follow the established pattern across the codebase.
24-28: LGTM!Permission check correctly validates the bot has
MANAGE_ROLESpermission.
40-44: LGTM!Controller invocation correctly delegates the delete operation with the provided reason.
47-49: LGTM!The
to_apilistimplementation correctly maps to the API registry variant.crates/dapi/src/api/guilds/add_guild_member_role.rs (1)
46-48: Role hierarchy check is correct.The
>=comparison correctly enforces Discord's role hierarchy rules, ensuring the bot can only assign roles that are strictly lower than its highest role. The error message also properly escapes backticks.crates/dapi/src/types/webhook.rs (1)
18-26: LGTM! TheMultiOptionchange is appropriate for edit operations.The change from
Option<String>toMultiOption<String>for theavatarfield is correct for PATCH/edit operations, as it allows distinguishing between:
- Not included in the request (don't modify the field)
- Explicitly set to null (clear the avatar)
- Set to a value (update the avatar)
The struct rename from
ModifyWebhooktoEditWebhookis also more idiomatic and aligns with Discord API terminology.crates/dapi/src/api/messages/delete_all_reactions_for_emoji.rs (1)
1-32: LGTM!The implementation follows the established pattern for reaction management endpoints. Permission check for
MANAGE_MESSAGESis correct per Discord API requirements, and the error handling is consistent with other similar endpoints likeDeleteAllReactions.crates/dapi/src/api/messages/delete_user_reaction.rs (1)
1-33: LGTM!The implementation correctly requires
MANAGE_MESSAGESfor removing another user's reaction, which aligns with Discord API requirements. The struct fields and controller delegation are properly implemented.crates/dapi/src/api/messages/delete_all_reactions.rs (1)
1-31: LGTM!Clean implementation following the established pattern. The
MANAGE_MESSAGESpermission requirement is correct for bulk reaction deletion.crates/dapi/src/api/messages/mod.rs (1)
1-14: LGTM!The module structure is well-organized with message operations (lines 1-7) followed by reaction operations (lines 9-14). All declared modules correspond to the implemented files in this PR.
crates/dapi/src/api/messages/create_reaction.rs (1)
1-32: LGTM!Permission requirements (
READ_MESSAGE_HISTORY | ADD_REACTIONS) are correct per Discord API documentation for adding reactions. The implementation follows the established pattern consistently.crates/dapi/src/types/message.rs (2)
216-224: LGTM!The
Fromimplementation provides a clean, direct mapping to serenity'sMessagePaginationtype, enabling seamless conversion for the controller layer.
208-214: The enum variant names (Before, After, Around) correctly match Discord's expected query parameter format. However, the#[serde(rename_all = "lowercase")]attribute on this enum does not control the Discord API query parameters—the actual query parameter serialization is handled by serenity'sMessagePaginationtype, since this enum is converted to serenity's type via theFromtrait before the HTTP request is made. The serde attribute is useful for the dapi crate's own serialization/deserialization but doesn't affect the Discord API call.crates/dapi/src/api/messages/get_channel_messages.rs (1)
14-32: The destructuring pattern correctly matches the return signature ofcheck_channel_permissions, which returns a 4-tuple of(PartialGuild, Member, GuildChannel, Permissions). No runtime panic risk exists here. The first two values are intentionally ignored (marked with_), and only the guild channel and permissions are used, which is valid Rust.Likely an incorrect or invalid review comment.
crates/dapi/src/types/guilds.rs (2)
113-119: LGTM: Struct definition is appropriate.The
ModifyRolePositionstruct correctly models the Discord API payload for modifying role positions, with an optional position field that can be omitted when not changing position.
121-125: LGTM: Equality implementation is reasonable.The equality check correctly compares both the role ID and position. Note that Discord role positions are non-negative, so the
i16tou16cast on line 123 should be safe in practice.crates/dapi/src/api/guilds/modify_guild_role_positions.rs (3)
1-8: LGTM: Imports and struct definition are appropriate.The imports correctly bring in necessary dependencies, and the struct definition follows the established pattern for API request types.
13-25: LGTM: Permission checks follow established patterns.The validation and permission checking logic correctly:
- Validates the audit log reason
- Ensures the bot user exists
- Verifies MANAGE_ROLES permission
- Retrieves the bot's highest role for hierarchy checks
38-47: LGTM: Controller delegation follows established patterns.The controller call correctly passes an iterator over the data and includes the audit log reason. The
to_apilistimplementation properly maps to the API enum variant.crates/dapi/src/api/guilds/modify_guild_member.rs (2)
92-106: Excellent role hierarchy validation.The role hierarchy validation correctly ensures the bot's highest role is strictly higher than any role being assigned. The detailed error message with role ID and name (with proper backtick escaping) provides good debugging information.
28-30: The validation correctly prevents invalid API calls. Thenickfield inEditMemberisOption<String>, whereNone(null in JSON) clears the nickname andSome(s)sets it. Empty strings are invalid for Discord's API, so this check prevents sending malformed payloads like{"nick": ""}. The validation is necessary and correct.Likely an incorrect or invalid review comment.
| let bot_highest_role = highest_role(&guild, &member) | ||
| .ok_or_else(|| "The bot must have roles in order to modify a guild role")?; | ||
|
|
||
| let mod_role = guild.roles.get(&self.role_id) | ||
| .ok_or_else(|| "The role being modified could not be found on the server")?; | ||
|
|
||
| if bot_highest_role <= *mod_role { | ||
| return Err("The bot must have a role that is higher than the role it is trying to modify".into()); | ||
| } |
There was a problem hiding this comment.
Fix error messages to reference "delete" instead of "modify".
The error messages incorrectly reference "modify" when the operation is "delete", which could confuse API consumers trying to understand why their delete operation failed.
🔎 Proposed fix
let bot_highest_role = highest_role(&guild, &member)
- .ok_or_else(|| "The bot must have roles in order to modify a guild role")?;
+ .ok_or_else(|| "The bot must have roles in order to delete a guild role")?;
let mod_role = guild.roles.get(&self.role_id)
- .ok_or_else(|| "The role being modified could not be found on the server")?;
+ .ok_or_else(|| "The role to be deleted could not be found on the server")?;
if bot_highest_role <= *mod_role {
- return Err("The bot must have a role that is higher than the role it is trying to modify".into());
+ return Err("The bot must have a role that is higher than the role it is trying to delete".into());
}🤖 Prompt for AI Agents
In crates/dapi/src/api/guilds/delete_guild_role.rs around lines 30 to 38, the
error messages currently say "modify" when the operation is deleting a role;
update the messages to reference "delete" instead (e.g., "The bot must have
roles in order to delete a guild role", "The role being deleted could not be
found on the server", and "The bot must have a role that is higher than the role
it is trying to delete") so they accurately describe the delete operation and
return the corrected strings.
| if roles.is_empty() { | ||
| return Err("Roles cannot be empty if provided".into()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file structure and the specific function
fd -t f "modify_guild_member.rs" --exec cat -n {} \;Repository: Anti-Raid/khronos
Length of output: 5204
🏁 Script executed:
# Search for roles field definition in types
rg -n "roles.*Option.*Vec" --type=rust -g '*/types.rs' -g '*/types/**' -A 3 -B 3Repository: Anti-Raid/khronos
Length of output: 209
🏁 Script executed:
# Look for Discord API documentation or comments about roles
rg -n "Roles cannot be empty\|empty.*role\|remove.*role" --type=rust -B 5 -A 5Repository: Anti-Raid/khronos
Length of output: 43
🏁 Script executed:
# Check for test cases related to roles validation
fd -t f "test" --type rust | xargs rg -l "roles\|modify_guild_member" | head -10Repository: Anti-Raid/khronos
Length of output: 232
🏁 Script executed:
# Find EditMember type definition
rg -n "struct EditMember\|pub.*roles.*Option" --type rust -B 3 -A 8 | head -50Repository: Anti-Raid/khronos
Length of output: 43
🏁 Script executed:
# Search for Discord API documentation or comments about roles
rg -n "discord.*api\|API.*modify.*member\|roles.*documentation" --type rust -i | head -20Repository: Anti-Raid/khronos
Length of output: 43
🏁 Script executed:
# Look at types.rs or similar files to find EditMember
fd "types.rs" -t fRepository: Anti-Raid/khronos
Length of output: 43
🏁 Script executed:
# Check for serenity-related documentation or comments about EditMember
rg -n "EditMember" --type rust -B 2 -A 10 | head -80Repository: Anti-Raid/khronos
Length of output: 5031
🏁 Script executed:
# Search for Discord API documentation or comments about roles behavior
rg -n "discord\|api\|endpoint" --type rust crates/dapi/src/api/guilds/modify_guild_member.rs -iRepository: Anti-Raid/khronos
Length of output: 43
🏁 Script executed:
# Look for test files related to modify_guild_member
fd -t f "test\|spec" | xargs rg -l "modify_guild_member\|EditMember.*role" 2>/dev/null | head -10Repository: Anti-Raid/khronos
Length of output: 43
🏁 Script executed:
# Search for any comments or documentation about empty roles
rg -n "remove.*role\|empty.*role\|clear.*role" --type rust -i | head -20Repository: Anti-Raid/khronos
Length of output: 43
🏁 Script executed:
# Check serenity-related code or comments about EditMember behavior
rg -n "serenity.*EditMember\|// Edit" --type rust -B 2 -A 5 | head -40Repository: Anti-Raid/khronos
Length of output: 43
🏁 Script executed:
# Look for PR or issue discussions mentioning roles
fd -t f "*.md" | xargs rg -l "role" | head -5Repository: Anti-Raid/khronos
Length of output: 478
🌐 Web query:
Discord API modify guild member roles empty array documentation
💡 Result:
Short answer: PATCH /guilds/{guild.id}/members/{user.id} with the JSON field "roles": [] sets the member's roles to an empty list (i.e., removes all non‑managed roles), but managed roles cannot be removed and the bot needs MANAGE_ROLES and a role higher than each target role to actually remove them. See Discord docs and implementations/threads below. [1][2][3]
Sources:
- Discord API Modify Guild Member (roles param = array of role IDs). [1]
- discord.js GuildMemberRoleManager.set([]) example (removes all roles). [2]
- GitHub issue/discussion about using an empty roles array and observed behavior (managed roles / audit-log notes). [3]
🌐 Web query:
Discord Bot roles parameter empty array remove all roles
💡 Result:
Use the "set roles" operation with an empty array.
- REST: PATCH the guild member's roles to an empty array (roles: []).
- discord.js: guildMember.roles.set([]). This will remove all removable roles. [1][2]
Important constraints:
- The bot needs Manage Roles permission and its highest role must be above any role it tries to remove. Managed roles (Discord-managed: e.g., Nitro boost, integrations) cannot be removed. Also be mindful of race/ordering issues when calling add/remove in quick succession (known discord.js edge cases). [1][2][3]
Sources:
[1] discord.js GuildMemberRoleManager docs (set / examples).
[2] StackOverflow — guildMember.roles.set([]) to remove all roles.
[3] discord.js GitHub issue about roles.add/remove adjacency causing unexpected removals.
🏁 Script executed:
# Check if there are any inline comments or documentation strings about roles
rg -n "roles\|Role" crates/dapi/src/types/guilds.rs -B 2 -A 2 | head -30Repository: Anti-Raid/khronos
Length of output: 43
Remove or modify the empty roles validation at lines 40-42.
Discord's API explicitly supports passing an empty roles array to remove all non-managed roles from a member (e.g., PATCH /guilds/{guild.id}/members/{user.id} with "roles": []). The current check if roles.is_empty() { return Err(...) } incorrectly rejects this valid use case. The subsequent role validation loop at lines 96-106 safely handles empty arrays (no iterations occur), so this check is both unnecessary and overly restrictive.
🤖 Prompt for AI Agents
In crates/dapi/src/api/guilds/modify_guild_member.rs around lines 40-42, remove
the explicit check that rejects empty roles (the `if roles.is_empty() { return
Err(...) }`) because Discord allows an empty roles array to clear roles; instead
allow roles to be empty and rely on the existing validation loop later (lines
~96-106) which safely handles empty slices. Simply delete or disable the
empty-roles rejection so the function accepts an empty Vec for roles while
keeping the subsequent per-role validation intact.
| if let Some(mute) = self.data.mute { | ||
| if mute { | ||
| needed_perms |= Permissions::MUTE_MEMBERS; | ||
| } | ||
| } | ||
|
|
||
| if let Some(deaf) = self.data.deaf { | ||
| if deaf { | ||
| needed_perms |= Permissions::DEAFEN_MEMBERS; | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing permission checks for unmute and undeafen operations.
The current logic only requires MUTE_MEMBERS and DEAFEN_MEMBERS permissions when setting mute or deaf to true. However, Discord requires these permissions for both muting/deafening AND unmuting/undeafening members. The current implementation allows modification of these fields when setting them to false without checking permissions.
🔎 Proposed fix
- if let Some(mute) = self.data.mute {
- if mute {
- needed_perms |= Permissions::MUTE_MEMBERS;
- }
+ if self.data.mute.is_some() {
+ needed_perms |= Permissions::MUTE_MEMBERS;
}
- if let Some(deaf) = self.data.deaf {
- if deaf {
- needed_perms |= Permissions::DEAFEN_MEMBERS;
- }
+ if self.data.deaf.is_some() {
+ needed_perms |= Permissions::DEAFEN_MEMBERS;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if let Some(mute) = self.data.mute { | |
| if mute { | |
| needed_perms |= Permissions::MUTE_MEMBERS; | |
| } | |
| } | |
| if let Some(deaf) = self.data.deaf { | |
| if deaf { | |
| needed_perms |= Permissions::DEAFEN_MEMBERS; | |
| } | |
| } | |
| if self.data.mute.is_some() { | |
| needed_perms |= Permissions::MUTE_MEMBERS; | |
| } | |
| if self.data.deaf.is_some() { | |
| needed_perms |= Permissions::DEAFEN_MEMBERS; | |
| } |
🤖 Prompt for AI Agents
In crates/dapi/src/api/guilds/modify_guild_member.rs around lines 47 to 57, the
code only requires MUTE_MEMBERS/DEAFEN_MEMBERS when mute/deaf are set to true,
which lets callers unmute/undeafen without permission; change the logic to
require the respective permission whenever the field is being modified (i.e., if
self.data.mute.is_some() then add Permissions::MUTE_MEMBERS to needed_perms;
similarly, if self.data.deaf.is_some() then add Permissions::DEAFEN_MEMBERS), so
both setting true and false require the permission.
| this.check_channel_permissions(bot_user.id, self.channel_id, Permissions::READ_MESSAGE_HISTORY | Permissions::ADD_REACTIONS) | ||
| .await?; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Discord API delete own reaction permissions requirements
💡 Result:
Short answer: You don't need MANAGE_MESSAGES to delete your own reaction. The REST endpoint DELETE /channels/{channel.id}/messages/{message.id}/reactions/{emoji}/@me only requires that the request is made by the account that added the reaction and that the account can access the channel (i.e. can view/read the channel). Deleting other users' reactions requires MANAGE_MESSAGES. [1][2][3]
Sources
- Discord API (library references & docs linking to the official endpoint behavior). [1]
- Community docs / API mirrors showing create/delete reaction permission notes (create requires READ_MESSAGE_HISTORY and ADD_REACTIONS conditionally; delete other users requires MANAGE_MESSAGES). [2]
- Library docs noting delete-user-reaction requires MANAGE_MESSAGES while delete-own-reaction is the current user. [3]
🏁 Script executed:
# Find and examine the CreateReaction implementation for comparison
find . -name "*.rs" | xargs grep -l "create_reaction" | head -5Repository: Anti-Raid/khronos
Length of output: 262
🏁 Script executed:
# Check the broader context of delete_own_reaction.rs
cat -n crates/dapi/src/api/messages/delete_own_reaction.rsRepository: Anti-Raid/khronos
Length of output: 1369
🏁 Script executed:
cat -n crates/dapi/src/api/messages/create_reaction.rsRepository: Anti-Raid/khronos
Length of output: 1356
Remove ADD_REACTIONS from permission check for deleting own reactions.
The Discord API only requires READ_MESSAGE_HISTORY to delete your own reaction. The current check includes ADD_REACTIONS, which is unnecessary and overly restrictive. While CreateReaction logically requires ADD_REACTIONS, DeleteOwnReaction does not per Discord's actual API requirements.
Change line 19 to check only Permissions::READ_MESSAGE_HISTORY.
🤖 Prompt for AI Agents
In crates/dapi/src/api/messages/delete_own_reaction.rs around lines 19-20, the
permission check incorrectly requires Permissions::ADD_REACTIONS in addition to
Permissions::READ_MESSAGE_HISTORY; update the call to
this.check_channel_permissions to only require Permissions::READ_MESSAGE_HISTORY
so deleting your own reaction isn't blocked by the unnecessary ADD_REACTIONS
requirement.
| }; | ||
|
|
||
| // Perform required checks | ||
| let (_, _, guild_channel, perms) = this.check_channel_permissions(bot_user.id, self.channel_id, Permissions::VIEW_CHANNEL).await?; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Discord API Get Channel Messages endpoint permission requirements
💡 Result:
Short answer: For GET /channels/{channel.id}/messages on a guild channel the caller needs the View Channel (VIEW_CHANNEL) permission; if they lack Read Message History (READ_MESSAGE_HISTORY) the endpoint will return no past messages. For bots also consider the Message Content privileged intent (if you need message content from many guilds/verified bots). [1][2][3]
Sources:
- Discord API docs overview. [1]
- Message Content privileged intent (bots). [2]
- Endpoint permission notes / community API references (GET messages requires VIEW_CHANNEL; READ_MESSAGE_HISTORY controls returned history). [3]
🏁 Script executed:
# Search for similar message-related endpoints in the codebase to see what permissions they check
fd -t f "messages\|reactions" crates/dapi/src/api/ | head -20Repository: Anti-Raid/khronos
Length of output: 43
🏁 Script executed:
# Look for other message read operations and their permission checks
rg "READ_MESSAGE_HISTORY|VIEW_CHANNEL" crates/dapi/src/api/messages/ -A 2 -B 2Repository: Anti-Raid/khronos
Length of output: 1926
🏁 Script executed:
# Find the implementation of check_channel_permissions to understand how it works
rg "fn check_channel_permissions" crates/ -A 10Repository: Anti-Raid/khronos
Length of output: 747
🏁 Script executed:
# Check all message endpoints to see the full pattern
fd -t f "\.rs" crates/dapi/src/api/messages/ | xargs basename -a | sortRepository: Anti-Raid/khronos
Length of output: 352
🏁 Script executed:
# Look at all message endpoints and their permission checks
rg "check_channel_permissions" crates/dapi/src/api/messages/ -B 1 -A 1Repository: Anti-Raid/khronos
Length of output: 3865
🏁 Script executed:
# Get the full implementation of check_channel_permissions
rg "fn check_channel_permissions" crates/dapi/src/context.rs -A 30Repository: Anti-Raid/khronos
Length of output: 1125
🏁 Script executed:
# Check if there's a reason get_reactions uses empty permissions
cat crates/dapi/src/api/messages/get_reactions.rs | head -40Repository: Anti-Raid/khronos
Length of output: 1367
Add READ_MESSAGE_HISTORY permission check for retrieving channel messages.
The Discord API requires both VIEW_CHANNEL and READ_MESSAGE_HISTORY permissions to retrieve past messages. Without READ_MESSAGE_HISTORY, the endpoint returns no messages. Update the permission check to include both:
- let (_, _, guild_channel, perms) = this.check_channel_permissions(bot_user.id, self.channel_id, Permissions::VIEW_CHANNEL).await?;
+ let (_, _, guild_channel, perms) = this.check_channel_permissions(bot_user.id, self.channel_id, Permissions::VIEW_CHANNEL | Permissions::READ_MESSAGE_HISTORY).await?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let (_, _, guild_channel, perms) = this.check_channel_permissions(bot_user.id, self.channel_id, Permissions::VIEW_CHANNEL).await?; | |
| let (_, _, guild_channel, perms) = this.check_channel_permissions(bot_user.id, self.channel_id, Permissions::VIEW_CHANNEL | Permissions::READ_MESSAGE_HISTORY).await?; |
🤖 Prompt for AI Agents
In crates/dapi/src/api/messages/get_channel_messages.rs around line 20, the
permission check only requests Permissions::VIEW_CHANNEL but must also require
Permissions::READ_MESSAGE_HISTORY to successfully retrieve past messages; update
the check_channel_permissions call to include both permissions (e.g., combine
VIEW_CHANNEL and READ_MESSAGE_HISTORY via the Permissions bitflag/OR) so the
method ensures the bot has both VIEW_CHANNEL and READ_MESSAGE_HISTORY before
attempting to fetch messages.
| this.check_channel_permissions(bot_user.id, self.channel_id, serenity::all::Permissions::empty()) | ||
| .await?; |
There was a problem hiding this comment.
Missing permission check for reading reactions.
The permission check uses Permissions::empty(), but Discord's API requires READ_MESSAGE_HISTORY to fetch reactions on a message. Without this check, the bot might attempt API calls that will fail, or worse, this could indicate a permission bypass.
Compare with get_channel_messages.rs which properly checks VIEW_CHANNEL, and create_reaction.rs which checks READ_MESSAGE_HISTORY | ADD_REACTIONS.
🔎 Proposed fix
- this.check_channel_permissions(bot_user.id, self.channel_id, serenity::all::Permissions::empty())
+ this.check_channel_permissions(bot_user.id, self.channel_id, serenity::all::Permissions::READ_MESSAGE_HISTORY)
.await?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.check_channel_permissions(bot_user.id, self.channel_id, serenity::all::Permissions::empty()) | |
| .await?; | |
| this.check_channel_permissions(bot_user.id, self.channel_id, serenity::all::Permissions::READ_MESSAGE_HISTORY) | |
| .await?; |
🤖 Prompt for AI Agents
In crates/dapi/src/api/messages/get_reactions.rs around lines 21-22, the
permission check currently uses Permissions::empty(), which misses the required
READ_MESSAGE_HISTORY (and should also ensure VIEW_CHANNEL) for fetching message
reactions; update the check to request the appropriate permissions (e.g.,
serenity::all::Permissions::READ_MESSAGE_HISTORY |
serenity::all::Permissions::VIEW_CHANNEL) so the function verifies the bot can
view the channel and read message history before calling the API.
| impl PartialOrd<Role> for ModifyRolePosition { | ||
| fn partial_cmp(&self, other: &Role) -> Option<std::cmp::Ordering> { | ||
| let self_pos = self.position.unwrap_or(other.position as u16); | ||
| match self_pos.partial_cmp(&(other.position as u16)) { | ||
| Some(std::cmp::Ordering::Equal) => other.id.partial_cmp(&self.id), | ||
| res => res, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Clarify the fallback behavior and reversed ID comparison.
The PartialOrd implementation has two concerns:
-
Confusing fallback (line 129): When
self.positionisNone, the code falls back toother.position, which makes the positions always equal in comparison. This means aModifyRolePositionwith no position specified would compare equal (in terms of position) to anyRole, then fall back to ID comparison. Is this the intended behavior for permission checks? -
Reversed ID comparison (line 131): The code compares
other.id.partial_cmp(&self.id)instead of the conventionalself.id.partial_cmp(&other.id). This reverses the sort order when positions are equal. Please add a comment explaining why this reversal is necessary.
These semantics become important in modify_guild_role_positions.rs line 33, where this comparison is used for permission checking.
🔎 Consider making the semantics more explicit
If the intent is to handle None as "no position change" during permission checks, consider adding a helper method that makes this explicit:
impl ModifyRolePosition {
/// Returns true if moving to the target position would be at or above the given role.
/// Returns false if no position change is specified (position is None).
pub fn would_be_at_or_above(&self, role: &Role) -> bool {
match self.position {
Some(pos) => pos >= role.position as u16,
None => false, // No position change, so no hierarchy violation
}
}
}This makes the permission-check logic in the API layer clearer and avoids relying on the potentially confusing PartialOrd semantics.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
crates/dapi/src/apilist.rs (2)
1-1: Consider breaking the long import line into multiple lines.The single-line import statement is difficult to read and maintain. Consider formatting it across multiple lines or grouping related imports.
📝 Suggested formatting
-use crate::{ApiReq, antiraid_check_channel_permissions::AntiRaidCheckChannelPermissions, antiraid_check_permissions::{AntiRaidCheckPermissions, AntiRaidCheckPermissionsAndHierarchy}, antiraid_get_fused_member::AntiRaidGetFusedMember, api::{auditlogs::GetAuditLog, automoderation::{get_auto_moderation_rule::GetAutoModerationRule, list_auto_moderation_rules::ListAutoModerationRules}, channels::edit_channel::EditChannel, guilds::modify_guild::ModifyGuild}, context::DiscordContext, controller::DiscordProvider}; +use crate::{ + ApiReq, + antiraid_check_channel_permissions::AntiRaidCheckChannelPermissions, + antiraid_check_permissions::{AntiRaidCheckPermissions, AntiRaidCheckPermissionsAndHierarchy}, + antiraid_get_fused_member::AntiRaidGetFusedMember, + api::{ + auditlogs::GetAuditLog, + automoderation::{get_auto_moderation_rule::GetAutoModerationRule, list_auto_moderation_rules::ListAutoModerationRules}, + channels::edit_channel::EditChannel, + guilds::modify_guild::ModifyGuild + }, + context::DiscordContext, + controller::DiscordProvider +};
26-26: Remove or track commented-out API variants.The commented variants
GetChannelInvitesandFollowAnnouncementChannelshould either be removed if not needed or tracked for future implementation via a TODO comment or issue.Do you want me to open an issue to track the implementation of these commented endpoints?
Also applies to: 29-29
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/dapi/src/api/mod.rscrates/dapi/src/apilist.rs
🔇 Additional comments (7)
crates/dapi/src/apilist.rs (6)
17-19: LGTM!The automoderation API variants are properly defined with fully qualified paths and correctly handled in the match statement.
23-28: LGTM!The channel API variants follow the established pattern and are properly integrated into the dispatch mechanism.
32-55: LGTM! Comprehensive guild API coverage.The guild-related variants provide extensive functionality covering guilds, channels, members, bans, and roles. All variants are properly handled in the match statement.
57-76: LGTM!The invites, messages, and reactions API variants are well-organized and properly integrated. Grouping reactions under the messages module is a logical choice.
78-101: LGTM!The interactions, commands, and webhooks variants complete the API surface expansion. All variants follow the established pattern and are properly dispatched.
122-200: All ApiReq trait implementations are properly in place and error handling is correct.The match statement correctly handles all 70 API variants, each implementing the
ApiReqtrait with proper asyncsendmethods. Validation is present where needed, and error handling is consistent across implementations through theop!macro which converts errors to the crate'sErrortype. No additional work is required.crates/dapi/src/api/mod.rs (1)
5-9: LGTM! All declared modules (webhooks,invites,messages,interactions,commands) exist as directories with propermod.rsimplementations. The new module declarations properly expose the expanded API surface.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
crates/dapi/src/api/guilds/modify_guild_member.rs (3)
40-42: The empty roles validation should be removed.This issue was previously identified: Discord's API explicitly supports passing an empty roles array to remove all non-managed roles from a member. The current check incorrectly rejects this valid use case.
47-57: Missing permission checks for unmute and undeafen operations.This issue was previously identified: The current logic only requires permissions when setting mute/deaf to true, but Discord requires these permissions for both operations (setting to true OR false). The current implementation allows unmuting/undeafening without proper authorization.
84-87: Incorrect permission check logic for member flags.This issue was previously identified: The permission check uses bitwise OR, so
perms.contains()returns true if ANY of the three permissions is present, but the error message states "and", implying all three are required. This mismatch between implementation and error message needs to be resolved.
🧹 Nitpick comments (1)
crates/dapi/src/api/guilds/add_guild_member_role.rs (1)
1-1: Consider removing the commented-out import.The commented import appears to be unused and can be removed to keep the code clean.
🔎 Proposed cleanup
-// use serenity::all::Permissions; use crate::{ApiReq, context::DiscordContext, controller::DiscordProvider, serenity_backports::{member_permissions, highest_role}};
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
crates/dapi/src/api/guilds/add_guild_member_role.rscrates/dapi/src/api/guilds/modify_guild_member.rscrates/dapi/src/api/guilds/remove_guild_member_role.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/dapi/src/api/guilds/remove_guild_member_role.rs
🧰 Additional context used
🧬 Code graph analysis (2)
crates/dapi/src/api/guilds/modify_guild_member.rs (1)
crates/dapi/src/context.rs (1)
controller(46-48)
crates/dapi/src/api/guilds/add_guild_member_role.rs (2)
crates/dapi/src/context.rs (1)
controller(46-48)crates/dapi/src/api/guilds/remove_guild_member_role.rs (1)
execute(14-60)
🔇 Additional comments (3)
crates/dapi/src/api/guilds/add_guild_member_role.rs (3)
4-9: LGTM!The struct definition is clean and uses appropriate Discord types from Serenity.
14-60: LGTM! Previous validation concern has been addressed.The execute method now includes comprehensive validation, including the target user existence check (lines 32-35) that was previously missing. The implementation follows a consistent pattern with
remove_guild_member_role.rsand properly validates:
- Reason string validity
- Bot user and member existence
- Target member existence in guild
- Bot's manage_roles permission
- Role hierarchy to ensure bot can manage the target role
62-64: LGTM!The
to_apilistmethod correctly maps to the API enum variant for routing.
|
LGTM |
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.