From cf4b4455d3940541093b9128ab26d58023218bdb Mon Sep 17 00:00:00 2001 From: Charles Bochet Date: Fri, 15 May 2026 18:31:48 +0200 Subject: [PATCH] fix(server): normalize composite defaultValues in manifest converter (unblock app re-install on 2.5-normalized workspaces) (#20615) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Context The runtime create-field path and the v2.5 `NormalizeCompositeFieldDefaultsCommand` workspace upgrade both run composite `defaultValue`s through `nullifyEmptyCompositeDefaultValue`. The manifest install/sync path was the only write path that skipped it: [`fromFieldManifestToUniversalFlatFieldMetadata`](https://github.com/twentyhq/twenty/blob/main/packages/twenty-server/src/engine/core-modules/application/application-manifest/converters/from-field-manifest-to-universal-flat-field-metadata.util.ts) passed `fieldManifest.defaultValue` through verbatim. For the SDK-emitted ACTOR system fields (`createdBy` / `updatedBy`), `twenty-sdk` ships `{ name: "''", source: "'MANUAL'" }`. After the runtime or the 2.5 normalize command stores them, the workspace row holds the canonical four-key form `{ context: null, name: null, source: "'MANUAL'", workspaceMemberId: null }`. The next install computes its TO map from the manifest, still gets the raw two-key shape, and diffs it against the normalized FROM. The dispatcher emits a `defaultValue` update on each system actor field; the flat-field-metadata validator rejects it with `FIELD_MUTATION_NOT_ALLOWED`, blocking every re-install of any application that defines a custom object on a v2.5-normalized workspace. ## Fix Normalize composite `defaultValue`s inside the converter, reusing the same `nullifyEmptyCompositeDefaultValue` helper the three other write paths already share: - [`get-default-flat-field-metadata-from-create-field-input.util.ts`](https://github.com/twentyhq/twenty/blob/main/packages/twenty-server/src/engine/metadata-modules/flat-field-metadata/utils/get-default-flat-field-metadata-from-create-field-input.util.ts) — `createOneObject` and `createOneField` GraphQL paths. - [`sanitize-raw-update-field-input.ts`](https://github.com/twentyhq/twenty/blob/main/packages/twenty-server/src/engine/metadata-modules/flat-field-metadata/utils/sanitize-raw-update-field-input.ts) — `updateOneField` GraphQL path. - [`2-5-workspace-command-1778000001000-normalize-composite-field-defaults.command.ts`](https://github.com/twentyhq/twenty/blob/main/packages/twenty-server/src/database/commands/upgrade-version-command/2-5/2-5-workspace-command-1778000001000-normalize-composite-field-defaults.command.ts) — the upgrade backfill that introduced the divergence. After the fix, the four write paths agree on the canonical shape, so re-installs are no-ops on system actor fields regardless of when the 2.5 normalize command ran. Non-composite types pass through unchanged. ## Test New spec `from-field-manifest-to-universal-flat-field-metadata.util.spec.ts` covers: - Empty-name actor defaults are normalized to the four-key canonical shape. - The converter is idempotent: feeding its own output back in produces the same result (so two consecutive syncs of the same manifest never emit a `defaultValue` update). - When the manifest omits `defaultValue`, the converter falls back to `generateDefaultValue` and normalizes the result. - Non-composite defaults pass through unchanged. ``` PASS src/engine/core-modules/application/application-manifest/converters/__tests__/from-field-manifest-to-universal-flat-field-metadata.util.spec.ts fromFieldManifestToUniversalFlatFieldMetadata composite defaultValue normalization ✓ normalizes empty-name actor defaults to the canonical four-key shape ✓ is idempotent: re-running the converter on its own output yields the same defaultValue ✓ falls back to the generated default and normalizes it when defaultValue is omitted ✓ leaves non-composite defaults untouched Tests: 4 passed ``` ## CI gap that let this through The integration suites covering manifest install (`appDevOnce` against the test workspace) never re-installed an existing app on a workspace whose composite fields had already been put through the 2.5 normalize command. They synced once, then ran assertions on the resulting state; the second sync that would have re-triggered the `defaultValue` diff was never exercised. If we want to catch this class of regression at the integration level too, we'd add a test that (1) syncs an app whose manifest includes an ACTOR system field with the raw SDK shape, (2) invokes `NormalizeCompositeFieldDefaultsCommand` directly on the test workspace, (3) re-syncs the same manifest, and (4) asserts no `FIELD_MUTATION_NOT_ALLOWED` errors. The unit-level idempotency check in this PR is the minimal version of that same coverage. Happy to ship that integration spec in a follow-up if it'd help. --- .../workflows/ci-example-app-postcard.yaml | 27 +++++ ...universal-flat-field-metadata.util.spec.ts | 108 ++++++++++++++++++ ...t-to-universal-flat-field-metadata.util.ts | 18 ++- ...rkspace-migration.integration-spec.ts.snap | 4 + 4 files changed, 155 insertions(+), 2 deletions(-) create mode 100644 packages/twenty-server/src/engine/core-modules/application/application-manifest/converters/__tests__/from-field-manifest-to-universal-flat-field-metadata.util.spec.ts diff --git a/.github/workflows/ci-example-app-postcard.yaml b/.github/workflows/ci-example-app-postcard.yaml index 28aaeac914..ab4ac47dc5 100644 --- a/.github/workflows/ci-example-app-postcard.yaml +++ b/.github/workflows/ci-example-app-postcard.yaml @@ -23,9 +23,11 @@ jobs: packages/twenty-sdk/** packages/twenty-client-sdk/** packages/twenty-shared/** + packages/twenty-server/** !packages/twenty-sdk/package.json !packages/twenty-client-sdk/package.json !packages/twenty-shared/package.json + !packages/twenty-server/package.json example-app-postcard: needs: changed-files-check @@ -83,6 +85,31 @@ jobs: working-directory: packages/twenty-apps/examples/postcard run: npx vitest run + - name: Configure remote for SDK CLI + run: | + mkdir -p ~/.twenty + cat > ~/.twenty/config.json <, +): FieldManifest & { objectUniversalIdentifier: string } => + ({ + universalIdentifier: FIELD_UID, + type: FieldMetadataType.TEXT, + name: 'demo', + label: 'Demo', + objectUniversalIdentifier: OBJECT_UID, + ...overrides, + }) as FieldManifest & { objectUniversalIdentifier: string }; + +describe('fromFieldManifestToUniversalFlatFieldMetadata', () => { + describe('composite defaultValue normalization', () => { + it('normalizes empty-name actor defaults to the canonical four-key shape', () => { + const result = fromFieldManifestToUniversalFlatFieldMetadata({ + fieldManifest: buildFieldManifest({ + type: FieldMetadataType.ACTOR, + name: 'createdBy', + label: 'Created by', + defaultValue: { name: "''", source: "'MANUAL'" }, + }), + applicationUniversalIdentifier: APP_UID, + now: NOW, + }); + + expect(result.defaultValue).toEqual({ + context: null, + name: null, + source: "'MANUAL'", + workspaceMemberId: null, + }); + }); + + it('is idempotent: re-running the converter on its own output yields the same defaultValue', () => { + const first = fromFieldManifestToUniversalFlatFieldMetadata({ + fieldManifest: buildFieldManifest({ + type: FieldMetadataType.ACTOR, + name: 'createdBy', + label: 'Created by', + defaultValue: { name: "''", source: "'MANUAL'" }, + }), + applicationUniversalIdentifier: APP_UID, + now: NOW, + }); + + const second = fromFieldManifestToUniversalFlatFieldMetadata({ + fieldManifest: buildFieldManifest({ + type: FieldMetadataType.ACTOR, + name: 'createdBy', + label: 'Created by', + defaultValue: first.defaultValue as FieldMetadataDefaultActor, + }), + applicationUniversalIdentifier: APP_UID, + now: NOW, + }); + + expect(second.defaultValue).toEqual(first.defaultValue); + }); + + it('falls back to the generated default and normalizes it when defaultValue is omitted', () => { + const result = fromFieldManifestToUniversalFlatFieldMetadata({ + fieldManifest: buildFieldManifest({ + type: FieldMetadataType.ACTOR, + name: 'updatedBy', + label: 'Updated by', + }), + applicationUniversalIdentifier: APP_UID, + now: NOW, + }); + + expect(result.defaultValue).toEqual({ + context: null, + name: "'System'", + source: "'MANUAL'", + workspaceMemberId: null, + }); + }); + + it('leaves non-composite defaults untouched', () => { + const result = fromFieldManifestToUniversalFlatFieldMetadata({ + fieldManifest: buildFieldManifest({ + type: FieldMetadataType.TEXT, + name: 'title', + label: 'Title', + defaultValue: "'todo'", + }), + applicationUniversalIdentifier: APP_UID, + now: NOW, + }); + + expect(result.defaultValue).toBe("'todo'"); + }); + }); +}); diff --git a/packages/twenty-server/src/engine/core-modules/application/application-manifest/converters/from-field-manifest-to-universal-flat-field-metadata.util.ts b/packages/twenty-server/src/engine/core-modules/application/application-manifest/converters/from-field-manifest-to-universal-flat-field-metadata.util.ts index 8242906457..c957d94a5e 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application-manifest/converters/from-field-manifest-to-universal-flat-field-metadata.util.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application-manifest/converters/from-field-manifest-to-universal-flat-field-metadata.util.ts @@ -8,7 +8,10 @@ import { ApplicationException, ApplicationExceptionCode, } from 'src/engine/core-modules/application/application.exception'; +import { type CompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/types/composite-field-metadata-type.type'; +import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util'; import { generateDefaultValue } from 'src/engine/metadata-modules/field-metadata/utils/generate-default-value'; +import { nullifyEmptyCompositeDefaultValue } from 'src/engine/metadata-modules/flat-field-metadata/utils/nullify-empty-composite-default-value.util'; import { PARTIAL_SYSTEM_FLAT_FIELD_METADATAS } from 'src/engine/metadata-modules/object-metadata/constants/partial-system-flat-field-metadatas.constant'; import { isMorphOrRelationFieldMetadataType } from 'src/engine/utils/is-morph-or-relation-field-metadata-type.util'; import { type UniversalFlatFieldMetadata } from 'src/engine/workspace-manager/workspace-migration/universal-flat-entity/types/universal-flat-field-metadata.type'; @@ -65,6 +68,18 @@ export const fromFieldManifestToUniversalFlatFieldMetadata = ({ relationTargetObjectMetadataUniversalIdentifier, } = getRelationTargetUniversalIdentifiers(fieldManifest); + // TODO: generate system fields server-side from the object manifest + // so the converter doesn't need to re-normalize composite defaults + // that the SDK couldn't have known the canonical shape of. + const rawDefaultValue = + fieldManifest.defaultValue ?? generateDefaultValue(fieldManifest.type); + const defaultValue = isCompositeFieldMetadataType(fieldManifest.type) + ? nullifyEmptyCompositeDefaultValue({ + defaultValue: rawDefaultValue, + fieldType: fieldManifest.type as CompositeFieldMetadataType, + }) + : rawDefaultValue; + return { universalIdentifier: fieldManifest.universalIdentifier, applicationUniversalIdentifier, @@ -75,8 +90,7 @@ export const fromFieldManifestToUniversalFlatFieldMetadata = ({ icon: fieldManifest.icon ?? null, standardOverrides: null, options: fieldManifest.options ?? null, - defaultValue: - fieldManifest.defaultValue ?? generateDefaultValue(fieldManifest.type), + defaultValue, universalSettings: fieldManifest.universalSettings ?? null, isCustom: true, isActive: true, diff --git a/packages/twenty-server/test/integration/metadata/suites/application/__snapshots__/successful-sync-application-workspace-migration.integration-spec.ts.snap b/packages/twenty-server/test/integration/metadata/suites/application/__snapshots__/successful-sync-application-workspace-migration.integration-spec.ts.snap index c0a2c22fa6..0b0f2c825e 100644 --- a/packages/twenty-server/test/integration/metadata/suites/application/__snapshots__/successful-sync-application-workspace-migration.integration-spec.ts.snap +++ b/packages/twenty-server/test/integration/metadata/suites/application/__snapshots__/successful-sync-application-workspace-migration.integration-spec.ts.snap @@ -206,6 +206,7 @@ exports[`syncApplication should delete old field and create equivalent one when "applicationUniversalIdentifier": Any, "createdAt": Any, "defaultValue": { + "context": null, "name": "'System'", "source": "'MANUAL'", "workspaceMemberId": null, @@ -236,6 +237,7 @@ exports[`syncApplication should delete old field and create equivalent one when "applicationUniversalIdentifier": Any, "createdAt": Any, "defaultValue": { + "context": null, "name": "'System'", "source": "'MANUAL'", "workspaceMemberId": null, @@ -562,6 +564,7 @@ exports[`syncApplication should return workspace migration actions on initial sy "applicationUniversalIdentifier": Any, "createdAt": Any, "defaultValue": { + "context": null, "name": "'System'", "source": "'MANUAL'", "workspaceMemberId": null, @@ -592,6 +595,7 @@ exports[`syncApplication should return workspace migration actions on initial sy "applicationUniversalIdentifier": Any, "createdAt": Any, "defaultValue": { + "context": null, "name": "'System'", "source": "'MANUAL'", "workspaceMemberId": null,