fix(server): normalize composite defaultValues in manifest converter (unblock app re-install on 2.5-normalized workspaces) (#20615)
## 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.
This commit is contained in:
Charles Bochet
@@ -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 <<EOF
|
||||
{
|
||||
"version": 1,
|
||||
"remotes": {
|
||||
"target": {
|
||||
"apiUrl": "${TWENTY_API_URL}",
|
||||
"apiKey": "${TWENTY_API_KEY}",
|
||||
"accessToken": "${TWENTY_API_KEY}"
|
||||
}
|
||||
},
|
||||
"defaultRemote": "target"
|
||||
}
|
||||
EOF
|
||||
|
||||
- name: Deploy postcard app (registry install path)
|
||||
working-directory: packages/twenty-apps/examples/postcard
|
||||
run: node ${{ github.workspace }}/packages/twenty-sdk/dist/cli.cjs deploy --remote target
|
||||
|
||||
- name: Install postcard app (registry install path)
|
||||
working-directory: packages/twenty-apps/examples/postcard
|
||||
run: node ${{ github.workspace }}/packages/twenty-sdk/dist/cli.cjs install --remote target
|
||||
|
||||
ci-example-app-postcard-status-check:
|
||||
if: always() && !cancelled()
|
||||
timeout-minutes: 5
|
||||
|
||||
+108
@@ -0,0 +1,108 @@
|
||||
import { type FieldManifest } from 'twenty-shared/application';
|
||||
import {
|
||||
type FieldMetadataDefaultActor,
|
||||
FieldMetadataType,
|
||||
} from 'twenty-shared/types';
|
||||
|
||||
import { fromFieldManifestToUniversalFlatFieldMetadata } from 'src/engine/core-modules/application/application-manifest/converters/from-field-manifest-to-universal-flat-field-metadata.util';
|
||||
|
||||
const APP_UID = '11111111-1111-1111-1111-111111111111';
|
||||
const OBJECT_UID = '22222222-2222-2222-2222-222222222222';
|
||||
const FIELD_UID = '33333333-3333-3333-3333-333333333333';
|
||||
const NOW = '2026-05-15T10:00:00.000Z';
|
||||
|
||||
const buildFieldManifest = (
|
||||
overrides: Partial<FieldManifest>,
|
||||
): 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'");
|
||||
});
|
||||
});
|
||||
});
|
||||
+16
-2
@@ -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,
|
||||
|
||||
+4
@@ -206,6 +206,7 @@ exports[`syncApplication should delete old field and create equivalent one when
|
||||
"applicationUniversalIdentifier": Any<String>,
|
||||
"createdAt": Any<String>,
|
||||
"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<String>,
|
||||
"createdAt": Any<String>,
|
||||
"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<String>,
|
||||
"createdAt": Any<String>,
|
||||
"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<String>,
|
||||
"createdAt": Any<String>,
|
||||
"defaultValue": {
|
||||
"context": null,
|
||||
"name": "'System'",
|
||||
"source": "'MANUAL'",
|
||||
"workspaceMemberId": null,
|
||||
|
||||
Reference in New Issue
Block a user