diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/2-26/2-26-workspace-command-1785255689000-reconcile-index-view-universal-identifier.command.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/2-26/2-26-workspace-command-1785255689000-reconcile-index-view-universal-identifier.command.ts index 70a0bd5b01..96bf5ca469 100644 --- a/packages/twenty-server/src/database/commands/upgrade-version-command/2-26/2-26-workspace-command-1785255689000-reconcile-index-view-universal-identifier.command.ts +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/2-26/2-26-workspace-command-1785255689000-reconcile-index-view-universal-identifier.command.ts @@ -27,14 +27,19 @@ type ReownUpdate = { update: { universalIdentifier?: string; isSystemSideEffect?: boolean; + key?: null; }; }; +type FlatViewFromMaps = NonNullable< + AllFlatEntityMaps['flatViewMaps']['byUniversalIdentifier'][string] +>; + @RegisteredWorkspaceCommand('2.26.0', 1785255689000) @Command({ name: 'upgrade:2-26:reconcile-index-view-universal-identifier', description: - 'Re-own the INDEX table views ("All {objectLabelPlural}", keyed on ViewKey.INDEX) of the twenty-standard and workspace-custom applications, and all their view fields, onto the engine convention: the view gets the name-free deterministic universal identifier (getSystemViewUniversalIdentifier, object identifier + INDEX key), each view field gets the derived getSystemViewFieldUniversalIdentifier keyed on the application of the field it DISPLAYS — not the row attribution, which diverges when a user shows a hidden standard column and mints a workspace-custom view field on a standard field — so an app or user column on a standard INDEX view converges too, and both get isSystemSideEffect: true, as if provisioned by the metadata side-effect engine. INDEX views of other applications are handled by the demote-and-backfill command. Children reference the view by primary key, so the re-own is a lossless update.', + 'Re-own the INDEX table views ("All {objectLabelPlural}", keyed on ViewKey.INDEX) of the twenty-standard and workspace-custom applications, and all their view fields, onto the engine convention: the view gets the name-free deterministic universal identifier (getSystemViewUniversalIdentifier, object identifier + INDEX key), each view field gets the derived getSystemViewFieldUniversalIdentifier keyed on the application of the field it DISPLAYS — not the row attribution, which diverges when a user shows a hidden standard column and mints a workspace-custom view field on a standard field — so an app or user column on a standard INDEX view converges too, and both get isSystemSideEffect: true, as if provisioned by the metadata side-effect engine. INDEX views of other applications are handled by the demote-and-backfill command. An INDEX view attributed to another application than its object (legacy caller-provided INDEX keys predating the flat view validator) is demoted to a plain view instead. Children reference the view by primary key, so the re-own is a lossless update.', }) export class ReconcileIndexViewUniversalIdentifierCommand extends ProvisionedWorkspaceCommandRunner { constructor( @@ -90,6 +95,7 @@ export class ReconcileIndexViewUniversalIdentifierCommand extends ProvisionedWor const { viewUpdates, viewFieldUpdates } = this.computeReownUpdates({ workspaceId, flatIndexViews, + flatViewMaps, flatViewFieldMaps, flatObjectMetadataMaps, flatFieldMetadataMaps, @@ -142,20 +148,22 @@ export class ReconcileIndexViewUniversalIdentifierCommand extends ProvisionedWor private computeReownUpdates({ workspaceId, flatIndexViews, + flatViewMaps, flatViewFieldMaps, flatObjectMetadataMaps, flatFieldMetadataMaps, }: { workspaceId: string; - flatIndexViews: NonNullable< - AllFlatEntityMaps['flatViewMaps']['byUniversalIdentifier'][string] - >[]; + flatIndexViews: FlatViewFromMaps[]; + flatViewMaps: AllFlatEntityMaps['flatViewMaps']; flatViewFieldMaps: AllFlatEntityMaps['flatViewFieldMaps']; flatObjectMetadataMaps: AllFlatEntityMaps['flatObjectMetadataMaps']; flatFieldMetadataMaps: AllFlatEntityMaps['flatFieldMetadataMaps']; }): { viewUpdates: ReownUpdate[]; viewFieldUpdates: ReownUpdate[] } { const viewUpdates: ReownUpdate[] = []; const viewFieldUpdates: ReownUpdate[] = []; + const claimedViewUniversalIdentifiers = new Set(); + const claimedViewFieldUniversalIdentifiers = new Set(); for (const flatView of flatIndexViews) { const flatObjectMetadata = @@ -170,6 +178,29 @@ export class ReconcileIndexViewUniversalIdentifierCommand extends ProvisionedWor continue; } + // An INDEX view belongs to the application of its object. A view + // attributed to another application (legacy caller-provided INDEX keys + // predating the flat view validator) cannot be the object's INDEX + // view: it is demoted to a plain caller-owned view, leaving the + // object's own INDEX view as the only holder of the key. + if ( + flatView.applicationUniversalIdentifier !== + flatObjectMetadata.applicationUniversalIdentifier + ) { + this.logger.warn( + `INDEX view ${flatView.id} is attributed to application ${flatView.applicationUniversalIdentifier} but its object belongs to application ${flatObjectMetadata.applicationUniversalIdentifier} in workspace ${workspaceId}, demoting it`, + ); + + const update: ReownUpdate['update'] = { key: null }; + + if (flatView.isSystemSideEffect) { + update.isSystemSideEffect = false; + } + + viewUpdates.push({ id: flatView.id, update }); + continue; + } + const derivedViewUniversalIdentifier = getSystemViewUniversalIdentifier({ objectMetadataApplicationUniversalIdentifier: flatObjectMetadata.applicationUniversalIdentifier, @@ -177,114 +208,158 @@ export class ReconcileIndexViewUniversalIdentifierCommand extends ProvisionedWor viewKey: ViewKey.INDEX, }); - const viewUpdate = this.computeViewReownUpdate({ - flatView, - derivedViewUniversalIdentifier, - }); + if (flatView.universalIdentifier === derivedViewUniversalIdentifier) { + claimedViewUniversalIdentifiers.add(derivedViewUniversalIdentifier); - if (isDefined(viewUpdate)) { - viewUpdates.push(viewUpdate); + if (!flatView.isSystemSideEffect) { + viewUpdates.push({ + id: flatView.id, + update: { isSystemSideEffect: true }, + }); + } + } else { + // The unique index on (workspaceId, universalIdentifier) covers + // soft-deleted rows too, and the flat maps are loaded withDeleted: + // any holder of the derived identifier makes the re-own impossible. + const isDerivedViewUniversalIdentifierTaken = + isDefined( + flatViewMaps.byUniversalIdentifier[derivedViewUniversalIdentifier], + ) || + claimedViewUniversalIdentifiers.has(derivedViewUniversalIdentifier); + + if (isDerivedViewUniversalIdentifierTaken) { + this.logger.warn( + `Derived identifier ${derivedViewUniversalIdentifier} of INDEX view ${flatView.id} is already held by another view in workspace ${workspaceId}, skipping`, + ); + continue; + } + + claimedViewUniversalIdentifiers.add(derivedViewUniversalIdentifier); + + const update: ReownUpdate['update'] = { + universalIdentifier: derivedViewUniversalIdentifier, + }; + + if (!flatView.isSystemSideEffect) { + update.isSystemSideEffect = true; + } + + viewUpdates.push({ id: flatView.id, update }); } viewFieldUpdates.push( - ...findManyFlatEntityByUniversalIdentifierInUniversalFlatEntityMaps({ - flatEntityMaps: flatViewFieldMaps, - universalIdentifiers: flatView.viewFieldUniversalIdentifiers, - }) - .map((flatViewField) => - this.computeViewFieldReownUpdate({ - workspaceId, - flatViewField, - derivedViewUniversalIdentifier, - flatFieldMetadataMaps, - }), - ) - .filter(isDefined), + ...this.computeViewFieldReownUpdates({ + workspaceId, + flatView, + derivedViewUniversalIdentifier, + flatViewFieldMaps, + flatFieldMetadataMaps, + claimedViewFieldUniversalIdentifiers, + }), ); } return { viewUpdates, viewFieldUpdates }; } - private computeViewReownUpdate({ + private computeViewFieldReownUpdates({ + workspaceId, flatView, derivedViewUniversalIdentifier, - }: { - flatView: NonNullable< - AllFlatEntityMaps['flatViewMaps']['byUniversalIdentifier'][string] - >; - derivedViewUniversalIdentifier: string; - }): ReownUpdate | undefined { - const update: ReownUpdate['update'] = {}; - - if (flatView.universalIdentifier !== derivedViewUniversalIdentifier) { - update.universalIdentifier = derivedViewUniversalIdentifier; - } - if (!flatView.isSystemSideEffect) { - update.isSystemSideEffect = true; - } - - if (Object.keys(update).length === 0) { - return undefined; - } - - return { id: flatView.id, update }; - } - - private computeViewFieldReownUpdate({ - workspaceId, - flatViewField, - derivedViewUniversalIdentifier, + flatViewFieldMaps, flatFieldMetadataMaps, + claimedViewFieldUniversalIdentifiers, }: { workspaceId: string; - flatViewField: NonNullable< - AllFlatEntityMaps['flatViewFieldMaps']['byUniversalIdentifier'][string] - >; + flatView: FlatViewFromMaps; derivedViewUniversalIdentifier: string; + flatViewFieldMaps: AllFlatEntityMaps['flatViewFieldMaps']; flatFieldMetadataMaps: AllFlatEntityMaps['flatFieldMetadataMaps']; - }): ReownUpdate | undefined { - if (isDefined(flatViewField.deletedAt)) { - return undefined; - } + claimedViewFieldUniversalIdentifiers: Set; + }): ReownUpdate[] { + const viewFieldUpdates: ReownUpdate[] = []; - const flatFieldMetadata = - flatFieldMetadataMaps.byUniversalIdentifier[ - flatViewField.fieldMetadataUniversalIdentifier - ]; - - if (!isDefined(flatFieldMetadata)) { - this.logger.warn( - `Missing field for INDEX view field ${flatViewField.id} in workspace ${workspaceId}, skipping`, - ); - - return undefined; - } - - const derivedViewFieldUniversalIdentifier = - getSystemViewFieldUniversalIdentifier({ - fieldMetadataApplicationUniversalIdentifier: - flatFieldMetadata.applicationUniversalIdentifier, - viewUniversalIdentifier: derivedViewUniversalIdentifier, - fieldMetadataUniversalIdentifier: - flatViewField.fieldMetadataUniversalIdentifier, + const flatViewFields = + findManyFlatEntityByUniversalIdentifierInUniversalFlatEntityMaps({ + flatEntityMaps: flatViewFieldMaps, + universalIdentifiers: flatView.viewFieldUniversalIdentifiers, }); - const update: ReownUpdate['update'] = {}; + for (const flatViewField of flatViewFields) { + if (isDefined(flatViewField.deletedAt)) { + continue; + } - if ( - flatViewField.universalIdentifier !== derivedViewFieldUniversalIdentifier - ) { - update.universalIdentifier = derivedViewFieldUniversalIdentifier; - } - if (!flatViewField.isSystemSideEffect) { - update.isSystemSideEffect = true; + const flatFieldMetadata = + flatFieldMetadataMaps.byUniversalIdentifier[ + flatViewField.fieldMetadataUniversalIdentifier + ]; + + if (!isDefined(flatFieldMetadata)) { + this.logger.warn( + `Missing field for INDEX view field ${flatViewField.id} in workspace ${workspaceId}, skipping`, + ); + continue; + } + + const derivedViewFieldUniversalIdentifier = + getSystemViewFieldUniversalIdentifier({ + fieldMetadataApplicationUniversalIdentifier: + flatFieldMetadata.applicationUniversalIdentifier, + viewUniversalIdentifier: derivedViewUniversalIdentifier, + fieldMetadataUniversalIdentifier: + flatViewField.fieldMetadataUniversalIdentifier, + }); + + if ( + flatViewField.universalIdentifier === + derivedViewFieldUniversalIdentifier + ) { + claimedViewFieldUniversalIdentifiers.add( + derivedViewFieldUniversalIdentifier, + ); + + if (!flatViewField.isSystemSideEffect) { + viewFieldUpdates.push({ + id: flatViewField.id, + update: { isSystemSideEffect: true }, + }); + } + continue; + } + + const isDerivedViewFieldUniversalIdentifierTaken = + isDefined( + flatViewFieldMaps.byUniversalIdentifier[ + derivedViewFieldUniversalIdentifier + ], + ) || + claimedViewFieldUniversalIdentifiers.has( + derivedViewFieldUniversalIdentifier, + ); + + if (isDerivedViewFieldUniversalIdentifierTaken) { + this.logger.warn( + `Derived identifier ${derivedViewFieldUniversalIdentifier} of view field ${flatViewField.id} is already held by another view field in workspace ${workspaceId}, skipping`, + ); + continue; + } + + claimedViewFieldUniversalIdentifiers.add( + derivedViewFieldUniversalIdentifier, + ); + + const update: ReownUpdate['update'] = { + universalIdentifier: derivedViewFieldUniversalIdentifier, + }; + + if (!flatViewField.isSystemSideEffect) { + update.isSystemSideEffect = true; + } + + viewFieldUpdates.push({ id: flatViewField.id, update }); } - if (Object.keys(update).length === 0) { - return undefined; - } - - return { id: flatViewField.id, update }; + return viewFieldUpdates; } } diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/2-26/__tests__/2-26-workspace-command-1785255689000-reconcile-index-view-universal-identifier.command.spec.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/2-26/__tests__/2-26-workspace-command-1785255689000-reconcile-index-view-universal-identifier.command.spec.ts index 07f5ed428a..d4383d934e 100644 --- a/packages/twenty-server/src/database/commands/upgrade-version-command/2-26/__tests__/2-26-workspace-command-1785255689000-reconcile-index-view-universal-identifier.command.spec.ts +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/2-26/__tests__/2-26-workspace-command-1785255689000-reconcile-index-view-universal-identifier.command.spec.ts @@ -43,6 +43,16 @@ const STANDARD_OBJECT_METADATA = { applicationUniversalIdentifier: STANDARD_APPLICATION_UNIVERSAL_IDENTIFIER, }; +const EXTERNAL_OBJECT_UNIVERSAL_IDENTIFIER = + '20202020-0000-4000-8000-0000000000bd'; + +// An object adopted into another application: its INDEX view can be left +// behind, still attributed to an engine application. +const EXTERNAL_OBJECT_METADATA = { + universalIdentifier: EXTERNAL_OBJECT_UNIVERSAL_IDENTIFIER, + applicationUniversalIdentifier: EXTERNAL_APPLICATION_UNIVERSAL_IDENTIFIER, +}; + // A custom object: its INDEX view was historically created with a random v4 // universal identifier by ObjectMetadataService.createOneObject. const CUSTOM_OBJECT_METADATA = { @@ -184,6 +194,7 @@ describe('ReconcileIndexViewUniversalIdentifierCommand', () => { flatObjectMetadataMaps: buildByUniversalIdentifierMap([ STANDARD_OBJECT_METADATA, CUSTOM_OBJECT_METADATA, + EXTERNAL_OBJECT_METADATA, ]), flatFieldMetadataMaps: buildByUniversalIdentifierMap(fieldMetadatas), }); @@ -324,6 +335,189 @@ describe('ReconcileIndexViewUniversalIdentifierCommand', () => { ); }); + it('demotes an INDEX view attributed to another application than its object', async () => { + mockWorkspaceCache({ + views: [ + buildFlatView({ + id: 'drifted-view-id', + universalIdentifier: 'drifted-view-uid', + key: ViewKey.INDEX, + objectMetadataUniversalIdentifier: EXTERNAL_OBJECT_UNIVERSAL_IDENTIFIER, + }), + ], + }); + + await runOnWorkspace(); + + expect(viewUpdateMock).toHaveBeenCalledTimes(1); + expect(viewUpdateMock).toHaveBeenCalledWith( + { id: 'drifted-view-id', workspaceId: WORKSPACE_ID }, + { key: null }, + ); + expect(viewFieldUpdateMock).not.toHaveBeenCalled(); + }); + + it('strips the system flag when demoting a drifted view previously stamped as system-owned', async () => { + mockWorkspaceCache({ + views: [ + buildFlatView({ + id: 'stamped-drifted-view-id', + universalIdentifier: 'stamped-drifted-view-uid', + key: ViewKey.INDEX, + isSystemSideEffect: true, + objectMetadataUniversalIdentifier: EXTERNAL_OBJECT_UNIVERSAL_IDENTIFIER, + }), + ], + }); + + await runOnWorkspace(); + + expect(viewUpdateMock).toHaveBeenCalledTimes(1); + expect(viewUpdateMock).toHaveBeenCalledWith( + { id: 'stamped-drifted-view-id', workspaceId: WORKSPACE_ID }, + { key: null, isSystemSideEffect: false }, + ); + }); + + it('demotes a workspace-custom INDEX view on a standard object next to the standard INDEX view', async () => { + mockWorkspaceCache({ + views: [ + buildFlatView({ + id: 'standard-view-id', + universalIdentifier: DERIVED_STANDARD_VIEW_UNIVERSAL_IDENTIFIER, + key: ViewKey.INDEX, + isSystemSideEffect: true, + }), + // A legacy caller-created view with key INDEX, attributed to the + // workspace-custom application but sitting on the standard object. + buildFlatView({ + id: 'legacy-caller-view-id', + universalIdentifier: 'legacy-caller-view-uid', + key: ViewKey.INDEX, + applicationUniversalIdentifier: CUSTOM_APPLICATION_UNIVERSAL_IDENTIFIER, + }), + ], + }); + + await runOnWorkspace(); + + expect(viewUpdateMock).toHaveBeenCalledTimes(1); + expect(viewUpdateMock).toHaveBeenCalledWith( + { id: 'legacy-caller-view-id', workspaceId: WORKSPACE_ID }, + { key: null }, + ); + }); + + it('skips an INDEX view whose derived identifier is held by a soft-deleted view', async () => { + mockWorkspaceCache({ + views: [ + buildFlatView({ + id: 'tombstone-view-id', + universalIdentifier: DERIVED_STANDARD_VIEW_UNIVERSAL_IDENTIFIER, + key: ViewKey.INDEX, + deletedAt: '2024-01-01T00:00:00.000Z', + }), + buildFlatView({ + id: 'active-view-id', + universalIdentifier: 'active-view-uid', + key: ViewKey.INDEX, + }), + ], + }); + + await runOnWorkspace(); + + // The tombstone still reserves the identifier in the unique index: the + // active view keeps its identifier rather than crashing the workspace. + expect(viewUpdateMock).not.toHaveBeenCalled(); + expect(invalidateCacheMock).not.toHaveBeenCalled(); + }); + + it('skips an INDEX view whose derived identifier is held by another active view', async () => { + mockWorkspaceCache({ + views: [ + buildFlatView({ + id: 'holder-view-id', + universalIdentifier: DERIVED_STANDARD_VIEW_UNIVERSAL_IDENTIFIER, + key: ViewKey.INDEX, + isSystemSideEffect: true, + }), + buildFlatView({ + id: 'duplicate-view-id', + universalIdentifier: 'duplicate-view-uid', + key: ViewKey.INDEX, + }), + ], + }); + + await runOnWorkspace(); + + expect(viewUpdateMock).not.toHaveBeenCalled(); + expect(invalidateCacheMock).not.toHaveBeenCalled(); + }); + + it('re-owns only the first of two INDEX views deriving the same identifier when neither holds it', async () => { + mockWorkspaceCache({ + views: [ + buildFlatView({ + id: 'first-view-id', + universalIdentifier: 'first-view-uid', + key: ViewKey.INDEX, + }), + buildFlatView({ + id: 'second-view-id', + universalIdentifier: 'second-view-uid', + key: ViewKey.INDEX, + }), + ], + }); + + await runOnWorkspace(); + + expect(viewUpdateMock).toHaveBeenCalledTimes(1); + expect(viewUpdateMock).toHaveBeenCalledWith( + { id: 'first-view-id', workspaceId: WORKSPACE_ID }, + { + universalIdentifier: DERIVED_STANDARD_VIEW_UNIVERSAL_IDENTIFIER, + isSystemSideEffect: true, + }, + ); + }); + + it('skips a view field whose derived identifier is held by a soft-deleted view field', async () => { + mockWorkspaceCache({ + views: [ + buildFlatView({ + id: 'view-id', + universalIdentifier: DERIVED_STANDARD_VIEW_UNIVERSAL_IDENTIFIER, + key: ViewKey.INDEX, + isSystemSideEffect: true, + viewFieldUniversalIdentifiers: [ + DERIVED_STANDARD_VIEW_FIELD_UNIVERSAL_IDENTIFIER, + 'active-view-field-uid', + ], + }), + ], + viewFields: [ + buildFlatViewField({ + id: 'tombstone-view-field-id', + universalIdentifier: DERIVED_STANDARD_VIEW_FIELD_UNIVERSAL_IDENTIFIER, + deletedAt: '2024-01-01T00:00:00.000Z', + }), + buildFlatViewField({ + id: 'active-view-field-id', + universalIdentifier: 'active-view-field-uid', + }), + ], + }); + + await runOnWorkspace(); + + expect(viewUpdateMock).not.toHaveBeenCalled(); + expect(viewFieldUpdateMock).not.toHaveBeenCalled(); + expect(invalidateCacheMock).not.toHaveBeenCalled(); + }); + it('re-owns only the active row of a soft-deleted + re-created view field pair', async () => { mockWorkspaceCache({ views: [