diff --git a/packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/__tests__/email-composer.service.spec.ts b/packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/__tests__/email-composer.service.spec.ts deleted file mode 100644 index ba1176602f..0000000000 --- a/packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/__tests__/email-composer.service.spec.ts +++ /dev/null @@ -1,94 +0,0 @@ -import { ConnectedAccountProvider } from 'twenty-shared/types'; - -import { EmailComposerService } from 'src/engine/core-modules/tool/tools/email-tool/email-composer.service'; - -const WORKSPACE_ID = '20202020-0000-4000-8000-000000000000'; -const CONNECTED_ACCOUNT_ID = '20202020-1111-4111-8111-111111111111'; - -const buildAccount = (id: string) => ({ - id, - handle: 'tim@apple.dev', - provider: ConnectedAccountProvider.GOOGLE, - scopes: ['email'], - connectionParameters: null, - messageChannels: [{ id: 'message-channel-1', handle: 'tim@apple.dev' }], -}); - -const baseParams = { - recipients: { to: 'test@example.com' }, - subject: 'Subject', - body: '

body

', - files: [], -}; - -const context = { workspaceId: WORKSPACE_ID }; - -describe('EmailComposerService connected account resolution', () => { - let service: EmailComposerService; - let connectedAccountRepository: { - findOne: jest.Mock; - find: jest.Mock; - }; - let globalWorkspaceOrmManager: { - executeInWorkspaceContext: jest.Mock; - getRepository: jest.Mock; - }; - - beforeEach(() => { - jest.clearAllMocks(); - - connectedAccountRepository = { findOne: jest.fn(), find: jest.fn() }; - globalWorkspaceOrmManager = { - executeInWorkspaceContext: jest.fn((callback) => callback()), - getRepository: jest.fn(), - }; - - service = new EmailComposerService( - globalWorkspaceOrmManager as never, - connectedAccountRepository as never, - { find: jest.fn() } as never, - {} as never, - ); - }); - - it('uses the connected account matching the provided id', async () => { - connectedAccountRepository.findOne.mockResolvedValue( - buildAccount(CONNECTED_ACCOUNT_ID), - ); - - const result = await service.composeEmail( - { ...baseParams, connectedAccountId: CONNECTED_ACCOUNT_ID }, - context, - ); - - expect(result.success).toBe(true); - expect(result.success && result.data.connectedAccount.id).toBe( - CONNECTED_ACCOUNT_ID, - ); - expect(connectedAccountRepository.findOne).toHaveBeenCalledWith( - expect.objectContaining({ - where: { id: CONNECTED_ACCOUNT_ID, workspaceId: WORKSPACE_ID }, - }), - ); - }); - - it('throws when the id is not a valid UUID', async () => { - await expect( - service.composeEmail( - { ...baseParams, connectedAccountId: 'not-a-uuid' }, - context, - ), - ).rejects.toThrow('Connected account id is not a valid UUID'); - }); - - it('throws when no connected account matches the provided id', async () => { - connectedAccountRepository.findOne.mockResolvedValue(null); - - await expect( - service.composeEmail( - { ...baseParams, connectedAccountId: CONNECTED_ACCOUNT_ID }, - context, - ), - ).rejects.toThrow(`No connected account found for id`); - }); -}); diff --git a/packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/email-composer.service.ts b/packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/email-composer.service.ts index 8293a26262..859e9bc9c8 100644 --- a/packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/email-composer.service.ts +++ b/packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/email-composer.service.ts @@ -9,7 +9,7 @@ import { ConnectedAccountProvider, type EmailAttachment, } from 'twenty-shared/types'; -import { isDefined, isValidUuid } from 'twenty-shared/utils'; +import { isDefined, isNonEmptyArray, isValidUuid } from 'twenty-shared/utils'; import { In, IsNull, LessThanOrEqual, type Repository } from 'typeorm'; import { z } from 'zod'; @@ -23,6 +23,7 @@ import { 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 { selectConnectedAccountIdForCaller } from 'src/engine/core-modules/tool/tools/email-tool/utils/select-connected-account-id-for-caller.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'; @@ -52,10 +53,13 @@ export class EmailComposerService { private readonly fileService: FileService, ) {} - private async getConnectedAccountOrThrow( - connectedAccountId: string, - workspaceId: string, - ): Promise { + private async getConnectedAccountOrThrow({ + connectedAccountId, + workspaceId, + }: { + connectedAccountId: string; + workspaceId: string; + }): Promise { if (!isValidUuid(connectedAccountId)) { throw new EmailToolException( `Connected account id is not a valid UUID`, @@ -89,25 +93,46 @@ export class EmailComposerService { ); } - private async getOrThrowFirstConnectedAccountId( - workspaceId: string, - ): Promise { + private async getDefaultConnectedAccountIdOrThrow({ + workspaceId, + userWorkspaceId, + }: { + workspaceId: string; + userWorkspaceId?: string; + }): Promise { const authContext = buildSystemAuthContext(workspaceId); return this.globalWorkspaceOrmManager.executeInWorkspaceContext( async () => { const allAccounts = await this.connectedAccountRepository.find({ where: { workspaceId, archivedAt: IsNull() }, + order: { createdAt: 'ASC', id: 'ASC' }, }); - if (!allAccounts || allAccounts.length === 0) { + if (!isNonEmptyArray(allAccounts)) { throw new EmailToolException( 'No connected accounts found for this workspace', EmailToolExceptionCode.CONNECTED_ACCOUNT_NOT_FOUND, ); } - return allAccounts[0].id; + if (!isDefined(userWorkspaceId)) { + return allAccounts[0].id; + } + + const connectedAccountId = selectConnectedAccountIdForCaller({ + connectedAccounts: allAccounts, + userWorkspaceId, + }); + + if (!isDefined(connectedAccountId)) { + throw new EmailToolException( + `No connected account available for user workspace '${userWorkspaceId}'`, + EmailToolExceptionCode.CONNECTED_ACCOUNT_NOT_FOUND, + ); + } + + return connectedAccountId; }, authContext, ); @@ -312,7 +337,7 @@ export class EmailComposerService { parameters: ComposeEmailParams, context: ToolExecutionContext, ): Promise { - const { workspaceId } = context; + const { workspaceId, userWorkspaceId } = context; const { subject, body, files, inReplyTo } = parameters; let { connectedAccountId } = parameters; @@ -351,14 +376,16 @@ export class EmailComposerService { const toRecipientsDisplay = recipients.to.join(', '); if (!connectedAccountId) { - connectedAccountId = - await this.getOrThrowFirstConnectedAccountId(workspaceId); + connectedAccountId = await this.getDefaultConnectedAccountIdOrThrow({ + workspaceId, + userWorkspaceId, + }); } - const connectedAccount = await this.getConnectedAccountOrThrow( + const connectedAccount = await this.getConnectedAccountOrThrow({ connectedAccountId, workspaceId, - ); + }); const messageChannel = connectedAccount.provider === ConnectedAccountProvider.EMAIL_GROUP diff --git a/packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/utils/__tests__/select-connected-account-id-for-caller.util.spec.ts b/packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/utils/__tests__/select-connected-account-id-for-caller.util.spec.ts new file mode 100644 index 0000000000..3fd3d78d6c --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/utils/__tests__/select-connected-account-id-for-caller.util.spec.ts @@ -0,0 +1,69 @@ +import { selectConnectedAccountIdForCaller } from 'src/engine/core-modules/tool/tools/email-tool/utils/select-connected-account-id-for-caller.util'; + +const USER_WORKSPACE_ID = '20202020-2222-4222-8222-222222222222'; +const OTHER_USER_WORKSPACE_ID = '20202020-3333-4333-8333-333333333333'; + +const ownAccount = { + id: 'own-account-id', + userWorkspaceId: USER_WORKSPACE_ID, + visibility: 'user' as const, +}; + +const colleagueAccount = { + id: 'colleague-account-id', + userWorkspaceId: OTHER_USER_WORKSPACE_ID, + visibility: 'user' as const, +}; + +const sharedAccount = { + id: 'shared-account-id', + userWorkspaceId: OTHER_USER_WORKSPACE_ID, + visibility: 'workspace' as const, +}; + +describe('selectConnectedAccountIdForCaller', () => { + it("returns the caller's own account even when another comes first", () => { + expect( + selectConnectedAccountIdForCaller({ + connectedAccounts: [colleagueAccount, ownAccount], + userWorkspaceId: USER_WORKSPACE_ID, + }), + ).toBe('own-account-id'); + }); + + it("prefers the caller's own account over a shared one", () => { + expect( + selectConnectedAccountIdForCaller({ + connectedAccounts: [sharedAccount, ownAccount], + userWorkspaceId: USER_WORKSPACE_ID, + }), + ).toBe('own-account-id'); + }); + + it('falls back to an account shared with the whole workspace', () => { + expect( + selectConnectedAccountIdForCaller({ + connectedAccounts: [colleagueAccount, sharedAccount], + userWorkspaceId: USER_WORKSPACE_ID, + }), + ).toBe('shared-account-id'); + }); + + it('returns undefined rather than a colleague account', () => { + expect( + selectConnectedAccountIdForCaller({ + connectedAccounts: [colleagueAccount], + userWorkspaceId: USER_WORKSPACE_ID, + }), + ).toBeUndefined(); + }); + + it('returns undefined when there is no account at all', () => { + expect( + selectConnectedAccountIdForCaller({ + connectedAccounts: [], + userWorkspaceId: USER_WORKSPACE_ID, + }), + ).toBeUndefined(); + }); +}); diff --git a/packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/utils/select-connected-account-id-for-caller.util.ts b/packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/utils/select-connected-account-id-for-caller.util.ts new file mode 100644 index 0000000000..b26c9c8ac6 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/utils/select-connected-account-id-for-caller.util.ts @@ -0,0 +1,25 @@ +import { isConnectedAccountUsableByCaller } from 'src/engine/metadata-modules/connected-account/utils/is-connected-account-usable-by-caller.util'; +import { type ConnectedAccountEntity } from 'src/engine/metadata-modules/connected-account/entities/connected-account.entity'; + +export const selectConnectedAccountIdForCaller = ({ + connectedAccounts, + userWorkspaceId, +}: { + connectedAccounts: Pick< + ConnectedAccountEntity, + 'id' | 'visibility' | 'userWorkspaceId' + >[]; + userWorkspaceId: string; +}): string | undefined => { + const ownAccount = connectedAccounts.find( + (connectedAccount) => connectedAccount.userWorkspaceId === userWorkspaceId, + ); + + const usableAccount = + ownAccount ?? + connectedAccounts.find((connectedAccount) => + isConnectedAccountUsableByCaller({ connectedAccount, userWorkspaceId }), + ); + + return usableAccount?.id; +}; diff --git a/packages/twenty-server/src/engine/metadata-modules/connected-account/connected-account-metadata.service.ts b/packages/twenty-server/src/engine/metadata-modules/connected-account/connected-account-metadata.service.ts index 70e3aeeccf..b3a0cb2040 100644 --- a/packages/twenty-server/src/engine/metadata-modules/connected-account/connected-account-metadata.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/connected-account/connected-account-metadata.service.ts @@ -17,6 +17,7 @@ import { } from 'src/engine/metadata-modules/connected-account/connected-account.exception'; import { ConnectedAccountEntity } from 'src/engine/metadata-modules/connected-account/entities/connected-account.entity'; import { type ConnectedAccountDeletedEvent } from 'src/engine/metadata-modules/connected-account/types/connected-account-deleted.type'; +import { isConnectedAccountUsableByCaller } from 'src/engine/metadata-modules/connected-account/utils/is-connected-account-usable-by-caller.util'; import { MESSAGE_CHANNEL_DELETED_EVENT } from 'src/engine/metadata-modules/message-channel/constants/message-channel-deleted.constant'; import { MessageChannelEntity } from 'src/engine/metadata-modules/message-channel/entities/message-channel.entity'; import { type MessageChannelDeletedEvent } from 'src/engine/metadata-modules/message-channel/types/message-channel-deleted.type'; @@ -95,8 +96,7 @@ export class ConnectedAccountMetadataService { } if ( - connectedAccount.visibility !== 'workspace' && - connectedAccount.userWorkspaceId !== userWorkspaceId + !isConnectedAccountUsableByCaller({ connectedAccount, userWorkspaceId }) ) { throw new ConnectedAccountException( `Connected account ${id} does not belong to user workspace ${userWorkspaceId}`, diff --git a/packages/twenty-server/src/engine/metadata-modules/connected-account/utils/__tests__/is-connected-account-usable-by-caller.util.spec.ts b/packages/twenty-server/src/engine/metadata-modules/connected-account/utils/__tests__/is-connected-account-usable-by-caller.util.spec.ts new file mode 100644 index 0000000000..63a0e8ecac --- /dev/null +++ b/packages/twenty-server/src/engine/metadata-modules/connected-account/utils/__tests__/is-connected-account-usable-by-caller.util.spec.ts @@ -0,0 +1,42 @@ +import { isConnectedAccountUsableByCaller } from 'src/engine/metadata-modules/connected-account/utils/is-connected-account-usable-by-caller.util'; + +const USER_WORKSPACE_ID = '20202020-2222-4222-8222-222222222222'; +const OTHER_USER_WORKSPACE_ID = '20202020-3333-4333-8333-333333333333'; + +describe('isConnectedAccountUsableByCaller', () => { + it('accepts an account the caller owns', () => { + expect( + isConnectedAccountUsableByCaller({ + connectedAccount: { + userWorkspaceId: USER_WORKSPACE_ID, + visibility: 'user', + }, + userWorkspaceId: USER_WORKSPACE_ID, + }), + ).toBe(true); + }); + + it('accepts an account shared with the whole workspace', () => { + expect( + isConnectedAccountUsableByCaller({ + connectedAccount: { + userWorkspaceId: OTHER_USER_WORKSPACE_ID, + visibility: 'workspace', + }, + userWorkspaceId: USER_WORKSPACE_ID, + }), + ).toBe(true); + }); + + it('rejects another user private account', () => { + expect( + isConnectedAccountUsableByCaller({ + connectedAccount: { + userWorkspaceId: OTHER_USER_WORKSPACE_ID, + visibility: 'user', + }, + userWorkspaceId: USER_WORKSPACE_ID, + }), + ).toBe(false); + }); +}); diff --git a/packages/twenty-server/src/engine/metadata-modules/connected-account/utils/is-connected-account-usable-by-caller.util.ts b/packages/twenty-server/src/engine/metadata-modules/connected-account/utils/is-connected-account-usable-by-caller.util.ts new file mode 100644 index 0000000000..dd22510c39 --- /dev/null +++ b/packages/twenty-server/src/engine/metadata-modules/connected-account/utils/is-connected-account-usable-by-caller.util.ts @@ -0,0 +1,14 @@ +import { type ConnectedAccountEntity } from 'src/engine/metadata-modules/connected-account/entities/connected-account.entity'; + +export const isConnectedAccountUsableByCaller = ({ + connectedAccount, + userWorkspaceId, +}: { + connectedAccount: Pick< + ConnectedAccountEntity, + 'visibility' | 'userWorkspaceId' + >; + userWorkspaceId: string; +}): boolean => + connectedAccount.visibility === 'workspace' || + connectedAccount.userWorkspaceId === userWorkspaceId; diff --git a/packages/twenty-server/test/integration/email-tool/suites/email-composer-connected-account.integration-spec.ts b/packages/twenty-server/test/integration/email-tool/suites/email-composer-connected-account.integration-spec.ts new file mode 100644 index 0000000000..0d2b67053e --- /dev/null +++ b/packages/twenty-server/test/integration/email-tool/suites/email-composer-connected-account.integration-spec.ts @@ -0,0 +1,157 @@ +import { EmailComposerService } from 'src/engine/core-modules/tool/tools/email-tool/email-composer.service'; + +import { getAppProviderByClassName } from 'test/integration/utils/get-app-provider-by-class-name.util'; + +const WORKSPACE_ID = '20202020-1c25-4d02-bf25-6aeccf7ea419'; + +const PHIL_USER_WORKSPACE_ID = '20202020-7169-42cf-bc47-1cfef15264b1'; +const PHIL_CONNECTED_ACCOUNT_ID = '20202020-cafc-4323-908d-e5b42ad69fdf'; + +const JONY_CONNECTED_ACCOUNT_ID = '20202020-0cc8-4d60-a3a4-803245698908'; + +const UNKNOWN_USER_WORKSPACE_ID = '20202020-0000-4000-8000-00000000dead'; +const UNKNOWN_CONNECTED_ACCOUNT_ID = '20202020-0000-4000-8000-00000000beef'; + +const baseParams = { + recipients: { to: 'customer@example.com' }, + subject: 'Subject', + body: '

body

', + files: [], +}; + +const getFirstWorkspaceConnectedAccountId = async (): Promise => { + const [{ id }] = await global.testDataSource.query( + `SELECT id FROM core."connectedAccount" + WHERE "workspaceId" = $1 AND "archivedAt" IS NULL + ORDER BY "createdAt" ASC, id ASC + LIMIT 1`, + [WORKSPACE_ID], + ); + + return id; +}; + +const setVisibility = async ( + connectedAccountId: string, + visibility: 'user' | 'workspace', +) => { + await global.testDataSource.query( + `UPDATE core."connectedAccount" SET visibility = $1 WHERE id = $2`, + [visibility, connectedAccountId], + ); +}; + +describe('EmailComposerService connected account resolution (integration)', () => { + let service: EmailComposerService; + + beforeAll(() => { + service = getAppProviderByClassName( + 'EmailComposerService', + ); + }); + + describe('when the caller names a connected account', () => { + it('uses that account, whoever owns it', async () => { + const result = await service.composeEmail( + { ...baseParams, connectedAccountId: JONY_CONNECTED_ACCOUNT_ID }, + { workspaceId: WORKSPACE_ID, userWorkspaceId: PHIL_USER_WORKSPACE_ID }, + ); + + expect(result.success).toBe(true); + expect(result.success && result.data.connectedAccount.id).toBe( + JONY_CONNECTED_ACCOUNT_ID, + ); + }); + + it('uses that account when there is no caller (workflow run)', async () => { + const result = await service.composeEmail( + { ...baseParams, connectedAccountId: JONY_CONNECTED_ACCOUNT_ID }, + { workspaceId: WORKSPACE_ID }, + ); + + expect(result.success).toBe(true); + expect(result.success && result.data.connectedAccount.id).toBe( + JONY_CONNECTED_ACCOUNT_ID, + ); + }); + + it('throws when the id is not a valid UUID', async () => { + await expect( + service.composeEmail( + { ...baseParams, connectedAccountId: 'not-a-uuid' }, + { + workspaceId: WORKSPACE_ID, + userWorkspaceId: PHIL_USER_WORKSPACE_ID, + }, + ), + ).rejects.toThrow('Connected account id is not a valid UUID'); + }); + + it('throws when no connected account matches the id', async () => { + await expect( + service.composeEmail( + { ...baseParams, connectedAccountId: UNKNOWN_CONNECTED_ACCOUNT_ID }, + { + workspaceId: WORKSPACE_ID, + userWorkspaceId: PHIL_USER_WORKSPACE_ID, + }, + ), + ).rejects.toThrow('No connected account found for id'); + }); + }); + + describe('when the caller names none', () => { + it('composes from the caller own account rather than the first of the workspace', async () => { + const result = await service.composeEmail(baseParams, { + workspaceId: WORKSPACE_ID, + userWorkspaceId: PHIL_USER_WORKSPACE_ID, + }); + + expect(result.success).toBe(true); + expect(result.success && result.data.connectedAccount.id).toBe( + PHIL_CONNECTED_ACCOUNT_ID, + ); + }); + + it('falls back to an account shared with the whole workspace', async () => { + await setVisibility(JONY_CONNECTED_ACCOUNT_ID, 'workspace'); + + try { + const result = await service.composeEmail(baseParams, { + workspaceId: WORKSPACE_ID, + userWorkspaceId: UNKNOWN_USER_WORKSPACE_ID, + }); + + expect(result.success).toBe(true); + expect(result.success && result.data.connectedAccount.id).toBe( + JONY_CONNECTED_ACCOUNT_ID, + ); + } finally { + await setVisibility(JONY_CONNECTED_ACCOUNT_ID, 'user'); + } + }); + + it('throws rather than composing from a colleague account', async () => { + await expect( + service.composeEmail(baseParams, { + workspaceId: WORKSPACE_ID, + userWorkspaceId: UNKNOWN_USER_WORKSPACE_ID, + }), + ).rejects.toThrow('No connected account available for user workspace'); + }); + + it('takes the first workspace account when there is no caller (workflow run)', async () => { + const firstWorkspaceConnectedAccountId = + await getFirstWorkspaceConnectedAccountId(); + + const result = await service.composeEmail(baseParams, { + workspaceId: WORKSPACE_ID, + }); + + expect(result.success).toBe(true); + expect(result.success && result.data.connectedAccount.id).toBe( + firstWorkspaceConnectedAccountId, + ); + }); + }); +});