From 2e7441380ffb84cf05196e86d710109717d81400 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Malfait?= Date: Wed, 1 Jul 2026 16:58:38 +0200 Subject: [PATCH] refactor(server): unify metadata override-blob computation (step 1 of override unification) (#22404) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Context Twenty currently has **two override mechanisms** for metadata: - `standardOverrides` (a bespoke JSONB column on `objectMetadata` / `fieldMetadata`) — i18n-aware, typed DTO with a per-locale `translations` map, resolved via `resolve-object/field-metadata-standard-override.util.ts`. - `OverridableEntity.overrides` (base class: `view`, `view-field`, `view-field-group`, `command-menu-item`, `page-layout-tab`, `page-layout-widget`) — a flat, i18n-free `{...entity, ...overrides}` spread, registry-driven via `isOverridable`. "One concept, two code paths → drift & confusion; reconciliation has to special-case." This PR is **step 1** of collapsing them. ## What this PR does (small, behavior-preserving) The add / remove / null-collapse **override-blob write logic was triplicated** across: - `sanitizeOverridableEntityInput` (`overrides`) - `sanitizeRawUpdateObjectInput` (object `standardOverrides`) - `sanitizeRawUpdateFieldInput` (field `standardOverrides`) This extracts it into a single `computeMetadataOverridesBlob` helper that all three now call. This is genuine **cross-mechanism convergence of the write path** — the first concrete reduction of the "two code paths". - Behavior-preserving: same diff semantics. The object/field paths used strict `===` on their string standard-override props; `isEqual` subsumes that for strings, and the overridable path already used `isEqual`. - Type-casts are contained **inside** the one helper; the three call sites stay clean and type-preserving. - 4 files: 1 new util + 3 refactors. ## ⚠️ Draft — verification status I could **not** run `typecheck` / `lint` / tests locally: the sandbox this was authored in cannot complete `yarn install` (network aborts mid-install, no `node_modules`). The change is small and reasoned, but **please let CI validate it** — that's why this is a draft. If CI flags a type/lint nit in the contained casts, it's isolated to `compute-metadata-overrides-blob.util.ts`. Per request: no code comments were added; the design/tradeoff discussion lives here. ## The full unification plan (this PR is step 1) The remaining steps are deliberately **not** in this PR because they need a live DB (migration) and the front-end codegen pipeline to verify — neither is available in the authoring sandbox. Documented here for review before we proceed: | Step | Change | Why staged | |------|--------|-----------| | **(this PR)** | Unify the write-path blob logic | Safe, behavior-preserving, no DB/FE | | Read path | One i18n-aware `resolveEffectiveEntity` (superset of the flat spread + the two i18n resolvers) | The i18n resolvers are entangled with typed translation-key narrowing; merging cleanly needs the storage/i18n generalization below | | Registry | Make object/field presentation props registry-driven (`facet` + `translatable`), like the overridable set already is | Depends on the facet annotation | | Storage | Object/field extend `OverridableEntity`; `standardOverrides` → `overrides` (translations preserved); **one data migration** | Needs DB verification; changes schema | | GraphQL + FE | Remove the `standardOverrides` field, expose `overrides`; regen `twenty-front` / client-SDK types; update the Settings → Data-Model rename UI | Needs codegen; see tradeoff below | ## Key tradeoffs / decisions to confirm 1. **GraphQL break on `standardOverrides` — accepted.** Per product call, external usage is negligible, so the later step will **remove** the field outright (no deprecated alias). The one real consumer is the Settings → Data-Model rename-label UI, updated in the same step. This drops the most complex part of the original plan (a virtual-alias resolver + deprecation window). 2. **`isActive` default.** `OverridableEntity` defaults `isActive` to `true`; object/field default it to `false`. The storage step must **explicitly override the default** and assert in the migration that no existing row's `isActive` changes. 3. **Overrides stay anonymous single-slot blobs** (no per-app attribution / multi-contributor 3-way merge). That limitation is unchanged here and is only worth revisiting if a concrete use case needs owner-tagged layering (real schema work, sized separately). 4. **Parity harness is the safety net for the storage step.** Because the read-path/storage merge touches the hot object/field resolve path and i18n precedence, that PR should land a golden-corpus parity gate (all locales, `isStandardApp`, empty/partial/full overrides) proving the unified resolver reproduces today's output byte-for-byte, before any switch. ## Not included (per request) - No service tests added. - No code comments added (rationale/tradeoffs are here, in the PR). ## Test plan - CI: `typecheck` + `lint` + the existing `sanitize-overridable-entity-input.util.spec.ts` (which exercises the shared logic through `sanitizeOverridableEntityInput`). - The object/field write paths have no dedicated unit spec; they're covered by the metadata integration suites (`successful-update-one-standard-object/field-metadata`). https://claude.ai/code/session_01E1pGBDLC3gEBs1w45G2W5Z --- _Generated by [Claude Code](https://claude.ai/code/session_01E1pGBDLC3gEBs1w45G2W5Z)_ Review in cubic --- .../utils/sanitize-raw-update-field-input.ts | 52 +++---------- .../utils/sanitize-raw-update-object-input.ts | 58 +++------------ .../compute-metadata-overrides-blob.util.ts | 74 +++++++++++++++++++ .../sanitize-overridable-entity-input.util.ts | 65 +++------------- 4 files changed, 104 insertions(+), 145 deletions(-) create mode 100644 packages/twenty-server/src/engine/metadata-modules/utils/compute-metadata-overrides-blob.util.ts diff --git a/packages/twenty-server/src/engine/metadata-modules/flat-field-metadata/utils/sanitize-raw-update-field-input.ts b/packages/twenty-server/src/engine/metadata-modules/flat-field-metadata/utils/sanitize-raw-update-field-input.ts index eeccaa0698..fdcff31544 100644 --- a/packages/twenty-server/src/engine/metadata-modules/flat-field-metadata/utils/sanitize-raw-update-field-input.ts +++ b/packages/twenty-server/src/engine/metadata-modules/flat-field-metadata/utils/sanitize-raw-update-field-input.ts @@ -16,6 +16,7 @@ import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-m import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util'; import { nullifyEmptyCompositeDefaultValue } from 'src/engine/metadata-modules/flat-field-metadata/utils/nullify-empty-composite-default-value.util'; import { belongsToTwentyStandardApp } from 'src/engine/metadata-modules/utils/belongs-to-twenty-standard-app.util'; +import { computeMetadataOverridesBlob } from 'src/engine/metadata-modules/utils/compute-metadata-overrides-blob.util'; type SanitizeRawUpdateFieldInputArgs = { rawUpdateFieldInput: UpdateFieldInput; @@ -81,51 +82,16 @@ export const sanitizeRawUpdateFieldInput = ({ ); } - const standardOverrides = FIELD_METADATA_STANDARD_OVERRIDES_PROPERTIES.reduce( - (standardOverrides, property) => { - const propertyValue = updatedEditableFieldProperties[property]; - - const isPropertyUpdated = - updatedEditableFieldProperties[property] !== undefined; - - if (!isPropertyUpdated) { - return standardOverrides; - } - delete updatedEditableFieldProperties[property]; - - if (propertyValue === existingFlatFieldMetadata[property]) { - if ( - isDefined(standardOverrides) && - Object.prototype.hasOwnProperty.call(standardOverrides, property) - ) { - const { [property]: _, ...restOverrides } = standardOverrides; - - return restOverrides; - } - - return standardOverrides; - } - - return { - ...standardOverrides, - [property]: propertyValue, - }; - }, - existingFlatFieldMetadata.standardOverrides, - ); - - if ( - isDefined(standardOverrides) && - Object.keys(standardOverrides).length === 0 - ) { - return { - standardOverrides: null, - updatedEditableFieldProperties, - }; - } + const { overrides: standardOverrides, remainingProperties } = + computeMetadataOverridesBlob({ + overridableProperties: FIELD_METADATA_STANDARD_OVERRIDES_PROPERTIES, + updatedProperties: updatedEditableFieldProperties, + existingEntity: existingFlatFieldMetadata, + existingOverrides: existingFlatFieldMetadata.standardOverrides, + }); return { standardOverrides, - updatedEditableFieldProperties, + updatedEditableFieldProperties: remainingProperties, }; }; diff --git a/packages/twenty-server/src/engine/metadata-modules/flat-object-metadata/utils/sanitize-raw-update-object-input.ts b/packages/twenty-server/src/engine/metadata-modules/flat-object-metadata/utils/sanitize-raw-update-object-input.ts index b468c7ff5e..daf81f7332 100644 --- a/packages/twenty-server/src/engine/metadata-modules/flat-object-metadata/utils/sanitize-raw-update-object-input.ts +++ b/packages/twenty-server/src/engine/metadata-modules/flat-object-metadata/utils/sanitize-raw-update-object-input.ts @@ -1,7 +1,4 @@ -import { - extractAndSanitizeObjectStringFields, - isDefined, -} from 'twenty-shared/utils'; +import { extractAndSanitizeObjectStringFields } from 'twenty-shared/utils'; import { FLAT_OBJECT_METADATA_EDITABLE_PROPERTIES } from 'src/engine/metadata-modules/flat-object-metadata/constants/flat-object-metadata-editable-properties.constant'; import { type FlatObjectMetadata } from 'src/engine/metadata-modules/flat-object-metadata/types/flat-object-metadata.type'; @@ -13,6 +10,7 @@ import { } from 'src/engine/metadata-modules/object-metadata/object-metadata.exception'; import { type ObjectMetadataStandardOverridesProperties } from 'src/engine/metadata-modules/object-metadata/types/object-metadata-standard-overrides-properties.types'; import { belongsToTwentyStandardApp } from 'src/engine/metadata-modules/utils/belongs-to-twenty-standard-app.util'; +import { computeMetadataOverridesBlob } from 'src/engine/metadata-modules/utils/compute-metadata-overrides-blob.util'; type SanitizeRawUpdateObjectInputArgs = { rawUpdateObjectInput: UpdateOneObjectInput; @@ -59,52 +57,16 @@ export const sanitizeRawUpdateObjectInput = ({ ); } - const standardOverrides = - OBJECT_METADATA_STANDARD_OVERRIDES_PROPERTIES.reduce( - (standardOverrides, property) => { - const propertyValue = updatedEditableObjectProperties[property]; - - const isPropertyUpdated = - updatedEditableObjectProperties[property] !== undefined; - - if (!isPropertyUpdated) { - return standardOverrides; - } - delete updatedEditableObjectProperties[property]; - - if (propertyValue === existingFlatObjectMetadata[property]) { - if ( - isDefined(standardOverrides) && - Object.prototype.hasOwnProperty.call(standardOverrides, property) - ) { - const { [property]: _, ...restOverrides } = standardOverrides; - - return restOverrides; - } - - return standardOverrides; - } - - return { - ...standardOverrides, - [property]: propertyValue, - }; - }, - existingFlatObjectMetadata.standardOverrides, - ); - - if ( - isDefined(standardOverrides) && - Object.keys(standardOverrides).length === 0 - ) { - return { - standardOverrides: null, - updatedEditableObjectProperties, - }; - } + const { overrides: standardOverrides, remainingProperties } = + computeMetadataOverridesBlob({ + overridableProperties: OBJECT_METADATA_STANDARD_OVERRIDES_PROPERTIES, + updatedProperties: updatedEditableObjectProperties, + existingEntity: existingFlatObjectMetadata, + existingOverrides: existingFlatObjectMetadata.standardOverrides, + }); return { standardOverrides, - updatedEditableObjectProperties, + updatedEditableObjectProperties: remainingProperties, }; }; diff --git a/packages/twenty-server/src/engine/metadata-modules/utils/compute-metadata-overrides-blob.util.ts b/packages/twenty-server/src/engine/metadata-modules/utils/compute-metadata-overrides-blob.util.ts new file mode 100644 index 0000000000..996f190b9d --- /dev/null +++ b/packages/twenty-server/src/engine/metadata-modules/utils/compute-metadata-overrides-blob.util.ts @@ -0,0 +1,74 @@ +import { fastDeepEqual, isDefined } from 'twenty-shared/utils'; + +type ComputeMetadataOverridesBlobArgs< + TProperties extends object, + TOverrides, +> = { + overridableProperties: readonly string[]; + updatedProperties: TProperties; + existingEntity: object; + existingOverrides: TOverrides | null; +}; + +export const computeMetadataOverridesBlob = < + TProperties extends object, + TOverrides = Record, +>({ + overridableProperties, + updatedProperties, + existingEntity, + existingOverrides, +}: ComputeMetadataOverridesBlobArgs): { + overrides: TOverrides | null; + remainingProperties: TProperties; +} => { + const remainingRecord: Record = { + ...(updatedProperties as unknown as Record), + }; + const existingRecord = existingEntity as Record; + + const overrides = overridableProperties.reduce | null>( + (acc, property) => { + if (remainingRecord[property] === undefined) { + return acc; + } + + const propertyValue = remainingRecord[property]; + + delete remainingRecord[property]; + + if (fastDeepEqual(propertyValue, existingRecord[property])) { + if ( + isDefined(acc) && + Object.prototype.hasOwnProperty.call(acc, property) + ) { + const { [property]: _removedProperty, ...restOverrides } = acc; + + return restOverrides; + } + + return acc; + } + + return { + ...acc, + [property]: propertyValue, + }; + }, + existingOverrides as unknown as Record | null, + ); + + const remainingProperties = remainingRecord as unknown as TProperties; + + if (isDefined(overrides) && Object.keys(overrides).length === 0) { + return { overrides: null, remainingProperties }; + } + + return { + overrides: overrides as unknown as TOverrides | null, + remainingProperties, + }; +}; diff --git a/packages/twenty-server/src/engine/metadata-modules/utils/sanitize-overridable-entity-input.util.ts b/packages/twenty-server/src/engine/metadata-modules/utils/sanitize-overridable-entity-input.util.ts index 94bb2d3c3f..687bff9680 100644 --- a/packages/twenty-server/src/engine/metadata-modules/utils/sanitize-overridable-entity-input.util.ts +++ b/packages/twenty-server/src/engine/metadata-modules/utils/sanitize-overridable-entity-input.util.ts @@ -1,8 +1,7 @@ -import isEqual from 'lodash.isequal'; import { type AllMetadataName } from 'twenty-shared/metadata'; -import { isDefined } from 'twenty-shared/utils'; import { ALL_OVERRIDABLE_PROPERTIES_BY_METADATA_NAME } from 'src/engine/metadata-modules/flat-entity/constant/all-overridable-properties-by-metadata-name.constant'; +import { computeMetadataOverridesBlob } from 'src/engine/metadata-modules/utils/compute-metadata-overrides-blob.util'; type FlatEntityWithOverrides = { [key: string]: unknown; @@ -26,63 +25,21 @@ export const sanitizeOverridableEntityInput = < overrides: Record | null; updatedEditableProperties: TProperties; } => { - const existingOverrides = existingFlatEntity.overrides; - if (!shouldOverride) { return { - overrides: existingOverrides, + overrides: existingFlatEntity.overrides, updatedEditableProperties, }; } - const sanitizedEditableProperties = { - ...updatedEditableProperties, - } as TProperties; + const { overrides, remainingProperties } = computeMetadataOverridesBlob({ + overridableProperties: ALL_OVERRIDABLE_PROPERTIES_BY_METADATA_NAME[ + metadataName + ] as string[], + updatedProperties: updatedEditableProperties, + existingEntity: existingFlatEntity, + existingOverrides: existingFlatEntity.overrides, + }); - const overridableProperties = ALL_OVERRIDABLE_PROPERTIES_BY_METADATA_NAME[ - metadataName - ] as string[]; - - const overrides = overridableProperties.reduce | null>((acc, property) => { - const isPropertyUpdated = - sanitizedEditableProperties[property] !== undefined; - - if (!isPropertyUpdated) { - return acc; - } - - const propertyValue = sanitizedEditableProperties[property]; - - delete sanitizedEditableProperties[property]; - - if (isEqual(propertyValue, existingFlatEntity[property])) { - if ( - isDefined(acc) && - Object.prototype.hasOwnProperty.call(acc, property) - ) { - const { [property]: _, ...restOverrides } = acc; - - return restOverrides; - } - - return acc; - } - - return { - ...acc, - [property]: propertyValue, - }; - }, existingOverrides); - - if (isDefined(overrides) && Object.keys(overrides).length === 0) { - return { - overrides: null, - updatedEditableProperties: sanitizedEditableProperties, - }; - } - - return { overrides, updatedEditableProperties: sanitizedEditableProperties }; + return { overrides, updatedEditableProperties: remainingProperties }; };