Fix threaded draft email replies (#22175)
## Summary Fixes Gmail and Microsoft draft replies so workflow-created drafts stay attached to the existing provider thread. Fixes twentyhq/core-team-issues#2597. ## Root cause The email composer already resolved `threadExternalId` and `references` from `inReplyTo`, but `DraftEmailTool` only forwarded `inReplyTo` to the outbound draft service. Gmail therefore created a raw draft without `message.threadId`, which lets the draft appear as a standalone compose instead of an inline thread reply. For Microsoft, the draft path used Graph `createReply`, but parent lookup filtered on a URL-encoded `internetMessageId`. That can miss the parent message and fall back to creating a new draft message instead of a reply draft. ## Changes - Forward `threadExternalId` and `references` from `DraftEmailTool` to outbound draft creation. - Set Gmail draft `message.threadId` when `threadExternalId` is available. - Make Microsoft parent lookup use Graph request query builders with OData string escaping, so `createReply` is reached reliably. - Add targeted Jest coverage for the Draft Email tool, Gmail draft threading, and Microsoft reply-draft creation. ## Validation - `NX_DAEMON=false /Users/thomascolasdesfrancs/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin/node ../../node_modules/nx/dist/bin/nx.js jest twenty-server -- --runTestsByPath src/engine/core-modules/tool/tools/email-tool/__tests__/draft-email-tool.spec.ts src/modules/messaging/message-outbound-manager/drivers/gmail/services/__tests__/gmail-message-outbound.service.spec.ts src/modules/messaging/message-outbound-manager/drivers/microsoft/services/__tests__/microsoft-message-outbound.service.spec.ts --runInBand` - `NX_DAEMON=false /Users/thomascolasdesfrancs/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin/node ./node_modules/nx/dist/bin/nx.js lint:diff-with-main twenty-server` <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22175?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Félix Malfait <FelixMalfait@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
1076866820
commit
eedd838189
+18
-3
@@ -79,13 +79,20 @@ describe('DraftEmailTool', () => {
|
||||
});
|
||||
|
||||
it('creates the draft when the resolved account has the compose scope', async () => {
|
||||
mockComposeEmail.mockResolvedValue({
|
||||
success: true,
|
||||
data: buildComposedEmail({
|
||||
const composedEmail = {
|
||||
...buildComposedEmail({
|
||||
id: 'account-1',
|
||||
provider: ConnectedAccountProvider.GOOGLE,
|
||||
scopes: [GMAIL_COMPOSE_SCOPE],
|
||||
}),
|
||||
inReplyTo: '<parent@example.com>',
|
||||
threadExternalId: 'thread-external-id',
|
||||
references: ['<ancestor@example.com>', '<parent@example.com>'],
|
||||
};
|
||||
|
||||
mockComposeEmail.mockResolvedValue({
|
||||
success: true,
|
||||
data: composedEmail,
|
||||
});
|
||||
|
||||
const result = await tool.execute(baseInput, {
|
||||
@@ -94,5 +101,13 @@ describe('DraftEmailTool', () => {
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(mockCreateDraft).toHaveBeenCalledTimes(1);
|
||||
expect(mockCreateDraft).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
inReplyTo: composedEmail.inReplyTo,
|
||||
threadExternalId: composedEmail.threadExternalId,
|
||||
references: composedEmail.references,
|
||||
}),
|
||||
composedEmail.connectedAccount,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
+2
@@ -114,6 +114,8 @@ export class DraftEmailTool implements Tool {
|
||||
html: data.sanitizedHtmlBody,
|
||||
attachments: data.attachments,
|
||||
inReplyTo: data.inReplyTo,
|
||||
threadExternalId: data.threadExternalId,
|
||||
references: data.references,
|
||||
},
|
||||
data.connectedAccount,
|
||||
);
|
||||
|
||||
+37
@@ -18,12 +18,18 @@ describe('GmailMessageOutboundService', () => {
|
||||
let service: GmailMessageOutboundService;
|
||||
|
||||
const mockSend = jest.fn().mockResolvedValue({ data: { id: 'message-id' } });
|
||||
const mockCreateDraft = jest
|
||||
.fn()
|
||||
.mockResolvedValue({ data: { id: 'draft-id' } });
|
||||
|
||||
const mockGmailClient = {
|
||||
users: {
|
||||
messages: {
|
||||
send: mockSend,
|
||||
},
|
||||
drafts: {
|
||||
create: mockCreateDraft,
|
||||
},
|
||||
getProfile: jest
|
||||
.fn()
|
||||
.mockResolvedValue({ data: { emailAddress: 'test@example.com' } }),
|
||||
@@ -69,6 +75,7 @@ describe('GmailMessageOutboundService', () => {
|
||||
|
||||
afterEach(() => {
|
||||
mockSend.mockClear();
|
||||
mockCreateDraft.mockClear();
|
||||
jest.restoreAllMocks();
|
||||
});
|
||||
|
||||
@@ -127,4 +134,34 @@ describe('GmailMessageOutboundService', () => {
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it('should create Gmail drafts in the existing thread when a thread id is provided', async () => {
|
||||
const sendMessageInput = {
|
||||
to: 'recipient@example.com',
|
||||
subject: 'Re: Existing thread',
|
||||
body: 'Plain text',
|
||||
html: '<p>HTML content</p>',
|
||||
attachments: [],
|
||||
inReplyTo: '<parent@example.com>',
|
||||
threadExternalId: 'gmail-thread-id',
|
||||
};
|
||||
|
||||
const connectedAccount = {
|
||||
id: 'connected-account-id',
|
||||
provider: ConnectedAccountProvider.GOOGLE,
|
||||
} as any;
|
||||
|
||||
await service.createDraft(sendMessageInput, connectedAccount);
|
||||
|
||||
expect(mockCreateDraft).toHaveBeenCalledTimes(1);
|
||||
expect(mockCreateDraft).toHaveBeenCalledWith({
|
||||
userId: 'me',
|
||||
requestBody: {
|
||||
message: {
|
||||
raw: Buffer.from('mocked-email-content').toString('base64url'),
|
||||
threadId: 'gmail-thread-id',
|
||||
},
|
||||
},
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
+5
-1
@@ -1,5 +1,6 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
|
||||
import { isNonEmptyString } from '@sniptt/guards';
|
||||
import { type gmail_v1, google } from 'googleapis';
|
||||
import MailComposer from 'nodemailer/lib/mail-composer';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
@@ -31,7 +32,7 @@ export class GmailMessageOutboundService implements MessageOutboundDriver {
|
||||
userId: 'me',
|
||||
requestBody: {
|
||||
raw: encodedMessage,
|
||||
...(sendMessageInput.threadExternalId
|
||||
...(isNonEmptyString(sendMessageInput.threadExternalId)
|
||||
? { threadId: sendMessageInput.threadExternalId }
|
||||
: {}),
|
||||
},
|
||||
@@ -58,6 +59,9 @@ export class GmailMessageOutboundService implements MessageOutboundDriver {
|
||||
requestBody: {
|
||||
message: {
|
||||
raw: encodedMessage,
|
||||
...(isNonEmptyString(sendMessageInput.threadExternalId)
|
||||
? { threadId: sendMessageInput.threadExternalId }
|
||||
: {}),
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
+123
@@ -0,0 +1,123 @@
|
||||
import { Test, type TestingModule } from '@nestjs/testing';
|
||||
|
||||
import { ConnectedAccountProvider } from 'twenty-shared/types';
|
||||
|
||||
import { MicrosoftOAuth2ClientProvider } from 'src/modules/connected-account/oauth2-client-manager/drivers/microsoft/microsoft-oauth2-client.provider';
|
||||
import { MicrosoftMessageOutboundService } from 'src/modules/messaging/message-outbound-manager/drivers/microsoft/services/microsoft-message-outbound.service';
|
||||
|
||||
describe('MicrosoftMessageOutboundService', () => {
|
||||
let service: MicrosoftMessageOutboundService;
|
||||
|
||||
const messagesRequest = {
|
||||
filter: jest.fn().mockReturnThis(),
|
||||
select: jest.fn().mockReturnThis(),
|
||||
top: jest.fn().mockReturnThis(),
|
||||
get: jest.fn(),
|
||||
post: jest.fn(),
|
||||
};
|
||||
|
||||
const replyRequest = {
|
||||
post: jest.fn(),
|
||||
};
|
||||
|
||||
const draftRequest = {
|
||||
patch: jest.fn(),
|
||||
};
|
||||
|
||||
const mockMicrosoftClient = {
|
||||
api: jest.fn((path: string) => {
|
||||
switch (path) {
|
||||
case '/me/messages':
|
||||
return messagesRequest;
|
||||
case '/me/messages/parent-message-id/createReply':
|
||||
return replyRequest;
|
||||
case '/me/messages/reply-draft-id':
|
||||
return draftRequest;
|
||||
default:
|
||||
throw new Error(`Unexpected Microsoft Graph path: ${path}`);
|
||||
}
|
||||
}),
|
||||
};
|
||||
|
||||
beforeEach(async () => {
|
||||
jest.clearAllMocks();
|
||||
|
||||
messagesRequest.filter.mockReturnThis();
|
||||
messagesRequest.select.mockReturnThis();
|
||||
messagesRequest.top.mockReturnThis();
|
||||
messagesRequest.get.mockResolvedValue({
|
||||
value: [{ id: 'parent-message-id' }],
|
||||
});
|
||||
replyRequest.post.mockResolvedValue({
|
||||
id: 'reply-draft-id',
|
||||
internetMessageId: '<reply@example.com>',
|
||||
conversationId: 'conversation-id',
|
||||
});
|
||||
draftRequest.patch.mockResolvedValue({
|
||||
id: 'reply-draft-id',
|
||||
internetMessageId: '<patched-reply@example.com>',
|
||||
conversationId: 'conversation-id',
|
||||
});
|
||||
|
||||
const module: TestingModule = await Test.createTestingModule({
|
||||
providers: [
|
||||
MicrosoftMessageOutboundService,
|
||||
{
|
||||
provide: MicrosoftOAuth2ClientProvider,
|
||||
useValue: {
|
||||
getClient: jest.fn().mockResolvedValue(mockMicrosoftClient),
|
||||
},
|
||||
},
|
||||
],
|
||||
}).compile();
|
||||
|
||||
service = module.get<MicrosoftMessageOutboundService>(
|
||||
MicrosoftMessageOutboundService,
|
||||
);
|
||||
});
|
||||
|
||||
it('creates Microsoft drafts as replies when a parent internet message id is provided', async () => {
|
||||
const connectedAccount = {
|
||||
id: 'connected-account-id',
|
||||
provider: ConnectedAccountProvider.MICROSOFT,
|
||||
} as any;
|
||||
|
||||
await service.createDraft(
|
||||
{
|
||||
to: 'recipient@example.com',
|
||||
subject: 'Re: Existing thread',
|
||||
body: 'Plain text',
|
||||
html: '<p>HTML content</p>',
|
||||
attachments: [],
|
||||
inReplyTo: "<parent's-message@example.com>",
|
||||
},
|
||||
connectedAccount,
|
||||
);
|
||||
|
||||
expect(mockMicrosoftClient.api).toHaveBeenCalledWith('/me/messages');
|
||||
expect(messagesRequest.filter).toHaveBeenCalledWith(
|
||||
"internetMessageId eq '<parent''s-message@example.com>'",
|
||||
);
|
||||
expect(messagesRequest.select).toHaveBeenCalledWith('id');
|
||||
expect(messagesRequest.top).toHaveBeenCalledWith(1);
|
||||
|
||||
expect(mockMicrosoftClient.api).toHaveBeenCalledWith(
|
||||
'/me/messages/parent-message-id/createReply',
|
||||
);
|
||||
expect(replyRequest.post).toHaveBeenCalledWith({});
|
||||
|
||||
expect(mockMicrosoftClient.api).toHaveBeenCalledWith(
|
||||
'/me/messages/reply-draft-id',
|
||||
);
|
||||
expect(draftRequest.patch).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
subject: 'Re: Existing thread',
|
||||
body: {
|
||||
contentType: 'HTML',
|
||||
content: '<p>HTML content</p>',
|
||||
},
|
||||
}),
|
||||
);
|
||||
expect(messagesRequest.post).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
+5
-4
@@ -97,12 +97,13 @@ export class MicrosoftMessageOutboundService implements MessageOutboundDriver {
|
||||
microsoftClient: MicrosoftGraphClient,
|
||||
internetMessageId: string,
|
||||
): Promise<string | undefined> {
|
||||
const encodedId = encodeURIComponent(internetMessageId);
|
||||
const escapedInternetMessageId = internetMessageId.split("'").join("''");
|
||||
|
||||
const response = await microsoftClient
|
||||
.api(
|
||||
`/me/messages?$filter=internetMessageId eq '${encodedId}'&$select=id&$top=1`,
|
||||
)
|
||||
.api('/me/messages')
|
||||
.filter(`internetMessageId eq '${escapedInternetMessageId}'`)
|
||||
.select('id')
|
||||
.top(1)
|
||||
.get();
|
||||
|
||||
return response?.value?.[0]?.id;
|
||||
|
||||
Reference in New Issue
Block a user