[BREAKING-CHANGE] Centralize system View viewField side effect (#23081)
# Introduction Closes https://github.com/twentyhq/core-team-issues/issues/2669 Part of the `isSystemSideEffect` engine-ownership effort. Until now, a custom object's default **INDEX** table view (`All {objectLabelPlural}`) and its view fields were built imperatively in `ObjectMetadataService` with random `v4()` identifiers, while `twenty-standard` authored its own copies with hardcoded literals. The two never converged, an object rename could drift the view, and nothing marked these rows as engine-owned. This PR makes the metadata side-effect engine the **single owner** of the INDEX view and its view fields, on name-free deterministic identifiers, for custom and standard objects alike. ## Core design - **Name-free deterministic identity.** The INDEX view identifier derives from `object identifier + ViewKey.INDEX` (`getSystemViewUniversalIdentifier`); each view-field identifier derives from `view identifier + field identifier` (`getViewFieldUniversalIdentifier`). An object rename (with a pinned object identifier) keeps the same view, losslessly. - **`isSystemSideEffect: true` is provenance.** Every INDEX view / view field the engine emits is flagged system-owned, so manifest deletion inference never drops it. The flag follows the view: a view field inherits its parent view's flag. - **The engine is the sole owner of the INDEX view.** It always emits it; a caller providing one with the same derived identifier is a genuine conflict surfaced by the engine's reserved-identifier collision, not silently deferred. ## Changes ### Shared (`twenty-shared`) - `getIndexViewUniversalIdentifier` → `getSystemViewUniversalIdentifier`, now taking a `viewKey` (generalizes to any singleton engine-owned view). - Standard field identifiers extracted into a new `STANDARD_OBJECT_FIELDS` constant, so both an object's `fields` and its INDEX view read the same field identifiers. - `buildStandardObjectIndexView` derives the standard INDEX view + view-field identifiers from `STANDARD_OBJECT_FIELDS`, replacing the hardcoded literals in `standard-object.constant.ts`. ### Metadata side-effect engine (custom objects) - **`objectSystemFieldsAndIndexViewOnCreate`** (replaces `objectSystemFieldsOnCreate`): on object creation, provisions the 7 reserved system fields **and** the INDEX view with one view field per displayable system field, all `isSystemSideEffect: true`. - **`fieldIndexViewFieldOnCreate`** (new): on field creation, provisions the field's INDEX view field. Object created in the same batch → visible, positioned before the system view fields; pre-existing object → hidden, appended (preserving the historical `createOneField` behavior). Both branches resolve the INDEX view by its derived identifier (single map access, never a scan). - **`fieldSystemViewFieldsOnDelete`** (new): on field deletion, cascade-deletes every engine-owned view field displaying it. - **`objectSystemSideEffectsOnDelete`** (extended): now also cascade-deletes the object's engine-owned views and their view fields (in addition to system fields, indexes, searchFieldMetadata). Every lookup walks a foreign-key aggregator down from the deleted object, so the work is proportional to what the object owns, never to workspace size. - Object-create and field-create positions are derived from the same caller-input field list, so the INDEX view layout is contiguous with no handler-ordering dependency. - `view` / `viewField` added to the side-effect companion metadata names for `fieldMetadata` and `objectMetadata`. ### Reserved-identifier invariant A caller can never define an entity whose identifier collides with one a system side effect produces: caller inputs are forced `isSystemSideEffect: false` at every entry point (API and app-manifest transpilers), and the engine raises `RESERVED_SYSTEM_UNIVERSAL_IDENTIFIER`, aborting the operation, when a system emission lands on a caller-claimed identifier. Covered by a new engine-level test. ### Caller-side provisioning removed The imperative INDEX view + view-field provisioning is removed from `ObjectMetadataService.createOneObject`. The record-page `FIELDS_WIDGET` view is intentionally left caller-side and deferred to the follow-up (see below). ### `twenty-standard` convergence Standard INDEX views and their view fields converge on the same derived-identifier + `isSystemSideEffect: true` scheme as the engine. `twenty-standard` syncs through the from/to migration path (which never runs the side-effect engine), so it authors this INDEX surface itself, matching what the engine produces for custom objects. ## Rollout Two `2.26.0` workspace commands, running after the `2.25` messageCampaign commands: - `upgrade:2-26:reconcile-index-view-universal-identifier` re-owns the INDEX views of the **twenty-standard and workspace-custom applications** and all their view fields to the derived identifiers with `isSystemSideEffect: true`, in a single per-workspace transaction. Each view field identifier is keyed on the application of the **displayed field** (an app or user column on a standard INDEX view converges too). Soft-deleted views and view fields are skipped: one can coexist with an active successor on the same derivation inputs and both would derive the same identifier. Children reference the view by primary key, so the re-own is lossless. - `upgrade:2-26:demote-and-backfill-application-index-view` handles **manifest-installed applications**, which never had their INDEX view auto-provisioned: every caller-authored INDEX view of another application is demoted to `key: null` (a plain additional view under its manifest identifier), then every application object gets the engine-owned INDEX view and its full view-field layout backfilled through the migration pipeline's legacy path (no side-effect expansion), views committed before view fields across applications since a view field belongs to the application owning its field. Idempotent and retry-safe: engine-owned INDEX views are neither demoted nor re-backfilled, and view creation and view-field creation are gated independently, so a retry after a partial failure still backfills the missing view fields of an already-committed view. Both support `--dry-run` and invalidate the full flat-maps closure (parents aggregate the re-owned identifiers, children resolve them as universal foreign keys, and page-layout widget universal configurations resolve view PKs at cache-build time). The `2.25` `upgrade:2-25:add-message-campaign-name-field` command is adapted to resolve the campaign INDEX view by its INDEX key on the object instead of by universal identifier: it now runs before the reconcile, on workspaces still holding legacy identifiers. ## ⚠️ Breaking change This PR **mutates 187 previously hardcoded universal identifiers** — the standard objects' INDEX views and their view fields (the literals removed from `standard-object.constant.ts`), now derived. - **Handled by the `2.26` commands above** for all existing workspaces. - **The INDEX key is now engine-reserved.** The flat view validator rejects caller-created INDEX views (API and manifest inputs are forced `isSystemSideEffect: false`) and enforces a single non-deleted INDEX view per object; `view.key` is no longer a comparable/updatable property, so no writer can promote or demote a view after creation. `ViewManifest.key` is deprecated and ignored (manifest views are always additional views, so old apps keep syncing and demoted views are not promoted back); the REST/GraphQL create path now rejects `key: INDEX`. In-repo example apps (`hello-world`, `document-generator`) no longer declare it. - **12 declared-but-never-seeded standard INDEX view field identifiers deleted** (the former `preservedViewFields` on `timelineActivity`, `workflowRun` and `workspaceMember`): after the reconcile, no workspace row references them. - **`computeFlatViewFieldsToCreate` now derives view field identifiers** instead of drawing `v4()` ones, which also changes what the committed `1-23` record-page backfill produces going forward (deliberate, documented in-code). - **Record-page views and view fields are not affected** (identifiers unchanged). - **In-repo apps: `twenty-last-contact` updated.** It was the only app declaring explicit INDEX view fields (10 columns across `allPeople` / `allCompanies` / `allOpportunities`) through manifest `viewFields`. Those target identifiers are now engine-owned and derived, so the manifest inputs no longer resolve and install failed with `View not found`. The app now declares only its fields; the engine's `fieldIndexViewFieldOnCreate` provisions the matching INDEX view field automatically. No other app under `packages/twenty-apps` references any of the 187 mutated identifiers, and apps that target standard views point at record-page views (e.g. `real-estate` → `opportunityRecordPageFields`) or their own objects (`twenty-partners`), all unchanged. ### Loss of granularity for app maintainers The engine now owns the INDEX view field of every field a caller adds to an object, so app maintainers lose direct control over those columns. Previously an app could target the engine-owned INDEX view with an explicit manifest `viewField` and set its `position` and `isVisible`. Now `fieldIndexViewFieldOnCreate` appends a **hidden** view field in caller-input order on field creation, so: - Columns an app previously showed at a **dedicated position** and **visible** (e.g. `twenty-last-contact`'s last-contact columns) become **hidden** and **appended in input order** after install. - There is currently **no manifest way to override** the engine-provisioned INDEX view field's position, visibility, or size. This is a deliberate regression accepted for the sake of single-ownership, and app maintainers should expect their INDEX columns to move/hide after upgrading. A follow-up override API will let maintainers reclaim per-field control over the engine-provisioned INDEX view field. ## Testing - Unit specs for each handler: object create (system fields + INDEX view/view fields, override, position offset), field create (same-batch vs existing-object, non-displayable noop, no-INDEX-view noop), field delete, object delete (fields/indexes/searchFieldMetadata/views/view fields cascade, reverse-relation view field on another object). - Engine-level test for the reserved-identifier collision. - `twenty-standard` guard test that its INDEX views/view fields stay on the derived scheme and stay system-owned. - Integration test: full engine provisioning of the INDEX view/view fields on object creation, same view id preserved across an object rename, and cascade delete on object deletion. ## Follow-up The full record-page stack (record-page view, its view fields, view field groups, page layout / tab / widget) is still built imperatively and moves into the engine in https://github.com/twentyhq/core-team-issues/issues/2721.
This commit is contained in:
Paul Rastoin
-1
@@ -54,7 +54,6 @@ type Assertions = [
|
||||
| 'deletedAt'
|
||||
| 'type'
|
||||
| 'position'
|
||||
| 'key'
|
||||
| 'isCompact'
|
||||
| 'openRecordIn'
|
||||
| 'kanbanAggregateOperation'
|
||||
|
||||
+186
@@ -0,0 +1,186 @@
|
||||
import { ViewKey, ViewType } from 'twenty-shared/types';
|
||||
|
||||
import { FlatViewValidatorService } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-view-validator.service';
|
||||
|
||||
const OBJECT_UNIVERSAL_IDENTIFIER = '00000000-0000-4000-8000-0000000000b1';
|
||||
const VIEW_UNIVERSAL_IDENTIFIER = '00000000-0000-4000-8000-0000000000c1';
|
||||
const OTHER_VIEW_UNIVERSAL_IDENTIFIER = '00000000-0000-4000-8000-0000000000c2';
|
||||
|
||||
type TestFlatView = {
|
||||
universalIdentifier: string;
|
||||
objectMetadataUniversalIdentifier: string;
|
||||
key: ViewKey | null;
|
||||
type: ViewType;
|
||||
isSystemSideEffect: boolean;
|
||||
deletedAt: string | null;
|
||||
};
|
||||
|
||||
const buildFlatView = (
|
||||
overrides: Partial<TestFlatView> = {},
|
||||
): TestFlatView => ({
|
||||
universalIdentifier: VIEW_UNIVERSAL_IDENTIFIER,
|
||||
objectMetadataUniversalIdentifier: OBJECT_UNIVERSAL_IDENTIFIER,
|
||||
key: null,
|
||||
type: ViewType.TABLE,
|
||||
isSystemSideEffect: false,
|
||||
deletedAt: null,
|
||||
...overrides,
|
||||
});
|
||||
|
||||
const mapsFrom = (
|
||||
entities: { universalIdentifier: string; [key: string]: unknown }[],
|
||||
) => ({
|
||||
byUniversalIdentifier: Object.fromEntries(
|
||||
entities.map((entity) => [entity.universalIdentifier, entity]),
|
||||
),
|
||||
});
|
||||
|
||||
const buildCreationArgs = ({
|
||||
flatViewToValidate,
|
||||
existingFlatViews = [],
|
||||
}: {
|
||||
flatViewToValidate: TestFlatView;
|
||||
existingFlatViews?: TestFlatView[];
|
||||
}) =>
|
||||
({
|
||||
flatEntityToValidate: flatViewToValidate,
|
||||
optimisticFlatEntityMapsAndRelatedFlatEntityMaps: {
|
||||
flatViewMaps: mapsFrom(existingFlatViews),
|
||||
flatFieldMetadataMaps: mapsFrom([]),
|
||||
flatObjectMetadataMaps: mapsFrom([
|
||||
{
|
||||
universalIdentifier: OBJECT_UNIVERSAL_IDENTIFIER,
|
||||
viewUniversalIdentifiers: existingFlatViews.map(
|
||||
(existingFlatView) => existingFlatView.universalIdentifier,
|
||||
),
|
||||
},
|
||||
]),
|
||||
},
|
||||
additionalCacheDataMaps: { featureFlagsMap: {} },
|
||||
}) as unknown as Parameters<
|
||||
FlatViewValidatorService['validateFlatViewCreation']
|
||||
>[0];
|
||||
|
||||
const buildUpdateArgs = ({
|
||||
flatEntityUpdate,
|
||||
existingFlatView,
|
||||
}: {
|
||||
flatEntityUpdate: Record<string, unknown>;
|
||||
existingFlatView: TestFlatView;
|
||||
}) =>
|
||||
({
|
||||
universalIdentifier: existingFlatView.universalIdentifier,
|
||||
flatEntityUpdate,
|
||||
optimisticFlatEntityMapsAndRelatedFlatEntityMaps: {
|
||||
flatViewMaps: mapsFrom([existingFlatView]),
|
||||
flatFieldMetadataMaps: mapsFrom([]),
|
||||
},
|
||||
additionalCacheDataMaps: { featureFlagsMap: {} },
|
||||
}) as unknown as Parameters<
|
||||
FlatViewValidatorService['validateFlatViewUpdate']
|
||||
>[0];
|
||||
|
||||
describe('FlatViewValidatorService INDEX key reservation', () => {
|
||||
let service: FlatViewValidatorService;
|
||||
|
||||
beforeEach(() => {
|
||||
service = new FlatViewValidatorService();
|
||||
});
|
||||
|
||||
describe('creation', () => {
|
||||
it('rejects a caller-provided INDEX view', () => {
|
||||
const result = service.validateFlatViewCreation(
|
||||
buildCreationArgs({
|
||||
flatViewToValidate: buildFlatView({
|
||||
key: ViewKey.INDEX,
|
||||
isSystemSideEffect: false,
|
||||
}),
|
||||
}),
|
||||
);
|
||||
|
||||
expect(result.errors).toHaveLength(1);
|
||||
expect(result.errors[0].message).toContain('reserved');
|
||||
});
|
||||
|
||||
it('accepts an engine-emitted INDEX view', () => {
|
||||
const result = service.validateFlatViewCreation(
|
||||
buildCreationArgs({
|
||||
flatViewToValidate: buildFlatView({
|
||||
key: ViewKey.INDEX,
|
||||
isSystemSideEffect: true,
|
||||
}),
|
||||
}),
|
||||
);
|
||||
|
||||
expect(result.errors).toEqual([]);
|
||||
});
|
||||
|
||||
it('rejects a second INDEX view on the same object', () => {
|
||||
const result = service.validateFlatViewCreation(
|
||||
buildCreationArgs({
|
||||
flatViewToValidate: buildFlatView({
|
||||
key: ViewKey.INDEX,
|
||||
isSystemSideEffect: true,
|
||||
}),
|
||||
existingFlatViews: [
|
||||
buildFlatView({
|
||||
universalIdentifier: OTHER_VIEW_UNIVERSAL_IDENTIFIER,
|
||||
key: ViewKey.INDEX,
|
||||
isSystemSideEffect: true,
|
||||
}),
|
||||
],
|
||||
}),
|
||||
);
|
||||
|
||||
expect(result.errors).toHaveLength(1);
|
||||
expect(result.errors[0].message).toContain('already has an INDEX view');
|
||||
});
|
||||
|
||||
it('ignores a soft-deleted INDEX view when checking the singleton', () => {
|
||||
const result = service.validateFlatViewCreation(
|
||||
buildCreationArgs({
|
||||
flatViewToValidate: buildFlatView({
|
||||
key: ViewKey.INDEX,
|
||||
isSystemSideEffect: true,
|
||||
}),
|
||||
existingFlatViews: [
|
||||
buildFlatView({
|
||||
universalIdentifier: OTHER_VIEW_UNIVERSAL_IDENTIFIER,
|
||||
key: ViewKey.INDEX,
|
||||
isSystemSideEffect: true,
|
||||
deletedAt: '2024-01-01T00:00:00.000Z',
|
||||
}),
|
||||
],
|
||||
}),
|
||||
);
|
||||
|
||||
expect(result.errors).toEqual([]);
|
||||
});
|
||||
|
||||
it('accepts a caller-provided view without a key', () => {
|
||||
const result = service.validateFlatViewCreation(
|
||||
buildCreationArgs({
|
||||
flatViewToValidate: buildFlatView({ key: null }),
|
||||
}),
|
||||
);
|
||||
|
||||
expect(result.errors).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('update', () => {
|
||||
it('accepts an update on an INDEX view', () => {
|
||||
const result = service.validateFlatViewUpdate(
|
||||
buildUpdateArgs({
|
||||
flatEntityUpdate: { name: 'Renamed default view' },
|
||||
existingFlatView: buildFlatView({
|
||||
key: ViewKey.INDEX,
|
||||
isSystemSideEffect: true,
|
||||
}),
|
||||
}),
|
||||
);
|
||||
|
||||
expect(result.errors).toEqual([]);
|
||||
});
|
||||
});
|
||||
});
|
||||
+36
@@ -5,6 +5,7 @@ import {
|
||||
FieldMetadataType,
|
||||
RelationType,
|
||||
ViewCalendarLayout,
|
||||
ViewKey,
|
||||
ViewType,
|
||||
} from 'twenty-shared/types';
|
||||
import { getViewLayoutFromViewType, isDefined } from 'twenty-shared/utils';
|
||||
@@ -459,6 +460,41 @@ export class FlatViewValidatorService {
|
||||
});
|
||||
}
|
||||
|
||||
if (flatViewToValidate.key === ViewKey.INDEX) {
|
||||
if (flatViewToValidate.isSystemSideEffect !== true) {
|
||||
validationResult.errors.push({
|
||||
code: ViewExceptionCode.INVALID_VIEW_DATA,
|
||||
message: t`The INDEX view key is reserved for the engine-owned default view; remove the key from the view definition`,
|
||||
userFriendlyMessage: msg`The INDEX view key is reserved for the default view`,
|
||||
});
|
||||
}
|
||||
|
||||
const objectAlreadyHasIndexFlatView =
|
||||
isDefined(optimisticFlatObjectMetadata) &&
|
||||
optimisticFlatObjectMetadata.viewUniversalIdentifiers.some(
|
||||
(viewUniversalIdentifier) => {
|
||||
const flatView = findFlatEntityByUniversalIdentifier({
|
||||
universalIdentifier: viewUniversalIdentifier,
|
||||
flatEntityMaps: optimisticFlatViewMaps,
|
||||
});
|
||||
|
||||
return (
|
||||
isDefined(flatView) &&
|
||||
flatView.key === ViewKey.INDEX &&
|
||||
!isDefined(flatView.deletedAt)
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
if (objectAlreadyHasIndexFlatView) {
|
||||
validationResult.errors.push({
|
||||
code: ViewExceptionCode.INVALID_VIEW_DATA,
|
||||
message: t`Object already has an INDEX view`,
|
||||
userFriendlyMessage: msg`This object already has a default view`,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
if (
|
||||
isDefined(
|
||||
flatViewToValidate.kanbanAggregateOperationFieldMetadataUniversalIdentifier,
|
||||
|
||||
Reference in New Issue
Block a user