refactor(server): unify metadata override-blob computation (step 1 of override unification) (#22404)

## 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)_  <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22404?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
2026-07-01 16:58:38 +02:00
parent 27dea0ed0b
commit 2e7441380f
4 changed files with 104 additions and 145 deletions
@@ -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,
  };
 };
@@ -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,
  };
 };
@@ -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<string, unknown>,
+>({
+  overridableProperties,
+  updatedProperties,
+  existingEntity,
+  existingOverrides,
+}: ComputeMetadataOverridesBlobArgs<TProperties, TOverrides>): {
+  overrides: TOverrides | null;
+  remainingProperties: TProperties;
+} => {
+  const remainingRecord: Record<string, unknown> = {
+    ...(updatedProperties as unknown as Record<string, unknown>),
+  };
+  const existingRecord = existingEntity as Record<string, unknown>;
+
+  const overrides = overridableProperties.reduce<Record<
+    string,
+    unknown
+  > | 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<string, unknown> | 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,
+  };
+};
@@ -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<string, unknown> | 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<Record<
-    string,
-    unknown
-  > | 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 };
 };