Skip to content

feat: change mid to string array #745

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

feat: change mid to string array #745

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9bd708d
feat: change mid to string array
medtaher123 Feb 11, 2025
0453ffb
test: adjust unit tests
medtaher123 Feb 12, 2025
fbd630c
test: adjust unit tests
medtaher123 Feb 12, 2025
d7f9ecd
test: adjust unit tests
medtaher123 Feb 12, 2025
b86fbe7
test: adjust unit tests
medtaher123 Feb 12, 2025
6ecfbc5
feat: add index on mid in Message schema
medtaher123 Feb 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/src/channel/lib/Handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ export default abstract class ChannelHandler<
envelope: StdOutgoingEnvelope,
options: any,
context: any,
): Promise<{ mid: string }>;
): Promise<{ mid: string | string[] }>;

/**
* Calls the channel handler to fetch attachments and stores them
Expand Down
18 changes: 16 additions & 2 deletions api/src/chat/controllers/message.controller.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ describe('MessageController', () => {

afterAll(closeInMongodConnection);

function toArray(value?: string | string[]): string[] {
return value ? (Array.isArray(value) ? value : [value]) : [];
}

describe('count', () => {
it('should count messages', async () => {
jest.spyOn(messageService, 'count');
Expand All @@ -162,8 +166,13 @@ describe('MessageController', () => {
expect(messageService.findOneAndPopulate).toHaveBeenCalledWith(
message.id,
);
const expectedFixture = messageFixtures.find(
({ mid }) =>
JSON.stringify(toArray(mid)) === JSON.stringify(message.mid),
);
expect(result).toEqualPayload({
...messageFixtures.find(({ mid }) => mid === message.mid),
...expectedFixture,
mid: toArray(expectedFixture?.mid),
sender,
recipient,
sentBy: user.id,
Expand All @@ -174,8 +183,13 @@ describe('MessageController', () => {
const result = await messageController.findOne(message.id, []);

expect(messageService.findOne).toHaveBeenCalledWith(message.id);
const expectedFixture = messageFixtures.find(
({ mid }) =>
JSON.stringify(toArray(mid)) === JSON.stringify(message.mid),
);
expect(result).toEqualPayload({
...messageFixtures.find(({ mid }) => mid === message.mid),
...expectedFixture,
mid: toArray(expectedFixture?.mid),
sender: sender.id,
recipient: recipient.id,
sentBy: user.id,
Expand Down
6 changes: 3 additions & 3 deletions api/src/chat/dto/message.dto.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright © 2024 Hexastack. All rights reserved.
* Copyright © 2025 Hexastack. All rights reserved.
*
* Licensed under the GNU Affero General Public License v3.0 (AGPLv3) with the following additional terms:
* 1. The name "Hexabot" is a trademark of Hexastack. You may not use this name in derivative works without express written permission.
Expand All @@ -11,8 +11,8 @@ import {
IsBoolean,
IsNotEmpty,
IsObject,
IsString,
IsOptional,
IsString,
} from 'class-validator';

import { IsObjectId } from '@/utils/validation-rules/is-object-id';
Expand All @@ -27,7 +27,7 @@ export class MessageCreateDto {
@ApiProperty({ description: 'Message id', type: String })
@IsOptional()
@IsString()
mid?: string;
mid?: string | string[];
Comment on lines 28 to +30
Copy link
Contributor

@coderabbitai coderabbitai bot Jul 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.


@ApiProperty({ description: 'Reply to Message id', type: String })
@IsOptional()
Expand Down
40 changes: 37 additions & 3 deletions api/src/chat/repositories/message.repository.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright © 2024 Hexastack. All rights reserved.
* Copyright © 2025 Hexastack. All rights reserved.
*
* Licensed under the GNU Affero General Public License v3.0 (AGPLv3) with the following additional terms:
* 1. The name "Hexabot" is a trademark of Hexastack. You may not use this name in derivative works without express written permission.
Expand Down Expand Up @@ -60,7 +60,11 @@ describe('MessageRepository', () => {
afterAll(closeInMongodConnection);

describe('findOneAndPopulate', () => {
it('should find one message by id, and populate its sender and recipient', async () => {
function toArray(value?: string | string[]): string[] {
return value ? (Array.isArray(value) ? value : [value]) : [];
}

it('should find one message by id, and populate its sender and recipient (with mid being a string)', async () => {
jest.spyOn(messageModel, 'findById');
const message = (await messageRepository.findOne({ mid: 'mid-1' }))!;
const sender = await subscriberRepository.findOne(message!['sender']);
Expand All @@ -71,8 +75,38 @@ describe('MessageRepository', () => {
const result = await messageRepository.findOneAndPopulate(message.id);

expect(messageModel.findById).toHaveBeenCalledWith(message.id, undefined);

const expectedFixture = messageFixtures.find(
({ mid }) =>
JSON.stringify(toArray(mid)) === JSON.stringify(message.mid),
);
expect(result).toEqualPayload({
...expectedFixture,
mid: toArray(expectedFixture?.mid),
sender,
recipient,
sentBy: user.id,
});
});
it('should find one message by id, and populate its sender and recipient (with mid being a string array)', async () => {
jest.spyOn(messageModel, 'findById');
const message = (await messageRepository.findOne({ mid: 'mid-2' }))!;
const sender = await subscriberRepository.findOne(message!['sender']);
const recipient = await subscriberRepository.findOne(
message!['recipient'],
);
const user = (await userRepository.findOne(message!['sentBy']))!;
const result = await messageRepository.findOneAndPopulate(message.id);

expect(messageModel.findById).toHaveBeenCalledWith(message.id, undefined);

const expectedFixture = messageFixtures.find(
({ mid }) =>
JSON.stringify(toArray(mid)) === JSON.stringify(message.mid),
);
expect(result).toEqualPayload({
...messageFixtures.find(({ mid }) => mid === message.mid),
...expectedFixture,
mid: toArray(expectedFixture?.mid),
sender,
recipient,
sentBy: user.id,
Expand Down
11 changes: 7 additions & 4 deletions api/src/chat/schemas/message.schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright © 2024 Hexastack. All rights reserved.
* Copyright © 2025 Hexastack. All rights reserved.
*
* Licensed under the GNU Affero General Public License v3.0 (AGPLv3) with the following additional terms:
* 1. The name "Hexabot" is a trademark of Hexastack. You may not use this name in derivative works without express written permission.
Expand All @@ -20,11 +20,11 @@ import { StdIncomingMessage, StdOutgoingMessage } from './types/message';
@Schema({ timestamps: true })
export class MessageStub extends BaseSchema {
@Prop({
type: String,
type: [String],
required: false,
//TODO : add default value for mid
})
mid?: string;
mid?: string | string[];
Comment on lines +23 to +27
Copy link
Contributor

@coderabbitai coderabbitai bot Jul 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.


@Prop({
type: MongooseSchema.Types.ObjectId,
Expand Down Expand Up @@ -96,9 +96,12 @@ export class MessageFull extends MessageStub {
sentBy?: string; // sendBy is never populate
}

const MessageSchema = SchemaFactory.createForClass(MessageStub);
MessageSchema.index({ mid: 1 });
Comment on lines +99 to +100
Copy link
Contributor

@coderabbitai coderabbitai bot Jul 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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.ts

Length 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.


export const MessageModel: ModelDefinition = LifecycleHookManager.attach({
name: Message.name,
schema: SchemaFactory.createForClass(MessageStub),
schema: MessageSchema,
});

export default MessageModel.schema;
Expand Down
10 changes: 9 additions & 1 deletion api/src/chat/services/message.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ describe('MessageService', () => {
afterEach(jest.clearAllMocks);
afterAll(closeInMongodConnection);

function toArray(value?: string | string[]): string[] {
return value ? (Array.isArray(value) ? value : [value]) : [];
}
describe('findOneAndPopulate', () => {
it('should find message by id, and populate its corresponding sender and recipient', async () => {
jest.spyOn(messageRepository, 'findOneAndPopulate');
Expand All @@ -114,8 +117,13 @@ describe('MessageService', () => {
message.id,
undefined,
);
const expectedFixture = messageFixtures.find(
({ mid }) =>
JSON.stringify(toArray(mid)) === JSON.stringify(message.mid),
);
expect(result).toEqualPayload({
...messageFixtures.find(({ mid }) => mid === message.mid),
...expectedFixture,
mid: toArray(expectedFixture?.mid),
sender,
recipient,
sentBy: user.id,
Expand Down
4 changes: 2 additions & 2 deletions api/src/extensions/channels/web/base-web-channel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ export default abstract class BaseWebChannelHandler<
...message,
author: anyMessage.sender,
read: true, // Temporary fix as read is false in the bd
mid: anyMessage.mid,
mid: anyMessage.mid?.[0],
createdAt: anyMessage.createdAt,
});
} else {
Expand All @@ -251,7 +251,7 @@ export default abstract class BaseWebChannelHandler<
...message,
author: 'chatbot',
read: true, // Temporary fix as read is false in the bd
mid: anyMessage.mid || this.generateId(),
mid: anyMessage.mid?.[0] || this.generateId(),
handover: !!anyMessage.handover,
createdAt: anyMessage.createdAt,
});
Expand Down
4 changes: 2 additions & 2 deletions api/src/utils/test/fixtures/message.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import mongoose from 'mongoose';

import { MessageCreateDto } from '@/chat/dto/message.dto';
import { MessageModel, Message } from '@/chat/schemas/message.schema';
import { Message, MessageModel } from '@/chat/schemas/message.schema';

import { getFixturesWithDefaultValues } from '../defaultValues';
import { TFixturesDefaultValues } from '../types';
Expand All @@ -27,7 +27,7 @@ const messages: MessageCreateDto[] = [
delivery: true,
},
{
mid: 'mid-2',
mid: ['mid-2', 'mid-2.1'],
sender: '1',
recipient: '1',
sentBy: '0',
Expand Down