-
Notifications
You must be signed in to change notification settings - Fork 0
16 implement workflow activities #18
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
Conversation
|
Warning Rate limit exceeded@Behzad-rabiei has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 31 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughThis update introduces a comprehensive Discord Gateway event ingestion and handling system, integrating Discord.js and Discord API v10 with Temporal workflows. It adds new modules for event mapping, persistence, and handler logic for channels, guild members, roles, and messages, including reactions. Supporting types, constants, and helper utilities are also provided. Example and index files are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant DiscordGateway as Discord Gateway
participant ExampleClient as Example Client
participant Temporal as Temporal Workflow
participant Handler as Event Handler
DiscordGateway->>ExampleClient: Dispatch Event (payload)
ExampleClient->>Temporal: Start Workflow (payload)
Temporal->>Handler: Route to event handler (by event type)
Handler->>Handler: Map and process event (channel/member/role/message)
Handler->>Handler: Persist/update/delete in DB as needed
Handler-->>Temporal: Complete event handling
Temporal-->>ExampleClient: Workflow complete
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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.
Actionable comments posted: 13
🧹 Nitpick comments (12)
src/activities/discord/index.ts (1)
1-15: Consider removing commented code or adding TODO comments.The extensive commented-out exports (lines 1-13) may clutter the codebase. Consider:
- Removing them if they're not planned for immediate implementation
- Adding TODO comments with timeframes if they represent planned features
- Moving them to a separate tracking document/issue
The active export on line 15 is clean and serves its purpose well.
If these exports represent planned features, consider adding context:
+// TODO: Implement direct persistence activities (planned for future release) // export { // createChannel, // updateChannel, // softDeleteChannel, // } from './persistence/channel'; +// ... (rest of commented exports) -// export { -// createRawInfo, -// updateRawInfo, -// deleteRawInfo, -// deleteRawInfos, -// } from './message'; export * from './gateway';src/activities/discord/gateway/ignoreUser.activity.ts (1)
14-21: Consider removing async/await since no async operations are performed.The function is marked as
asyncbut doesn't perform any asynchronous operations. This can be simplified for better performance.-export async function isUserIgnored( +export function isUserIgnored( guildId: string, userId: Snowflake, -): Promise<boolean> { +): boolean { const ignored = getIgnoredUsersForGuild(guildId).includes(userId); if (ignored) logger.debug({ guildId, userId }, 'user ignored'); return ignored; }src/workflows/discord/gateway/handlers/guildMember.handler.ts (1)
15-41: Consider using a namespace or plain functions instead of a static-only class.The implementation logic is correct and follows good patterns for mapping and persistence. However, since this class only contains static methods, consider using a namespace or plain exported functions for better TypeScript practices.
-export class GuildMemberHandler { - static async add(data: GatewayGuildMemberAddDispatchData): Promise<void> { +export async function addGuildMember(data: GatewayGuildMemberAddDispatchData): Promise<void> { const mappedData = await activitiesProxy.mapGuildMemberCreate(data); await activitiesProxy.createMember(data.guild_id, mappedData); - } +} - static async update( +export async function updateGuildMember( data: GatewayGuildMemberUpdateDispatchData, ): Promise<void> { const mappedData = await activitiesProxy.mapGuildMemberUpdate(data); await activitiesProxy.updateMember( data.guild_id, { discordId: data.user.id }, mappedData, ); - } +} - static async remove( +export async function removeGuildMember( data: GatewayGuildMemberRemoveDispatchData, ): Promise<void> { await activitiesProxy.updateMember( data.guild_id, { discordId: data.user.id }, { deletedAt: new Date() }, ); - } -} +}🧰 Tools
🪛 Biome (1.9.4)
[error] 15-41: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
src/workflows/discord/gateway/handlers/role.handler.ts (1)
15-37: Consistent implementation with good separation of concerns.The role handler follows the same well-structured pattern as other handlers with proper mapping and persistence. Like the guild member handler, consider refactoring to use plain functions instead of a static-only class.
-export class RoleHandler { - static async create(data: GatewayGuildRoleCreateDispatchData): Promise<void> { +export async function createRole(data: GatewayGuildRoleCreateDispatchData): Promise<void> { const mappedData = await activitiesProxy.mapRoleCreate(data); await activitiesProxy.createRole(data.guild_id, mappedData); - } +} - static async update(data: GatewayGuildRoleUpdateDispatchData): Promise<void> { +export async function updateRole(data: GatewayGuildRoleUpdateDispatchData): Promise<void> { const mappedData = await activitiesProxy.mapRoleUpdate(data); await activitiesProxy.updateRole( data.guild_id, { roleId: data.role.id }, mappedData, ); - } +} - static async delete(data: GatewayGuildRoleDeleteDispatchData): Promise<void> { +export async function deleteRole(data: GatewayGuildRoleDeleteDispatchData): Promise<void> { await activitiesProxy.updateRole( data.guild_id, { roleId: data.role_id }, { deletedAt: new Date() }, ); - } -} +}🧰 Tools
🪛 Biome (1.9.4)
[error] 15-37: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
src/workflows/discord/gateway/handlers/channel.handler.ts (1)
16-38: Consider using namespace or module pattern instead of static-only class.The static analysis tool correctly identifies that this class contains only static members. Consider refactoring to use a namespace or module pattern for better code organization and to avoid the overhead of class instantiation.
-export class ChannelHandler { - static async create(data: GatewayChannelCreateDispatchData): Promise<void> { +export namespace ChannelHandler { + export async function create(data: GatewayChannelCreateDispatchData): Promise<void> { const mappedData = await activitiesProxy.mapChannelCreate(data); await activitiesProxy.createChannel(data.guild_id, mappedData); } - static async update(data: GatewayChannelUpdateDispatchData): Promise<void> { + export async function update(data: GatewayChannelUpdateDispatchData): Promise<void> { const mappedData = await activitiesProxy.mapChannelUpdate(data); await activitiesProxy.updateChannel( data.guild_id, { channelId: data.id }, mappedData, ); } - static async delete(data: GatewayChannelDeleteDispatchData): Promise<void> { + export async function delete(data: GatewayChannelDeleteDispatchData): Promise<void> { await activitiesProxy.updateChannel( data.guild_id, { channelId: data.id }, { deletedAt: new Date() }, ); } -} +}🧰 Tools
🪛 Biome (1.9.4)
[error] 16-38: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
src/workflows/discord/gateway/mappers/guildMember.mapper.ts (1)
31-31: Consider type consistency for joined_at field.The
joinedAtfield handling is inconsistent between add and update mappers. In the add mapper, it's always converted to a Date, but in the update mapper, it can be undefined.Consider making the type handling more explicit:
- joinedAt: payload.joined_at ? new Date(payload.joined_at) : undefined, + joinedAt: payload.joined_at ? new Date(payload.joined_at) : null,This ensures consistency with the null pattern used elsewhere in the mappers.
src/examples/client/discordGateway.example.ts (2)
70-72: Fix unsafe property access with optional chaining.The static analysis tool correctly identifies that this can be simplified and made safer with optional chaining.
- if (data.guild && data.guild.id) { - return data.guild.id; - } + if (data.guild?.id) { + return data.guild.id; + }🧰 Tools
🪛 Biome (1.9.4)
[error] 70-70: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
112-116: Consider uncommenting event filtering for production use.The event filtering logic is commented out, which means all Discord events will trigger workflows. This could be resource-intensive in production.
Consider adding a configuration option to enable/disable filtering:
const FILTER_EVENTS = process.env.FILTER_DISCORD_EVENTS === 'true'; // Then in the dispatch handler: if (!FILTER_EVENTS || isAllowedEvent(payload.t)) { console.log(`Received event ${payload.t} from shard ${shardId}`); ingestEvent(payload); }src/activities/discord/gateway/persistence.activity.ts (1)
167-171: Consider fallback creation on update miss for consistency.The
updateRawInfofunction doesn't implement fallback creation when no document is found to update, unlike other update functions. This could lead to missing data if the document doesn't exist.Consider adding fallback creation for consistency:
if (result.modifiedCount === 0) { - logger.warn({ guildId, filter }, 'No rawinfo document found to update'); + logger.warn({ guildId, filter }, 'No rawinfo document found to update, attempting creation'); + await repo.create(updateData); } else { logger.debug({ guildId, filter }, 'Updated rawinfo document'); }Note: This requires ensuring
updateDatacontains all required fields for creation.src/workflows/discord/gateway/mappers/message.mapper.ts (2)
1-33: Remove commented-out code.This large block of commented code should be removed. If this code is needed for reference, it should be in version control history or documentation, not left as comments in the production code.
-// import { Message, Role, TextChannel, User } from 'discord.js'; -// import { GatewayMessageCreateDispatchData } from 'discord-api-types/v10'; - -// import { IRawInfo } from '@togethercrew.dev/db'; - -// export interface ThreadInfo { -// threadId: string | null; -// threadName: string | null; -// channelId: string; -// channelName: string | null; -// } - -// export function toIRawInfo(message: GatewayMessageCreateDispatchData): IRawInfo { -// return { -// type: message.type, -// author: message.author.id, -// content: message.content, -// createdDate: new Date(message.timestamp), -// role_mentions: message.mention_roles, -// user_mentions: message.mentions, -// replied_user: message.type === 19 ? message.referenced_message?.id : null, -// reactions: message.reactions, -// messageId: message.id, -// channelId: thread?.channelId ?? message.channelId, -// channelName: -// thread?.channelName ?? -// (message.channel instanceof TextChannel ? message.channel.name : null), -// threadId: thread?.threadId ?? null, -// threadName: thread?.threadName ?? null, -// isGeneratedByWebhook: Boolean(message.webhookId), -// }; -// } - import {
99-101: Address TODO: Populate channel and thread information.The
channelName,threadId, andthreadNamefields are hardcoded tonull. These fields should be populated from the appropriate data source.Would you like me to help implement the logic to fetch and populate these fields, or create an issue to track this TODO?
src/activities/discord/gateway/mapping.activity.ts (1)
33-85: Consider adding debug logging to all mapping activities for consistency.Currently, only message-related activities have debug logging. For better observability and debugging, consider adding similar logging to channel, guild member, and role mapping activities.
Example for channel operations:
export async function mapChannelCreate( payload: GatewayChannelCreateDispatchData, ) { + logger.debug({ channelId: payload.id }, 'Mapping channel create event'); return ChannelMappers.add(payload); }
📜 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 (28)
package.json(1 hunks)src/activities/discord/gateway/channelSelection.activity.ts(1 hunks)src/activities/discord/gateway/ignoreUser.activity.ts(1 hunks)src/activities/discord/gateway/index.ts(1 hunks)src/activities/discord/gateway/mapping.activity.ts(1 hunks)src/activities/discord/gateway/persistence.activity.ts(1 hunks)src/activities/discord/index.ts(1 hunks)src/activities/index.ts(1 hunks)src/config/config.service.ts(1 hunks)src/examples/client/discordGateway.example.ts(1 hunks)src/libs/helpers/AssertNever.ts(1 hunks)src/libs/telegram/services/chat_member.service.test.ts(0 hunks)src/shared/constants/discordGateway.constant.ts(1 hunks)src/shared/types/discord/EventIngestion.discord.ts(1 hunks)src/shared/types/discord/discordEvents.ts(1 hunks)src/worker.ts(1 hunks)src/workflows/discord/gateway/handlers/channel.handler.ts(1 hunks)src/workflows/discord/gateway/handlers/guildMember.handler.ts(1 hunks)src/workflows/discord/gateway/handlers/index.ts(1 hunks)src/workflows/discord/gateway/handlers/message.handler.ts(1 hunks)src/workflows/discord/gateway/handlers/role.handler.ts(1 hunks)src/workflows/discord/gateway/index.ts(1 hunks)src/workflows/discord/gateway/mappers/channel.mapper.ts(1 hunks)src/workflows/discord/gateway/mappers/guildMember.mapper.ts(1 hunks)src/workflows/discord/gateway/mappers/index.ts(1 hunks)src/workflows/discord/gateway/mappers/message.mapper.ts(1 hunks)src/workflows/discord/gateway/mappers/role.mapper.ts(1 hunks)src/workflows/discord/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/libs/telegram/services/chat_member.service.test.ts
🧰 Additional context used
🧬 Code Graph Analysis (8)
src/worker.ts (1)
src/config/logger.config.ts (1)
logger(4-10)
src/workflows/discord/gateway/index.ts (1)
src/workflows/discord/gateway/handlers/index.ts (1)
eventHandlers(8-31)
src/activities/discord/gateway/channelSelection.activity.ts (1)
src/config/logger.config.ts (1)
logger(4-10)
src/workflows/discord/gateway/handlers/index.ts (4)
src/workflows/discord/gateway/handlers/channel.handler.ts (1)
ChannelHandler(16-38)src/workflows/discord/gateway/handlers/guildMember.handler.ts (1)
GuildMemberHandler(15-41)src/workflows/discord/gateway/handlers/role.handler.ts (1)
RoleHandler(15-37)src/workflows/discord/gateway/handlers/message.handler.ts (1)
MessageHandler(31-120)
src/activities/discord/gateway/ignoreUser.activity.ts (2)
src/config/logger.config.ts (1)
logger(4-10)src/shared/constants/discordGateway.constant.ts (1)
GUILD_IGNORED_USERS(6-8)
src/shared/types/discord/EventIngestion.discord.ts (1)
src/shared/types/discord/discordEvents.ts (2)
DiscordEventType(5-23)EventPayloadMap(25-44)
src/examples/client/discordGateway.example.ts (1)
src/workflows/discord/gateway/index.ts (1)
DiscordGatewayEventWorkflow(5-13)
src/activities/discord/gateway/persistence.activity.ts (1)
src/config/logger.config.ts (1)
logger(4-10)
🪛 Biome (1.9.4)
src/workflows/discord/gateway/handlers/role.handler.ts
[error] 15-37: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
src/workflows/discord/gateway/handlers/channel.handler.ts
[error] 16-38: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
src/workflows/discord/gateway/handlers/guildMember.handler.ts
[error] 15-41: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
src/workflows/discord/gateway/handlers/message.handler.ts
[error] 31-120: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
src/examples/client/discordGateway.example.ts
[error] 70-70: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (35)
src/config/config.service.ts (1)
1-5: No functional impact from import reordering
The imports have been reordered and inline comments removed without altering behavior. The externaldotenvimport is correctly placed before the relative imports (./schemaand./transform), and the singleton pattern and environment validation logic remain unchanged.package.json (2)
40-40:Details
❓ Verification inconclusive
Verify breaking changes in @togethercrew.dev/db upgrade.
The significant version jump from 3.3.0 to 3.10.0 for
@togethercrew.dev/dbmay introduce breaking changes that could affect existing database operations.
🏁 Script executed:
#!/bin/bash # Description: Check for breaking changes in database package usage echo "Searching for database connection and model usage patterns..." rg -A 3 -B 1 "@togethercrew.dev/db" --type ts rg -A 5 "Connection\.|connect\(" src/ --type tsLength of output: 11807
Verify Breaking Changes in @togethercrew.dev/db Upgrade
I found widespread usage of
@togethercrew.dev/dbacross the codebase – please review the changelog between v3.3.0 and v3.10.0 to ensure none of these are affected:• src/worker.ts
–import { Connection as MongoConnection }andNativeConnection.connect()
• src/workflows/**
– Interfaces:IPlatform,IRawInfo,IGuildMember,IRole,IChannel
• src/shared/types/**
– Types:PlatformNames,Queue
• src/libs/mongo/PlatformService.ts
–Connection,IPlatform,Platform
• src/activities/**
– Models and enums:Platform,TokenTypeNames, plus various CRUD callsPlease check for breaking changes to the Connection API, model interfaces, and enum definitions in the package’s changelog, then run or add integration tests around database operations to confirm nothing has broken.
33-34:Details
✅ Verification successful
Verify Discord.js package compatibility and versions.
The Discord.js v14 ecosystem packages are being added with specific versions. Please ensure these versions are compatible with each other and represent the latest stable releases.
Also applies to: 44-45
🏁 Script executed:
#!/bin/bash # Description: Verify Discord.js package versions and compatibility echo "Checking latest Discord.js versions..." npm view discord.js version npm view @discordjs/rest version npm view @discordjs/ws version npm view discord-api-types version echo -e "\nChecking for known issues between these packages..." npm view discord.js peerDependencies npm view @discordjs/rest peerDependencies npm view @discordjs/ws peerDependenciesLength of output: 493
Discord.js versions are up to date and compatible.
- discord.js @14.19.3 (latest stable)
- @discordjs/rest @2.5.0 (latest stable)
- @discordjs/ws @2.0.2 (latest stable)
- discord-api-types @0.38.10 (latest stable)
- No peerDependencies reported for these packages, indicating no compatibility issues.
src/activities/index.ts (1)
6-6:Details
✅ Verification successful
LGTM! Consistent export pattern.
The export follows the same pattern as other activity modules. Ensure the
./discordmodule exists and properly exports its activities.
🏁 Script executed:
#!/bin/bash # Description: Verify discord activities module exists and has proper structure echo "Checking if discord activities module exists..." fd "index.ts" src/activities/discord/ echo -e "\nChecking discord activities exports..." if [ -f "src/activities/discord/index.ts" ]; then cat src/activities/discord/index.ts fi echo -e "\nListing discord activity files..." fd . src/activities/discord/ --type f --extension tsLength of output: 1219
All set:
discordactivities module verified
- Confirmed
src/activities/discord/index.tsexists and exports./gateway- Listed gateway activity files under
src/activities/discord/gateway/No further changes needed—ready to merge.
src/workflows/discord/index.ts (1)
2-2:Details
✅ Verification successful
LGTM! Consistent export pattern.
The export follows the same pattern as the existing workflow export. Ensure the
./gatewaymodule exists and properly exports gateway workflows.
🏁 Script executed:
#!/bin/bash # Description: Verify gateway workflows module exists and has proper structure echo "Checking if gateway workflows module exists..." fd "index.ts" src/workflows/discord/gateway/ echo -e "\nChecking gateway workflows exports..." if [ -f "src/workflows/discord/gateway/index.ts" ]; then cat src/workflows/discord/gateway/index.ts fi echo -e "\nListing gateway workflow files..." fd . src/workflows/discord/gateway/ --type f --extension tsLength of output: 1609
Gateway export verified and approved
The
src/workflows/discord/gatewaymodule exists and itsindex.tsexports theDiscordGatewayEventWorkflow. The lineexport * from './gateway';in
src/workflows/discord/index.tswill correctly re-export all named exports from that module. No further changes required.src/worker.ts (2)
1-1: LGTM! Import formatting improvements.The import order change and activities import formatting are cosmetic improvements that enhance readability.
Also applies to: 4-4
6-6:Details
✅ Verification successful
Verify logger import change consistency.
The logger import was changed from a named import to a default import. Ensure this is consistent with the logger configuration export.
🏁 Script executed:
#!/bin/bash # Description: Verify logger export type in config echo "Checking logger config exports..." cat src/config/logger.config.ts echo -e "\nSearching for other logger imports in the codebase..." rg "from.*logger\.config" --type tsLength of output: 3776
Logger import consistency verified. The
src/config/logger.config.tsfile provides a default export (export default logger;), and all logger imports across the codebase use default imports. No changes are needed.src/activities/discord/gateway/index.ts (1)
1-4: Clean module organization with barrel exports.The barrel export pattern provides a clean and organized way to consolidate Discord gateway activities. This makes imports cleaner elsewhere in the codebase.
src/workflows/discord/gateway/mappers/index.ts (1)
1-4: LGTM! Clean and consistent export structure.The index file properly consolidates all mapper exports with a consistent naming pattern and clear organization.
src/shared/types/discord/EventIngestion.discord.ts (1)
2-8: Well-designed generic interface for type-safe event handling.The interface properly uses TypeScript generics with constraints and provides a clean structure for Discord gateway event inputs. The type system ensures payload types are correctly mapped to their corresponding event types.
src/activities/discord/gateway/channelSelection.activity.ts (1)
11-37: Excellent implementation with proper error handling and logging.The function demonstrates several best practices:
- Proper async/await usage with error handling
- Sensible default behavior (returns
truewhen no channels are explicitly selected)- Comprehensive logging for debugging and error tracking
- Fallback to
truein error cases ensures system robustness- Clean logic using optional chaining and array methods
The channel selection logic correctly handles both scenarios: when no channels are configured (process all) and when specific channels are selected.
src/workflows/discord/gateway/handlers/index.ts (1)
8-31: Comprehensive and well-structured event-to-handler mapping.The implementation provides several strengths:
- Complete coverage of Discord gateway events (channels, guild members, roles, messages, reactions)
- Clean organization grouping related events together
- Proper use of
as constfor type safety and immutability- Consistent naming convention matching the handler implementations
- Clear separation of concerns with dedicated handler classes
This centralized mapping enables clean event dispatching in the workflow system.
src/activities/discord/gateway/ignoreUser.activity.ts (1)
1-21: Well-structured activity module with good separation of concerns.The implementation correctly handles ignored user checking with proper type safety and logging. The helper function provides a safe fallback for missing guild configurations.
src/workflows/discord/gateway/handlers/guildMember.handler.ts (1)
10-13: Good configuration for Temporal activities.The 1-minute timeout and 5 retry attempts provide a reasonable balance between reliability and performance for Discord API operations.
src/workflows/discord/gateway/handlers/role.handler.ts (1)
24-27:Details
✅ Verification successful
Verify consistency in role identification between update and delete.
In the
updatemethod, you access the role ID viadata.role.id, but in thedeletemethod, you usedata.role_id. This suggests different payload structures between update and delete events.
🏁 Script executed:
#!/bin/bash # Description: Verify the structure of Discord role event payloads to ensure correct property access # Expected: Confirm that update events have data.role.id and delete events have data.role_id ast-grep --pattern $'GatewayGuildRoleUpdateDispatchData' ast-grep --pattern $'GatewayGuildRoleDeleteDispatchData'Length of output: 716
🏁 Script executed:
#!/bin/bash # Extract the relevant methods in role.handler.ts to inspect update & delete handlers sed -n '1,200p' src/workflows/discord/gateway/handlers/role.handler.tsLength of output: 1206
No changes needed for role ID access
The discrepancy (
data.role.idvs.data.role_id) reflects Discord’s event payload shapes:
- Role Update events include the full role object (
data.role.id).- Role Delete events only provide
data.role_id.The current code correctly handles each case.
src/shared/types/discord/discordEvents.ts (3)
5-23: Comprehensive Discord event type coverage.The union type properly covers all the major Discord gateway events needed for the system, with good use of the Discord.js Events enum for consistency.
36-43: Consistent mapping of message events to IRawInfo.All message-related events appropriately map to
IRawInfo, providing a unified interface for message data across different event types.
25-44:Details
❓ Verification inconclusive
Verify the mapping of UserUpdate to IGuildMember.
Most of the event-to-payload mappings look correct, but
Events.UserUpdatemapping toIGuildMemberseems semantically questionable. User updates are global Discord user changes, not guild-specific member changes.
🏁 Script executed:
#!/bin/bash # Description: Check how UserUpdate events are actually used in the codebase # Expected: Verify if UserUpdate should map to a different interface or if this is intentional rg -A 5 -B 5 "Events\.UserUpdate|UserUpdate" --type tsLength of output: 1567
🏁 Script executed:
#!/bin/bash # Display the top of the file to inspect imports head -n20 src/shared/types/discord/discordEvents.ts # Find the IUser interface definition rg -A3 -B3 "export interface IUser" --type ts # Find the IGuildMember interface definition rg -A3 -B3 "export interface IGuildMember" --type tsLength of output: 733
Verify payload mapping for Events.UserUpdate
The
Events.UserUpdateevent delivers a global Discord User, not a guild-specific member. Currently it’s mapped toIGuildMember, which may be misleading. Please:
- Confirm whether
@togethercrew.dev/dbexports aIUser(or similar) type for Discord users.- If it does, change the mapping to:
[Events.UserUpdate]: IUser;- If no user-specific interface exists, consider:
- Using
IRawInfofor the raw payload, or- Defining a new
IUserinterface to model global user updates.Location:
• src/shared/types/discord/discordEvents.ts – line where[Events.UserUpdate]: IGuildMember;is declared.src/workflows/discord/gateway/handlers/channel.handler.ts (1)
11-14: Good configuration for activity proxy settings.The timeout and retry configuration is well-configured for Discord gateway operations, providing sufficient time and attempts for network operations.
src/workflows/discord/gateway/mappers/role.mapper.ts (3)
10-16: Good use of helper function to avoid code duplication.The
mapRolehelper function effectively reduces code duplication between create and update operations while maintaining type safety.
33-37: Consistent naming pattern for mapper object.The RoleMappers object provides a clean interface with intuitive method names (
add,update,remove) that align well with CRUD operations.
18-24:Details
❓ Verification inconclusive
Verify if all necessary role properties are mapped.
The current mapping only includes
roleId,name, andcolor. Discord roles have additional properties likepermissions,position,hoist,mentionable, etc. that might be important for your application.
🏁 Script executed:
#!/bin/bash # Description: Check if other role properties are used elsewhere in the codebase # Expected: Find usage of additional role properties to determine if mapping is complete rg -A 5 -B 5 "permissions|position|hoist|mentionable" --type ts | head -20Length of output: 885
🏁 Script executed:
#!/bin/bash # Locate the mapRole implementation to see which properties are mapped rg -C5 "function mapRole" --type ts # Locate the IRole interface/type definition to compare expected properties rg -C5 "interface IRole" --type tsLength of output: 1732
🏁 Script executed:
#!/bin/bash # Inspect the mapRole implementation to see which properties are mapped sed -n '1,200p' src/workflows/discord/gateway/mappers/role.mapper.ts # Locate the IRole interface/type definition to compare expected properties rg -n 'interface IRole' -C5 --type tsLength of output: 1082
Ensure all Discord role fields are mapped in
mapRoleThe
mapRolehelper (insrc/workflows/discord/gateway/mappers/role.mapper.ts) currently only maps:{ roleId: role.id, name: role.name, color: role.color, }Discord’s
APIRoleincludes additional properties (e.g.permissions,position,hoist,mentionable, etc.). Please:
- Verify the
IRoledefinition in@togethercrew.dev/dbto see which fields it expects.- Extend
mapRoleto include any missing properties, for example:const mapRole = (role: …) => ({ roleId: role.id, name: role.name, color: role.color, permissions: role.permissions, position: role.position, hoist: role.hoist, mentionable: role.mentionable, // …other required fields });- If
IRoleis intentionally limited, confirm that omitting these fields is acceptable.src/shared/constants/discordGateway.constant.ts (2)
10-28: Comprehensive coverage of Discord gateway events.The supported events list covers all major Discord gateway events for channels, guild members, roles, messages, and reactions, providing good coverage for Discord bot functionality.
30-30: Good use of type inference for union type.The
SupportedGatewayEventtype alias correctly creates a union type from the const assertion, ensuring type safety for event handling.src/workflows/discord/gateway/mappers/channel.mapper.ts (3)
8-16: Good handling of nullable fields and consistent mapping pattern.The use of nullish coalescing (
??) for optional fields likenameandparent_idis appropriate, and the helper function follows the same good pattern as the role mapper.
37-41: Consistent mapper object pattern maintained.The ChannelMappers object follows the same clean pattern as RoleMappers, providing consistency across the codebase.
14-14:Details
❓ Verification inconclusive
Verify permission overwrites structure compatibility.
The
permission_overwritesfield is mapped directly without transformation. Ensure that the Discord API structure matches your internalIChannelinterface expectations.
🏁 Script executed:
#!/bin/bash # Description: Check the IChannel interface definition for permission overwrites # Expected: Find the IChannel interface to verify compatibility rg -A 10 "interface IChannel" --type tsLength of output: 41
To locate the
IChanneldefinition (interface or type) and inspect itspermission_overwritesshape, let’s broaden the grep search:
🏁 Script executed:
#!/bin/bash # Search for IChannel interface or type alias definitions rg -n "interface IChannel" -A 5 rg -n "export interface IChannel" -A 5 rg -n "type IChannel" -A 5 # Also find any occurrence of permission_overwrites in type/interface rg -n "permission_overwrites" -A 3Length of output: 456
Let’s inspect the mapper and locate where
IChannelis defined or imported:
🏁 Script executed:
#!/bin/bash # Show the channel mapper file to see imports, mapper function and return type sed -n '1,200p' src/workflows/discord/gateway/mappers/channel.mapper.ts # Search the entire codebase for IChannel references rg -n "IChannel" --type ts rg -n "import .*IChannel" --type ts -C 2Length of output: 3027
Confirm permissionOverwrites structure matches IChannel
You’re directly assigningchannel.permission_overwrites(Discord’sAPIOverwrite[]) to your internalIChannel.permissionOverwrites. Since theIChanneltype lives in@togethercrew.dev/db, please verify that itspermissionOverwritesproperty accepts the same shape as Discord’spermission_overwrites.• File: src/workflows/discord/gateway/mappers/channel.mapper.ts
• Line:permissionOverwrites: channel.permission_overwrites,src/workflows/discord/gateway/mappers/guildMember.mapper.ts (1)
44-48: Good mapper organization.The export of
GuildMemberMappersobject provides a clean interface for accessing all mapper functions in a structured way.src/workflows/discord/gateway/handlers/message.handler.ts (2)
20-29: Good guard function implementation.The
guardMessagefunction provides excellent separation of concerns by centralizing channel selection and user ignore checks. The logic is clear and the optional author ID parameter handles different event types appropriately.
16-19: Appropriate activity configuration.The timeout and retry configuration (1 minute timeout, 5 max attempts) is well-suited for Discord API operations which can experience temporary failures.
src/examples/client/discordGateway.example.ts (1)
89-103: Good workflow ID generation and error handling.The workflow ID generation includes timestamp for uniqueness, and the promise handling with proper logging for both success and failure cases is well implemented.
src/activities/discord/gateway/persistence.activity.ts (1)
22-22: Excellent structured logging implementation.The logging throughout the file is well-structured with appropriate log levels (debug, warn, error) and includes relevant context data like
guildId,messageId, etc. The child logger creation with activity context is a good practice.Also applies to: 137-140, 170-170
src/workflows/discord/gateway/mappers/message.mapper.ts (1)
254-261: Well-structured mapper exports!The MessageMappers object provides a clean, organized interface for all message-related mapping functions.
src/activities/discord/gateway/mapping.activity.ts (2)
1-31: Clean imports and proper logger setup!Good practice using a child logger with appropriate context for tracing Discord event mapping activities.
87-174: Well-implemented message mapping activities!Good implementation with:
- Comprehensive debug logging with relevant context (messageId, emoji, userId)
- Proper handling of delete operations returning IDs directly
- Correct parameter passing for reaction operations that need existing data
| import { logger } from './config/logger.config'; | ||
| import parentLogger from './config/logger.config'; | ||
|
|
||
| const logger = parentLogger.child({ mdoule: 'worker' }); |
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 logger property name.
There's a typo in the logger property: mdoule should be module.
-const logger = parentLogger.child({ mdoule: 'worker' });
+const logger = parentLogger.child({ module: 'worker' });📝 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.
| const logger = parentLogger.child({ mdoule: 'worker' }); | |
| - const logger = parentLogger.child({ mdoule: 'worker' }); | |
| + const logger = parentLogger.child({ module: 'worker' }); |
🤖 Prompt for AI Agents
In src/worker.ts at line 8, fix the typo in the logger property name by changing
'mdoule' to 'module' in the child logger creation to ensure correct property
naming.
| export function assertNever(x: any): never { | ||
| throw new Error(`Unhandled value: ${x}`); | ||
| } |
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.
🛠️ Refactor suggestion
Improve type safety and error handling for exhaustive checking.
The current implementation has several issues that reduce its effectiveness for exhaustive type checking:
- The parameter type
anydefeats the purpose - it should beneverto ensure this function is only called when all cases are exhausted - Interpolating complex objects in the error message could result in
[object Object] - Missing documentation explaining its intended usage
Apply this diff to improve type safety and error handling:
+/**
+ * Utility function for exhaustive type checking.
+ * Should only be called in unreachable code paths when all cases are handled.
+ * @param x - The value that should never be reached
+ * @throws Error indicating an unhandled case
+ */
-export function assertNever(x: any): never {
- throw new Error(`Unhandled value: ${x}`);
+export function assertNever(x: never): never {
+ throw new Error(`Unhandled value: ${JSON.stringify(x)}`);
}📝 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.
| export function assertNever(x: any): never { | |
| throw new Error(`Unhandled value: ${x}`); | |
| } | |
| /** | |
| * Utility function for exhaustive type checking. | |
| * Should only be called in unreachable code paths when all cases are handled. | |
| * @param x - The value that should never be reached | |
| * @throws Error indicating an unhandled case | |
| */ | |
| export function assertNever(x: never): never { | |
| throw new Error(`Unhandled value: ${JSON.stringify(x)}`); | |
| } |
🤖 Prompt for AI Agents
In src/libs/helpers/AssertNever.ts lines 1 to 3, change the parameter type from
any to never to enforce that this function is only called when all cases are
exhausted. Update the error message to safely serialize the input value, for
example by using JSON.stringify or a similar method, to avoid displaying [object
Object]. Add a comment above the function explaining that it is used for
exhaustive type checking to ensure all possible cases are handled.
| const { t: event, d } = payload; | ||
| if (!(event in eventHandlers)) { | ||
| throw new Error(`Unsupported gateway event: ${event as string}`); | ||
| } | ||
| await (eventHandlers as any)[event](d); | ||
| } |
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.
🛠️ Refactor suggestion
Improve type safety and error handling.
The current implementation has several type safety and robustness issues:
- Line 12 uses
(eventHandlers as any)which defeats TypeScript's type checking - No validation that
payloadhas the expected structure - Error message could be more descriptive
Apply this diff to improve type safety:
export async function DiscordGatewayEventWorkflow(
payload: GatewayDispatchPayload,
): Promise<void> {
- const { t: event, d } = payload;
+ const { t: event, d } = payload;
+
+ if (!event || !d) {
+ throw new Error('Invalid payload: missing event type or data');
+ }
+
if (!(event in eventHandlers)) {
- throw new Error(`Unsupported gateway event: ${event as string}`);
+ throw new Error(`Unsupported gateway event: ${event}. Supported events: ${Object.keys(eventHandlers).join(', ')}`);
}
- await (eventHandlers as any)[event](d);
+
+ const handler = eventHandlers[event as keyof typeof eventHandlers];
+ await handler(d);
}🤖 Prompt for AI Agents
In src/workflows/discord/gateway/index.ts around lines 8 to 13, improve type
safety by removing the use of 'as any' on eventHandlers and instead properly
typing eventHandlers to ensure the event key exists. Add validation to confirm
that payload has the expected structure with properties 't' and 'd' before
accessing them. Enhance the error message to include the invalid event value and
possibly the entire payload for better debugging context.
| @@ -0,0 +1,8 @@ | |||
| import { DiscordEventType, EventPayloadMap } from './DiscordEvents'; | |||
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.
💡 Verification agent
🧩 Analysis chain
Verify import path casing for cross-platform compatibility.
The import path uses './DiscordEvents' but based on the relevant code snippets, the actual filename appears to be discordEvents.ts. This casing mismatch could cause issues on case-sensitive file systems (Linux/macOS).
🏁 Script executed:
#!/bin/bash
# Description: Check the actual filename and verify import path consistency
# Expected: Find the correct filename and flag any casing mismatches
# Find the actual Discord events file
fd -t f -e ts DiscordEvents src/shared/types/discord/
fd -t f -e ts discordEvents src/shared/types/discord/
# Check for import statements using this path
rg -A 2 -B 2 "from.*DiscordEvents" src/shared/types/discord/Length of output: 523
Fix Import Path Casing
To avoid runtime errors on case-sensitive file systems, the import path must match the actual filename (discordEvents.ts).
- In
src/shared/types/discord/EventIngestion.discord.ts, update the import:
-import { DiscordEventType, EventPayloadMap } from './DiscordEvents';
+import { DiscordEventType, EventPayloadMap } from './discordEvents';📝 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.
| import { DiscordEventType, EventPayloadMap } from './DiscordEvents'; | |
| import { DiscordEventType, EventPayloadMap } from './discordEvents'; |
🤖 Prompt for AI Agents
In src/shared/types/discord/EventIngestion.discord.ts at line 1, the import path
'./DiscordEvents' does not match the actual filename 'discordEvents.ts'. Update
the import statement to use the correct casing './discordEvents' to ensure
compatibility on case-sensitive file systems.
| static async delete(data: GatewayChannelDeleteDispatchData): Promise<void> { | ||
| await activitiesProxy.updateChannel( | ||
| data.guild_id, | ||
| { channelId: data.id }, | ||
| { deletedAt: new Date() }, | ||
| ); | ||
| } |
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.
🛠️ Refactor suggestion
Potential issue with delete operation data consistency.
The delete handler directly calls updateChannel with a deletion timestamp, but this bypasses the mapping activity that might handle additional cleanup or validation logic. Consider using a dedicated delete activity or ensuring the mapping layer handles deletion properly.
static async delete(data: GatewayChannelDeleteDispatchData): Promise<void> {
+ const mappedData = await activitiesProxy.mapChannelDelete(data);
await activitiesProxy.updateChannel(
data.guild_id,
{ channelId: data.id },
- { deletedAt: new Date() },
+ mappedData,
);
}📝 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.
| static async delete(data: GatewayChannelDeleteDispatchData): Promise<void> { | |
| await activitiesProxy.updateChannel( | |
| data.guild_id, | |
| { channelId: data.id }, | |
| { deletedAt: new Date() }, | |
| ); | |
| } | |
| static async delete(data: GatewayChannelDeleteDispatchData): Promise<void> { | |
| const mappedData = await activitiesProxy.mapChannelDelete(data); | |
| await activitiesProxy.updateChannel( | |
| data.guild_id, | |
| { channelId: data.id }, | |
| mappedData, | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/workflows/discord/gateway/handlers/channel.handler.ts around lines 31 to
37, the delete method directly updates the channel with a deletion timestamp,
bypassing the mapping layer that may perform necessary cleanup or validation.
Refactor the delete method to use a dedicated delete activity or ensure the
mapping layer is invoked to handle deletion logic properly, maintaining data
consistency and triggering any additional required processes.
| ); | ||
| } | ||
|
|
||
| const DISCORD_TOKEN = 'xx'; |
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.
🛠️ Refactor suggestion
Replace hardcoded token with environment variable.
The hardcoded Discord token should be replaced with an environment variable for security and usability.
- const DISCORD_TOKEN = 'xx';
+ const DISCORD_TOKEN = process.env.DISCORD_TOKEN || 'your-bot-token-here';Also consider adding a check to ensure the token is provided:
if (!DISCORD_TOKEN || DISCORD_TOKEN === 'your-bot-token-here') {
throw new Error('Please set DISCORD_TOKEN environment variable');
}🤖 Prompt for AI Agents
In src/examples/client/discordGateway.example.ts at line 105, replace the
hardcoded Discord token string with a reference to an environment variable, such
as process.env.DISCORD_TOKEN. Additionally, add a check after retrieving the
token to verify it is set and not a placeholder value, throwing an error if the
token is missing to prevent runtime issues and improve security.
| export async function updateRole( | ||
| guildId: Snowflake, | ||
| filter: FilterQuery<IRole>, | ||
| data: IRoleUpdateBody, | ||
| ): Promise<void> { | ||
| try { | ||
| const dbConnection = | ||
| await DatabaseManager.getInstance().getGuildDb(guildId); | ||
| const repo = makeRoleRepository(dbConnection); | ||
| const res = await repo.updateOne(filter, data); | ||
| if (!res.modifiedCount) await repo.create(data); | ||
| } catch (err: any) { | ||
| if (err.code === 11000) { | ||
| logger.warn({ guildId, data }, 'Role already exists'); | ||
| } else { | ||
| throw err; | ||
| } | ||
| } | ||
| } |
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 inconsistent error handling in updateRole function.
The updateRole function has different error handling compared to other update functions. It wraps the entire operation in a try-catch and handles duplicate key errors during the fallback creation, while other update functions only handle errors during creation.
Make the error handling consistent with other update functions:
-export async function updateRole(
- guildId: Snowflake,
- filter: FilterQuery<IRole>,
- data: IRoleUpdateBody,
-): Promise<void> {
- try {
- const dbConnection =
- await DatabaseManager.getInstance().getGuildDb(guildId);
- const repo = makeRoleRepository(dbConnection);
- const res = await repo.updateOne(filter, data);
- if (!res.modifiedCount) await repo.create(data);
- } catch (err: any) {
- if (err.code === 11000) {
- logger.warn({ guildId, data }, 'Role already exists');
- } else {
- throw err;
- }
- }
-}
+export async function updateRole(
+ guildId: Snowflake,
+ filter: FilterQuery<IRole>,
+ data: IRoleUpdateBody,
+): Promise<void> {
+ const dbConnection = await DatabaseManager.getInstance().getGuildDb(guildId);
+ const repo = makeRoleRepository(dbConnection);
+
+ const res = await repo.updateOne(filter, data);
+ if (!res.modifiedCount) {
+ try {
+ await repo.create(data);
+ } catch (err: any) {
+ if (err.code === 11000) {
+ logger.warn({ guildId, roleId: data.roleId }, 'Role already exists');
+ } else {
+ throw err;
+ }
+ }
+ }
+}📝 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.
| export async function updateRole( | |
| guildId: Snowflake, | |
| filter: FilterQuery<IRole>, | |
| data: IRoleUpdateBody, | |
| ): Promise<void> { | |
| try { | |
| const dbConnection = | |
| await DatabaseManager.getInstance().getGuildDb(guildId); | |
| const repo = makeRoleRepository(dbConnection); | |
| const res = await repo.updateOne(filter, data); | |
| if (!res.modifiedCount) await repo.create(data); | |
| } catch (err: any) { | |
| if (err.code === 11000) { | |
| logger.warn({ guildId, data }, 'Role already exists'); | |
| } else { | |
| throw err; | |
| } | |
| } | |
| } | |
| export async function updateRole( | |
| guildId: Snowflake, | |
| filter: FilterQuery<IRole>, | |
| data: IRoleUpdateBody, | |
| ): Promise<void> { | |
| const dbConnection = await DatabaseManager.getInstance().getGuildDb(guildId); | |
| const repo = makeRoleRepository(dbConnection); | |
| const res = await repo.updateOne(filter, data); | |
| if (!res.modifiedCount) { | |
| try { | |
| await repo.create(data); | |
| } catch (err: any) { | |
| if (err.code === 11000) { | |
| logger.warn({ guildId, roleId: data.roleId }, 'Role already exists'); | |
| } else { | |
| throw err; | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/activities/discord/gateway/persistence.activity.ts around lines 108 to
126, the updateRole function currently wraps the entire update and create logic
in a try-catch block, handling duplicate key errors during creation. To make
error handling consistent with other update functions, remove the outer
try-catch and only wrap the create call in a try-catch that handles duplicate
key errors. This means performing the update first without catching errors, then
if no documents were modified, attempt to create the role inside a try-catch
that logs duplicate key errors and rethrows others.
| const updateData: any = {}; | ||
|
|
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.
Use proper typing instead of any.
Using any type defeats TypeScript's type safety. Use IRawInfoUpdateBody or Partial<IRawInfo> instead.
- const updateData: any = {};
+ const updateData: IRawInfoUpdateBody = {};📝 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.
| const updateData: any = {}; | |
| const updateData: IRawInfoUpdateBody = {}; |
🤖 Prompt for AI Agents
In src/workflows/discord/gateway/mappers/message.mapper.ts at line 110, replace
the type annotation of the updateData variable from any to a more specific type
such as IRawInfoUpdateBody or Partial<IRawInfo> to leverage TypeScript's type
safety features. This involves importing the appropriate interface if not
already imported and updating the declaration to use the chosen type instead of
any.
| // Create a minimal IRawInfo if none exists (fallback case) | ||
| const newReaction = formatReaction([payload.user_id], emojiStr); | ||
| return { | ||
| type: 0, | ||
| author: payload.message_author_id || payload.user_id, | ||
| content: '', | ||
| createdDate: new Date(), | ||
| user_mentions: [], | ||
| role_mentions: [], | ||
| reactions: [newReaction], | ||
| replied_user: null, | ||
| messageId: payload.message_id, | ||
| channelId: payload.channel_id, | ||
| channelName: null, | ||
| threadId: null, | ||
| threadName: null, | ||
| isGeneratedByWebhook: false, | ||
| }; |
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.
🛠️ Refactor suggestion
Avoid hardcoding message type value.
The hardcoded type: 0 should be replaced with a proper constant or enum value from Discord's message types.
+import { MessageType } from 'discord-api-types/v10';
+
// Create a minimal IRawInfo if none exists (fallback case)
const newReaction = formatReaction([payload.user_id], emojiStr);
return {
- type: 0,
+ type: MessageType.Default,
author: payload.message_author_id || payload.user_id,Also, consider if creating a minimal IRawInfo is the right approach when the message doesn't exist. It might be better to handle this case differently or throw an error.
📝 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.
| // Create a minimal IRawInfo if none exists (fallback case) | |
| const newReaction = formatReaction([payload.user_id], emojiStr); | |
| return { | |
| type: 0, | |
| author: payload.message_author_id || payload.user_id, | |
| content: '', | |
| createdDate: new Date(), | |
| user_mentions: [], | |
| role_mentions: [], | |
| reactions: [newReaction], | |
| replied_user: null, | |
| messageId: payload.message_id, | |
| channelId: payload.channel_id, | |
| channelName: null, | |
| threadId: null, | |
| threadName: null, | |
| isGeneratedByWebhook: false, | |
| }; | |
| import { MessageType } from 'discord-api-types/v10'; | |
| // Create a minimal IRawInfo if none exists (fallback case) | |
| const newReaction = formatReaction([payload.user_id], emojiStr); | |
| return { | |
| type: MessageType.Default, | |
| author: payload.message_author_id || payload.user_id, | |
| content: '', | |
| createdDate: new Date(), | |
| user_mentions: [], | |
| role_mentions: [], | |
| reactions: [newReaction], | |
| replied_user: null, | |
| messageId: payload.message_id, | |
| channelId: payload.channel_id, | |
| channelName: null, | |
| threadId: null, | |
| threadName: null, | |
| isGeneratedByWebhook: false, | |
| }; |
🤖 Prompt for AI Agents
In src/workflows/discord/gateway/mappers/message.mapper.ts around lines 167 to
184, replace the hardcoded message type value 0 with the appropriate constant or
enum from Discord's message types to improve code clarity and maintainability.
Additionally, review the logic for creating a minimal IRawInfo as a fallback;
consider handling the absence of a message more explicitly, such as by throwing
an error or using a different approach, rather than silently creating a minimal
object.
| // Helper function to format reaction strings according to the schema | ||
| const formatReaction = (userIds: string[], emoji: string): string => { | ||
| return `${userIds.join(',')},${emoji}`; | ||
| }; | ||
|
|
||
| // Helper function to find existing reaction by emoji | ||
| const findReactionByEmoji = ( | ||
| reactions: string[], | ||
| emoji: string, | ||
| ): string | undefined => { | ||
| return reactions.find((reaction) => { | ||
| const parts = reaction.split(','); | ||
| return parts[parts.length - 1] === emoji; | ||
| }); | ||
| }; | ||
|
|
||
| // Helper function to get user IDs from a reaction string | ||
| const getUserIdsFromReaction = (reaction: string): string[] => { | ||
| const parts = reaction.split(','); | ||
| return parts.slice(0, -1); // All parts except the last one (emoji) | ||
| }; | ||
|
|
||
| // Helper function to get emoji from a reaction string | ||
| const getEmojiFromReaction = (reaction: string): string => { | ||
| const parts = reaction.split(','); | ||
| return parts[parts.length - 1]; // Last part is emoji | ||
| }; |
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.
🛠️ Refactor suggestion
Consider using a more robust reaction storage format.
The current comma-separated string format for reactions is fragile and could break if emoji names contain commas or other special characters. Consider using a more structured format.
Consider using JSON serialization or a different delimiter that's less likely to appear in emoji names:
-// Helper function to format reaction strings according to the schema
-const formatReaction = (userIds: string[], emoji: string): string => {
- return `${userIds.join(',')},${emoji}`;
-};
+// Helper function to format reaction strings according to the schema
+const formatReaction = (userIds: string[], emoji: string): string => {
+ // Use a delimiter that's unlikely to appear in emoji names
+ return `${userIds.join('|')}||${emoji}`;
+};
-// Helper function to find existing reaction by emoji
-const findReactionByEmoji = (
- reactions: string[],
- emoji: string,
-): string | undefined => {
- return reactions.find((reaction) => {
- const parts = reaction.split(',');
- return parts[parts.length - 1] === emoji;
- });
-};
+// Helper function to find existing reaction by emoji
+const findReactionByEmoji = (
+ reactions: string[],
+ emoji: string,
+): string | undefined => {
+ return reactions.find((reaction) => {
+ const parts = reaction.split('||');
+ return parts[1] === emoji;
+ });
+};
-// Helper function to get user IDs from a reaction string
-const getUserIdsFromReaction = (reaction: string): string[] => {
- const parts = reaction.split(',');
- return parts.slice(0, -1); // All parts except the last one (emoji)
-};
+// Helper function to get user IDs from a reaction string
+const getUserIdsFromReaction = (reaction: string): string[] => {
+ const parts = reaction.split('||');
+ return parts[0].split('|').filter(id => id); // Split user IDs and filter empty
+};
-// Helper function to get emoji from a reaction string
-const getEmojiFromReaction = (reaction: string): string => {
- const parts = reaction.split(',');
- return parts[parts.length - 1]; // Last part is emoji
-};
+// Helper function to get emoji from a reaction string
+const getEmojiFromReaction = (reaction: string): string => {
+ const parts = reaction.split('||');
+ return parts[1] || ''; // Second part is emoji
+};Alternatively, consider storing reactions as a more structured format in the database schema if possible.
📝 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.
| // Helper function to format reaction strings according to the schema | |
| const formatReaction = (userIds: string[], emoji: string): string => { | |
| return `${userIds.join(',')},${emoji}`; | |
| }; | |
| // Helper function to find existing reaction by emoji | |
| const findReactionByEmoji = ( | |
| reactions: string[], | |
| emoji: string, | |
| ): string | undefined => { | |
| return reactions.find((reaction) => { | |
| const parts = reaction.split(','); | |
| return parts[parts.length - 1] === emoji; | |
| }); | |
| }; | |
| // Helper function to get user IDs from a reaction string | |
| const getUserIdsFromReaction = (reaction: string): string[] => { | |
| const parts = reaction.split(','); | |
| return parts.slice(0, -1); // All parts except the last one (emoji) | |
| }; | |
| // Helper function to get emoji from a reaction string | |
| const getEmojiFromReaction = (reaction: string): string => { | |
| const parts = reaction.split(','); | |
| return parts[parts.length - 1]; // Last part is emoji | |
| }; | |
| // Helper function to format reaction strings according to the schema | |
| const formatReaction = (userIds: string[], emoji: string): string => { | |
| // Use a delimiter that's unlikely to appear in emoji names | |
| return `${userIds.join('|')}||${emoji}`; | |
| }; | |
| // Helper function to find existing reaction by emoji | |
| const findReactionByEmoji = ( | |
| reactions: string[], | |
| emoji: string, | |
| ): string | undefined => { | |
| return reactions.find((reaction) => { | |
| const parts = reaction.split('||'); | |
| return parts[1] === emoji; | |
| }); | |
| }; | |
| // Helper function to get user IDs from a reaction string | |
| const getUserIdsFromReaction = (reaction: string): string[] => { | |
| const parts = reaction.split('||'); | |
| return parts[0].split('|').filter(id => id); // Split user IDs and filter out any empty strings | |
| }; | |
| // Helper function to get emoji from a reaction string | |
| const getEmojiFromReaction = (reaction: string): string => { | |
| const parts = reaction.split('||'); | |
| return parts[1] || ''; // Second part is emoji | |
| }; |
🤖 Prompt for AI Agents
In src/workflows/discord/gateway/mappers/message.mapper.ts between lines 45 and
71, the current reaction handling uses comma-separated strings which can break
if emojis contain commas. To fix this, refactor the reaction storage to use JSON
serialization for reactions, storing user IDs and emoji as an object string
instead of a comma-separated string. Update all helper functions to parse and
stringify JSON objects accordingly, ensuring robust handling of special
characters and improving maintainability.
Summary by CodeRabbit
New Features
Chores
Style