Repository navigation
feat(bans): add the ability to view information about a banned user - #395
Akinator31 merged 10 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds end-to-end “ban info” support by persisting member snapshots at ban time, exposing banned-user data via a new admin API endpoint, and surfacing it in both the web panel and Discord (slash + text) commands.
Changes:
- Add SQLite tables + Rust DB operations to track members and persist banned-user records (including roles, reason, executor).
- Add
/api/admin/bansprotected by a newViewBanspanel permission, and a new Panel “Bans” page/route. - Add
baninfo(slash + text) command and aGuildBanHandlerto record ban events.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| migrations/20260418120000_banned_users.sql | Introduces tracked_members and banned_users tables + username indexes to persist ban-related snapshots. |
| crates/rustmail_types/src/api/panel_permissions.rs | Adds ViewBans permission string mappings for backend permission checks. |
| crates/rustmail_panel/src/types.rs | Adds ViewBans to the panel’s deserializable permission enum and display label. |
| crates/rustmail_panel/src/pages/panel.rs | Adds /panel/bans route and renders the new bans page. |
| crates/rustmail_panel/src/pages/administration.rs | Adds view_bans to permission management UI/options and enum mapping. |
| crates/rustmail_panel/src/i18n/fr/fr.json | Adds navbar + panel strings for bans page and new permission label (FR). |
| crates/rustmail_panel/src/i18n/en/en.json | Adds navbar + panel strings for bans page and new permission label (EN). |
| crates/rustmail_panel/src/components/navbar.rs | Adds conditional navbar entries (desktop + mobile) for the bans page gated by ViewBans. |
| crates/rustmail_panel/src/components/mod.rs | Exposes the new bans component module. |
| crates/rustmail_panel/src/components/bans.rs | Implements the “Banned Users” page: permission check + fetch/render of /api/admin/bans. |
| crates/rustmail/src/i18n/language/fr.rs | Adds French dictionary messages for the baninfo command output. |
| crates/rustmail/src/i18n/language/en.rs | Adds English dictionary messages for the baninfo command output. |
| crates/rustmail/src/handlers/mod.rs | Registers the new GuildBanHandler module export. |
| crates/rustmail/src/handlers/guild_messages_handler.rs | Registers baninfo/bi as a text command. |
| crates/rustmail/src/handlers/guild_ban_handler.rs | New event handler to upsert tracked members and persist banned-user records on ban events. |
| crates/rustmail/src/db/repr.rs | Adds TrackedMember and BannedUser representations for DB operations and API/command use. |
| crates/rustmail/src/db/operations/mod.rs | Exposes the new banned_users operations module. |
| crates/rustmail/src/db/operations/banned_users.rs | Implements CRUD/search operations for tracked members and banned users. |
| crates/rustmail/src/commands/mod.rs | Adds the baninfo command module exports. |
| crates/rustmail/src/commands/baninfo/mod.rs | New baninfo command module (slash + text). |
| crates/rustmail/src/commands/baninfo/text_command/mod.rs | Exposes baninfo text-command implementation. |
| crates/rustmail/src/commands/baninfo/text_command/baninfo.rs | Implements !baninfo/!bi lookup + response embed generation. |
| crates/rustmail/src/commands/baninfo/slash_command/mod.rs | Exposes baninfo slash-command implementation. |
| crates/rustmail/src/commands/baninfo/slash_command/baninfo.rs | Implements /baninfo registration + execution logic using shared resolver/response. |
| crates/rustmail/src/bot.rs | Registers BaninfoCommand and attaches GuildBanHandler to the Serenity client. |
| crates/rustmail/src/api/utils/panel_permissions.rs | Grants ViewBans to super-admin/admin permission sets. |
| crates/rustmail/src/api/routes/admin.rs | Adds /bans admin route protected by ViewBans. |
| crates/rustmail/src/api/handler/admin/mod.rs | Wires the new bans admin handler module. |
| crates/rustmail/src/api/handler/admin/bans.rs | Implements /api/admin/bans list endpoint with role + staff-name resolution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if let Err(e) = save_banned_user(&record, pool).await { | ||
| eprintln!("Failed to save banned user {}: {:?}", banned_user.id, e); | ||
| } | ||
|
|
||
| let _ = delete_tracked_member(&guild_id_str, &user_id_str, pool).await; |
There was a problem hiding this comment.
delete_tracked_member is executed unconditionally after attempting save_banned_user. If save_banned_user fails (DB transient error, schema mismatch, etc.), this still deletes the tracked member record and can permanently lose the only copy of roles/nickname/join data you were trying to preserve. Only delete from tracked_members after the ban record was successfully persisted (or at least gate deletion on save_banned_user returning Ok).
| if let Err(e) = save_banned_user(&record, pool).await { | |
| eprintln!("Failed to save banned user {}: {:?}", banned_user.id, e); | |
| } | |
| let _ = delete_tracked_member(&guild_id_str, &user_id_str, pool).await; | |
| match save_banned_user(&record, pool).await { | |
| Ok(()) => { | |
| if let Err(e) = delete_tracked_member(&guild_id_str, &user_id_str, pool).await { | |
| eprintln!( | |
| "Failed to delete tracked member {} from guild {} after saving banned user: {:?}", | |
| banned_user.id, guild_id, e | |
| ); | |
| } | |
| } | |
| Err(e) => { | |
| eprintln!("Failed to save banned user {}: {:?}", banned_user.id, e); | |
| } | |
| } |
| ON CONFLICT(guild_id, user_id) DO UPDATE SET | ||
| username = excluded.username, | ||
| global_name = excluded.global_name, | ||
| nickname = excluded.nickname, | ||
| avatar_url = excluded.avatar_url, | ||
| roles = excluded.roles, | ||
| joined_at = excluded.joined_at, | ||
| banned_at = excluded.banned_at, | ||
| banned_by = excluded.banned_by, | ||
| ban_reason = excluded.ban_reason, | ||
| roles_unknown = excluded.roles_unknown |
There was a problem hiding this comment.
In the ON CONFLICT ... DO UPDATE clause for banned_users, joined_at is overwritten with excluded.joined_at even when the new value is NULL. If a user gets banned again and the new record lacks joined_at, this will wipe previously stored join info. Consider using joined_at = COALESCE(excluded.joined_at, banned_users.joined_at) (similar to the tracked_members upsert) to preserve existing data when the incoming value is NULL.
| pub async fn baninfo( | ||
| ctx: Context, | ||
| msg: Message, | ||
| config: &Config, | ||
| _handler: Arc<GuildMessagesHandler>, | ||
| ) -> ModmailResult<()> { | ||
| let pool = config | ||
| .db_pool | ||
| .as_ref() | ||
| .ok_or_else(database_connection_failed)?; | ||
|
|
||
| let query = extract_reply_content(&msg.content, &config.command.prefix, &["baninfo", "bi"]) | ||
| .unwrap_or_default(); | ||
|
|
||
| if query.is_empty() { | ||
| return Err(ModmailError::Command(CommandError::MissingArguments)); | ||
| } | ||
|
|
||
| let guild_id = config.bot.get_community_guild_id().to_string(); | ||
| let matches = resolve_banned_users(&guild_id, &query, pool).await?; | ||
|
|
||
| send_baninfo_response(&ctx, config, msg.channel_id, &query, &matches).await; | ||
| Ok(()) |
There was a problem hiding this comment.
The text !baninfo/!bi command can be triggered from any guild channel because guild-message commands are dispatched purely by prefix. This implementation doesn't restrict execution to the staff guild (or staff thread channels), so regular community members could query and receive ban details if the bot can post in that channel. Add an explicit guild/channel authorization check (e.g., require msg.guild_id == staff_guild_id or require the channel to be a staff thread) and return a permission error otherwise.
| pub async fn resolve_banned_users( | ||
| guild_id: &str, | ||
| query: &str, | ||
| pool: &sqlx::SqlitePool, | ||
| ) -> ModmailResult<Vec<BannedUser>> { | ||
| let trimmed = query.trim().trim_start_matches('@'); | ||
| if let Ok(parsed) = trimmed.parse::<u64>() { | ||
| let user_id_str = parsed.to_string(); | ||
| if let Some(user) = get_banned_user_by_id(guild_id, &user_id_str, pool).await? { | ||
| return Ok(vec![user]); | ||
| } | ||
| } | ||
| find_banned_users_by_username(guild_id, trimmed, pool).await |
There was a problem hiding this comment.
resolve_banned_users only trims a leading '@' before attempting to parse a numeric ID. Common Discord inputs like <@123> / <@!123> won't parse, so looking up by “user id” via mention will silently fall back to username search and likely fail. Consider extracting the ID from mention formats (regex like <@!?(\d+)>) before the numeric parse.
| pub async fn resolve_banned_users( | |
| guild_id: &str, | |
| query: &str, | |
| pool: &sqlx::SqlitePool, | |
| ) -> ModmailResult<Vec<BannedUser>> { | |
| let trimmed = query.trim().trim_start_matches('@'); | |
| if let Ok(parsed) = trimmed.parse::<u64>() { | |
| let user_id_str = parsed.to_string(); | |
| if let Some(user) = get_banned_user_by_id(guild_id, &user_id_str, pool).await? { | |
| return Ok(vec![user]); | |
| } | |
| } | |
| find_banned_users_by_username(guild_id, trimmed, pool).await | |
| fn normalize_banned_user_query(query: &str) -> &str { | |
| let trimmed = query.trim(); | |
| if let Some(inner) = trimmed | |
| .strip_prefix("<@!") | |
| .and_then(|value| value.strip_suffix('>')) | |
| .or_else(|| trimmed.strip_prefix("<@").and_then(|value| value.strip_suffix('>'))) | |
| { | |
| return inner; | |
| } | |
| trimmed.trim_start_matches('@') | |
| } | |
| pub async fn resolve_banned_users( | |
| guild_id: &str, | |
| query: &str, | |
| pool: &sqlx::SqlitePool, | |
| ) -> ModmailResult<Vec<BannedUser>> { | |
| let normalized = normalize_banned_user_query(query); | |
| if let Ok(parsed) = normalized.parse::<u64>() { | |
| let user_id_str = parsed.to_string(); | |
| if let Some(user) = get_banned_user_by_id(guild_id, &user_id_str, pool).await? { | |
| return Ok(vec![user]); | |
| } | |
| } | |
| find_banned_users_by_username(guild_id, normalized, pool).await |
| let community_guild_obj = GuildId::new(community_guild_id); | ||
| let roles_map = match community_guild_obj.roles(&bot_http).await { | ||
| Ok(r) => r, | ||
| Err(_) => HashMap::new(), | ||
| }; | ||
|
|
||
| let staff_guild_obj = GuildId::new(staff_guild_id); | ||
| let staff_members = match staff_guild_obj.members(&bot_http, None, None).await { | ||
| Ok(m) => m, | ||
| Err(_) => Vec::new(), | ||
| }; | ||
| let staff_map: HashMap<String, String> = staff_members | ||
| .into_iter() | ||
| .map(|m| (m.user.id.to_string(), m.user.name.clone())) | ||
| .collect(); |
There was a problem hiding this comment.
handle_list_bans performs multiple Discord API calls on every request (fetching all community roles and all staff guild members) just to resolve role names and banned_by. This can be slow and may hit Discord rate limits as the bans page is used more. Prefer resolving only the IDs you need (e.g., collect unique banned_by IDs from the DB results and fetch those users), and/or cache roles/staff-name lookups for a short TTL.
| Ok(mut users) => { | ||
| for user in &mut users { | ||
| // Resolve roles | ||
| user.roles = user | ||
| .roles | ||
| .iter() | ||
| .map(|role_id_str| { | ||
| if let Ok(id) = role_id_str.parse::<u64>() { | ||
| if let Some(role) = roles_map.get(&serenity::all::RoleId::new(id)) { | ||
| return role.name.clone(); | ||
| } | ||
| } | ||
| role_id_str.clone() | ||
| }) | ||
| .collect(); | ||
|
|
||
| // Resolve banned_by | ||
| if let Some(banned_by_id) = &user.banned_by { | ||
| if let Some(staff_name) = staff_map.get(banned_by_id) { | ||
| user.banned_by = Some(staff_name.clone()); | ||
| } | ||
| } | ||
| } | ||
| (StatusCode::OK, Json(users)).into_response() |
There was a problem hiding this comment.
This handler mutates BannedUser.banned_by from a Discord user ID into a display name before returning JSON. Since the same BannedUser type is also used elsewhere with banned_by meaning an ID (e.g., for building Discord mentions), this makes the field semantics inconsistent across endpoints and harder to reason about. Consider returning a dedicated DTO for the panel API (e.g., banned_by_id + banned_by_name) while keeping the DB representation unchanged.
| Ok(mut users) => { | |
| for user in &mut users { | |
| // Resolve roles | |
| user.roles = user | |
| .roles | |
| .iter() | |
| .map(|role_id_str| { | |
| if let Ok(id) = role_id_str.parse::<u64>() { | |
| if let Some(role) = roles_map.get(&serenity::all::RoleId::new(id)) { | |
| return role.name.clone(); | |
| } | |
| } | |
| role_id_str.clone() | |
| }) | |
| .collect(); | |
| // Resolve banned_by | |
| if let Some(banned_by_id) = &user.banned_by { | |
| if let Some(staff_name) = staff_map.get(banned_by_id) { | |
| user.banned_by = Some(staff_name.clone()); | |
| } | |
| } | |
| } | |
| (StatusCode::OK, Json(users)).into_response() | |
| Ok(users) => { | |
| let panel_users: Vec<serde_json::Value> = users | |
| .into_iter() | |
| .map(|user| { | |
| let resolved_roles: Vec<String> = user | |
| .roles | |
| .iter() | |
| .map(|role_id_str| { | |
| if let Ok(id) = role_id_str.parse::<u64>() { | |
| if let Some(role) = roles_map.get(&serenity::all::RoleId::new(id)) { | |
| return role.name.clone(); | |
| } | |
| } | |
| role_id_str.clone() | |
| }) | |
| .collect(); | |
| let banned_by_id = user.banned_by.clone(); | |
| let banned_by_name = banned_by_id | |
| .as_ref() | |
| .and_then(|id| staff_map.get(id).cloned()); | |
| let mut user_json = match serde_json::to_value(&user) { | |
| Ok(value) => value, | |
| Err(_) => serde_json::json!({}), | |
| }; | |
| if let Some(user_obj) = user_json.as_object_mut() { | |
| user_obj.insert("roles".to_string(), serde_json::json!(resolved_roles)); | |
| user_obj.remove("banned_by"); | |
| user_obj.insert("banned_by_id".to_string(), serde_json::json!(banned_by_id)); | |
| user_obj.insert( | |
| "banned_by_name".to_string(), | |
| serde_json::json!(banned_by_name), | |
| ); | |
| } | |
| user_json | |
| }) | |
| .collect(); | |
| (StatusCode::OK, Json(panel_users)).into_response() |
|
@copilot apply changes based on the comments in this thread |
…d API DTOs Agent-Logs-Url: https://github.com/Rustmail/rustmail/sessions/d8ab99ea-adb5-42be-9d8b-83f61ea8296f Co-authored-by: Akinator31 <99099121+Akinator31@users.noreply.github.com>
Implemented the requested review-thread fixes in commit Included changes: guarded tracked-member deletion on successful ban save, preserved UI screenshot: N/A (no visible UI layout change). |
This pull request introduces a new "ban info" feature to the project, allowing staff to view detailed information about banned users through both API endpoints and Discord commands. It adds a new
/bansadmin API route, abaninfoslash and text command, and the necessary permission and handler infrastructure to support these features. Additionally, it updates permissions to control access to ban information.Ban Info Feature Implementation:
baninfocommand (both slash and text) that allows staff to query and display detailed information about banned users, including roles, ban reason, and who issued the ban.BaninfoCommandin the bot initialization, enabling its use in Discord.Admin API Enhancements:
/bansadmin API endpoint (handle_list_bans) to retrieve and resolve information about all banned users, including role and staff name resolution./bansendpoint into the admin router with a newViewBanspanel permission for access control.Bot Event Handling:
GuildBanHandlerevent handler to process ban-related events in Discord.Command and Module Organization:
baninfomodule to the main commands module for unified access and re-exported its components.These changes collectively provide staff with powerful tools for managing and auditing bans, both through Discord and the API, with appropriate permission checks.