fix(server): make all view children reparentable across a workspace migration sync (#22600)
## Summary Uniformizes the workspace migration engine so **every** view child entity — `viewField`, `viewFieldGroup`, `viewGroup`, `viewFilter`, `viewSort`, `viewFilterGroup` — can be reparented from one view to another within a single manifest sync, including when the previous parent view is deleted in the same sync. ### Context When an app manifest deletes a view and reparents its children onto another view in the same sync (e.g. replacing a custom `FIELDS_WIDGET` view with a standard one), the sync failed with a builder validation error `View field to update parent view not found`. Root causes: 1. `viewField`, `viewFieldGroup` and `viewGroup` had `viewId.toCompare: false`, so the diff never detected the parent-view change and never emitted a reparent update (the already-reparentable siblings `viewFilter`/`viewSort`/`viewFilterGroup` had `toCompare: true`). 2. `validateFlatViewFieldGroupUpdate` resolved the *old* parent view (it ignored the update patch), inconsistent with the other view-child validators. 3. Once the builder no longer errors, the runner would fail silently: `view.delete` ran **before** the child reparent updates, and `viewId` is `onDelete: CASCADE`, so the old view's deletion cascade-deleted the children before they could be reparented (silent data loss, since `repository.update` on a missing row is a no-op). ### Changes - **`all-entity-properties-configuration-by-metadata-name.constant.ts`**: set `viewId.toCompare: true` for `viewField`, `viewFieldGroup`, `viewGroup`. Because `viewId` maps to `universalProperty: 'viewUniversalIdentifier'`, the diff compares **only** `viewUniversalIdentifier` (never the raw FK). Snapshot updated accordingly. - **`flat-view-field-group-validator.service.ts`**: merge `flatEntityUpdate` and resolve the **new** parent view, matching the `viewField`/`viewGroup`/`viewSort` validators. - **`compute-ordered-migration-actions.util.ts`**: move `view.delete` to run **after** all view-child create/update actions so a child can be reparented off a view that is being deleted in the same sync. Child `delete → create → update` order is preserved (needed for `viewField`'s partial-unique `(fieldMetadataId, viewId)`). - **New integration test** `successful-manifest-reparent-view-children.integration-spec.ts` covering reparenting of every view child (a) between two persisting views and (b) when the source view is deleted in the same sync. ## Test plan - [x] `nx typecheck twenty-server` - [x] oxlint + oxfmt on changed files - [x] Unit snapshot regenerated: `all-universal-flat-entity-properties-to-compare-and-stringify.constant.spec` - [x] New integration test passes (both scenarios) - [x] Verified the delete-source scenario **fails** on the old action ordering (children cascade-deleted, `Received length: 0`) and **passes** after the reorder — confirming it's a genuine regression guard Made with [Cursor](https://cursor.com) <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22600?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> <!-- End of auto-generated description by cubic. -->
This commit is contained in:
+3
@@ -362,6 +362,7 @@ exports[`ALL_UNIVERSAL_FLAT_ENTITY_PROPERTIES_TO_COMPARE_AND_STRINGIFY should ma
|
||||
"aggregateOperation",
|
||||
"viewFieldGroupUniversalIdentifier",
|
||||
"deletedAt",
|
||||
"viewUniversalIdentifier",
|
||||
"isActive",
|
||||
"universalOverrides",
|
||||
],
|
||||
@@ -376,6 +377,7 @@ exports[`ALL_UNIVERSAL_FLAT_ENTITY_PROPERTIES_TO_COMPARE_AND_STRINGIFY should ma
|
||||
"isVisible",
|
||||
"isActive",
|
||||
"deletedAt",
|
||||
"viewUniversalIdentifier",
|
||||
"overrides",
|
||||
],
|
||||
"propertiesToStringify": [
|
||||
@@ -414,6 +416,7 @@ exports[`ALL_UNIVERSAL_FLAT_ENTITY_PROPERTIES_TO_COMPARE_AND_STRINGIFY should ma
|
||||
"fieldValue",
|
||||
"position",
|
||||
"deletedAt",
|
||||
"viewUniversalIdentifier",
|
||||
],
|
||||
"propertiesToStringify": [],
|
||||
},
|
||||
|
||||
+3
-3
@@ -477,7 +477,7 @@ export const ALL_ENTITY_PROPERTIES_CONFIGURATION_BY_METADATA_NAME = {
|
||||
toStringify: false,
|
||||
},
|
||||
viewId: {
|
||||
toCompare: false,
|
||||
toCompare: true,
|
||||
universalProperty: 'viewUniversalIdentifier',
|
||||
toStringify: false,
|
||||
},
|
||||
@@ -544,7 +544,7 @@ export const ALL_ENTITY_PROPERTIES_CONFIGURATION_BY_METADATA_NAME = {
|
||||
universalProperty: 'fieldMetadataUniversalIdentifier',
|
||||
},
|
||||
viewId: {
|
||||
toCompare: false,
|
||||
toCompare: true,
|
||||
toStringify: false,
|
||||
universalProperty: 'viewUniversalIdentifier',
|
||||
},
|
||||
@@ -592,7 +592,7 @@ export const ALL_ENTITY_PROPERTIES_CONFIGURATION_BY_METADATA_NAME = {
|
||||
universalProperty: undefined,
|
||||
},
|
||||
viewId: {
|
||||
toCompare: false,
|
||||
toCompare: true,
|
||||
toStringify: false,
|
||||
universalProperty: 'viewUniversalIdentifier',
|
||||
},
|
||||
|
||||
+1
-1
@@ -23,7 +23,6 @@ export const computeOrderedMigrationActions = (
|
||||
///
|
||||
|
||||
// Views
|
||||
...aggregatedOrchestratorActionsReport.view.delete,
|
||||
...aggregatedOrchestratorActionsReport.view.create,
|
||||
...aggregatedOrchestratorActionsReport.view.update,
|
||||
...aggregatedOrchestratorActionsReport.viewField.delete,
|
||||
@@ -44,6 +43,7 @@ export const computeOrderedMigrationActions = (
|
||||
...aggregatedOrchestratorActionsReport.viewSort.create,
|
||||
...aggregatedOrchestratorActionsReport.viewSort.update,
|
||||
...aggregatedOrchestratorActionsReport.viewSort.delete,
|
||||
...aggregatedOrchestratorActionsReport.view.delete,
|
||||
///
|
||||
|
||||
// Logic functions
|
||||
|
||||
+13
-1
@@ -95,6 +95,7 @@ export class FlatViewFieldGroupValidatorService {
|
||||
|
||||
public validateFlatViewFieldGroupUpdate({
|
||||
universalIdentifier,
|
||||
flatEntityUpdate,
|
||||
optimisticFlatEntityMapsAndRelatedFlatEntityMaps: {
|
||||
flatViewFieldGroupMaps: optimisticFlatViewFieldGroupMaps,
|
||||
flatViewMaps,
|
||||
@@ -125,8 +126,19 @@ export class FlatViewFieldGroupValidatorService {
|
||||
return validationResult;
|
||||
}
|
||||
|
||||
const updatedFlatViewFieldGroup = {
|
||||
...existingFlatViewFieldGroup,
|
||||
...flatEntityUpdate,
|
||||
};
|
||||
|
||||
validationResult.flatEntityMinimalInformation = {
|
||||
...validationResult.flatEntityMinimalInformation,
|
||||
viewUniversalIdentifier:
|
||||
updatedFlatViewFieldGroup.viewUniversalIdentifier,
|
||||
};
|
||||
|
||||
const flatView = findFlatEntityByUniversalIdentifier({
|
||||
universalIdentifier: existingFlatViewFieldGroup.viewUniversalIdentifier,
|
||||
universalIdentifier: updatedFlatViewFieldGroup.viewUniversalIdentifier,
|
||||
flatEntityMaps: flatViewMaps,
|
||||
});
|
||||
|
||||
|
||||
+356
@@ -0,0 +1,356 @@
|
||||
import { buildBaseManifest } from 'test/integration/metadata/suites/application/utils/build-base-manifest.util';
|
||||
import { cleanupApplicationAndAppRegistration } from 'test/integration/metadata/suites/application/utils/cleanup-application-and-app-registration.util';
|
||||
import { setupApplicationForSync } from 'test/integration/metadata/suites/application/utils/setup-application-for-sync.util';
|
||||
import { syncApplication } from 'test/integration/metadata/suites/application/utils/sync-application.util';
|
||||
import { findManyObjectMetadataWithIndexes } from 'test/integration/metadata/suites/object-metadata/utils/find-many-object-metadata-with-indexes.util';
|
||||
import { findViews } from 'test/integration/metadata/suites/view/utils/find-views.util';
|
||||
import { type FieldManifest, type Manifest } from 'twenty-shared/application';
|
||||
import { STANDARD_OBJECTS } from 'twenty-shared/metadata';
|
||||
import {
|
||||
FieldMetadataType,
|
||||
ViewFilterGroupLogicalOperator,
|
||||
ViewFilterOperand,
|
||||
ViewSortDirection,
|
||||
ViewType,
|
||||
} from 'twenty-shared/types';
|
||||
import { v4 as uuidv4 } from 'uuid';
|
||||
|
||||
const TEST_APP_ID = uuidv4();
|
||||
const TEST_ROLE_ID = uuidv4();
|
||||
const TEST_FIELD_ID = uuidv4();
|
||||
|
||||
const SOURCE_VIEW_ID = uuidv4();
|
||||
const TARGET_VIEW_ID = uuidv4();
|
||||
|
||||
const VIEW_FIELD_ID = uuidv4();
|
||||
const VIEW_FIELD_GROUP_ID = uuidv4();
|
||||
const VIEW_GROUP_ID = uuidv4();
|
||||
const VIEW_FILTER_ID = uuidv4();
|
||||
const VIEW_SORT_ID = uuidv4();
|
||||
const VIEW_FILTER_GROUP_ID = uuidv4();
|
||||
|
||||
const SOURCE_VIEW_NAME = 'Reparent Source View';
|
||||
const TARGET_VIEW_NAME = 'Reparent Target View';
|
||||
|
||||
const CUSTOM_FIELD_NAME = 'integrationReparentColumn';
|
||||
|
||||
const PERSON_OBJECT_UNIVERSAL_IDENTIFIER =
|
||||
STANDARD_OBJECTS.person.universalIdentifier;
|
||||
|
||||
const personFieldManifest: FieldManifest = {
|
||||
universalIdentifier: TEST_FIELD_ID,
|
||||
type: FieldMetadataType.TEXT,
|
||||
name: CUSTOM_FIELD_NAME,
|
||||
label: 'Integration Reparent Column',
|
||||
description: 'Custom field used to reparent view children between views',
|
||||
icon: 'IconStar',
|
||||
objectUniversalIdentifier: PERSON_OBJECT_UNIVERSAL_IDENTIFIER,
|
||||
};
|
||||
|
||||
// Each of the six view-xxx child entities is reparentable: moving a child
|
||||
// (same universalIdentifier) from one view to another must only update its
|
||||
// viewId (the row is preserved, not recreated).
|
||||
const buildViewChildren = () => ({
|
||||
fields: [
|
||||
{
|
||||
universalIdentifier: VIEW_FIELD_ID,
|
||||
fieldMetadataUniversalIdentifier: TEST_FIELD_ID,
|
||||
position: 1,
|
||||
isVisible: true,
|
||||
size: 150,
|
||||
},
|
||||
],
|
||||
fieldGroups: [
|
||||
{
|
||||
universalIdentifier: VIEW_FIELD_GROUP_ID,
|
||||
name: 'Reparent Field Group',
|
||||
position: 0,
|
||||
isVisible: true,
|
||||
},
|
||||
],
|
||||
groups: [
|
||||
{
|
||||
universalIdentifier: VIEW_GROUP_ID,
|
||||
fieldValue: 'reparent-group-value',
|
||||
position: 0,
|
||||
isVisible: true,
|
||||
},
|
||||
],
|
||||
filters: [
|
||||
{
|
||||
universalIdentifier: VIEW_FILTER_ID,
|
||||
fieldMetadataUniversalIdentifier: TEST_FIELD_ID,
|
||||
operand: ViewFilterOperand.CONTAINS,
|
||||
value: 'reparent',
|
||||
},
|
||||
],
|
||||
sorts: [
|
||||
{
|
||||
universalIdentifier: VIEW_SORT_ID,
|
||||
fieldMetadataUniversalIdentifier: TEST_FIELD_ID,
|
||||
direction: ViewSortDirection.ASC,
|
||||
},
|
||||
],
|
||||
filterGroups: [
|
||||
{
|
||||
universalIdentifier: VIEW_FILTER_GROUP_ID,
|
||||
logicalOperator: ViewFilterGroupLogicalOperator.AND,
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
const SOURCE_VIEW = {
|
||||
universalIdentifier: SOURCE_VIEW_ID,
|
||||
name: SOURCE_VIEW_NAME,
|
||||
objectUniversalIdentifier: PERSON_OBJECT_UNIVERSAL_IDENTIFIER,
|
||||
type: ViewType.TABLE,
|
||||
icon: 'IconLayoutList',
|
||||
};
|
||||
|
||||
const TARGET_VIEW = {
|
||||
universalIdentifier: TARGET_VIEW_ID,
|
||||
name: TARGET_VIEW_NAME,
|
||||
objectUniversalIdentifier: PERSON_OBJECT_UNIVERSAL_IDENTIFIER,
|
||||
type: ViewType.TABLE,
|
||||
icon: 'IconLayoutList',
|
||||
};
|
||||
|
||||
const buildManifestWithViews = (views: Manifest['views']): Manifest =>
|
||||
buildBaseManifest({
|
||||
appId: TEST_APP_ID,
|
||||
roleId: TEST_ROLE_ID,
|
||||
overrides: {
|
||||
fields: [personFieldManifest],
|
||||
views,
|
||||
},
|
||||
});
|
||||
|
||||
// Both views exist; the children live under the chosen view.
|
||||
const buildManifestWithChildrenOnView = (
|
||||
childrenViewUniversalIdentifier: string,
|
||||
): Manifest => {
|
||||
const children = buildViewChildren();
|
||||
|
||||
return buildManifestWithViews([
|
||||
childrenViewUniversalIdentifier === SOURCE_VIEW_ID
|
||||
? { ...SOURCE_VIEW, ...children }
|
||||
: SOURCE_VIEW,
|
||||
childrenViewUniversalIdentifier === TARGET_VIEW_ID
|
||||
? { ...TARGET_VIEW, ...children }
|
||||
: TARGET_VIEW,
|
||||
]);
|
||||
};
|
||||
|
||||
// The source view is removed from the manifest (deleted) while its children are
|
||||
// reparented onto the target view in the very same sync.
|
||||
const buildManifestWithSourceViewDeletedAndChildrenOnTarget = (): Manifest =>
|
||||
buildManifestWithViews([{ ...TARGET_VIEW, ...buildViewChildren() }]);
|
||||
|
||||
const VIEW_WITH_CHILDREN_GQL_FIELDS = `
|
||||
id
|
||||
name
|
||||
viewFields { id viewId }
|
||||
viewFieldGroups { id viewId }
|
||||
viewGroups { id viewId }
|
||||
viewFilters { id viewId }
|
||||
viewSorts { id viewId }
|
||||
viewFilterGroups { id viewId }
|
||||
`;
|
||||
|
||||
type ViewWithChildren = {
|
||||
id: string;
|
||||
name: string;
|
||||
viewFields: { id: string; viewId: string }[];
|
||||
viewFieldGroups: { id: string; viewId: string }[];
|
||||
viewGroups: { id: string; viewId: string }[];
|
||||
viewFilters: { id: string; viewId: string }[];
|
||||
viewSorts: { id: string; viewId: string }[];
|
||||
viewFilterGroups: { id: string; viewId: string }[];
|
||||
};
|
||||
|
||||
const CHILD_COLLECTION_KEYS = [
|
||||
'viewFields',
|
||||
'viewFieldGroups',
|
||||
'viewGroups',
|
||||
'viewFilters',
|
||||
'viewSorts',
|
||||
'viewFilterGroups',
|
||||
] as const satisfies readonly (keyof ViewWithChildren)[];
|
||||
|
||||
const findPersonObjectId = async () => {
|
||||
const objects = await findManyObjectMetadataWithIndexes({
|
||||
expectToFail: false,
|
||||
});
|
||||
|
||||
const person = objects.find(
|
||||
(object) =>
|
||||
object.universalIdentifier === PERSON_OBJECT_UNIVERSAL_IDENTIFIER,
|
||||
);
|
||||
|
||||
if (!person) {
|
||||
throw new Error('Standard Person object not found in workspace');
|
||||
}
|
||||
|
||||
return person.id;
|
||||
};
|
||||
|
||||
const findPersonViewsWithChildren = async (
|
||||
personObjectId: string,
|
||||
): Promise<ViewWithChildren[]> => {
|
||||
const { data } = await findViews({
|
||||
objectMetadataId: personObjectId,
|
||||
gqlFields: VIEW_WITH_CHILDREN_GQL_FIELDS,
|
||||
expectToFail: false,
|
||||
});
|
||||
|
||||
return (data?.getViews ?? []) as unknown as ViewWithChildren[];
|
||||
};
|
||||
|
||||
const findReparentViews = async (personObjectId: string) => {
|
||||
const views = await findPersonViewsWithChildren(personObjectId);
|
||||
|
||||
const sourceView = views.find((view) => view.name === SOURCE_VIEW_NAME);
|
||||
const targetView = views.find((view) => view.name === TARGET_VIEW_NAME);
|
||||
|
||||
if (!sourceView || !targetView) {
|
||||
throw new Error('Reparent source and/or target views not found');
|
||||
}
|
||||
|
||||
return { sourceView, targetView };
|
||||
};
|
||||
|
||||
const getChildIdsByCollection = (view: ViewWithChildren) =>
|
||||
Object.fromEntries(
|
||||
CHILD_COLLECTION_KEYS.map((childCollectionKey) => [
|
||||
childCollectionKey,
|
||||
view[childCollectionKey][0].id,
|
||||
]),
|
||||
) as Record<(typeof CHILD_COLLECTION_KEYS)[number], string>;
|
||||
|
||||
describe('Manifest reparent - all view children are reparentable', () => {
|
||||
beforeEach(async () => {
|
||||
await setupApplicationForSync({
|
||||
applicationUniversalIdentifier: TEST_APP_ID,
|
||||
name: 'Test Application',
|
||||
description: 'App for testing view children reparenting across views',
|
||||
sourcePath: 'test-manifest-reparent-view-children',
|
||||
});
|
||||
}, 60000);
|
||||
|
||||
afterEach(async () => {
|
||||
await cleanupApplicationAndAppRegistration({
|
||||
applicationUniversalIdentifier: TEST_APP_ID,
|
||||
});
|
||||
});
|
||||
|
||||
it('moves every view child from the source view to the target view by only updating viewId', async () => {
|
||||
const personObjectId = await findPersonObjectId();
|
||||
|
||||
const { errors: firstSyncErrors } = await syncApplication({
|
||||
manifest: buildManifestWithChildrenOnView(SOURCE_VIEW_ID),
|
||||
expectToFail: false,
|
||||
});
|
||||
|
||||
expect(firstSyncErrors).toBeUndefined();
|
||||
|
||||
const {
|
||||
sourceView: sourceAfterFirstSync,
|
||||
targetView: targetAfterFirstSync,
|
||||
} = await findReparentViews(personObjectId);
|
||||
|
||||
// Every child collection starts on the source view with exactly one row,
|
||||
// and the target view has none.
|
||||
for (const childCollectionKey of CHILD_COLLECTION_KEYS) {
|
||||
expect(sourceAfterFirstSync[childCollectionKey]).toHaveLength(1);
|
||||
expect(sourceAfterFirstSync[childCollectionKey][0].viewId).toBe(
|
||||
sourceAfterFirstSync.id,
|
||||
);
|
||||
expect(targetAfterFirstSync[childCollectionKey]).toHaveLength(0);
|
||||
}
|
||||
|
||||
const childIdsByCollectionBeforeReparent =
|
||||
getChildIdsByCollection(sourceAfterFirstSync);
|
||||
|
||||
const { errors: secondSyncErrors } = await syncApplication({
|
||||
manifest: buildManifestWithChildrenOnView(TARGET_VIEW_ID),
|
||||
expectToFail: false,
|
||||
});
|
||||
|
||||
expect(secondSyncErrors).toBeUndefined();
|
||||
|
||||
const { sourceView: sourceAfterReparent, targetView: targetAfterReparent } =
|
||||
await findReparentViews(personObjectId);
|
||||
|
||||
for (const childCollectionKey of CHILD_COLLECTION_KEYS) {
|
||||
// The source view no longer owns any child.
|
||||
expect(sourceAfterReparent[childCollectionKey]).toHaveLength(0);
|
||||
|
||||
// The target view now owns exactly the same row (same id), proving the
|
||||
// reparent updated viewId instead of deleting and recreating the child.
|
||||
expect(targetAfterReparent[childCollectionKey]).toHaveLength(1);
|
||||
expect(targetAfterReparent[childCollectionKey][0].id).toBe(
|
||||
childIdsByCollectionBeforeReparent[childCollectionKey],
|
||||
);
|
||||
expect(targetAfterReparent[childCollectionKey][0].viewId).toBe(
|
||||
targetAfterReparent.id,
|
||||
);
|
||||
}
|
||||
}, 60000);
|
||||
|
||||
it('reparents every view child onto the target view when the source view is deleted in the same sync', async () => {
|
||||
const personObjectId = await findPersonObjectId();
|
||||
|
||||
const { errors: firstSyncErrors } = await syncApplication({
|
||||
manifest: buildManifestWithChildrenOnView(SOURCE_VIEW_ID),
|
||||
expectToFail: false,
|
||||
});
|
||||
|
||||
expect(firstSyncErrors).toBeUndefined();
|
||||
|
||||
const { sourceView: sourceAfterFirstSync } =
|
||||
await findReparentViews(personObjectId);
|
||||
|
||||
const childIdsByCollectionBeforeReparent =
|
||||
getChildIdsByCollection(sourceAfterFirstSync);
|
||||
|
||||
// Second sync deletes the source view and reparents its children onto the
|
||||
// target view at once. Because viewId is onDelete: CASCADE, this only
|
||||
// succeeds if the reparent updates run before the source view deletion.
|
||||
const { errors: secondSyncErrors } = await syncApplication({
|
||||
manifest: buildManifestWithSourceViewDeletedAndChildrenOnTarget(),
|
||||
expectToFail: false,
|
||||
});
|
||||
|
||||
expect(secondSyncErrors).toBeUndefined();
|
||||
|
||||
const viewsAfterReparent =
|
||||
await findPersonViewsWithChildren(personObjectId);
|
||||
|
||||
const sourceAfterReparent = viewsAfterReparent.find(
|
||||
(view) => view.name === SOURCE_VIEW_NAME,
|
||||
);
|
||||
const targetAfterReparent = viewsAfterReparent.find(
|
||||
(view) => view.name === TARGET_VIEW_NAME,
|
||||
);
|
||||
|
||||
// The source view has been deleted.
|
||||
expect(sourceAfterReparent).toBeUndefined();
|
||||
|
||||
if (!targetAfterReparent) {
|
||||
throw new Error('Reparent target view not found after sync');
|
||||
}
|
||||
|
||||
for (const childCollectionKey of CHILD_COLLECTION_KEYS) {
|
||||
// Each child survived the source view deletion and now lives on the
|
||||
// target view as the very same row (same id) instead of being
|
||||
// cascade-deleted along with the source view.
|
||||
expect(targetAfterReparent[childCollectionKey]).toHaveLength(1);
|
||||
expect(targetAfterReparent[childCollectionKey][0].id).toBe(
|
||||
childIdsByCollectionBeforeReparent[childCollectionKey],
|
||||
);
|
||||
expect(targetAfterReparent[childCollectionKey][0].viewId).toBe(
|
||||
targetAfterReparent.id,
|
||||
);
|
||||
}
|
||||
}, 60000);
|
||||
});
|
||||
Reference in New Issue
Block a user