Send Email UI IMAP/SMTP message threading fix (#20784)
**Problem:** When using Twenty Send Email UI IMAP/SMTP message threading is broken on Twenty side as well as recipient email client **Twenty side fix:** - SMTP has no concept of `externalThreadId` sendEmail resolver always returns null, this breaks threading Fix is to pass `parentThreadExternalId` to `resolveOutboundThreadExternalId` for SMTP/IMAP path **Recipient email client fix:** - Fetch associated threads as per RFC spec to write `References` header ``` From: johndoe@domain.com To: janedoe@domain.com Subject: Test References: <root@...> <mid1@...> <parent@...> ```
This commit is contained in:
+51
-25
@@ -2,6 +2,7 @@ import { Injectable, Logger } from '@nestjs/common';
|
||||
import { InjectRepository } from '@nestjs/typeorm';
|
||||
|
||||
import { toPlainText } from '@react-email/render';
|
||||
import { isNonEmptyString } from '@sniptt/guards';
|
||||
import DOMPurify from 'dompurify';
|
||||
import { MAX_EMAIL_RECIPIENTS } from 'twenty-shared/constants';
|
||||
import {
|
||||
@@ -10,7 +11,7 @@ import {
|
||||
FileFolder,
|
||||
} from 'twenty-shared/types';
|
||||
import { isDefined, isValidUuid } from 'twenty-shared/utils';
|
||||
import { In, type Repository } from 'typeorm';
|
||||
import { In, LessThanOrEqual, type Repository } from 'typeorm';
|
||||
import { z } from 'zod';
|
||||
|
||||
import { FileEntity } from 'src/engine/core-modules/file/entities/file.entity';
|
||||
@@ -19,18 +20,24 @@ import {
|
||||
EmailToolException,
|
||||
EmailToolExceptionCode,
|
||||
} from 'src/engine/core-modules/tool/tools/email-tool/exceptions/email-tool.exception';
|
||||
import { EmailComposerResult } from 'src/engine/core-modules/tool/tools/email-tool/types/email-composer-result.type';
|
||||
import { type ComposeEmailParams } from 'src/engine/core-modules/tool/tools/email-tool/types/compose-email-params.type';
|
||||
import { EmailComposerResult } from 'src/engine/core-modules/tool/tools/email-tool/types/email-composer-result.type';
|
||||
import { parseCommaSeparatedEmails } from 'src/engine/core-modules/tool/tools/email-tool/utils/parse-comma-separated-emails.util';
|
||||
import { type ToolExecutionContext } from 'src/engine/core-modules/tool/types/tool-execution-context.type';
|
||||
import { ConnectedAccountEntity } from 'src/engine/metadata-modules/connected-account/entities/connected-account.entity';
|
||||
import { GlobalWorkspaceOrmManager } from 'src/engine/twenty-orm/global-workspace-datasource/global-workspace-orm.manager';
|
||||
import { buildSystemAuthContext } from 'src/engine/twenty-orm/utils/build-system-auth-context.util';
|
||||
import { MessagingAccountAuthenticationService } from 'src/modules/messaging/message-import-manager/services/messaging-account-authentication.service';
|
||||
import { type MessageChannelMessageAssociationWorkspaceEntity } from 'src/modules/messaging/common/standard-objects/message-channel-message-association.workspace-entity';
|
||||
import { type MessageWorkspaceEntity } from 'src/modules/messaging/common/standard-objects/message.workspace-entity';
|
||||
import { MessagingAccountAuthenticationService } from 'src/modules/messaging/message-import-manager/services/messaging-account-authentication.service';
|
||||
import { type MessageAttachment } from 'src/modules/messaging/message-import-manager/types/message';
|
||||
import { streamToBuffer } from 'src/utils/stream-to-buffer';
|
||||
|
||||
type ParentThreadContext = {
|
||||
threadExternalId?: string;
|
||||
references?: string[];
|
||||
};
|
||||
|
||||
@Injectable()
|
||||
export class EmailComposerService {
|
||||
private readonly logger = new Logger(EmailComposerService.name);
|
||||
@@ -232,13 +239,13 @@ export class EmailComposerService {
|
||||
return attachments;
|
||||
}
|
||||
|
||||
// Look up the provider-specific thread ID (e.g. Gmail threadId) from the
|
||||
// parent message so replies can be explicitly threaded in the provider API.
|
||||
private async getThreadExternalId(
|
||||
// Resolve parent's root thread id (Gmail/MS native or stored) + RFC 5322 §3.6.4
|
||||
// References chain so replies thread on both Twenty and recipient mail clients.
|
||||
private async getParentThreadContext(
|
||||
workspaceId: string,
|
||||
inReplyTo: string,
|
||||
messageChannelId: string,
|
||||
): Promise<string | undefined> {
|
||||
): Promise<ParentThreadContext> {
|
||||
const authContext = buildSystemAuthContext(workspaceId);
|
||||
|
||||
return this.globalWorkspaceOrmManager.executeInWorkspaceContext(
|
||||
@@ -253,8 +260,12 @@ export class EmailComposerService {
|
||||
where: { headerMessageId: inReplyTo },
|
||||
});
|
||||
|
||||
if (!parentMessage) {
|
||||
return undefined;
|
||||
if (
|
||||
!isDefined(parentMessage) ||
|
||||
!isDefined(parentMessage.messageThreadId) ||
|
||||
!isDefined(parentMessage.receivedAt)
|
||||
) {
|
||||
return {};
|
||||
}
|
||||
|
||||
const associationRepository =
|
||||
@@ -263,14 +274,29 @@ export class EmailComposerService {
|
||||
'messageChannelMessageAssociation',
|
||||
);
|
||||
|
||||
const association = await associationRepository.findOne({
|
||||
where: {
|
||||
messageId: parentMessage.id,
|
||||
messageChannelId,
|
||||
},
|
||||
});
|
||||
const [association, ancestorMessages] = await Promise.all([
|
||||
associationRepository.findOne({
|
||||
where: { messageId: parentMessage.id, messageChannelId },
|
||||
select: { messageThreadExternalId: true },
|
||||
}),
|
||||
messageRepository.find({
|
||||
where: {
|
||||
messageThreadId: parentMessage.messageThreadId,
|
||||
receivedAt: LessThanOrEqual(parentMessage.receivedAt),
|
||||
},
|
||||
select: { headerMessageId: true },
|
||||
order: { receivedAt: 'ASC' },
|
||||
}),
|
||||
]);
|
||||
|
||||
return association?.messageThreadExternalId ?? undefined;
|
||||
const references = ancestorMessages
|
||||
.map((message) => message.headerMessageId)
|
||||
.filter(isNonEmptyString);
|
||||
|
||||
return {
|
||||
threadExternalId: association?.messageThreadExternalId ?? undefined,
|
||||
references: references.length > 0 ? references : undefined,
|
||||
};
|
||||
},
|
||||
authContext,
|
||||
);
|
||||
@@ -397,15 +423,14 @@ export class EmailComposerService {
|
||||
const plainTextBody = toPlainText(sanitizedHtmlBody);
|
||||
const sanitizedSubject = purify.sanitize(subject || '');
|
||||
|
||||
let threadExternalId: string | undefined;
|
||||
|
||||
if (inReplyTo && isDefined(messageChannel)) {
|
||||
threadExternalId = await this.getThreadExternalId(
|
||||
workspaceId,
|
||||
inReplyTo,
|
||||
messageChannel.id,
|
||||
);
|
||||
}
|
||||
const { threadExternalId, references } =
|
||||
isDefined(inReplyTo) && isDefined(messageChannel)
|
||||
? await this.getParentThreadContext(
|
||||
workspaceId,
|
||||
inReplyTo,
|
||||
messageChannel.id,
|
||||
)
|
||||
: {};
|
||||
|
||||
return {
|
||||
success: true,
|
||||
@@ -421,6 +446,7 @@ export class EmailComposerService {
|
||||
shouldPersistMessage: isDefined(messageChannel),
|
||||
inReplyTo,
|
||||
threadExternalId,
|
||||
references,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
+1
@@ -13,4 +13,5 @@ export type ComposedEmail = {
|
||||
shouldPersistMessage: boolean;
|
||||
inReplyTo?: string;
|
||||
threadExternalId?: string;
|
||||
references?: string[];
|
||||
};
|
||||
|
||||
+2
@@ -26,6 +26,7 @@ export class SendEmailService {
|
||||
attachments: data.attachments,
|
||||
inReplyTo: data.inReplyTo,
|
||||
threadExternalId: data.threadExternalId,
|
||||
references: data.references,
|
||||
},
|
||||
data.connectedAccount,
|
||||
);
|
||||
@@ -45,6 +46,7 @@ export class SendEmailService {
|
||||
connectedAccount: data.connectedAccount,
|
||||
messageChannelId: data.messageChannelId!,
|
||||
inReplyTo: data.inReplyTo,
|
||||
parentThreadExternalId: data.threadExternalId,
|
||||
workspaceId,
|
||||
});
|
||||
} catch (persistenceError) {
|
||||
|
||||
+1
@@ -9,5 +9,6 @@ export type PersistSentMessageInput = {
|
||||
connectedAccount: Pick<ConnectedAccountEntity, 'id' | 'handle'>;
|
||||
messageChannelId: string;
|
||||
inReplyTo?: string;
|
||||
parentThreadExternalId?: string;
|
||||
workspaceId: string;
|
||||
};
|
||||
|
||||
+1
@@ -14,4 +14,5 @@ export type SendMessageInput = {
|
||||
}[];
|
||||
inReplyTo?: string;
|
||||
threadExternalId?: string;
|
||||
references?: string[];
|
||||
};
|
||||
|
||||
+16
@@ -79,6 +79,22 @@ describe('formatSentMessage', () => {
|
||||
expect(message.headerMessageId).toBe('<msg-2@mail.example>');
|
||||
});
|
||||
|
||||
it('should persist IMAP/SMTP replies under the parent thread external id rather than the immediate parent Message-ID', () => {
|
||||
const message = formatSentMessage(
|
||||
buildInput({
|
||||
sendResult: {
|
||||
headerMessageId: '<reply@mail.example>',
|
||||
messageExternalId: undefined,
|
||||
threadExternalId: undefined,
|
||||
},
|
||||
inReplyTo: '<parent@mail.example>',
|
||||
parentThreadExternalId: '<root@mail.example>',
|
||||
}),
|
||||
);
|
||||
|
||||
expect(message.messageThreadExternalId).toBe('<root@mail.example>');
|
||||
});
|
||||
|
||||
it('should copy subject and body verbatim and start with no folder associations', () => {
|
||||
const message = formatSentMessage(buildInput());
|
||||
|
||||
|
||||
+15
-1
@@ -14,7 +14,21 @@ describe('resolveOutboundThreadExternalId', () => {
|
||||
expect(result).toBe('gmail-thread-id');
|
||||
});
|
||||
|
||||
it('should fall back to inReplyTo so IMAP/SMTP replies attach to the parent thread', () => {
|
||||
it('should prefer the parent-derived thread id over inReplyTo so IMAP/SMTP replies attach to the original thread root, not the immediate parent', () => {
|
||||
const result = resolveOutboundThreadExternalId({
|
||||
sendResult: {
|
||||
headerMessageId: '<reply@mail.example>',
|
||||
messageExternalId: undefined,
|
||||
threadExternalId: undefined,
|
||||
},
|
||||
parentThreadExternalId: '<root@mail.example>',
|
||||
inReplyTo: '<parent@mail.example>',
|
||||
});
|
||||
|
||||
expect(result).toBe('<root@mail.example>');
|
||||
});
|
||||
|
||||
it('should fall back to inReplyTo when the parent is unknown locally so the reply still anchors on the parent Message-ID per RFC 5322', () => {
|
||||
const result = resolveOutboundThreadExternalId({
|
||||
sendResult: {
|
||||
headerMessageId: '<reply@mail.example>',
|
||||
|
||||
+1
@@ -46,6 +46,7 @@ export const formatSentMessage = (
|
||||
headerMessageId,
|
||||
messageThreadExternalId: resolveOutboundThreadExternalId({
|
||||
sendResult: input.sendResult,
|
||||
parentThreadExternalId: input.parentThreadExternalId,
|
||||
inReplyTo: input.inReplyTo,
|
||||
}),
|
||||
subject: input.subject,
|
||||
|
||||
+7
-2
@@ -4,17 +4,22 @@ import { type SendMessageResult } from 'src/modules/messaging/message-outbound-m
|
||||
|
||||
export const resolveOutboundThreadExternalId = ({
|
||||
sendResult,
|
||||
parentThreadExternalId,
|
||||
inReplyTo,
|
||||
}: {
|
||||
sendResult: SendMessageResult;
|
||||
parentThreadExternalId?: string;
|
||||
inReplyTo?: string;
|
||||
}): string => {
|
||||
if (isNonEmptyString(sendResult.threadExternalId)) {
|
||||
return sendResult.threadExternalId;
|
||||
}
|
||||
|
||||
// IMAP/SMTP have no server-side thread id. Reuse the parent's Message-ID so
|
||||
// the reply attaches to the same thread the parent stored under.
|
||||
// IMAP/SMTP: anchor on parent's thread id so replies stay in the original thread.
|
||||
if (isNonEmptyString(parentThreadExternalId)) {
|
||||
return parentThreadExternalId;
|
||||
}
|
||||
|
||||
if (isNonEmptyString(inReplyTo)) {
|
||||
return inReplyTo;
|
||||
}
|
||||
|
||||
+6
-1
@@ -1,4 +1,5 @@
|
||||
import { type SendMessageInput } from 'src/modules/messaging/message-outbound-manager/types/send-message-input.type';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
export const toMailComposerOptions = (
|
||||
from: string,
|
||||
@@ -24,7 +25,11 @@ export const toMailComposerOptions = (
|
||||
...(sendMessageInput.inReplyTo
|
||||
? {
|
||||
inReplyTo: sendMessageInput.inReplyTo,
|
||||
references: sendMessageInput.inReplyTo,
|
||||
references:
|
||||
isDefined(sendMessageInput.references) &&
|
||||
sendMessageInput.references.length > 0
|
||||
? sendMessageInput.references
|
||||
: sendMessageInput.inReplyTo,
|
||||
}
|
||||
: {}),
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user