From 8a74ea882948990dd83796cc803a48bc4b8b43fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Malfait?= Date: Thu, 28 May 2026 20:49:56 +0200 Subject: [PATCH] fix(contact-creation): enrich missing names on auto-created contacts (#21018) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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" ` — 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 --- ...create-company-and-contact.service.spec.ts | 254 ++++++++++++++++++ .../create-company-and-contact.service.ts | 96 ++++++- .../services/create-person.service.ts | 37 +++ ...parsed-name-from-display-name.util.spec.ts | 56 ++++ .../get-parsed-name-from-display-name.util.ts | 20 +- 5 files changed, 457 insertions(+), 6 deletions(-) diff --git a/packages/twenty-server/src/modules/contact-creation-manager/services/__tests__/create-company-and-contact.service.spec.ts b/packages/twenty-server/src/modules/contact-creation-manager/services/__tests__/create-company-and-contact.service.spec.ts index 4926ffe514..1ab28b8424 100644 --- a/packages/twenty-server/src/modules/contact-creation-manager/services/__tests__/create-company-and-contact.service.spec.ts +++ b/packages/twenty-server/src/modules/contact-creation-manager/services/__tests__/create-company-and-contact.service.spec.ts @@ -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, + ): 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' }, + }, + ]); + }); + }); }); }); diff --git a/packages/twenty-server/src/modules/contact-creation-manager/services/create-company-and-contact.service.ts b/packages/twenty-server/src/modules/contact-creation-manager/services/create-company-and-contact.service.ts index e891961adc..7285c8d4ab 100644 --- a/packages/twenty-server/src/modules/contact-creation-manager/services/create-company-and-contact.service.ts +++ b/packages/twenty-server/src/modules/contact-creation-manager/services/create-company-and-contact.service.ts @@ -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, diff --git a/packages/twenty-server/src/modules/contact-creation-manager/services/create-person.service.ts b/packages/twenty-server/src/modules/contact-creation-manager/services/create-person.service.ts index a1c3d0ab9e..eaad107477 100644 --- a/packages/twenty-server/src/modules/contact-creation-manager/services/create-person.service.ts +++ b/packages/twenty-server/src/modules/contact-creation-manager/services/create-person.service.ts @@ -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[]> { + 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, ): Promise { diff --git a/packages/twenty-server/src/modules/contact-creation-manager/utils/__tests__/get-parsed-name-from-display-name.util.spec.ts b/packages/twenty-server/src/modules/contact-creation-manager/utils/__tests__/get-parsed-name-from-display-name.util.spec.ts index d1550a40a0..4f2809ab3c 100644 --- a/packages/twenty-server/src/modules/contact-creation-manager/utils/__tests__/get-parsed-name-from-display-name.util.spec.ts +++ b/packages/twenty-server/src/modules/contact-creation-manager/utils/__tests__/get-parsed-name-from-display-name.util.spec.ts @@ -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 } }) => { diff --git a/packages/twenty-server/src/modules/contact-creation-manager/utils/get-parsed-name-from-display-name.util.ts b/packages/twenty-server/src/modules/contact-creation-manager/utils/get-parsed-name-from-display-name.util.ts index 8131cff929..a787f2ef28 100644 --- a/packages/twenty-server/src/modules/contact-creation-manager/utils/get-parsed-name-from-display-name.util.ts +++ b/packages/twenty-server/src/modules/contact-creation-manager/utils/get-parsed-name-from-display-name.util.ts @@ -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+/);