feat(category): associate roles to categories - #401
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for associating Discord role IDs with ticket categories, exposing this via DB schema + backend APIs, and wiring it into both bot commands and the panel UI (including mentioning linked roles when a new ticket thread is created).
Changes:
- Introduces
ticket_category_rolesstorage + new DB operations for listing/adding/removing/clearing/setting role links. - Adds category-role management via API routes/handlers and via slash + text
category roles ...commands. - Updates panel UI/i18n to view/add/remove roles per category and optionally set roles on category creation.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| migrations/20260423120000_category_roles.sql | Adds ticket_category_roles table with FK cascade + index |
| crates/rustmail_panel/src/i18n/en/en.json | Panel strings for linked roles UI + create modal field |
| crates/rustmail_panel/src/i18n/fr/fr.json | French translations for linked roles UI + create modal field |
| crates/rustmail_panel/src/components/categories.rs | Panel UI: fetch/display/add/remove roles; create modal role parsing + post-create set |
| crates/rustmail/src/modules/categories.rs | Mentions linked roles when a new ticket thread is created |
| crates/rustmail/src/i18n/language/en.rs | Help/usage + messages for category roles text/slash commands |
| crates/rustmail/src/i18n/language/fr.rs | French equivalents for category roles messages |
| crates/rustmail/src/db/operations/ticket_categories.rs | Adds role-link CRUD ops; attempts role cleanup on category delete |
| crates/rustmail/src/commands/category/text_command/category.rs | Adds category roles add/remove/list/clear text subcommands |
| crates/rustmail/src/commands/category/slash_command/category.rs | Adds roles subcommand group for slash command |
| crates/rustmail/src/api/routes/categories.rs | Registers new /categories/{id}/roles... endpoints |
| crates/rustmail/src/api/handler/categories/categories.rs | Implements role-link list/add/remove/set handlers + validation |
| .run/Run.run.xml | Changes IDE run config profile to release |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| roles_loaded.set(true); | ||
| role_error.set(None); | ||
| } | ||
| Err(e) => role_error.set(Some(e)), |
There was a problem hiding this comment.
In the error path for loading roles, roles_loaded is never set to true, so the UI can remain in the “Loading roles…” state indefinitely (and may show loading + error at the same time). Consider setting roles_loaded to true (and optionally clearing roles) when fetch_category_roles returns an error.
| Err(e) => role_error.set(Some(e)), | |
| Err(e) => { | |
| roles.set(Vec::new()); | |
| roles_loaded.set(true); | |
| role_error.set(Some(e)); | |
| } |
| { | ||
| let load_roles = load_roles.clone(); | ||
| use_effect_with(c.id.clone(), move |_| { | ||
| load_roles.emit(()); | ||
| || () | ||
| }); | ||
| } |
There was a problem hiding this comment.
Each CategoryCard triggers its own /api/categories/{id}/roles fetch on mount. When listing many categories, this becomes an N+1 request pattern and can slow down the page. Consider deferring role loading until the roles section is expanded, caching results, or adding a batched endpoint to fetch roles for all categories in one request.
| pub async fn delete_category(id: &str, pool: &SqlitePool) -> ModmailResult<bool> { | ||
| let _ = sqlx::query("DELETE FROM ticket_category_roles WHERE category_id = ?") | ||
| .bind(id) | ||
| .execute(pool) | ||
| .await; | ||
|
|
There was a problem hiding this comment.
delete_category runs a manual DELETE FROM ticket_category_roles ... but ignores any error/result (let _ = ...). This can silently leave orphaned role links if the deletion fails for reasons other than “no such table”. Either rely solely on the FK ON DELETE CASCADE behavior, or propagate/log the error and return a failure if the cleanup query fails.
| let now = Utc::now().timestamp(); | ||
| for role_id in role_ids { | ||
| sqlx::query( | ||
| r#" | ||
| INSERT INTO ticket_category_roles (category_id, role_id, created_at) | ||
| VALUES (?, ?, ?) | ||
| ON CONFLICT(category_id, role_id) DO NOTHING | ||
| "#, | ||
| ) | ||
| .bind(category_id) | ||
| .bind(role_id) | ||
| .bind(now) | ||
| .execute(&mut *tx) |
There was a problem hiding this comment.
list_category_role_ids orders by created_at, but set_category_roles assigns the same created_at timestamp (now) to every inserted role in the loop. After a set/replace, ordering can become non-deterministic. Consider either ordering by a stable column (e.g., role_id) or storing an explicit position / using incrementing timestamps within the loop to preserve deterministic ordering.
| .route("/{id}/roles", get(list_category_roles_handler)) | ||
| .route("/{id}/roles", post(add_category_role_handler)) | ||
| .route("/{id}/roles", put(set_category_roles_handler)) | ||
| .route( | ||
| "/{id}/roles/{role_id}", | ||
| delete(remove_category_role_handler), | ||
| ) |
There was a problem hiding this comment.
PR description mentions an API endpoint for “clearing” category roles, but the router only exposes GET/POST/PUT on /{id}/roles and DELETE on /{id}/roles/{role_id} (no clear route like DELETE /{id}/roles). Either add the clear endpoint (e.g., map to clear_category_roles) or update the description to clarify that PUT { role_ids: [] } is the supported way to clear roles.
| <component name="ProjectRunConfigurationManager"> | ||
| <configuration default="false" name="Run" type="CargoCommandRunConfiguration" factoryName="Cargo Command" nameIsGenerated="true"> | ||
| <option name="buildProfileId" value="dev" /> | ||
| <option name="buildProfileId" value="release" /> |
There was a problem hiding this comment.
Switching the IDE run configuration default build profile to release makes local iteration/debugging slower and changes the default developer experience for the repo. If this wasn’t intentional/project-wide, consider keeping dev here (or adding a separate Release run config instead).
| <option name="buildProfileId" value="release" /> | |
| <option name="buildProfileId" value="dev" /> |
|
@copilot apply changes based on the comments in this thread |
… run config Agent-Logs-Url: https://github.com/Rustmail/rustmail/sessions/5e99ff9e-8804-4ca7-b131-ff06287ea3b0 Co-authored-by: Akinator31 <99099121+Akinator31@users.noreply.github.com>
Applied all changes from the review in commit cf96865:
|
This pull request introduces a comprehensive system for managing roles linked to ticket categories, including new API endpoints, slash and text command support, and database integration. It also ensures that when a category is deleted, its associated roles are cleaned up. The most important changes are summarized below.
API and Backend Enhancements:
categories.rsfor listing, adding, removing, setting, and clearing roles associated with categories. These endpoints validate role IDs and ensure the category exists before making changes.ticket_categories.rsso that deleting a category also deletes its associated roles, preventing orphaned records.Slash Command Improvements:
rolessubcommand group with actions to add, remove, list, and clear roles linked to a category. Implemented the corresponding logic for each action.Text Command Improvements:
rolesactions (add,remove,list,clear) to the text-based category command, including role ID parsing and user feedback.Routing and Configuration:
These changes provide a robust and consistent interface for managing category-role associations across both API and command interfaces.