-
Notifications
You must be signed in to change notification settings - Fork 0
Modmail Feature MVP #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Caution Review failedFailed to post review comments WalkthroughA comprehensive refactor introducing Jest testing infrastructure, a new Arc3 bot wrapper library, and a complete modmail system with multi-language support. Migrates from CommonJS to ESM, adds configuration and blacklist management hooks, and removes legacy example code. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (DM)
participant Arc3 as Arc3 Bot
participant ModmailMS as ModmailMessageService
participant ModmailRepo as ModmailRepo
participant MongoDB as MongoDB
participant Guild as Guild (Channel)
participant Webhook as Webhook
User->>Arc3: Send DM message
Arc3->>ModmailMS: ProcessModmailMessageRecieved
ModmailMS->>ModmailRepo: Check active modmails
ModmailRepo->>MongoDB: Query active modmails
MongoDB-->>ModmailRepo: Return modmail record
ModmailRepo-->>ModmailMS: Return modmail
alt No Active Modmail
ModmailMS->>User: Show modmail select menu
else Active Modmail Exists
ModmailMS->>Webhook: Forward message via webhook
Webhook->>Guild: Post in modmail channel
ModmailMS->>ModmailRepo: Log transcript entry
ModmailRepo->>MongoDB: Save transcript
end
alt Error Occurred
ModmailMS->>ModmailMS: TryCleanupModmail
ModmailMS->>Guild: Delete modmail channel
ModmailMS->>User: Notify failure
end
sequenceDiagram
participant Moderator as Moderator (Channel)
participant Arc3 as Arc3 Bot
participant ModmailMS as ModmailMessageService
participant ModmailRepo as ModmailRepo
participant User as User (DM)
Moderator->>Arc3: Reply in modmail channel
Arc3->>ModmailMS: ProcessModmailMessageRecieved
ModmailMS->>ModmailRepo: Get active modmail
ModmailRepo-->>ModmailMS: Return modmail (User DM context)
ModmailMS->>User: Forward message via DM
ModmailMS->>ModmailRepo: Create transcript entry
ModmailRepo-->>ModmailMS: Transcript saved
Moderator->>Moderator: Click Save/Ban button
Moderator->>Arc3: Interaction triggered
Arc3->>ModmailMS: ProcessModmailSaveButton
ModmailMS->>ModmailMS: TryCleanupModmail
ModmailMS->>User: Notify session closed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR implements a modmail feature MVP that enables private communication between Discord users and server moderators through the bot. The implementation adds a complete TypeScript-based modmail system including MongoDB persistence with Mongoose schemas (v1 and v2 variants for Modmail, Transcript, Blacklist, GuildConfig), service layers for message routing and interaction handling, repository pattern for data access with caching, internationalization support (English, French, Chinese locales), React-style hooks for cached data access, Discord event handlers, slash commands for configuration, and comprehensive Jest testing infrastructure. The PR also migrates all existing JavaScript schema files from CommonJS to TypeScript ES modules as part of a broader codebase modernization effort. The architecture follows a layered pattern: Arc3 class bootstraps the bot → ModmailEvents listens to Discord events → ModmailMessageService and ModmailInteractionService handle business logic → ModmailRepo manages persistence → hooks provide cached data access across the system. The modmail workflow creates dedicated Discord channels with webhooks for bidirectional message relay, tracks active sessions in MongoDB, supports moderator comments, logs transcripts, and handles cleanup.
Important Files Changed
| Filename | Score | Overview |
|---|---|---|
| src/lib/arc3.ts | 1/5 | Core bot initialization with critical unbound event handlers and fire-and-forget async loops causing runtime failures |
| src/lib/hooks/useGuildConfig.ts | 1/5 | Guild config caching with invalid ObjectId timestamps and no upsert logic causing duplicate entries |
| src/locales/zh.json | 1/5 | Chinese translations with critical spelling errors in keys ('sucess', 'recieved') breaking runtime lookups |
| src/lib/repositories/ModmailRepo.ts | 2/5 | Modmail persistence layer with incorrect ObjectId generation and unbounded cache growth |
| src/lib/service/ModmailMessageService.ts | 2/5 | Message routing service with spelling errors in method names, unsafe array access, and overly broad triggers |
| src/lib/service/ModmailInteractionService.ts | 2/5 | Interaction handler with typo variable naming, improper blacklist handling, and missing error feedback |
| src/lib/util/ModmailUtils.ts | 2/5 | Core modmail utilities with async forEach issues, invalid webhook properties, and placeholder transcript URLs |
| src/lib/discord/events/ModmailEvents.ts | 2/5 | Event orchestration with deferred interactions not properly responded to before timeout |
| src/lib/discord/autocomplete/setConfigKeyAutoComplete.ts | 2/5 | Autocomplete handler with for-in enum iteration bug and unawaited response causing promise rejections |
| src/lib/hooks/useActiveModmails.ts | 2/5 | Active modmails caching with unawaited initialization causing empty cache returns |
| src/lib/hooks/useBlacklist.ts | 2/5 | Blacklist caching with race conditions from async initialization and hardcoded global key assumptions |
| src/lib/hooks/useTextContent.ts | 2/5 | i18n system with blocking synchronous file I/O and multi-digit placeholder corruption bug |
| src/lib/logger/DiscordLoggerImpl.ts | 2/5 | Pino logger adapter incorrectly passing rest parameters as arrays causing malformed logs |
| src/locales/fr.json | 2/5 | French translations with spelling errors in keys matching en.json/zh.json causing cross-locale inconsistency |
| src/lib/discord/commands/configs.ts | 2/5 | Config command with null-coalescing to "0" creating invalid entries and no error handling |
| src/lib/hooks/tests/useActiveModmails.test.ts | 2/5 | Comprehensive hook tests with incorrect Arc3 mock path causing test failures |
| src/lib/hooks/tests/useGuildConfig.test.ts | 3/5 | Guild config tests with incorrect Arc3 mock path and weak assertions for error cases |
| src/lib/hooks/tests/useBlacklist.test.ts | 3/5 | Blacklist tests with Arc3 path issues and missing positive blacklist case coverage |
| src/lib/schema/v2/Transcript.ts | 3/5 | New transcript schema with inconsistent field naming and potential v1 model collision |
| src/lib/schema/v1/Modmail.ts | 3/5 | Modmail schema with explicit _id preventing auto-generation and missing performance indexes |
| src/lib/schema/v1/GuildConfig.ts | 3/5 | Config schema with explicit _id override and no validation constraints or indexing |
| src/lib/schema/v1/Blacklist.ts | 3/5 | Blacklist schema with redundant _id field and missing indexes for frequently queried fields |
| src/lib/util/DiscordUtils.ts | 3/5 | Discord helper functions with unsafe type assertions and no permission error handling |
| src/lib/discord/guards/blacklist.ts | 3/5 | Blacklist guard with hardcoded English locale and non-ephemeral replies exposing status |
| src/ui/ModmailUi.ts | 3/5 | UI factory functions with unchecked clientInstance throwing errors and undefined modmailId in button IDs |
| src/main.ts | 3/5 | Simplified bootstrap delegating to Arc3 but removing explicit error handling and using nested namespace access |
| package.json | 4/5 | Dependency additions for modmail infrastructure well-structured but requiring operational verification |
| src/lib/schema/v1/Comment.ts | 4/5 | Clean CommonJS to ES module migration maintaining schema structure with no type safety additions |
| src/lib/schema/v1/Commandstat.ts | 4/5 | Module syntax migration with no TypeScript types added but no breaking changes |
| src/lib/schema/v1/Application.ts | 4/5 | ES module conversion with date fields still as strings instead of Date types |
| src/lib/schema/v1/Approval.ts | 4/5 | Clean migration to TypeScript modules preserving existing schema without validation |
| src/lib/schema/v1/Transcript.ts | 4/5 | Module conversion with inconsistent naming conventions persisting from original |
| src/lib/schema/v2/UserData.ts | 4.5/5 | TypeScript migration complete with no type annotations but no breaking changes |
| src/lib/schema/v1/Guild.ts | 4.5/5 | Clean CommonJS to ES module conversion with minor missing semicolon on line 2 |
| jest.config.js | 4/5 | Proper Jest ESM configuration for TypeScript with appropriate test file patterns |
| src/lib/hooks/tests/useTextContent.test.ts | 4/5 | Comprehensive i18n tests with spelling errors matching actual locale files |
| src/locales/en.json | 4/5 | English locale file with two spelling errors ('sucess', 'recieved') requiring correction |
| tsconfig.json | 5/5 | Safe addition of resolveJsonModule for locale imports with casing normalization |
| src/lib/schema/v1/Insight.ts | 5/5 | Minimal risk JavaScript to TypeScript conversion preserving schema structure |
| src/lib/schema/v1/Notes.ts | 5/5 | Safe module system conversion with no functional changes to note schema |
| src/lib/logger/index.ts | 4/5 | Simple barrel export with missing EOF newline and spacing inconsistency |
| jest.setup.js | 5/5 | Empty placeholder setup file for future test configuration |
| src/commands/slashes.ts | 5/5 | Safe deletion of example/scaffolding ping command |
| src/events/common.ts | 5/5 | Safe removal of unused example event handler boilerplate |
Confidence score: 1/5
- This PR is not safe to merge and will cause immediate production failures due to critical runtime errors in core files
- Score reflects multiple high-severity issues: unbound event handlers in Arc3 causing
thisreference errors, invalid ObjectId timestamp generation causing database write failures, unawaited async operations creating race conditions and empty cache returns, spelling errors in locale keys causing translation lookup failures, and fire-and-forget interaction handlers causing Discord timeout errors. Additionally, the Arc3 mock path in all three test files is incorrect, meaning tests won't properly isolate dependencies and may not catch real issues. - Pay critical attention to src/lib/arc3.ts (event handler binding), src/lib/hooks/useGuildConfig.ts (ObjectId timestamps), src/lib/hooks/useActiveModmails.ts (cache initialization), all three locale files (key spelling), src/lib/repositories/ModmailRepo.ts (ObjectId generation), src/lib/discord/events/ModmailEvents.ts (interaction responses), and all test files (Arc3 mock paths)
Sequence Diagram
sequenceDiagram
participant User
participant Discord DM
participant ModmailMessageService
participant ModmailRepo
participant ModmailUtils
participant Guild Channel
participant Webhook
participant Moderator
participant ModmailInteractionService
User->>Discord DM: Sends message with "modmail"
Discord DM->>ModmailMessageService: ProcessModmailDmMessageRecieved
ModmailMessageService->>ModmailRepo: getActiveModmails
ModmailRepo-->>ModmailMessageService: No active modmail found
ModmailMessageService->>ModmailUtils: SendModmailSelectMenu
ModmailUtils->>User: Send server selection menu
User->>ModmailInteractionService: Select server from menu
ModmailInteractionService->>ModmailUtils: initModmailAsync
ModmailUtils->>Guild Channel: CreateTextChannel
ModmailUtils->>Guild Channel: CreateWebhook
ModmailUtils->>ModmailRepo: CreateModmail
ModmailRepo-->>ModmailInteractionService: Return modmail object
ModmailInteractionService->>User: Send "Modmail Sent" embed
ModmailInteractionService->>Guild Channel: Send modmail menu with buttons
User->>Discord DM: Send modmail message
Discord DM->>ModmailMessageService: ProcessModmailDmMessageRecieved
ModmailMessageService->>ModmailRepo: getActiveModmails
ModmailRepo-->>ModmailMessageService: Active modmail found
ModmailMessageService->>Webhook: SendAttachmentsAndMessageToWebhook
Webhook->>Guild Channel: Post user message
ModmailMessageService->>ModmailRepo: CreateTranscript
ModmailMessageService->>User: React with "📨"
Moderator->>Guild Channel: Reply to modmail
Guild Channel->>ModmailMessageService: ProcessModmailChannelMessageRecieved
ModmailMessageService->>User: SendAttachmentsAndMessageToUser
ModmailMessageService->>ModmailRepo: CreateTranscript
ModmailMessageService->>Guild Channel: React with "📨"
User->>Discord DM: Edit previous message
Discord DM->>ModmailMessageService: ProcessModmailMessageUpdated
ModmailMessageService->>Webhook: Edit webhook message
ModmailMessageService->>ModmailRepo: CreateTranscript
ModmailMessageService->>User: React with "✏️"
Moderator->>Guild Channel: Click "Save" button
Guild Channel->>ModmailInteractionService: ProcessModmailSaveButton
ModmailInteractionService->>ModmailUtils: LogModmailTranscript
ModmailUtils->>Guild Channel: Post transcript embed
ModmailInteractionService->>ModmailUtils: TryCleanupModmail
ModmailUtils->>Guild Channel: Delete modmail channel
ModmailUtils->>ModmailRepo: Delete modmail record
ModmailInteractionService->>ModmailRepo: clearActiveModmailsCache
Additional Comments (31)
-
src/lib/schema/v1/Notes.ts, line 3-10 (link)style: consider adding TypeScript types for the schema fields—using
mongoose.Schema<UserNoteDocument>with an interface would provide type safety -
src/lib/schema/v1/Approval.ts, line 3-8 (link)style: Schema fields lack type enforcement and timestamps. All fields are loose
Stringtypes with no required validation, anddateshould use nativeDatetype with{ timestamps: true }for automatic createdAt/updatedAt tracking. -
src/lib/schema/v1/Insight.ts, line 3-13 (link)style: define a TypeScript interface for the Insight document to enforce type safety across the codebase (currently
data: Objectis untyped) -
src/lib/schema/v1/Comment.ts, line 3-8 (link)style: Schema fields lack type safety and constraints.
commentDateshould use{ type: Date, default: Date.now }instead of String, and consider addingrequired:trueto critical fields likeappealIdanduserSnowflake. -
src/lib/schema/v1/Commandstat.ts, line 6 (link)style:
args: Objectis too loose. Consider defining a proper type or usingmongoose.Schema.Types.Mixedexplicitly, and add validation if the structure is known. What is the expected structure ofargs? -
src/lib/schema/v1/Transcript.ts, line 3-13 (link)style: Schema field names are inconsistent (some camelCase:
modmailId,messagecontent; some lowercase:sendersnowflake,createdat,GuildSnowflake). This makes the schema harder to query and maintain. Consider standardizing to camelCase (e.g.,senderSnowflake,createdAt,guildSnowflake). -
src/lib/schema/v1/Transcript.ts, line 3-13 (link)style: Schema lacks type safety and validation. Consider defining an interface and using TypeScript types with proper mongoose types (e.g.,
{ type: String, required: true }) to catch runtime errors and improve IntelliSense. -
src/lib/schema/v2/UserData.ts, line 3-9 (link)style: Add TypeScript types for schema fields. Consider defining an interface for UserData and using
mongoose.Schema<IUserData>to get proper type-checking for documents -
src/lib/schema/v1/Application.ts, line 6 (link)style:
submitDate,joindatestored as String instead of Date type; storing timestamps as strings prevents date queries and sorting -
src/lib/service/ModmailMessageService.ts, line 346-347 (link)logic: Case-sensitive substring check means 'Modmail' won't trigger but 'mod' or 'mail' in any other word (like 'modern', 'email') will create false positives
-
src/lib/util/ModmailUtils.ts, line 35 (link)syntax: JSDoc param mentions
clientInstancebut this parameter was removed from the function signature -
src/lib/util/ModmailUtils.ts, line 38 (link)syntax: Return type description says 'true if successful, falseotherwise' but actually returns
Modmail | undefined -
src/lib/util/ModmailUtils.ts, line 143-148 (link)logic: Async callbacks in
forEachare not awaited - attachment sends will execute concurrently without error handling. Usefor...ofloop orPromise.all -
src/lib/util/ModmailUtils.ts, line 155 (link)syntax:
isMessageis not a valid property in discord.js webhook.send() options. Is this a custom property used elsewhere in the codebase, or was this intended to be a different property? -
src/lib/util/ModmailUtils.ts, line 194 (link)syntax: Typo in JSDoc - extra
.1at end of comment -
src/lib/util/ModmailUtils.ts, line 250 (link)logic: When
failed=false, condition$lte: new Date()matches all modmails up to now, potentially cleaning up unrelated active modmails. Should the success case use a different timestamp constraint or additional filters to avoid unintended cleanup? -
src/lib/service/ModmailInteractionService.ts, line 35 (link)syntax: typo: 'isBlacklsted' should be 'isBlacklisted'
-
src/lib/service/ModmailInteractionService.ts, line 50 (link)syntax: Error constructor only accepts message string; passing 'modmail' object as second argument will be ignored and won't appear in error
-
src/lib/service/ModmailInteractionService.ts, line 67-72 (link)logic: catch block doesn't notify user of failure; consider calling interaction.editReply or interaction.followUp with error message
-
src/lib/service/ModmailInteractionService.ts, line 87 (link)logic: no validation that customId has at least 3 parts; split could return fewer elements causing undefined extraction
-
src/lib/hooks/__tests__/useTextContent.test.ts, line 12 (link)syntax: typo: 'sucess' should be 'success' (same typo may exist in actual locale files)
-
src/lib/hooks/__tests__/useTextContent.test.ts, line 17 (link)syntax: typo: 'recieved' should be 'received' (verify spelling in actual locale files too)
-
src/lib/discord/events/ModmailEvents.ts, line 59 (link)syntax: Typo: 'Recieved' should be 'Received'
-
src/lib/discord/events/ModmailEvents.ts, line 102-105 (link)logic: Same issue as line 88-91: deferred reply with fire-and-forget processing can cause 'interaction failed' errors
-
src/lib/arc3.ts, line 83-84 (link)logic:
onGuildMemberAddwill losethisbinding when passed to event listener. Use arrow function or bind it. -
src/lib/arc3.ts, line 98-100 (link)logic:
onInteractionCreatewill losethisbinding. Although it re-bindsthistoClientin the signature, the event listener won't preserve this. -
src/lib/arc3.ts, line 127-136 (link)logic:
forEachwith async callback creates fire-and-forget promises. Usefor...oforPromise.allto properly handle async operations and errors. -
src/lib/arc3.ts, line 129 (link)style: Fetching all guilds inside the loop causes N+1 database queries. Move
GuildInfo.find()outside the loop (line 127). -
src/lib/arc3.ts, line 121 (link)logic: MongoDB connection error is logged but not propagated. If connection fails, the bot continues running without a database. Should the bot shut down if MongoDB connection fails, or is degraded operation acceptable?
-
src/lib/arc3.ts, line 88 (link)logic: Passing empty array
[]bypasses the duplicate check inNewGuild. Should pass actual cached guilds or handle differently. -
src/lib/arc3.ts, line 159 (link)style:
ObjectId.createFromTimecreates predictable IDs. Usenew ObjectId()for uniqueness unless time-based ordering is required.
45 files reviewed, 102 comments
| @@ -0,0 +1,3 @@ | |||
| import { DiscordLoggerImpl } from "./DiscordLoggerImpl.js"; | |||
|
|
|||
| export {DiscordLoggerImpl as Logger} No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax: missing newline at end of file
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/logger/index.ts
Line: 3:3
Comment:
**syntax:** missing newline at end of file
How can I resolve this? If you propose a fix, please make it concise.| @@ -0,0 +1,3 @@ | |||
| import { DiscordLoggerImpl } from "./DiscordLoggerImpl.js"; | |||
|
|
|||
| export {DiscordLoggerImpl as Logger} No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: missing space after opening brace and before closing brace in export statement. Consider: export { DiscordLoggerImpl as Logger }
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/logger/index.ts
Line: 3:3
Comment:
**style:** missing space after opening brace and before closing brace in export statement. Consider: `export { DiscordLoggerImpl as Logger }`
How can I resolve this? If you propose a fix, please make it concise.| const mongoose = require('mongoose'); | ||
| require('mongoose-long')(mongoose); | ||
| import mongoose from 'mongoose'; | ||
| import mongooseLong from 'mongoose-long' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax: Missing semicolon at end of import statement
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/schema/v1/Guild.ts
Line: 2:2
Comment:
**syntax:** Missing semicolon at end of import statement
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| } | ||
|
|
||
| async CreateTranscript( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Method name uses PascalCase instead of camelCase, inconsistent with JavaScript conventions and the other public method getActiveModmails.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/repositories/ModmailRepo.ts
Line: 88:88
Comment:
**style:** Method name uses PascalCase instead of camelCase, inconsistent with JavaScript conventions and the other public method `getActiveModmails`.
How can I resolve this? If you propose a fix, please make it concise.| // We have established the user has an active modmail, is not a bot, and does not wish to close the session | ||
| await this.ProcessModmailMessageToGuild( | ||
| message, | ||
| modmails.filter((x: InstanceType<typeof Modmail>)=> x.usersnowflake?.toString() === message.author.id)[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Array access [0] assumes at least one match exists, but filter could return empty array. Use .find() instead or add safety check.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/service/ModmailMessageService.ts
Line: 237:237
Comment:
**logic:** Array access [0] assumes at least one match exists, but filter could return empty array. Use .find() instead or add safety check.
How can I resolve this? If you propose a fix, please make it concise.| private async ProcessModmailMessageToGuild(this: ModmailMessageService, message: Message, modmail: InstanceType<typeof Modmail>) { | ||
|
|
||
| // Get the active modmail webhook | ||
| const webhook = await Arc3.Arc3.clientInstance.fetchWebhook(modmail.webhooksnowflake?.toString() ?? "0"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Missing error handling for webhook fetch failure - will throw if webhook doesn't exist
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/service/ModmailMessageService.ts
Line: 251:251
Comment:
**logic:** Missing error handling for webhook fetch failure - will throw if webhook doesn't exist
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 35
🧹 Nitpick comments (35)
src/lib/schema/v1/Notes.ts (1)
1-1: ESM default export ok; add caching to prevent model overwrite in tests.Adopt cached model creation to avoid OverwriteModelError with Jest/hot reload.
Recommended pattern (outside this hunk):
const UserNote = (mongoose.models['user_note'] as mongoose.Model<any>) ?? mongoose.model('user_note', userNoteSchema); export default UserNote;Optional: field naming is inconsistent across models (
usersnowflakevsuserSnowflake). Consider aligning naming to a single convention later.Also applies to: 14-14
src/lib/schema/v2/UserData.ts (1)
1-1: ESM migration verified; add Mongoose model caching pattern as optional best practice.The ESM import and default export are correct. However, no importers of UserData.ts were found in the codebase, so the immediate verification concern does not apply. The missing model caching guard is not currently critical but should be addressed as a best practice when this file is integrated elsewhere to prevent OverwriteModelError in hot-reload scenarios.
Recommended optional refactor (when file is actively imported):
const UserData = (mongoose.models['userdata'] as mongoose.Model<any>) ?? mongoose.model('userdata', userDataSchema); export default UserData;src/lib/schema/v1/Appeal.ts (1)
1-1: ESM migration verified; add cached model pattern for consistency.The default export migration is correct and safe—Appeal is not imported anywhere in the codebase. Adding the cached model pattern is still recommended to prevent OverwriteModelError in test runs and align with best practices:
const Appeal = (mongoose.models['Appeal'] as mongoose.Model<any>) ?? mongoose.model('Appeal', AppealSchema); export default Appeal;src/lib/util/DiscordUtils.ts (1)
4-12: Remove redundant type cast.The explicit cast to
Promise<TextChannel>is unnecessary sinceguild.channels.create()withChannelType.GuildTextalready returnsPromise<TextChannel>. The wrapping parentheses are also superfluous.Apply this diff to simplify the code:
export async function CreateTextChannel(guild: Guild, name: string, options?: Partial<GuildChannelCreateOptions>) : Promise<TextChannel> { - return await (guild.channels.create({ + return await guild.channels.create({ name, type: ChannelType.GuildText, ...options - }) as Promise<TextChannel>); + }); }src/lib/discord/autocomplete/setConfigKeyAutoComplete.ts (1)
8-10: Make enum iteration and autocomplete response robust (filter, cap 25, await).
- for..in on numeric enums includes reverse numeric keys.
- Autocomplete must return ≤25 items and should filter by the focused input.
Apply:
-export function SetConfigKeyAutoComplete(interaction: AutocompleteInteraction) { - - const values : string[] = []; - - for (let item in GuildConfigKey) { - values.push(item) - } - - // Respond with enum values - interaction.respond( - values.map(choice => ({ - name: choice, - value: choice, - })) - ); -} +export async function SetConfigKeyAutoComplete(interaction: AutocompleteInteraction) { + const focused = interaction.options.getFocused()?.toString().toLowerCase() ?? ""; + // Avoid reverse numeric mappings; keep only string keys + const keys = Object.keys(GuildConfigKey).filter((k) => isNaN(Number(k))); + const choices = keys + .filter((k) => k.toLowerCase().includes(focused)) + .slice(0, 25) + .map((k) => ({ name: k, value: k })); + await interaction.respond(choices); +}If
GuildConfigKeyis a string enum already, this is still fine; if it's numeric, the filter is required. Please confirm the enum type insrc/lib/hooks/useGuildConfig.ts.Also applies to: 13-18
src/main.ts (1)
8-10: HandlerunAsync()errors to avoid unhandled rejections.Attach a catch; move the log into then.
-arc.runAsync().finally(() => { - console.log("Running"); -}); +arc + .runAsync() + .then(() => console.log("Running")) + .catch((err) => { + console.error("Arc3 failed to start:", err); + process.exitCode = 1; + });src/lib/hooks/__tests__/useActiveModmails.test.ts (1)
50-52: Minor test hygiene.
afterEach(() => jest.clearAllMocks())duplicates thebeforeEachreset; consider removing the afterEach call.src/lib/discord/guards/blacklist.ts (2)
18-20: Use the interaction’s locale instead of hardcoding EN.Prefer
interactionParams.locale/guildLocaleto select texts.- const { text } = useTextContent(Locale.EN).actions; + const loc = (interactionParams.guildLocale ?? interactionParams.locale ?? Locale.EN) as Locale; + const { text } = useTextContent(loc).actions;
32-34: Make blacklist reply ephemeral.Avoid exposing blacklist status publicly.
- await interactionParams.reply({ - content: text('arc.blacklist'), - }) + await interactionParams.reply({ + content: text('arc.blacklist'), + ephemeral: true, + });src/lib/hooks/useActiveModmails.ts (2)
20-22: Handle async init errors.Fire-and-forget
buildCache()can reject silently.- if (!activeModmailsCache.isCached("activemodmails")) { - buildCache(); - } + if (!activeModmmailsCache.isCached("activemodmails")) { + buildCache().catch((e) => logger.error("Failed to build activemodmails cache", e)); + }
24-32: Expose a getter for consumers.Return a
getActiveModmails()action rather than requiring callers to reach into the cache.export const useActiveModmails = () => { async function buildCache() { return activeModmailsCache.cacheable(async () => { return await Modmail.find(); }, "activemodmails", { cachePolicy: 'cache-only'}) } + async function getActiveModmails() { + return buildCache(); + } + if (!activeModmailsCache.isCached("activemodmails")) { buildCache(); } return { actions: { - buildCache + buildCache, + getActiveModmails }, states: { activeModmailsCache, logger } }src/lib/discord/commands/configs.ts (1)
37-46: Optional: add error handling around setConfig.Surface a user-friendly failure if persistence throws.
- await setConfig(interaction.guildId?? "0", key, value); - - await interaction.editReply({ - content: text('arc.command.setconfig.sucess'), - }); + try { + await setConfig(interaction.guildId ?? "0", key, value); + await interaction.editReply({ content: text('arc.command.setconfig.success') }); + } catch (e) { + await interaction.editReply({ content: text('arc.error.generic') ?? 'Failed to set config.' }); + }src/lib/hooks/useGuildConfig.ts (1)
32-45: Enum vs stored keys: validate consistency.You cast
x.configkeytoGuildConfigKey, but DB may contain other strings (e.g., tests usemodmail_channel,log_channel). Consider migration/normalization.Example normalization:
const rawKey = (x.configkey ?? "unknown") as string; const configKey = normalizeKey(rawKey) as GuildConfigKey; // where normalizeKey maps legacy keys to enum valuessrc/lib/logger/DiscordLoggerImpl.ts (1)
13-17: Optional: return ILogger from child for consistent typing.Returning pino Logger leaks implementation; returning
ILoggerkeeps callers decoupled.- child(loggerName: string) : Logger { - return this._logger.child({ - source: loggerName - }); - } + child(loggerName: string): ILogger { + const l = this._logger.child({ source: loggerName }); + // minimal adapter preserving ILogger shape + return { + error: (...a: unknown[]) => l.error(...(a as any[])), + info: (...a: unknown[]) => l.info(...(a as any[])), + log: (...a: unknown[]) => l.debug(...(a as any[])), + warn: (...a: unknown[]) => l.warn(...(a as any[])), + } as ILogger; + }src/lib/discord/events/ModmailEvents.ts (2)
99-100: Regex pattern is too permissive; use a literal and anchors.Current string escapes collapse at runtime, so
.becomes wildcard. Tighten the selector.- @ButtonComponent({ id: new RegExp("modmail\.save\..*") }) + @ButtonComponent({ id: /^modmail\.save\..+$/ })
6-15: Initialize mongoose-long once in a central module; remove unused Long/ObjectId here.Multiple invocations are unnecessary; centralize the plugin registration (e.g., DB bootstrap) and drop unused type exports from this file.
src/lib/hooks/useTextContent.ts (3)
11-14: Locale file resolution will break when running compiled code.Using
./src/locales/...assumes cwd and source paths. Resolve relative to this module to work fromdist/.-for (let locale in Locale) { - locale = locale.toLowerCase(); - translationFiles[locale] = JSON.parse(readFileSync(`./src/locales/${locale}.json`).toString()) as Translations; -} +import { fileURLToPath } from "url"; +import path from "path"; +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); +for (let locale in Locale) { + const lc = locale.toLowerCase(); + const p = path.resolve(__dirname, "../../locales", `${lc}.json`); + translationFiles[lc] = JSON.parse(readFileSync(p, "utf-8")) as Translations; +}
18-26: Add fallback when a locale file/key is missing.Avoid crashing on missing locale or leaking raw codes in user UX.
- const translations = translationFiles[locale]; + const translations = translationFiles[locale] ?? translationFiles[Locale.EN]; @@ - if (!(code in translations)) - return code; + if (!(code in translations)) { + return String(code); + }
1-1: Remove unused import.
sreaddirSyncis never used.-import { readdirSync, readFileSync } from "fs"; +import { readFileSync } from "fs";src/lib/service/ModmailInteractionService.ts (1)
43-73: Prefer async/await over mixed .then/.catch for readability and error scope.This block mixes paradigms. Converting to try/catch clarifies control flow and error cleanup.
- initModmailAsync( - guild, - interaction.user, - this.modmailRepo - ).then(async (modmail) => { - if (modmail === undefined) { - throw new Error("Modmail init unsucessfull", modmail); - } - await interaction.editReply({ embeds: [ModmailSentEmbed()] }); - const modmailChannel = ( await interaction.client.channels - .fetch(modmail.channelsnowflake?.toString() ?? "0") - ) as TextChannel; - const modmailMenu = ModmailMenuEmbed(modmail.usersnowflake?.toString()?? "0", modmail._id?.toString()) - await modmailChannel.send(modmailMenu); - }) - .catch(async (e) => { - await TryCleanupModmail(interaction, interaction.user.id, e); - this.modmailRepo.clearActiveModmailsCache() - }); + try { + const modmail = await initModmailAsync(guild, interaction.user, this.modmailRepo); + if (!modmail) throw new Error("Modmail init unsuccessful"); + await interaction.editReply({ embeds: [ModmailSentEmbed()] }); + const chan = await interaction.client.channels.fetch(modmail.channelsnowflake?.toString() ?? "0"); + const modmailChannel = chan as TextChannel; + await modmailChannel.send(ModmailMenuEmbed(modmail.usersnowflake?.toString() ?? "0", modmail._id?.toString())); + } catch (e) { + await TryCleanupModmail(interaction, interaction.user.id, e); + this.modmailRepo.clearActiveModmailsCache(); + }src/lib/hooks/useBlacklist.ts (1)
27-51: Cache policy can starve updates under long-lived processes.
cache-onlywith manual invalidation works, but consider TTL or SWR to protect against missed invalidations.src/ui/ModmailUi.ts (3)
71-85: Guard against missing modmailId to avoid “undefined” custom IDs.Ensure IDs are valid and within 100 chars.
- new ButtonBuilder() - .setCustomId(`modmail.save.${modmailId}`) + new ButtonBuilder() + .setCustomId(`modmail.save.${modmailId ?? 'unknown'}`) .setStyle(ButtonStyle.Secondary) @@ - new ButtonBuilder() - .setCustomId(`modmail.ban.${modmailId}`) + new ButtonBuilder() + .setCustomId(`modmail.ban.${modmailId ?? 'unknown'}`) @@ - new ButtonBuilder() - .setCustomId(`modmail.ping.${modmailId}`) + new ButtonBuilder() + .setCustomId(`modmail.ping.${modmailId ?? 'unknown'}`)Alternatively, throw if
modmailIdis falsy to catch upstream issues early.
1-1: Remove unused import.
Useris not used.-import { ActionRowBuilder, ButtonBuilder, ButtonStyle, EmbedBuilder, MessageActionRowComponentBuilder, User } from "discord.js"; +import { ActionRowBuilder, ButtonBuilder, ButtonStyle, EmbedBuilder, MessageActionRowComponentBuilder } from "discord.js";
103-127: Consistent embed return types.You sometimes return
toJSON()and sometimesEmbedBuilder. Both work, but consistency simplifies callers. Prefer returning builders and let discord.js encode them.src/lib/arc3.ts (2)
18-22: Reduce confusion: duplicateclientInstance(static vs instance).Having both a static and instance property named
clientInstanceis confusing. Consider renaming the instance field toclient.- public static clientInstance : Client; - private readonly clientInstance : Client; + public static clientInstance : Client; + private readonly client : Client; @@ - this.clientInstance = Arc3.clientInstance; + this.client = Arc3.clientInstance;Update call sites accordingly.
Also applies to: 64-66
88-91: Prevent duplicate guild records on join.Passing
[]guarantees insert. Either query before calling (as above) or makeNewGuildself-check with a DBfindOne. Consider a unique index onguildsnowflake.src/lib/repositories/ModmailRepo.ts (3)
16-19: Unify logger types (pino vs custom).
this.logger: Logger(from pino) is assignedArc3.Arc3.clientLogger.child(...)(custom logger). Align on one logger interface/type to avoid structural typing surprises.Also applies to: 27-31
117-128: Prefer Map for recent message mapping (and add reverse index).Object lookup + linear reverse scan is O(n). Use two Maps for O(1) both ways.
- private recentMessagesCache : Record<string, string>; + private recentUserToModmail = new Map<string, string>(); + private recentModmailToUser = new Map<string, string>(); @@ - this.recentMessagesCache = { - - } as Record<string, string>; + this.recentUserToModmail.clear(); + this.recentModmailToUser.clear(); @@ - public addRecentMessage(userMessageId: string, modmailMessageId: string) { - this.recentMessagesCache[userMessageId] = modmailMessageId; - } + public addRecentMessage(userMessageId: string, modmailMessageId: string) { + this.recentUserToModmail.set(userMessageId, modmailMessageId); + this.recentModmailToUser.set(modmailMessageId, userMessageId); + } @@ - public getRecentModmailMessageID(userMessageID: string) { - return this.recentMessagesCache[userMessageID]; - } + public getRecentModmailMessageID(userMessageID: string) { + return this.recentUserToModmail.get(userMessageID); + } @@ - public getRecentUserMessageID(modmailMessageID: string) { - const entry = Object.entries(this.recentMessagesCache).find( ([_, v]) => v === modmailMessageID); - return entry ? entry[0] : undefined; - } + public getRecentUserMessageID(modmailMessageID: string) { + return this.recentModmailToUser.get(modmailMessageID); + }
59-62: Rebuild cache after invalidation.After clearing
activeModmailsCache, consider rebuilding to keep subsequent reads hot.- activeModmailsCache.clear(); + activeModmailsCache.clear(); + await this.getActiveModmails();src/lib/util/ModmailUtils.ts (3)
70-75: Sanitize channel name for Discord constraints.Usernames can contain spaces/uppercase/unsupported chars. Slugify to a safe
channel-name.- `${text('arc.modmail.channel.name')}-${user.username}`, + `${text('arc.modmail.channel.name')}-${user.username}` + .toLowerCase() + .replace(/\s+/g, "-") + .replace(/[^a-z0-9-_]/g, "") + .slice(0, 90),
96-104: No need to await a property; also ensure configs exist.
guilds.cacheis synchronous; removeawait. Consider skipping guilds not present inguildConfigs.- const guilds = await Arc3.Arc3.clientInstance.guilds.cache; + const guilds = Arc3.Arc3.clientInstance.guilds.cache;
196-211: DM may fail; add error handling.Users with closed DMs will reject the send. Catch and log.
- await message.author.send({ + await message.author.send({ components: [buttonRow], content: text('arc.modmail.menu.select.placeholder') - }); + }).catch(e => logger.warn(e, "Failed to send modmail select menu (DMs closed?)"));src/lib/service/ModmailMessageService.ts (3)
26-37: Fix typos in method names ('Recieved' → 'Received').Minor but pervasive; improves readability and searchability. Rename methods and update call sites within this file.
-ProcessModmailMessageRecieved +ProcessModmailMessageReceived -ProcessModmailChannelMessageRecieved +ProcessModmailChannelMessageReceived -ProcessModmailDmMessageRecieved +ProcessModmailDmMessageReceivedAlso applies to: 185-205, 213-240, 248-278, 285-314, 322-337, 344-353
248-276: Add allowedMentions on reactions failure path already handled elsewhere; keep webhook send safe.Ensure downstream sends don’t ping users; you're already using utils for webhook send. No change needed here if utils patch is applied. Consider catching
reactfailures too.- }).catch(async (e) => { + }).catch(async (e) => { this.logger.error(e, "Error sending message in modmail: " + modmail._id?.toString()) - await message.react(text('arc.modmail.delivery.emoji.failed')); + try { await message.react(text('arc.modmail.delivery.emoji.failed')); } catch {} });
285-314: Symmetric react error handling.Mirrors the guild path; wrap in try/catch to avoid unhandled rejections on missing perms.
- await message.react(text('arc.modmail.delivery.emoji.failed')); + try { await message.react(text('arc.modmail.delivery.emoji.failed')); } catch {}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (45)
jest.config.js(1 hunks)jest.setup.js(1 hunks)package.json(1 hunks)src/commands/slashes.ts(0 hunks)src/events/common.ts(0 hunks)src/lib/arc3.ts(1 hunks)src/lib/discord/autocomplete/setConfigKeyAutoComplete.ts(1 hunks)src/lib/discord/commands/configs.ts(1 hunks)src/lib/discord/events/ModmailEvents.ts(1 hunks)src/lib/discord/guards/blacklist.ts(1 hunks)src/lib/hooks/__tests__/useActiveModmails.test.ts(1 hunks)src/lib/hooks/__tests__/useBlacklist.test.ts(1 hunks)src/lib/hooks/__tests__/useGuildConfig.test.ts(1 hunks)src/lib/hooks/__tests__/useTextContent.test.ts(1 hunks)src/lib/hooks/useActiveModmails.ts(1 hunks)src/lib/hooks/useBlacklist.ts(1 hunks)src/lib/hooks/useGuildConfig.ts(1 hunks)src/lib/hooks/useTextContent.ts(1 hunks)src/lib/logger/DiscordLoggerImpl.ts(1 hunks)src/lib/logger/index.ts(1 hunks)src/lib/repositories/ModmailRepo.ts(1 hunks)src/lib/schema/v1/Appeal.ts(2 hunks)src/lib/schema/v1/Application.ts(2 hunks)src/lib/schema/v1/Approval.ts(2 hunks)src/lib/schema/v1/Blacklist.ts(1 hunks)src/lib/schema/v1/Commandstat.ts(2 hunks)src/lib/schema/v1/Comment.ts(2 hunks)src/lib/schema/v1/Guild.ts(2 hunks)src/lib/schema/v1/GuildConfig.ts(1 hunks)src/lib/schema/v1/Insight.ts(2 hunks)src/lib/schema/v1/Modmail.ts(1 hunks)src/lib/schema/v1/Notes.ts(2 hunks)src/lib/schema/v1/Transcript.ts(2 hunks)src/lib/schema/v2/Transcript.ts(1 hunks)src/lib/schema/v2/UserData.ts(2 hunks)src/lib/service/ModmailInteractionService.ts(1 hunks)src/lib/service/ModmailMessageService.ts(1 hunks)src/lib/util/DiscordUtils.ts(1 hunks)src/lib/util/ModmailUtils.ts(1 hunks)src/locales/en.json(1 hunks)src/locales/fr.json(1 hunks)src/locales/zh.json(1 hunks)src/main.ts(1 hunks)src/ui/ModmailUi.ts(1 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (2)
- src/events/common.ts
- src/commands/slashes.ts
🧰 Additional context used
🧬 Code graph analysis (18)
src/lib/schema/v1/Blacklist.ts (1)
src/lib/discord/guards/blacklist.ts (1)
Blacklist(12-36)
src/lib/discord/guards/blacklist.ts (2)
src/lib/hooks/useBlacklist.ts (1)
useBlacklist(18-99)src/lib/hooks/useTextContent.ts (1)
useTextContent(16-49)
src/lib/hooks/useGuildConfig.ts (2)
src/lib/arc3.ts (1)
Arc3(16-192)src/lib/discord/commands/configs.ts (1)
setConfig(18-48)
src/lib/discord/commands/configs.ts (4)
src/lib/discord/guards/blacklist.ts (1)
Blacklist(12-36)src/lib/discord/autocomplete/setConfigKeyAutoComplete.ts (1)
SetConfigKeyAutoComplete(4-20)src/lib/hooks/useGuildConfig.ts (1)
useGuildConfig(18-90)src/lib/hooks/useTextContent.ts (1)
useTextContent(16-49)
src/lib/hooks/__tests__/useBlacklist.test.ts (1)
src/lib/hooks/useBlacklist.ts (1)
useBlacklist(18-99)
src/ui/ModmailUi.ts (2)
src/lib/arc3.ts (1)
Arc3(16-192)src/lib/hooks/useTextContent.ts (1)
useTextContent(16-49)
src/lib/logger/DiscordLoggerImpl.ts (1)
src/lib/logger/index.ts (1)
DiscordLoggerImpl(3-3)
src/lib/hooks/__tests__/useTextContent.test.ts (1)
src/lib/hooks/useTextContent.ts (2)
useTextContent(16-49)Translations(51-97)
src/lib/hooks/useActiveModmails.ts (1)
src/lib/arc3.ts (1)
Arc3(16-192)
src/lib/hooks/useBlacklist.ts (2)
src/lib/arc3.ts (1)
Arc3(16-192)src/lib/discord/guards/blacklist.ts (1)
Blacklist(12-36)
src/lib/service/ModmailInteractionService.ts (6)
src/lib/repositories/ModmailRepo.ts (1)
ModmailRepo(14-130)src/lib/arc3.ts (1)
Arc3(16-192)src/lib/hooks/useBlacklist.ts (1)
useBlacklist(18-99)src/lib/hooks/useTextContent.ts (1)
useTextContent(16-49)src/lib/util/ModmailUtils.ts (3)
initModmailAsync(41-89)TryCleanupModmail(242-286)LogModmailTranscript(214-240)src/ui/ModmailUi.ts (2)
ModmailSentEmbed(5-29)ModmailMenuEmbed(31-92)
src/main.ts (1)
src/lib/arc3.ts (1)
Arc3(16-192)
src/lib/discord/events/ModmailEvents.ts (4)
src/lib/service/ModmailMessageService.ts (1)
ModmailMessageService(11-355)src/lib/service/ModmailInteractionService.ts (1)
ModmailInteractionService(12-113)src/lib/repositories/ModmailRepo.ts (1)
ModmailRepo(14-130)src/lib/arc3.ts (1)
Arc3(16-192)
src/lib/hooks/__tests__/useGuildConfig.test.ts (1)
src/lib/hooks/useGuildConfig.ts (1)
useGuildConfig(18-90)
src/lib/util/ModmailUtils.ts (6)
src/lib/arc3.ts (1)
Arc3(16-192)src/lib/repositories/ModmailRepo.ts (1)
ModmailRepo(14-130)src/lib/hooks/useGuildConfig.ts (1)
useGuildConfig(18-90)src/lib/hooks/useTextContent.ts (1)
useTextContent(16-49)src/lib/util/DiscordUtils.ts (2)
CreateTextChannel(4-12)CreateWebhook(14-19)src/ui/ModmailUi.ts (3)
ModmailModeratorMessageEmbed(130-151)ModmailTranscriptEmbed(109-128)ModmailFailedEmbed(94-107)
src/lib/hooks/__tests__/useActiveModmails.test.ts (1)
src/lib/hooks/useActiveModmails.ts (1)
useActiveModmails(12-34)
src/lib/repositories/ModmailRepo.ts (2)
src/lib/hooks/useActiveModmails.ts (1)
useActiveModmails(12-34)src/lib/arc3.ts (1)
Arc3(16-192)
src/lib/service/ModmailMessageService.ts (5)
src/lib/repositories/ModmailRepo.ts (1)
ModmailRepo(14-130)src/lib/arc3.ts (1)
Arc3(16-192)src/lib/hooks/useTextContent.ts (1)
useTextContent(16-49)src/ui/ModmailUi.ts (1)
ModmailModeratorMessageEmbed(130-151)src/lib/util/ModmailUtils.ts (3)
SendAttachmentsAndMessageToWebhook(140-161)SendAttachmentsAndMessageToUser(163-189)SendModmailSelectMenu(196-212)
🔇 Additional comments (21)
src/lib/schema/v1/Application.ts (2)
16-16: Trivial formatting.Semicolon addition is fine. No action needed.
1-1: ESM usage approved; no problematic imports detected.Verification confirms:
- Lines 1, 16, 20 use correct ESM default import/export syntax
- No named imports (
{ Application }) or CJS requires of this module exist in the codebase- All schema/v1 consumers follow the same ESM default import pattern
The suggested model caching pattern is a good practice to prevent test-time OverwriteModelError but remains optional.
tsconfig.json (1)
4-4: Review comment is incorrect—codebase doesn't use ES import syntax for JSON.The review suggests configuring TypeScript for JSON assertions and NodeNext resolution, but the codebase loads JSON via
readFileSync()at runtime (insrc/lib/hooks/useTextContent.ts), not through ES import statements. This meansverbatimModuleSyntax: trueandmoduleResolution: "NodeNext"are unnecessary for the actual use case.The only valid point is the minor casing issue:
"module": "esnext"should be"ESNext"for canonical/schema compliance.Likely an incorrect or invalid review comment.
src/lib/schema/v1/Insight.ts (1)
1-1: LGTM! Clean ESM migration.The migration from CommonJS to ES modules is correct and consistent with the broader refactoring effort across the PR.
Also applies to: 17-17
src/lib/schema/v1/Blacklist.ts (1)
1-17: LGTM! Schema structure is appropriate.The Blacklist model correctly uses
mongoose-longfor Discord snowflake handling. Note that without an explicit collection name in the schema options, Mongoose will auto-pluralize the model name to"blacklists".src/lib/schema/v1/Modmail.ts (1)
1-17: LGTM! Schema structure is well-designed.The Modmail model correctly uses
mongoose-longfor Discord snowflake handling and includes automatic timestamps for tracking creation and updates. Note that Mongoose will auto-pluralize the collection name to"modmails".src/lib/schema/v1/Comment.ts (1)
1-1: LGTM! Clean ESM migration.The migration from CommonJS to ES modules is correct and aligns with the project-wide refactoring.
Also applies to: 12-12
src/lib/schema/v1/Guild.ts (1)
1-4: LGTM! Proper mongoose-long initialization added.The ESM migration correctly adds
mongoose-longinitialization to support the existingLongtype usage in the schema. This ensures Discord snowflakes are properly handled.Also applies to: 19-19
src/lib/schema/v1/Commandstat.ts (1)
1-1: LGTM! Clean ESM migration.The migration from CommonJS to ES modules is correct and follows the established pattern across the PR.
Also applies to: 14-14
src/lib/schema/v1/Approval.ts (1)
1-1: LGTM! Clean ESM migration.The migration from CommonJS to ES modules is correct and completes the consistent refactoring pattern across all schema files.
Also applies to: 12-12
src/locales/zh.json (1)
4-4: The review comment's warning about inconsistencies is not supported by verification.The script output shows that all locale files (zh.json, fr.json, en.json) consistently use the same misspelled keys (
"arc.command.setconfig.sucess"and"arc.modmail.delivery.recieved"), and all application code references match these exact misspellings. There are no cross-file inconsistencies; the code atconfigs.ts:45andModmailUi.ts:19-22correctly references the keys as spelled in the JSON files, and type definitions inuseTextContent.tsmatch as well.While the keys are indeed misspelled (correct spelling would be "success" and "received"), fixing them would require coordinated changes across all locale files, code references, and test files—not because of inconsistency, but because the typos are uniform throughout the codebase.
Likely an incorrect or invalid review comment.
src/lib/util/DiscordUtils.ts (1)
14-19: LGTM!The webhook creation wrapper is clean and correctly implements the async/await pattern.
jest.setup.js (1)
1-2: LGTM!An empty setup file is acceptable for initial Jest configuration. This can be populated with global test utilities or mocks as needed in future iterations.
src/lib/logger/index.ts (1)
1-3: LGTM!The re-export pattern provides a clean public API surface for the logger implementation.
src/lib/schema/v1/Transcript.ts (1)
1-17: LGTM!The migration from CommonJS (
module.exports) to ESM (export default) is correctly implemented and aligns with the broader ESM migration across the codebase.src/lib/schema/v1/GuildConfig.ts (2)
1-4: LGTM!Proper integration of mongoose-long for 64-bit integer support, which is necessary for Discord snowflakes.
8-13: No issues found. The explicit_idfield is correctly handled throughout the codebase.The single creation path in the codebase (in
src/lib/hooks/useGuildConfig.tsat line 54-59) consistently provides an_idvalue when instantiating GuildConfig documents usingObjectId.createFromTime(). No alternative creation patterns (.create(),.insertMany()) exist that could bypass this requirement. The explicit schema definition is intentional and poses no risk.src/lib/schema/v2/Transcript.ts (1)
1-20: Original review comment is incorrect—no model registration conflict exists.The concern assumes both
v1/Transcript.tsandv2/Transcript.tswould be imported simultaneously. However, verification shows:
v1/Transcript.tsis not imported anywhere in the codebasev2/Transcript.tsis imported only inModmailRepo.ts- No file imports both versions
Since v1 is never executed, Mongoose never registers the duplicate model. The v1 schema appears to be legacy code no longer in use; only v2 is actively deployed.
Likely an incorrect or invalid review comment.
src/lib/hooks/__tests__/useBlacklist.test.ts (1)
93-105: Sanity-check: init shape assertions look good.Actions/states coverage is appropriate.
jest.config.js (1)
1-24: ESM + ts-jest setup looks consistent.No blockers spotted.
src/lib/service/ModmailMessageService.ts (1)
2-5: Configuration mismatch: Review incorrectly identifies the module resolution mode.The review claims "In NodeNext ESM" but the project's
tsconfig.jsonuses"module": "esnext"with"moduleResolution": "Node"(the older node10 resolution). NodeNext and Node16 moduleResolution modes enforce.jsextensions; the node10 mode does not. The current mixed import patterns (some with.js, some without) are already valid for this configuration and do not require modification.Likely an incorrect or invalid review comment.
| "@jest/globals": "^30.1.2", | ||
| "@types/jest": "^30.0.0", | ||
| "@types/node": "^22.10.10", | ||
| "jest": "^30.1.3", | ||
| "nodemon": "^3.1.9", | ||
| "prettier": "^3.4.2", | ||
| "ts-jest": "^29.4.2", | ||
| "ts-node": "^10.9.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Align ts-jest with Jest 30 (prevents test runs).
You’re on Jest 30; ts-jest 29 targets Jest 29.
- "@jest/globals": "^30.1.2",
- "@types/jest": "^30.0.0",
+ "@jest/globals": "^30.1.2",
+ "@types/jest": "^30.0.0",
"@types/node": "^22.10.10",
- "jest": "^30.1.3",
+ "jest": "^30.1.3",
"nodemon": "^3.1.9",
"prettier": "^3.4.2",
- "ts-jest": "^29.4.2",
+ "ts-jest": "^30.0.0",
"ts-node": "^10.9.2",
"typescript": "5.7.3"🏁 Script executed:
#!/bin/bash
# Confirm version compatibility and current installed versions
node -v
npm -v
jq -r '.devDependencies["jest"], .devDependencies["ts-jest"]' package.jsonLength of output: 30
🌐 Web query:
ts-jest Jest 30 compatibility version
💡 Result:
Short answer: ts-jest has not released a stable v30 — the maintained ts-jest releases are in the 29.x line and are intended to match Jest 29. So Jest 30 compatibility requires waiting for a ts-jest v30 (or a 29→30 compatibility release) or using a workaround (keep Jest at 29.x, or force-install with risks). [1][2]
Sources:
- ts-jest releases / latest tag (latest: 29.4.x). [1]
- Jest 30 upgrade notes (breaking changes / new requirements for Jest 30). [2]
References:
[1] ts-jest package/releases (latest: 29.x).
[2] Jest — Upgrading from v29 to v30.
🌐 Web query:
ts-jest 30.0.0 npm release
💡 Result:
There is no ts-jest 30.0.0 release. The ts-jest releases are currently in the 29.x series (latest tags show v29.4.5; npm/Snyk metadata also list 29.4.x as the newest). [1][2]
Sources:
- ts-jest GitHub tags (latest tags show v29.4.5). [1]
- ts-jest package metadata / registry listings (latest version 29.4.x). [2]
Jest 30 is incompatible with ts-jest 29 — requires immediate resolution.
The current configuration pairs Jest 30 (^30.1.3) with ts-jest 29 (^29.4.2). ts-jest latest releases are in the 29.x line and are intended to match Jest 29, and Jest 30 compatibility requires waiting for a ts-jest v30. No stable ts-jest 30.0.0 release currently exists; the latest ts-jest release is 29.4.5.
Choose one:
- Downgrade Jest to 29.x to restore compatibility with ts-jest 29.x
- Await ts-jest 30 release before upgrading Jest to 30
🤖 Prompt for AI Agents
package.json lines 28-35: the project currently uses Jest ^30.1.3 with ts-jest
^29.4.2 which are incompatible; change the Jest dependency to a 29.x release to
match ts-jest (e.g., set "jest" to a latest 29.x like "^29.5.0") and ensure
@jest/globals remains on a 29.x-compatible version (adjust if necessary), then
run npm install and the test suite to verify everything passes; alternatively,
if you prefer keeping Jest 30, hold off until ts-jest publishes a stable v30 and
revert this change then.
| { | ||
| "arc.bot.name": "ARC", | ||
| "arc.blacklist": "Vous êtes sur la liste noire et ne pouvez pas utiliser cette commande !", | ||
| "arc.command.setconfig.sucess": "La configuration a été définie avec succès", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo in locale key.
The key contains a typo: "sucess" should be "success". This typo appears to be consistent across all locale files (en.json, fr.json), suggesting it needs correction in the entire localization system.
- "arc.command.setconfig.sucess": "La configuration a été définie avec succès",
+ "arc.command.setconfig.success": "La configuration a été définie avec succès",📝 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.
| "arc.command.setconfig.sucess": "La configuration a été définie avec succès", | |
| "arc.command.setconfig.success": "La configuration a été définie avec succès", |
🤖 Prompt for AI Agents
In src/locales/fr.json around line 4 the locale key
"arc.command.setconfig.sucess" contains a typo; rename the key to
"arc.command.setconfig.success" in this file and the equivalent keys in all
locale files (e.g., src/locales/en.json and any other language files), then run
a repository-wide search to update every code reference to the corrected key
(including tests and i18n lookup calls) so nothing breaks at runtime; ensure
translations remain the same and run the i18n tests/linter to verify no missing
keys.
| "arc.modmail.delivery.recieved.description": "Votre demande de modmail a bien été reçue ! Veuillez patienter, un membre du personnel vous aidera sous peu.", | ||
| "arc.modmail.delivery.recieved.footer": "v0.1 Merci d'utiliser ARC", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo in locale keys.
Both keys contain a typo: "recieved" should be "received". This typo appears across all locale files and should be corrected throughout the localization system.
- "arc.modmail.delivery.recieved.description": "Votre demande de modmail a bien été reçue ! Veuillez patienter, un membre du personnel vous aidera sous peu.",
- "arc.modmail.delivery.recieved.footer": "v0.1 Merci d'utiliser ARC",
+ "arc.modmail.delivery.received.description": "Votre demande de modmail a bien été reçue ! Veuillez patienter, un membre du personnel vous aidera sous peu.",
+ "arc.modmail.delivery.received.footer": "v0.1 Merci d'utiliser ARC",📝 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.
| "arc.modmail.delivery.recieved.description": "Votre demande de modmail a bien été reçue ! Veuillez patienter, un membre du personnel vous aidera sous peu.", | |
| "arc.modmail.delivery.recieved.footer": "v0.1 Merci d'utiliser ARC", | |
| "arc.modmail.delivery.received.description": "Votre demande de modmail a bien été reçue ! Veuillez patienter, un membre du personnel vous aidera sous peu.", | |
| "arc.modmail.delivery.received.footer": "v0.1 Merci d'utiliser ARC", |
🤖 Prompt for AI Agents
In src/locales/fr.json around lines 9-10 the locale keys use the misspelling
"recieved" instead of "received"; rename the keys to
"arc.modmail.delivery.received.description" and
"arc.modmail.delivery.received.footer" in this file and in every other locale
file, then update all code references (i18n lookups, tests, and any static
strings) to use the corrected keys, and run the localization/test suite to
ensure no missing-key regressions.
Summary by CodeRabbit
New Features
Tests
Chores