fix: address code review — skip unnecessary queries, add tests

Performance:
- Add `skip` option to useFirstConnectedAccount and
  useOpenEmailInAppOrFallback so the connected-account Apollo query is
  only fired when actually needed.
- useGetSecondaryRecordTableCellButton now skips the query for non-email
  field types (phones, links).
- EmailsFieldDisplay skips the query when the click action is COPY or
  OPEN_LINK.

Clarity:
- Add explicit OPEN_LINK fallthrough comment in EmailsFieldDisplay.

Tests (18 new):
- getPrimaryEmailFromRecord: 7 cases (happy path + all null/edge cases)
- useResolveDefaultEmailRecipient: 7 cases (person/company/opportunity
  resolution, unknown types, null recordId, loading, skip propagation)
- useOpenEmailInAppOrFallback: 4 cases (side-panel open, mailto
  fallback, loading fallback, skip propagation)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

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 });
+  });
+});

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);
+  });
+});

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;
 

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(

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();
+  });
+});

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<HTMLElement>,
@@ -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 <a href="mailto:…"> behaviour handle it.
   };
 
   return <EmailsDisplay value={fieldValue} onEmailClick={handleEmailClick} />;

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;