Skip to content

Commit 2fb9110

Browse files
CopilotAkinator31
andauthored
fix: address category review feedback across API, commands, flow, and panel
Agent-Logs-Url: https://github.com/Rustmail/rustmail/sessions/7f12ccf8-887b-4e84-8b8f-9f4629b1f1a4 Co-authored-by: Akinator31 <99099121+Akinator31@users.noreply.github.com>
1 parent ac17f6a commit 2fb9110

9 files changed

Lines changed: 95 additions & 29 deletions

File tree

crates/rustmail/src/api/handler/categories/categories.rs

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ pub async fn create_category_handler(
7878
State(bot_state): State<Arc<Mutex<BotState>>>,
7979
Json(req): Json<CreateCategoryRequest>,
8080
) -> Result<Json<CategoryDto>, (StatusCode, String)> {
81-
if req.name.trim().is_empty() {
81+
let name = req.name.trim();
82+
if name.is_empty() {
8283
return Err((StatusCode::BAD_REQUEST, "Name required".to_string()));
8384
}
8485
if req.discord_category_id.parse::<u64>().is_err() {
@@ -97,15 +98,19 @@ pub async fn create_category_handler(
9798
));
9899
}
99100

100-
if let Ok(Some(_)) = get_category_by_name(&req.name, &p).await {
101+
if get_category_by_name(name, &p)
102+
.await
103+
.map_err(internal)?
104+
.is_some()
105+
{
101106
return Err((
102107
StatusCode::CONFLICT,
103108
"Category with this name already exists".to_string(),
104109
));
105110
}
106111

107112
let created = create_category(
108-
&req.name,
113+
name,
109114
req.description.as_deref(),
110115
req.emoji.as_deref(),
111116
&req.discord_category_id,
@@ -138,6 +143,22 @@ pub async fn update_category_handler(
138143
.map_err(internal)?
139144
.ok_or((StatusCode::NOT_FOUND, "Category not found".to_string()))?;
140145

146+
let trimmed_name = req.name.as_deref().map(str::trim);
147+
if let Some(name) = trimmed_name {
148+
if name.is_empty() {
149+
return Err((StatusCode::BAD_REQUEST, "Name required".to_string()));
150+
}
151+
152+
if let Some(conflict) = get_category_by_name(name, &p).await.map_err(internal)? {
153+
if conflict.id != existing.id {
154+
return Err((
155+
StatusCode::CONFLICT,
156+
"Category with this name already exists".to_string(),
157+
));
158+
}
159+
}
160+
}
161+
141162
if let Some(true) = req.enabled {
142163
if !existing.enabled {
143164
let enabled_count = count_enabled_categories(&p).await.map_err(internal)?;
@@ -161,7 +182,7 @@ pub async fn update_category_handler(
161182

162183
update_category(
163184
&id,
164-
req.name.as_deref(),
185+
trimmed_name,
165186
req.description.as_ref().map(|o| o.as_deref()),
166187
req.emoji.as_ref().map(|o| o.as_deref()),
167188
req.discord_category_id.as_deref(),

crates/rustmail/src/commands/category/slash_command/category.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,10 +441,19 @@ async fn sub_rename(
441441
Some(v) => v,
442442
None => return reply(ctx, command, config, "category.text_usage", None).await,
443443
};
444+
let new = new.trim().to_string();
445+
if new.is_empty() {
446+
return reply(ctx, command, config, "category.text_usage", None).await;
447+
}
444448
let cat = match get_category_by_name(&old, pool).await? {
445449
Some(c) => c,
446450
None => return reply(ctx, command, config, "category.not_found", None).await,
447451
};
452+
if let Some(existing) = get_category_by_name(&new, pool).await? {
453+
if existing.id != cat.id {
454+
return reply(ctx, command, config, "category.already_exists", None).await;
455+
}
456+
}
448457
update_category(&cat.id, Some(&new), None, None, None, None, None, pool).await?;
449458
let mut params = HashMap::new();
450459
params.insert("name".to_string(), new);

crates/rustmail/src/commands/category/text_command/category.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,11 @@ async fn handle_rename(
211211
Some(c) => c,
212212
None => return send_translated(ctx, config, msg, "category.not_found", None).await,
213213
};
214+
if let Some(existing) = get_category_by_name(new, pool).await? {
215+
if existing.id != cat.id {
216+
return send_translated(ctx, config, msg, "category.already_exists", None).await;
217+
}
218+
}
214219
update_category(&cat.id, Some(new), None, None, None, None, None, pool).await?;
215220
let mut params = HashMap::new();
216221
params.insert("name".to_string(), new.to_string());

crates/rustmail/src/db/operations/ticket_categories.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ pub async fn get_category_by_name(
151151
SELECT id, name, description, emoji, discord_category_id,
152152
position, enabled, created_at, updated_at
153153
FROM ticket_categories
154-
WHERE name = ?
154+
WHERE name = ? COLLATE NOCASE
155155
LIMIT 1
156156
"#,
157157
)

crates/rustmail/src/modules/categories.rs

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,18 @@ pub async fn maybe_start_category_selection(ctx: &Context, config: &Config, msg:
7272
}
7373

7474
if let Ok(Some(_)) = get_pending_selection(msg.author.id.get() as i64, pool).await {
75-
let _ = append_queued_message(msg.author.id.get() as i64, &msg.id.to_string(), pool).await;
76-
return true;
75+
match append_queued_message(msg.author.id.get() as i64, &msg.id.to_string(), pool).await {
76+
Ok(_) => return true,
77+
Err(err) => {
78+
eprintln!(
79+
"failed to append queued message for user {} and message {}: {}",
80+
msg.author.id.get(),
81+
msg.id,
82+
err
83+
);
84+
return false;
85+
}
86+
}
7787
}
7888

7989
let categories = match list_enabled_categories(pool).await {
@@ -196,22 +206,15 @@ pub async fn finalize_with_category(
196206
None => return Ok(()),
197207
};
198208

199-
if !delete_pending_selection(user_id, pool)
200-
.await
201-
.unwrap_or(false)
202-
{
203-
return Ok(());
204-
}
205-
206209
let user = UserId::new(user_id as u64);
207210

208211
let (discord_override, ticket_cat_id) = if let Some(id) = category_id {
209212
match get_category_by_id(id, pool).await? {
210-
Some(cat) => {
213+
Some(cat) if cat.enabled => {
211214
let parent = cat.discord_category_id.parse::<u64>().ok();
212215
(parent, Some(cat.id.clone()))
213216
}
214-
None => (None, None),
217+
Some(_) | None => (None, None),
215218
}
216219
} else {
217220
(None, None)
@@ -237,6 +240,10 @@ pub async fn finalize_with_category(
237240
}
238241
}
239242

243+
if !delete_pending_selection(user_id, pool).await? {
244+
return Err("Failed to clear pending selection".into());
245+
}
246+
240247
Ok(())
241248
}
242249

crates/rustmail/src/modules/threads.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,16 +43,16 @@ pub async fn create_or_get_thread_for_user(
4343
.create_channel(&ctx.http, channel_builder)
4444
.await?;
4545

46-
let _ = create_thread_for_user(&channel, user_id.get() as i64, &username, pool)
46+
let thread_id = create_thread_for_user(&channel, user_id.get() as i64, &username, pool)
4747
.await
4848
.map_err(|e| {
4949
eprintln!("Error creating thread: {}", e);
5050
e
5151
})?;
5252

5353
if let Some(cat_id) = ticket_category_id {
54-
if let Some(thread) = get_thread_by_channel_id(&channel.id.to_string(), pool).await {
55-
let _ = set_thread_category(&thread.id, Some(cat_id), pool).await;
54+
if let Err(e) = set_thread_category(&thread_id, Some(cat_id), pool).await {
55+
eprintln!("Failed to set thread category for thread {thread_id}: {e}");
5656
}
5757
}
5858

crates/rustmail_panel/src/components/categories.rs

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -42,20 +42,34 @@ struct UpdateCategoryRequest {
4242
enabled: Option<bool>,
4343
}
4444

45-
async fn fetch_categories() -> Option<Vec<CategoryDto>> {
46-
let resp = Request::get("/api/categories").send().await.ok()?;
45+
async fn fetch_categories() -> Result<Vec<CategoryDto>, String> {
46+
let resp = Request::get("/api/categories")
47+
.send()
48+
.await
49+
.map_err(|e| e.to_string())?;
4750
if resp.status() != 200 {
48-
return None;
51+
let status = resp.status();
52+
let body = resp.text().await.unwrap_or_default();
53+
return Err(format!("HTTP {}: {}", status, body));
4954
}
50-
resp.json::<Vec<CategoryDto>>().await.ok()
55+
resp.json::<Vec<CategoryDto>>()
56+
.await
57+
.map_err(|e| e.to_string())
5158
}
5259

53-
async fn fetch_settings() -> Option<CategorySettingsDto> {
54-
let resp = Request::get("/api/categories/settings").send().await.ok()?;
60+
async fn fetch_settings() -> Result<CategorySettingsDto, String> {
61+
let resp = Request::get("/api/categories/settings")
62+
.send()
63+
.await
64+
.map_err(|e| e.to_string())?;
5565
if resp.status() != 200 {
56-
return None;
66+
let status = resp.status();
67+
let body = resp.text().await.unwrap_or_default();
68+
return Err(format!("HTTP {}: {}", status, body));
5769
}
58-
resp.json::<CategorySettingsDto>().await.ok()
70+
resp.json::<CategorySettingsDto>()
71+
.await
72+
.map_err(|e| e.to_string())
5973
}
6074

6175
#[function_component(CategoriesPage)]
@@ -102,22 +116,28 @@ pub fn categories_page() -> Html {
102116
let settings = settings.clone();
103117
let loading = loading.clone();
104118
let error = error.clone();
119+
let i18n = i18n.clone();
105120
Callback::from(move |_| {
106121
let categories = categories.clone();
107122
let settings = settings.clone();
108123
let loading = loading.clone();
109124
let error = error.clone();
125+
let i18n = i18n.clone();
110126
spawn_local(async move {
111127
loading.set(true);
112128
let cats = fetch_categories().await;
113129
let s = fetch_settings().await;
114130
match (cats, s) {
115-
(Some(c), Some(s)) => {
131+
(Ok(c), Ok(s)) => {
116132
categories.set(c);
117133
settings.set(Some(s));
118134
error.set(None);
119135
}
120-
_ => error.set(Some("Failed to load".to_string())),
136+
(Err(e), _) | (_, Err(e)) => error.set(Some(format!(
137+
"{}: {}",
138+
i18n.t("panel.categories.error_load"),
139+
e
140+
))),
121141
}
122142
loading.set(false);
123143
});

crates/rustmail_panel/src/i18n/en/en.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,7 @@
288288
"discord_category_id": "Discord Category ID",
289289
"error_name_required": "Name is required",
290290
"error_discord_id_required": "Discord category ID is required",
291+
"error_load": "Failed to load categories",
291292
"error_create": "Failed to create category",
292293
"modal": {
293294
"title": "Create Category",

migrations/20260413120000_ticket_categories.sql

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ CREATE TABLE IF NOT EXISTS ticket_categories (
1212
updated_at INTEGER NOT NULL
1313
);
1414

15+
CREATE UNIQUE INDEX IF NOT EXISTS idx_ticket_categories_name_unique
16+
ON ticket_categories(name COLLATE NOCASE);
17+
1518
CREATE INDEX IF NOT EXISTS idx_ticket_categories_enabled
1619
ON ticket_categories(enabled, position);
1720

0 commit comments

Comments
 (0)