fix(contact-creation): enrich missing names on auto-created contacts (#21018)

## Summary Three related fixes to the auto-creation of People records from calendar events and email messages, all centred on the data-quality problem of contacts being created with missing or malformed names. ### 1. Enrich names on existing contacts (commit 1) Previously: when an email or calendar import matched an existing Person by email, the existing record was left untouched — even if the new source carried a better name. This is the root cause of contacts like `"Félix"` (no last name) sticking around forever: the `To:`/`Cc:` headers of outbound emails rarely include a display name, and Google Calendar only returns `displayName` for attendees already in the organizer's address book. So the first sighting often creates a Person as `{firstName: "felix", lastName: ""}`, and a later inbound `From: "Félix Malfait" <felix@twenty.com>` — which would have produced the right name — gets silently dropped because the Person already exists. The new `computePeopleToEnrichNames` bucket and `CreatePersonService.enrichPeopleNames` method fill in missing `firstName`/`lastName` fields from the new parsed name, with conservative rules: - Only enrich when the existing Person's `createdBy.source` is `CALENDAR` or `EMAIL` — `MANUAL`, `IMPORT`, `API`, `WORKFLOW`, etc. are never touched. - Only fill empty fields. Non-empty `firstName`/`lastName` are never overwritten. - Soft-deleted contacts continue to be handled by the existing restore path. ### 2. Handle multi-comma "Last, First, Suffix" display names (commit 2) The comma-inverted swap in the parser previously required *exactly* one comma. Names like `"Smith, Jane, Jr."`, `"O'Brien, Mary, MD"` or `"Doe, John, Patrick"` fell through to the space-split fallback, which stored the comma in `firstName` (e.g. `"Smith,"`) and produced garbled records (the avatar shows a single "B" and the name reads `"Barbey, Julien"` because the entire string lives in `firstName`). The regex now splits on the first comma and treats the remainder as the first name, collapsing any further commas to spaces. Single-comma behaviour is unchanged. ### 3. Perf: skip the parser when an existing record is already populated (commit 3) `computePeopleToEnrichNames` runs on every cron-driven email/calendar import batch. The first version called the display-name parser for every matched existing person, even when both `firstName` and `lastName` were already set — i.e. the steady-state case after the initial enrichment pass. Reordered so the cheap "both fields populated" check short-circuits before any parsing happens. Same behaviour, fewer parser calls on the hot path. ## Test plan - [x] 8 new unit tests for the enrichment bucket: empty `lastName` enrichment, both `EMAIL` and `CALENDAR` sources, non-overwrite of non-empty fields, skip on `MANUAL`/`IMPORT`, skip when the new source also has no last name, skip for soft-deleted, fill `firstName` while preserving `lastName`, handle null `name` field - [x] 5 new parser tests for multi-comma forms: `"Last, First, Suffix"`, credential suffixes (`MD`), three-token forms, whitespace around inner commas, `:GROUP` tag interaction - [x] 1 new parser test on the single-comma path covering multi-word first names (`"Smith, Mary Jane"`) - [x] All 15 existing parser tests still pass - [x] All 116 tests in `contact-creation-manager` pass - [x] `npx nx typecheck twenty-server` - [x] `npx oxlint --type-aware` + `npx oxfmt --check` on changed files - [ ] Manual: trigger a fresh contact creation from an outbound email with no display name, then a subsequent inbound email from the same address with a full display name, and confirm the Person's last name gets populated
2026-05-28 20:49:56 +02:00
parent f4ead89956
commit 8a74ea8829
5 changed files with 457 additions and 6 deletions
@@ -30,6 +30,7 @@ describe('CreateCompanyAndPersonService', () => {
    };
    const mockCreatePersonService = {
      restorePeople: jest.fn(),
+      enrichPeopleNames: jest.fn(),
    };

    const module: TestingModule = await Test.createTestingModule({
@@ -148,5 +149,258 @@ describe('CreateCompanyAndPersonService', () => {
        result.contactsThatNeedPersonRestore.map((c) => c.handle),
      ).toContain('jane.smith@company.com');
    });
+
+    describe('peopleToEnrichNames', () => {
+      const contact: Contact = {
+        handle: 'felix@twenty.com',
+        displayName: 'Félix Malfait',
+      };
+
+      const buildExistingPerson = (
+        overrides: Record<string, unknown>,
+      ): PersonWorkspaceEntity =>
+        ({
+          id: 'existing-person-1',
+          emails: {
+            primaryEmail: 'felix@twenty.com',
+            additionalEmails: null,
+          },
+          name: { firstName: 'Félix', lastName: '' },
+          createdBy: { source: FieldActorSource.EMAIL },
+          deletedAt: null,
+          ...overrides,
+        }) as unknown as PersonWorkspaceEntity;
+
+      it('should enrich an empty lastName on an EMAIL-created contact', () => {
+        const result =
+          service.computeContactsThatNeedPersonCreateAndRestoreAndWorkDomainNamesToCreate(
+            [contact],
+            [buildExistingPerson({})],
+            FieldActorSource.CALENDAR,
+            mockConnectedAccount,
+            null,
+          );
+
+        expect(result.peopleToEnrichNames).toEqual([
+          {
+            personId: 'existing-person-1',
+            name: { firstName: 'Félix', lastName: 'Malfait' },
+          },
+        ]);
+      });
+
+      it('should enrich a contact created from CALENDAR too', () => {
+        const result =
+          service.computeContactsThatNeedPersonCreateAndRestoreAndWorkDomainNamesToCreate(
+            [contact],
+            [
+              buildExistingPerson({
+                createdBy: {
+                  source: FieldActorSource.CALENDAR,
+                } as PersonWorkspaceEntity['createdBy'],
+              }),
+            ],
+            FieldActorSource.EMAIL,
+            mockConnectedAccount,
+            null,
+          );
+
+        expect(result.peopleToEnrichNames).toHaveLength(1);
+      });
+
+      it('should not overwrite an existing non-empty lastName', () => {
+        const result =
+          service.computeContactsThatNeedPersonCreateAndRestoreAndWorkDomainNamesToCreate(
+            [{ handle: 'felix@twenty.com', displayName: 'Felix Smith' }],
+            [
+              buildExistingPerson({
+                name: { firstName: 'Félix', lastName: 'Malfait' },
+              }),
+            ],
+            FieldActorSource.EMAIL,
+            mockConnectedAccount,
+            null,
+          );
+
+        expect(result.peopleToEnrichNames).toEqual([]);
+      });
+
+      it('should not touch a manually-created contact', () => {
+        const result =
+          service.computeContactsThatNeedPersonCreateAndRestoreAndWorkDomainNamesToCreate(
+            [contact],
+            [
+              buildExistingPerson({
+                createdBy: {
+                  source: FieldActorSource.MANUAL,
+                } as PersonWorkspaceEntity['createdBy'],
+              }),
+            ],
+            FieldActorSource.EMAIL,
+            mockConnectedAccount,
+            null,
+          );
+
+        expect(result.peopleToEnrichNames).toEqual([]);
+      });
+
+      it('should not touch an IMPORT-created contact', () => {
+        const result =
+          service.computeContactsThatNeedPersonCreateAndRestoreAndWorkDomainNamesToCreate(
+            [contact],
+            [
+              buildExistingPerson({
+                createdBy: {
+                  source: FieldActorSource.IMPORT,
+                } as PersonWorkspaceEntity['createdBy'],
+              }),
+            ],
+            FieldActorSource.EMAIL,
+            mockConnectedAccount,
+            null,
+          );
+
+        expect(result.peopleToEnrichNames).toEqual([]);
+      });
+
+      it('should skip enrichment when the new displayName provides no last name either', () => {
+        const result =
+          service.computeContactsThatNeedPersonCreateAndRestoreAndWorkDomainNamesToCreate(
+            [{ handle: 'felix@twenty.com', displayName: 'Félix' }],
+            [buildExistingPerson({})],
+            FieldActorSource.EMAIL,
+            mockConnectedAccount,
+            null,
+          );
+
+        expect(result.peopleToEnrichNames).toEqual([]);
+      });
+
+      it('should enrich a soft-deleted contact that will be restored in this same batch', () => {
+        // Restore runs before enrich in createCompaniesAndPeople — by the time
+        // the enrichment UPDATE fires, the row is no longer soft-deleted.
+        const result =
+          service.computeContactsThatNeedPersonCreateAndRestoreAndWorkDomainNamesToCreate(
+            [contact],
+            [buildExistingPerson({ deletedAt: new Date() })],
+            FieldActorSource.EMAIL,
+            mockConnectedAccount,
+            null,
+          );
+
+        expect(result.peopleToEnrichNames).toEqual([
+          {
+            personId: 'existing-person-1',
+            name: { firstName: 'Félix', lastName: 'Malfait' },
+          },
+        ]);
+      });
+
+      it('should fill firstName when missing and preserve existing lastName', () => {
+        const result =
+          service.computeContactsThatNeedPersonCreateAndRestoreAndWorkDomainNamesToCreate(
+            [contact],
+            [
+              buildExistingPerson({
+                name: { firstName: '', lastName: 'Malfait' },
+              }),
+            ],
+            FieldActorSource.EMAIL,
+            mockConnectedAccount,
+            null,
+          );
+
+        expect(result.peopleToEnrichNames).toEqual([
+          {
+            personId: 'existing-person-1',
+            name: { firstName: 'Félix', lastName: 'Malfait' },
+          },
+        ]);
+      });
+
+      it('should handle a person with a null name field', () => {
+        const result =
+          service.computeContactsThatNeedPersonCreateAndRestoreAndWorkDomainNamesToCreate(
+            [contact],
+            [buildExistingPerson({ name: null })],
+            FieldActorSource.EMAIL,
+            mockConnectedAccount,
+            null,
+          );
+
+        expect(result.peopleToEnrichNames).toEqual([
+          {
+            personId: 'existing-person-1',
+            name: { firstName: 'Félix', lastName: 'Malfait' },
+          },
+        ]);
+      });
+
+      it('should not emit two enrichments when one Person matches both primary and additional emails in the batch', () => {
+        const existingPersonWithMultipleEmails = buildExistingPerson({
+          emails: {
+            primaryEmail: 'felix@twenty.com',
+            additionalEmails: ['felix.personal@example.com'],
+          },
+        });
+
+        const result =
+          service.computeContactsThatNeedPersonCreateAndRestoreAndWorkDomainNamesToCreate(
+            [
+              { handle: 'felix@twenty.com', displayName: 'Félix Malfait' },
+              {
+                handle: 'felix.personal@example.com',
+                displayName: 'Félix Other',
+              },
+            ],
+            [existingPersonWithMultipleEmails],
+            FieldActorSource.EMAIL,
+            mockConnectedAccount,
+            null,
+          );
+
+        expect(result.peopleToEnrichNames).toEqual([
+          {
+            personId: 'existing-person-1',
+            name: { firstName: 'Félix', lastName: 'Malfait' },
+          },
+        ]);
+      });
+
+      it('should merge partial enrichments across contacts mapping to the same Person', () => {
+        const existingPersonWithMultipleEmails = buildExistingPerson({
+          name: { firstName: '', lastName: '' },
+          emails: {
+            primaryEmail: 'felix@twenty.com',
+            additionalEmails: ['felix.personal@example.com'],
+          },
+        });
+
+        const result =
+          service.computeContactsThatNeedPersonCreateAndRestoreAndWorkDomainNamesToCreate(
+            [
+              // First contact only carries a first name.
+              { handle: 'felix@twenty.com', displayName: 'Félix' },
+              // Second contact (additional email) carries both — the lastName
+              // should fill in even though the firstName slot is already taken.
+              {
+                handle: 'felix.personal@example.com',
+                displayName: 'Félix Malfait',
+              },
+            ],
+            [existingPersonWithMultipleEmails],
+            FieldActorSource.EMAIL,
+            mockConnectedAccount,
+            null,
+          );
+
+        expect(result.peopleToEnrichNames).toEqual([
+          {
+            personId: 'existing-person-1',
+            name: { firstName: 'Félix', lastName: 'Malfait' },
+          },
+        ]);
+      });
+    });
  });
 });
@@ -6,7 +6,8 @@ import chunk from 'lodash.chunk';
 import compact from 'lodash.compact';
 import {
  ConnectedAccountProvider,
-  type FieldActorSource,
+  FieldActorSource,
+  type FullNameMetadata,
 } from 'twenty-shared/types';
 import { isDefined } from 'twenty-shared/utils';
 import { type DeepPartial, type Repository } from 'typeorm';
@@ -112,6 +113,7 @@ export class CreateCompanyAndPersonService {
        const {
          contactsThatNeedPersonCreate,
          contactsThatNeedPersonRestore,
+          peopleToEnrichNames,
          workDomainNamesToCreate,
          shouldCreateOrRestorePeopleByHandleMap,
        } =
@@ -157,6 +159,11 @@ export class CreateCompanyAndPersonService {
          workspaceId,
        );

+        await this.createPersonService.enrichPeopleNames(
+          peopleToEnrichNames,
+          workspaceId,
+        );
+
        return { ...createdPeople, ...restoredPeople };
      },
      authContext,
@@ -296,6 +303,11 @@ export class CreateCompanyAndPersonService {
      return !isNull(existingPerson.deletedAt);
    });

+    const peopleToEnrichNames = this.computePeopleToEnrichNames(
+      uniqueContacts,
+      shouldCreateOrRestorePeopleByHandleMap,
+    );
+
    const workDomainNamesToCreate = compact(
      [...contactsThatNeedPersonCreate, ...contactsThatNeedPersonRestore]
        .map((contact) => {
@@ -321,11 +333,93 @@ export class CreateCompanyAndPersonService {
    return {
      contactsThatNeedPersonCreate,
      contactsThatNeedPersonRestore,
+      peopleToEnrichNames,
      workDomainNamesToCreate,
      shouldCreateOrRestorePeopleByHandleMap,
    };
  }

+  // Stages per-personId name enrichments for existing People auto-created via
+  // CALENDAR or EMAIL. Empty fields are filled from new sources (first
+  // non-empty value wins across multiple contacts mapping to the same Person);
+  // populated fields are never overwritten.
+  private computePeopleToEnrichNames(
+    uniqueContacts: Contact[],
+    shouldCreateOrRestorePeopleByHandleMap: Map<
+      string,
+      { existingPerson: PersonWorkspaceEntity }
+    >,
+  ): { personId: string; name: FullNameMetadata }[] {
+    const enrichmentByPersonId = new Map<
+      string,
+      { firstName: string; lastName: string }
+    >();
+
+    for (const contact of uniqueContacts) {
+      const existingPerson = shouldCreateOrRestorePeopleByHandleMap.get(
+        contact.handle.toLowerCase(),
+      )?.existingPerson;
+
+      if (!isDefined(existingPerson)) {
+        continue;
+      }
+
+      // Soft-deleted matches are restored earlier in the same job, so the
+      // enrichment UPDATE runs against an un-deleted row.
+      const existingSource = existingPerson.createdBy?.source;
+
+      if (
+        existingSource !== FieldActorSource.CALENDAR &&
+        existingSource !== FieldActorSource.EMAIL
+      ) {
+        continue;
+      }
+
+      const staged = enrichmentByPersonId.get(existingPerson.id);
+      const currentFirstName =
+        staged?.firstName ?? existingPerson.name?.firstName ?? '';
+      const currentLastName =
+        staged?.lastName ?? existingPerson.name?.lastName ?? '';
+      const firstNameIsEmpty = !isNonEmptyString(currentFirstName);
+      const lastNameIsEmpty = !isNonEmptyString(currentLastName);
+
+      if (!firstNameIsEmpty && !lastNameIsEmpty) {
+        continue;
+      }
+
+      const { firstName: parsedFirstName, lastName: parsedLastName } =
+        getFirstNameAndLastNameFromHandleAndDisplayName(
+          contact.handle,
+          contact.displayName,
+        );
+
+      const enrichedFirstName =
+        firstNameIsEmpty && isNonEmptyString(parsedFirstName)
+          ? parsedFirstName
+          : currentFirstName;
+      const enrichedLastName =
+        lastNameIsEmpty && isNonEmptyString(parsedLastName)
+          ? parsedLastName
+          : currentLastName;
+
+      if (
+        enrichedFirstName === currentFirstName &&
+        enrichedLastName === currentLastName
+      ) {
+        continue;
+      }
+
+      enrichmentByPersonId.set(existingPerson.id, {
+        firstName: enrichedFirstName,
+        lastName: enrichedLastName,
+      });
+    }
+
+    return Array.from(enrichmentByPersonId.entries()).map(
+      ([personId, name]) => ({ personId, name }),
+    );
+  }
+
  formatPeopleToCreateFromContacts({
    contactsToCreate,
    createdBy,
@@ -1,5 +1,6 @@
 import { Injectable } from '@nestjs/common';

+import { type FullNameMetadata } from 'twenty-shared/types';
 import { DeepPartial } from 'typeorm';

 import { GlobalWorkspaceOrmManager } from 'src/engine/twenty-orm/global-workspace-datasource/global-workspace-orm.manager';
@@ -89,6 +90,42 @@ export class CreatePersonService {
    );
  }

+  public async enrichPeopleNames(
+    peopleToEnrich: { personId: string; name: FullNameMetadata }[],
+    workspaceId: string,
+  ): Promise<DeepPartial<PersonWorkspaceEntity>[]> {
+    if (peopleToEnrich.length === 0) {
+      return [];
+    }
+
+    const authContext = buildSystemAuthContext(workspaceId);
+
+    return this.globalWorkspaceOrmManager.executeInWorkspaceContext(
+      async () => {
+        const personRepository =
+          await this.globalWorkspaceOrmManager.getRepository(
+            workspaceId,
+            PersonWorkspaceEntity,
+            {
+              shouldBypassPermissionChecks: true,
+            },
+          );
+
+        const enrichedPeople = await personRepository.updateMany(
+          peopleToEnrich.map(({ personId, name }) => ({
+            criteria: personId,
+            partialEntity: { name },
+          })),
+          undefined,
+          ['id'],
+        );
+
+        return enrichedPeople.raw;
+      },
+      authContext,
+    );
+  }
+
  private async getLastPersonPosition(
    personRepository: WorkspaceRepository<PersonWorkspaceEntity>,
  ): Promise<number> {
@@ -63,6 +63,62 @@ describe('getParsedNameFromDisplayName', () => {
          expected: { firstName: 'John', lastName: 'Doe' },
        },
      },
+      {
+        title:
+          'should keep a multi-word first name intact when paired with a last name',
+        context: {
+          displayName: 'Smith, Mary Jane',
+          expected: { firstName: 'Mary Jane', lastName: 'Smith' },
+        },
+      },
+    ];
+
+    test.each(testCases)('$title', ({ context: { displayName, expected } }) => {
+      expect(getParsedNameFromDisplayName(displayName)).toEqual(expected);
+    });
+  });
+
+  describe('multi-comma comma-inverted forms', () => {
+    const testCases: TestCase[] = [
+      {
+        title:
+          'should treat the segment before the first comma as the last name and merge the rest into the first name',
+        context: {
+          displayName: 'Smith, Jane, Jr.',
+          expected: { firstName: 'Jane Jr.', lastName: 'Smith' },
+        },
+      },
+      {
+        title: 'should keep credential suffixes attached to the first name',
+        context: {
+          displayName: "O'Brien, Mary, MD",
+          expected: { firstName: 'Mary MD', lastName: "O'Brien" },
+        },
+      },
+      {
+        title:
+          'should fold a three-part "Last, First, Middle" form into a multi-word first name',
+        context: {
+          displayName: 'Doe, John, Patrick',
+          expected: { firstName: 'John Patrick', lastName: 'Doe' },
+        },
+      },
+      {
+        title:
+          'should collapse extra whitespace around the inner commas as it merges',
+        context: {
+          displayName: 'Smith ,  Jane  ,  Jr.',
+          expected: { firstName: 'Jane Jr.', lastName: 'Smith' },
+        },
+      },
+      {
+        title:
+          'should still strip a trailing :GROUP tag after a multi-comma swap',
+        context: {
+          displayName: 'Smith, Jane, Jr.:GROUP',
+          expected: { firstName: 'Jane Jr.', lastName: 'Smith' },
+        },
+      },
    ];

    test.each(testCases)('$title', ({ context: { displayName, expected } }) => {
@@ -24,13 +24,23 @@ export const getParsedNameFromDisplayName = (
    lastName: stripTrailingGroupTag(parsed.lastName),
  });

-  const commaMatch = cleaned.match(/^([^,]+),\s*([^,]+)$/);
+  // Comma-inverted forms "Last, First[, Suffix]". Splits on the first comma;
+  // any further commas collapse to spaces so firstName is comma-free.
+  const commaMatch = cleaned.match(/^([^,]+),\s*(.+)$/);

  if (isDefined(commaMatch)) {
-    return withGroupTagsStripped({
-      firstName: commaMatch[2].trim(),
-      lastName: commaMatch[1].trim(),
-    });
+    const lastName = commaMatch[1].trim();
+    const firstName = commaMatch[2]
+      .trim()
+      .replace(/\s*,\s*/g, ' ')
+      .replace(/\s+/g, ' ');
+
+    if (isNonEmptyString(firstName) && isNonEmptyString(lastName)) {
+      return withGroupTagsStripped({
+        firstName,
+        lastName,
+      });
+    }
  }

  const [firstToken, ...rest] = cleaned.split(/\s+/);