feat(new_thread): open and close silently when staff hasn't engaged the user - #406
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a “silent thread” mode so staff can open a support thread without notifying the user, and ensures close notifications are only sent once staff has actually engaged the user (i.e., successfully delivered a DM reply).
Changes:
- Add a
threads.silentdatabase flag and plumb it through thread creation. - Mark threads as “engaged” (clear
silent) once a staff reply DM is successfully sent; suppress close DMs while still silent. - Update new-thread success messaging in EN/FR to reflect silent opening behavior.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| migrations/20260429120000_thread_silent.sql | Adds silent column to threads to persist the silent state. |
| crates/rustmail/src/db/operations/threads.rs | Extends create_thread_for_user with silent, adds is_thread_silent + mark_thread_engaged. |
| crates/rustmail/src/commands/new_thread/text_command/new_thread.rs | Opens threads silently (no DM), sets DB silent flag accordingly. |
| crates/rustmail/src/commands/new_thread/slash_command/new_thread.rs | Opens threads silently (no DM), updates success message key. |
| crates/rustmail/src/commands/new_thread/common.rs | Adds notify_user switch to welcome message handling. |
| crates/rustmail/src/commands/reply/text_command/reply.rs | Clears silent flag after a successful DM reply. |
| crates/rustmail/src/commands/reply/slash_command/reply.rs | Clears silent flag after a successful DM reply. |
| crates/rustmail/src/commands/anonreply/text_command/anonreply.rs | Clears silent flag only when the DM portion succeeds. |
| crates/rustmail/src/commands/close/text_command/close.rs | Suppresses close DM when thread is still silent (including scheduled closures). |
| crates/rustmail/src/commands/close/slash_command/close.rs | Same suppression logic as text close. |
| crates/rustmail/src/modules/scheduled_closures.rs | Avoids sending close DMs when thread remains silent. |
| crates/rustmail/src/modules/threads.rs | Updates thread record creation calls for new silent parameter. |
| crates/rustmail/src/api/handler/externals/tickets/create.rs | Updates thread creation call signature and welcome message call signature. |
| crates/rustmail/src/i18n/language/en.rs | Updates new_thread.success_without_dm text to indicate silent open. |
| crates/rustmail/src/i18n/language/fr.rs | Same as EN update for French. |
Comments suppressed due to low confidence (1)
crates/rustmail/src/db/operations/threads.rs:149
create_thread_for_userreturnsresbased on inserting intothreads, but the subsequent insert intothread_statususes the newly generatedthread_idregardless of whether the first insert returned an existing thread ID (unique open-thread constraint). Also, any failure to insert intothread_statusis currently ignored, which can leave a thread without athread_statusrow and break later operations that expect it. Consider restructuring so you compute the effective thread id first (inserted vs existing), then ensurethread_statusexists for that id (propagate errors or handle the expected “already exists” case explicitly).
let channel_id = channel.id.to_string();
let thread_id = Uuid::new_v4().to_string();
let silent_int: i64 = if silent { 1 } else { 0 };
let res = match sqlx::query(
"INSERT INTO threads (id, user_id, user_name, channel_id, silent) VALUES (?, ?, ?, ?, ?)",
)
.bind(&thread_id)
.bind(user_id)
.bind(user_name)
.bind(&channel_id)
.bind(silent_int)
.execute(&pool.clone())
.await
{
Ok(_) => Ok(thread_id.clone()),
Err(Error::Database(db_err))
if db_err.code() == Some(std::borrow::Cow::Borrowed("2067")) =>
{
if let Some(existing_thread_id) =
sqlx::query_scalar("SELECT id FROM threads WHERE user_id = ? AND status = 1")
.bind(user_id)
.fetch_optional(pool)
.await?
{
Ok(existing_thread_id)
} else {
Err(Error::Database(db_err))
}
}
Err(e) => Err(e),
};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if notify_user { | ||
| let _ = MessageBuilder::system_message(ctx, config) | ||
| .translated_content( | ||
| "new_thread.welcome_message", | ||
| Some(¶ms), | ||
| None, | ||
| Some(channel.guild_id.get()), | ||
| ) | ||
| .await | ||
| .to_user(user.id) | ||
| .send(true) | ||
| .await; |
There was a problem hiding this comment.
In the DM branch (if notify_user { ... }), translated_content is called with user_id: None, so get_translated_message will always fall back to the default language (it ignores guild_id for language selection when user_id is None). This means the welcome DM won’t respect the recipient’s language preferences. Pass Some(user.id) when translating content intended for the user (and keep guild_id as needed for server-scoped strings).
| .ok() | ||
| .flatten() | ||
| .map(|v| v != 0) | ||
| .unwrap_or(false) |
There was a problem hiding this comment.
is_thread_silent currently swallows database errors / missing rows and defaults to false. For the new “silent by default” behavior, this default can cause the bot to send user notifications when it can’t verify the silent flag (e.g., transient DB error), which undermines the feature’s intent. Consider returning Result<bool, sqlx::Error> (and handling it at call sites) or defaulting to true on read errors so you fail closed (no user notification).
| .unwrap_or(false) | |
| .unwrap_or(true) |
|
@copilot apply changes based on the comments in this thread |
- Pass Some(user.id) when translating DM content so welcome DM respects the recipient's language preferences instead of always falling back to the default language - Change is_thread_silent to unwrap_or(true) so DB errors fail closed, preventing accidental user notifications when the silent flag cannot be read Agent-Logs-Url: https://github.com/Rustmail/rustmail/sessions/611e4c0f-7bca-4626-b22f-8070ccd7676c Co-authored-by: Akinator31 <99099121+Akinator31@users.noreply.github.com>
Applied both changes from the review thread in commit af556be:
|
No description provided.