feat: change mid to string array #745
Conversation
WalkthroughThe changes expand support for the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Service
participant Repository
participant Schema
Client->>Controller: Send/Create/Fetch message (mid: string | string[])
Controller->>Service: Pass message DTO (mid: string | string[])
Service->>Repository: Store/Retrieve message (mid: string | string[])
Repository->>Schema: Save/Query message (mid: string | string[])
Schema-->>Repository: Return message (mid: string | string[])
Repository-->>Service: Return message (mid: string | string[])
Service-->>Controller: Return message (mid: string | string[])
Controller-->>Client: Return message (mid: string | string[])
Poem
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
api/src/channel/lib/Handler.ts(1 hunks)api/src/chat/controllers/message.controller.spec.ts(3 hunks)api/src/chat/dto/message.dto.ts(3 hunks)api/src/chat/repositories/message.repository.spec.ts(3 hunks)api/src/chat/schemas/message.schema.ts(3 hunks)api/src/chat/services/message.service.spec.ts(2 hunks)api/src/extensions/channels/web/base-web-channel.ts(2 hunks)api/src/utils/test/fixtures/message.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (5)
api/src/chat/controllers/message.controller.spec.ts (3)
Learnt from: yassinedorbozgithub
PR: Hexastack/Hexabot#1137
File: api/src/utils/test/utils.ts:262-262
Timestamp: 2025-06-19T07:24:22.565Z
Learning: In the Hexabot codebase's `buildTestingMocks` function, the type cast `autoInjectFrom as ToUnionArray<typeof autoInjectFrom>` is necessary due to the discriminated union with literal array types like `'providers'[]` and `'controllers'[]`. Without this cast, TypeScript compilation fails when calling `.includes()` with string literals not in the constrained type.
Learnt from: yassinedorbozgithub
PR: Hexastack/Hexabot#1137
File: api/src/utils/test/utils.ts:262-262
Timestamp: 2025-06-19T07:24:22.565Z
Learning: In TypeScript discriminated unions with literal array types like `'providers'[]` vs `'controllers'[]`, calling `.includes()` with string literals not in the constrained type causes compilation errors. Type assertions with utility types like `ToUnionArray` are necessary to unify these types for common array operations.
Learnt from: yassinedorbozgithub
PR: Hexastack/Hexabot#1137
File: api/src/utils/test/utils.ts:262-262
Timestamp: 2025-06-19T07:24:22.565Z
Learning: In TypeScript discriminated unions with different array literal types (like `'providers'[]` vs `'controllers'[]`), calling `.includes()` with values not in the literal type will cause compilation errors. Type assertions may be necessary to unify these types for common operations.
api/src/chat/dto/message.dto.ts (1)
Learnt from: medchedli
PR: Hexastack/Hexabot#1096
File: api/src/chat/repositories/block.repository.ts:0-0
Timestamp: 2025-07-03T17:41:54.853Z
Learning: In the Hexabot block schema, a block is designed so that it can never simultaneously have both an `attachedBlock` and non-empty `nextBlocks`; the two are mutually exclusive.
api/src/chat/schemas/message.schema.ts (1)
Learnt from: medchedli
PR: Hexastack/Hexabot#1096
File: api/src/chat/repositories/block.repository.ts:0-0
Timestamp: 2025-07-03T17:41:54.853Z
Learning: In the Hexabot block schema, a block is designed so that it can never simultaneously have both an `attachedBlock` and non-empty `nextBlocks`; the two are mutually exclusive.
api/src/chat/repositories/message.repository.spec.ts (1)
Learnt from: medchedli
PR: Hexastack/Hexabot#1096
File: api/src/chat/repositories/block.repository.ts:0-0
Timestamp: 2025-07-03T17:41:54.853Z
Learning: In the Hexabot block schema, a block is designed so that it can never simultaneously have both an `attachedBlock` and non-empty `nextBlocks`; the two are mutually exclusive.
api/src/chat/services/message.service.spec.ts (2)
Learnt from: yassinedorbozgithub
PR: Hexastack/Hexabot#1137
File: api/src/utils/test/utils.ts:262-262
Timestamp: 2025-06-19T07:24:22.565Z
Learning: In the Hexabot codebase's `buildTestingMocks` function, the type cast `autoInjectFrom as ToUnionArray<typeof autoInjectFrom>` is necessary due to the discriminated union with literal array types like `'providers'[]` and `'controllers'[]`. Without this cast, TypeScript compilation fails when calling `.includes()` with string literals not in the constrained type.
Learnt from: yassinedorbozgithub
PR: Hexastack/Hexabot#1137
File: api/src/utils/test/utils.ts:262-262
Timestamp: 2025-06-19T07:24:22.565Z
Learning: In TypeScript discriminated unions with literal array types like `'providers'[]` vs `'controllers'[]`, calling `.includes()` with string literals not in the constrained type causes compilation errors. Type assertions with utility types like `ToUnionArray` are necessary to unify these types for common array operations.
🧬 Code Graph Analysis (1)
api/src/chat/services/message.service.spec.ts (1)
api/src/utils/test/fixtures/message.ts (1)
messageFixtures(53-56)
🔇 Additional comments (12)
api/src/utils/test/fixtures/message.ts (1)
30-30: Good addition of array format for testing.The fixture update correctly demonstrates the new capability to handle multiple message IDs, which aligns with the PR objective for supporting channels like Slack that require multiple separate messages.
api/src/chat/services/message.service.spec.ts (2)
108-110: Well-implemented helper function.The
toArrayhelper function correctly normalizes the union type (string | string[]) to a consistent array format for testing, ensuring proper handling of both scenarios.
120-126: Correct test logic update.The test logic properly uses the helper function to normalize
midvalues for consistent comparison, supporting both single and multiple message ID scenarios.api/src/channel/lib/Handler.ts (1)
217-217: Verify Handler.sendMessage implementations
Please confirm that every class extendingHandlerinapi/src/channel/lib/Handler.tshas itssendMessageoverride updated to return{ mid: string | string[] }. Manual review is required to ensure no handlers still expect a single-string return.api/src/chat/repositories/message.repository.spec.ts (3)
63-65: Well-implemented normalization helper.The
toArrayfunction provides a clean way to normalizemidvalues consistently, handling both string and array cases correctly.
67-90: Good test coverage for both mid types.Splitting the test into separate cases for string and array
midvalues ensures comprehensive coverage of the new dual-type functionality. The JSON.stringify comparison approach correctly handles array equality.
91-114: Consistent test implementation.The second test case follows the same pattern as the first, maintaining consistency in the test suite structure and approach.
api/src/extensions/channels/web/base-web-channel.ts (2)
245-245: Correct extraction of first mid element.Using
anyMessage.mid?.[0]safely extracts the first message ID from the array while handling the case wheremidmight be undefined or a single string. This is appropriate for the web channel format.
254-254: Proper fallback for outgoing messages.The logic correctly extracts the first
midelement and falls back to generating a new ID whenmidis absent or empty. This maintains consistency with the incoming message handling above.api/src/chat/controllers/message.controller.spec.ts (3)
144-146: Consistent normalization helper implementation.The
toArrayfunction matches the implementation in the repository test file, providing consistency across the test suite.
169-175: Well-adapted test logic for dual-type mid support.The test correctly uses the
toArrayhelper and JSON.stringify comparison to handle both string and arraymidvalues, ensuring the fixture matching works properly.
186-192: Consistent test pattern application.The second test case follows the same pattern as the first, maintaining consistency in the test implementation approach.
| @IsOptional() | ||
| @IsString() | ||
| mid?: string; | ||
| mid?: string | string[]; |
There was a problem hiding this comment.
Fix validation decorator for union type.
The @IsString() decorator only validates string types, but the mid property now accepts string | string[]. This will cause validation failures when mid is provided as an array.
Apply this diff to fix the validation:
- @IsString()
- mid?: string | string[];
+ @IsString({ each: true })
+ mid?: string | string[];Or use a custom validation approach:
- @IsString()
+ @IsString({ each: true })
+ @Transform(({ value }) => Array.isArray(value) ? value : [value])
mid?: string | string[];Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In api/src/chat/dto/message.dto.ts around lines 28 to 30, the mid property is
typed as string or string array but only has the @IsString() decorator, which
fails validation for arrays. Replace @IsString() with a validation decorator
that supports both string and string array types, such as @IsString({ each: true
}) combined with a check for array or string, or implement a custom validator to
handle the union type correctly.
| const MessageSchema = SchemaFactory.createForClass(MessageStub); | ||
| MessageSchema.index({ mid: 1 }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider database migration strategy.
The schema change from string to array may require migrating existing data. Verify how existing messages with string mid values will be handled.
🏁 Script executed:
#!/bin/bash
# Description: Check for existing database migration scripts or strategies
# Expected: Migration scripts should handle converting string mid to array format
fd -e js -e ts migration | xargs grep -l "mid\|message"Length of output: 204
🏁 Script executed:
#!/bin/bash
# Inspect migration files for handling of converting string mid to array
# 1. Show occurrences of 'mid' in all migration-related code
grep -R "mid" -n api/src/migration
# 2. Show occurrences of 'message' in the specific versioned migration
grep -R "message" -n api/src/migration/migrations/1735836154221-v-2-2-0.migration.ts
# 3. Dump the first 200 lines of the migration to review its up/down logic
sed -n '1,200p' api/src/migration/migrations/1735836154221-v-2-2-0.migration.tsLength of output: 9834
Add migration to convert existing message.mid values from string to array
We didn’t find any migration handling the mid type change—existing documents will break when mid becomes an array. Please extend your migration script to wrap any string mid in an array:
• File: api/src/migration/migrations/1735836154221-v-2-2-0.migration.ts
After your other MessageModel updates, add something like:
// Convert string mid to array
const MessageModel = mongoose.model<Message>(Message.name, messageSchema);
await MessageModel.updateMany(
{ mid: { $type: 'string' } },
[
{ $set: { mid: { $cond: { if: { $isArray: '$mid' }, then: '$mid', else: ['$mid'] } } } }
]
);
logger.log('Migrated string mid to array for existing messages');🤖 Prompt for AI Agents
In api/src/migration/migrations/1735836154221-v-2-2-0.migration.ts after your
existing MessageModel updates, add a migration step to convert any string mid
fields to arrays. Use mongoose's updateMany with a pipeline that checks if mid
is already an array; if not, wrap it in an array. This ensures existing
documents with string mids are compatible with the new schema expecting an
array. Also, add a log statement confirming the migration ran successfully.
| type: [String], | ||
| required: false, | ||
| //TODO : add default value for mid | ||
| }) | ||
| mid?: string; | ||
| mid?: string | string[]; |
There was a problem hiding this comment.
Fix schema type inconsistency.
There's a mismatch between the Mongoose schema definition and TypeScript type:
- Mongoose schema:
type: [String](always expects array) - TypeScript type:
string | string[](union type)
This could cause runtime errors when saving string values.
Apply this diff to fix the schema definition:
@Prop({
- type: [String],
+ type: MongooseSchema.Types.Mixed,
required: false,
//TODO : add default value for mid
})Or use a custom schema type that handles both cases:
@Prop({
- type: [String],
+ type: MongooseSchema.Types.Mixed,
+ validate: {
+ validator: function(v: any) {
+ return typeof v === 'string' || (Array.isArray(v) && v.every(item => typeof item === 'string'));
+ },
+ message: 'mid must be a string or array of strings'
+ },
required: false,
//TODO : add default value for mid
})🤖 Prompt for AI Agents
In api/src/chat/schemas/message.schema.ts around lines 23 to 27, fix the
inconsistency between the Mongoose schema and TypeScript type for the mid field.
Update the Mongoose schema type to accept both a single string and an array of
strings, or adjust the TypeScript type to match the schema by making mid always
an array of strings. Ensure the schema and TypeScript types align to prevent
runtime errors when saving string values.
3 participants
Motivation
Changed the mid field in the Message schema from string to [string].
Reason for the Change:
In some channels (like Slack), certain elements must be sent as multiple separate messages. For example:
To handle this, we now store mid as an array of strings in our message object, allowing multiple message IDs to be linked to a single logical message in our database.
Summary by CodeRabbit
New Features
midcan now be a string or an array of strings) across message creation, storage, and retrieval.Bug Fixes
midis a string or an array.Tests
Chores