diff --git a/packages/twenty-front/src/modules/activities/emails/hooks/__tests__/useOpenEmailInAppOrFallback.test.tsx b/packages/twenty-front/src/modules/activities/emails/hooks/__tests__/useOpenEmailInAppOrFallback.test.tsx new file mode 100644 index 00000000000..93282622ac5 --- /dev/null +++ b/packages/twenty-front/src/modules/activities/emails/hooks/__tests__/useOpenEmailInAppOrFallback.test.tsx @@ -0,0 +1,102 @@ +import { act, renderHook } from '@testing-library/react'; + +import { useOpenEmailInAppOrFallback } from '@/activities/emails/hooks/useOpenEmailInAppOrFallback'; + +const MOCK_CONNECTED_ACCOUNT_ID = 'connected-account-1'; + +const mockOpenComposeEmailInSidePanel = jest.fn(); + +jest.mock('@/side-panel/hooks/useOpenComposeEmailInSidePanel', () => ({ + useOpenComposeEmailInSidePanel: () => ({ + openComposeEmailInSidePanel: mockOpenComposeEmailInSidePanel, + }), +})); + +const mockUseFirstConnectedAccount = jest.fn(); + +jest.mock('@/activities/emails/hooks/useFirstConnectedAccount', () => ({ + useFirstConnectedAccount: (opts: unknown) => + mockUseFirstConnectedAccount(opts), +})); + +describe('useOpenEmailInAppOrFallback', () => { + let windowOpenSpy: jest.SpyInstance; + + beforeEach(() => { + jest.clearAllMocks(); + windowOpenSpy = jest.spyOn(window, 'open').mockImplementation(); + }); + + afterEach(() => { + windowOpenSpy.mockRestore(); + }); + + it('should open the side-panel composer when a connected account exists', () => { + mockUseFirstConnectedAccount.mockReturnValue({ + connectedAccountId: MOCK_CONNECTED_ACCOUNT_ID, + connectedAccountHandle: 'user@example.com', + loading: false, + }); + + const { result } = renderHook(() => useOpenEmailInAppOrFallback()); + + act(() => { + result.current.openEmail('alice@test.com'); + }); + + expect(mockOpenComposeEmailInSidePanel).toHaveBeenCalledWith({ + connectedAccountId: MOCK_CONNECTED_ACCOUNT_ID, + defaultTo: 'alice@test.com', + }); + expect(windowOpenSpy).not.toHaveBeenCalled(); + }); + + it('should fall back to mailto when no connected account exists', () => { + mockUseFirstConnectedAccount.mockReturnValue({ + connectedAccountId: null, + connectedAccountHandle: null, + loading: false, + }); + + const { result } = renderHook(() => useOpenEmailInAppOrFallback()); + + act(() => { + result.current.openEmail('bob@test.com'); + }); + + expect(windowOpenSpy).toHaveBeenCalledWith('mailto:bob@test.com', '_blank'); + expect(mockOpenComposeEmailInSidePanel).not.toHaveBeenCalled(); + }); + + it('should fall back to mailto when query is still loading', () => { + mockUseFirstConnectedAccount.mockReturnValue({ + connectedAccountId: null, + connectedAccountHandle: null, + loading: true, + }); + + const { result } = renderHook(() => useOpenEmailInAppOrFallback()); + + act(() => { + result.current.openEmail('eager@test.com'); + }); + + expect(windowOpenSpy).toHaveBeenCalledWith( + 'mailto:eager@test.com', + '_blank', + ); + expect(mockOpenComposeEmailInSidePanel).not.toHaveBeenCalled(); + }); + + it('should pass skip option to useFirstConnectedAccount', () => { + mockUseFirstConnectedAccount.mockReturnValue({ + connectedAccountId: null, + connectedAccountHandle: null, + loading: false, + }); + + renderHook(() => useOpenEmailInAppOrFallback({ skip: true })); + + expect(mockUseFirstConnectedAccount).toHaveBeenCalledWith({ skip: true }); + }); +}); diff --git a/packages/twenty-front/src/modules/activities/emails/hooks/__tests__/useResolveDefaultEmailRecipient.test.ts b/packages/twenty-front/src/modules/activities/emails/hooks/__tests__/useResolveDefaultEmailRecipient.test.ts new file mode 100644 index 00000000000..c5f97e1acbf --- /dev/null +++ b/packages/twenty-front/src/modules/activities/emails/hooks/__tests__/useResolveDefaultEmailRecipient.test.ts @@ -0,0 +1,171 @@ +import { renderHook } from '@testing-library/react'; + +import { useResolveDefaultEmailRecipient } from '@/activities/emails/hooks/useResolveDefaultEmailRecipient'; +import { CoreObjectNameSingular } from 'twenty-shared/types'; + +const mockUseFindOneRecord = jest.fn(); +const mockUseFindManyRecords = jest.fn(); + +jest.mock('@/object-record/hooks/useFindOneRecord', () => ({ + useFindOneRecord: (args: unknown) => mockUseFindOneRecord(args), +})); + +jest.mock('@/object-record/hooks/useFindManyRecords', () => ({ + useFindManyRecords: (args: unknown) => mockUseFindManyRecords(args), +})); + +describe('useResolveDefaultEmailRecipient', () => { + beforeEach(() => { + jest.clearAllMocks(); + + mockUseFindOneRecord.mockReturnValue({ + record: null, + loading: false, + }); + mockUseFindManyRecords.mockReturnValue({ + records: [], + loading: false, + }); + }); + + it('should return the person primary email for a Person record', () => { + mockUseFindOneRecord.mockImplementation( + (args: { objectNameSingular: string }) => { + if (args.objectNameSingular === CoreObjectNameSingular.Person) { + return { + record: { emails: { primaryEmail: 'person@example.com' } }, + loading: false, + }; + } + + return { record: null, loading: false }; + }, + ); + + const { result } = renderHook(() => + useResolveDefaultEmailRecipient({ + objectNameSingular: CoreObjectNameSingular.Person, + recordId: 'person-id', + }), + ); + + expect(result.current.defaultTo).toBe('person@example.com'); + expect(result.current.loading).toBe(false); + }); + + it('should return the first company employee email for a Company record', () => { + mockUseFindManyRecords.mockReturnValue({ + records: [{ emails: { primaryEmail: 'employee@company.com' } }], + loading: false, + }); + + const { result } = renderHook(() => + useResolveDefaultEmailRecipient({ + objectNameSingular: CoreObjectNameSingular.Company, + recordId: 'company-id', + }), + ); + + expect(result.current.defaultTo).toBe('employee@company.com'); + }); + + it('should return the opportunity point of contact email', () => { + mockUseFindOneRecord.mockImplementation( + (args: { objectNameSingular: string }) => { + if (args.objectNameSingular === CoreObjectNameSingular.Opportunity) { + return { + record: { + pointOfContact: { + emails: { primaryEmail: 'contact@opp.com' }, + }, + }, + loading: false, + }; + } + + return { record: null, loading: false }; + }, + ); + + const { result } = renderHook(() => + useResolveDefaultEmailRecipient({ + objectNameSingular: CoreObjectNameSingular.Opportunity, + recordId: 'opp-id', + }), + ); + + expect(result.current.defaultTo).toBe('contact@opp.com'); + }); + + it('should return empty string for unknown object types', () => { + const { result } = renderHook(() => + useResolveDefaultEmailRecipient({ + objectNameSingular: 'customObject', + recordId: 'some-id', + }), + ); + + expect(result.current.defaultTo).toBe(''); + expect(result.current.loading).toBe(false); + }); + + it('should return empty string when recordId is null', () => { + const { result } = renderHook(() => + useResolveDefaultEmailRecipient({ + objectNameSingular: CoreObjectNameSingular.Person, + recordId: null, + }), + ); + + expect(result.current.defaultTo).toBe(''); + }); + + it('should report loading when the relevant query is in flight', () => { + mockUseFindOneRecord.mockImplementation( + (args: { objectNameSingular: string }) => { + if (args.objectNameSingular === CoreObjectNameSingular.Person) { + return { record: null, loading: true }; + } + + return { record: null, loading: false }; + }, + ); + + const { result } = renderHook(() => + useResolveDefaultEmailRecipient({ + objectNameSingular: CoreObjectNameSingular.Person, + recordId: 'person-id', + }), + ); + + expect(result.current.loading).toBe(true); + }); + + it('should pass skip=true to queries for non-matching object types', () => { + renderHook(() => + useResolveDefaultEmailRecipient({ + objectNameSingular: CoreObjectNameSingular.Person, + recordId: 'person-id', + }), + ); + + // Person query should NOT be skipped + const personCall = mockUseFindOneRecord.mock.calls.find( + (call: { objectNameSingular: string }[]) => + call[0].objectNameSingular === CoreObjectNameSingular.Person, + ); + + expect(personCall?.[0].skip).toBe(false); + + // Opportunity query SHOULD be skipped + const oppCall = mockUseFindOneRecord.mock.calls.find( + (call: { objectNameSingular: string }[]) => + call[0].objectNameSingular === CoreObjectNameSingular.Opportunity, + ); + + expect(oppCall?.[0].skip).toBe(true); + + // Company people query SHOULD be skipped + expect(mockUseFindManyRecords.mock.calls[0][0].skip).toBe(true); + }); +}); diff --git a/packages/twenty-front/src/modules/activities/emails/hooks/useFirstConnectedAccount.ts b/packages/twenty-front/src/modules/activities/emails/hooks/useFirstConnectedAccount.ts index 0a09f02c23f..583e8a93a3f 100644 --- a/packages/twenty-front/src/modules/activities/emails/hooks/useFirstConnectedAccount.ts +++ b/packages/twenty-front/src/modules/activities/emails/hooks/useFirstConnectedAccount.ts @@ -2,10 +2,18 @@ import { useQuery } from '@apollo/client/react'; import { GET_MY_CONNECTED_ACCOUNTS } from '@/settings/accounts/graphql/queries/getMyConnectedAccounts'; -export const useFirstConnectedAccount = () => { +type UseFirstConnectedAccountOptions = { + skip?: boolean; +}; + +export const useFirstConnectedAccount = ( + options?: UseFirstConnectedAccountOptions, +) => { const { data, loading } = useQuery<{ myConnectedAccounts: { id: string; handle: string }[]; - }>(GET_MY_CONNECTED_ACCOUNTS); + }>(GET_MY_CONNECTED_ACCOUNTS, { + skip: options?.skip, + }); const firstAccount = data?.myConnectedAccounts?.[0] ?? null; diff --git a/packages/twenty-front/src/modules/activities/emails/hooks/useOpenEmailInAppOrFallback.ts b/packages/twenty-front/src/modules/activities/emails/hooks/useOpenEmailInAppOrFallback.ts index 45fafddbf39..c480374e600 100644 --- a/packages/twenty-front/src/modules/activities/emails/hooks/useOpenEmailInAppOrFallback.ts +++ b/packages/twenty-front/src/modules/activities/emails/hooks/useOpenEmailInAppOrFallback.ts @@ -4,12 +4,23 @@ import { useFirstConnectedAccount } from '@/activities/emails/hooks/useFirstConn import { useOpenComposeEmailInSidePanel } from '@/side-panel/hooks/useOpenComposeEmailInSidePanel'; import { isDefined } from 'twenty-shared/utils'; +type UseOpenEmailInAppOrFallbackOptions = { + // When true the underlying connected-account query is skipped entirely, + // avoiding an unnecessary network request for non-email field types or when + // the click action is not OPEN_IN_APP. + skip?: boolean; +}; + // Opens the in-app email composer for the given email address. When the user // has no connected account we cannot send through the side panel, so we fall // back to the OS-level mailto handler instead of redirecting to settings — // the user explicitly opted into "Open in app" and the link is still useful. -export const useOpenEmailInAppOrFallback = () => { - const { connectedAccountId, loading } = useFirstConnectedAccount(); +export const useOpenEmailInAppOrFallback = ( + options?: UseOpenEmailInAppOrFallbackOptions, +) => { + const { connectedAccountId, loading } = useFirstConnectedAccount({ + skip: options?.skip, + }); const { openComposeEmailInSidePanel } = useOpenComposeEmailInSidePanel(); const openEmail = useCallback( diff --git a/packages/twenty-front/src/modules/activities/emails/utils/__tests__/getPrimaryEmailFromRecord.test.ts b/packages/twenty-front/src/modules/activities/emails/utils/__tests__/getPrimaryEmailFromRecord.test.ts new file mode 100644 index 00000000000..09e1b40edee --- /dev/null +++ b/packages/twenty-front/src/modules/activities/emails/utils/__tests__/getPrimaryEmailFromRecord.test.ts @@ -0,0 +1,52 @@ +import { getPrimaryEmailFromRecord } from '@/activities/emails/utils/getPrimaryEmailFromRecord'; +import { type ObjectRecord } from '@/object-record/types/ObjectRecord'; + +const makeRecord = (emails: unknown): ObjectRecord => + ({ + id: 'test-id', + emails, + }) as unknown as ObjectRecord; + +describe('getPrimaryEmailFromRecord', () => { + it('should return the primary email when present', () => { + const record = makeRecord({ primaryEmail: 'alice@example.com' }); + + expect(getPrimaryEmailFromRecord(record)).toBe('alice@example.com'); + }); + + it('should return null when emails is null', () => { + const record = makeRecord(null); + + expect(getPrimaryEmailFromRecord(record)).toBeNull(); + }); + + it('should return null when emails is undefined', () => { + const record = makeRecord(undefined); + + expect(getPrimaryEmailFromRecord(record)).toBeNull(); + }); + + it('should return null when emails is not an object', () => { + const record = makeRecord('not-an-object'); + + expect(getPrimaryEmailFromRecord(record)).toBeNull(); + }); + + it('should return null when primaryEmail is missing', () => { + const record = makeRecord({ additionalEmails: ['b@example.com'] }); + + expect(getPrimaryEmailFromRecord(record)).toBeNull(); + }); + + it('should return null when primaryEmail is an empty string', () => { + const record = makeRecord({ primaryEmail: '' }); + + expect(getPrimaryEmailFromRecord(record)).toBeNull(); + }); + + it('should return null when primaryEmail is not a string', () => { + const record = makeRecord({ primaryEmail: 42 }); + + expect(getPrimaryEmailFromRecord(record)).toBeNull(); + }); +}); diff --git a/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/components/EmailsFieldDisplay.tsx b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/components/EmailsFieldDisplay.tsx index f53067900f9..7ce13231913 100644 --- a/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/components/EmailsFieldDisplay.tsx +++ b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/components/EmailsFieldDisplay.tsx @@ -9,7 +9,6 @@ import { useCopyToClipboard } from '~/hooks/useCopyToClipboard'; export const EmailsFieldDisplay = () => { const { fieldValue, fieldDefinition } = useEmailsFieldDisplay(); const { copyToClipboard } = useCopyToClipboard(); - const { openEmail } = useOpenEmailInAppOrFallback(); const { t } = useLingui(); // Email fields default to opening the in-app composer when no setting is @@ -19,6 +18,13 @@ export const EmailsFieldDisplay = () => { fieldDefinition.metadata.settings?.clickAction ?? FieldMetadataSettingsOnClickAction.OPEN_IN_APP; + const isOpenInApp = + onClickAction === FieldMetadataSettingsOnClickAction.OPEN_IN_APP; + + // Only fire the connected-account query when the click action is + // OPEN_IN_APP — COPY and OPEN_LINK don't need it. + const { openEmail } = useOpenEmailInAppOrFallback({ skip: !isOpenInApp }); + const handleEmailClick = ( email: string, event: React.MouseEvent, @@ -30,10 +36,14 @@ export const EmailsFieldDisplay = () => { return; } - if (onClickAction === FieldMetadataSettingsOnClickAction.OPEN_IN_APP) { + if (isOpenInApp) { event.preventDefault(); openEmail(email); + + return; } + + // OPEN_LINK: let the native behaviour handle it. }; return ; diff --git a/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/useGetSecondaryRecordTableCellButton.ts b/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/useGetSecondaryRecordTableCellButton.ts index 78bfedc6807..b8234881a4f 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/useGetSecondaryRecordTableCellButton.ts +++ b/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/useGetSecondaryRecordTableCellButton.ts @@ -19,7 +19,12 @@ import { useCopyToClipboard } from '~/hooks/useCopyToClipboard'; export const useGetSecondaryRecordTableCellButton = () => { const { fieldDefinition, recordId } = useContext(FieldContext); const { copyToClipboard } = useCopyToClipboard(); - const { openEmail } = useOpenEmailInAppOrFallback(); + + const isEmailField = isFieldEmails(fieldDefinition); + + // Only fire the connected-account query for email fields — phone and link + // fields never use the in-app composer so the request would be wasted. + const { openEmail } = useOpenEmailInAppOrFallback({ skip: !isEmailField }); const fieldValue = useRecordFieldValue< FieldPhonesValue | FieldEmailsValue | FieldLinksValue | undefined @@ -28,7 +33,7 @@ export const useGetSecondaryRecordTableCellButton = () => { if ( (!isFieldPhones(fieldDefinition) && !isFieldLinks(fieldDefinition) && - !isFieldEmails(fieldDefinition)) || + !isEmailField) || !isDefined(fieldValue) ) { return []; @@ -36,7 +41,6 @@ export const useGetSecondaryRecordTableCellButton = () => { // Email fields default to opening the in-app composer; other field types // default to opening the platform link handler. - const isEmailField = isFieldEmails(fieldDefinition); const defaultClickAction = isEmailField ? FieldMetadataSettingsOnClickAction.OPEN_IN_APP : FieldMetadataSettingsOnClickAction.OPEN_LINK;