From 29e03270639c98e54844f5fafa3fdc8012a046d9 Mon Sep 17 00:00:00 2001 From: martmull Date: Thu, 25 Jun 2026 12:04:18 +0200 Subject: [PATCH] fix(server): allow moving menu items into a folder created in the same sync (#22130) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Context Fixes [core-team-issues#2593](https://github.com/twentyhq/core-team-issues/issues/2593). When reorganizing navigation menu items by moving existing items into a **newly created folder** within a single deploy, the sync failed with `Parent navigation menu item not found`, forcing a two-step deploy (create the folder first, then move the items into it). ## Root cause Migration entities are validated in the fixed order **delete → update → create** (`workspace-entity-migration-builder.service.ts`). When items are moved into a new folder in one sync, the items are *updated* (adding `folderUniversalIdentifier`) while the folder is *created* — but the update phase runs before the create phase, so the folder isn't yet in the optimistic maps. The **creation** validator already handles "parent doesn't exist yet" by also checking `remainingFlatEntityMapsToValidate`. The **update** validator couldn't: `FlatEntityUpdateValidationArgs` explicitly omitted that field, so it only looked at the optimistic maps and threw. ## Changes - `universal-flat-entity-update-validation-args.type.ts` — stop omitting `remainingFlatEntityMapsToValidate` from the update args. - `workspace-entity-migration-builder.service.ts` — pass `createdFlatEntityMaps` (entities being created in the same migration) into update validation. - `flat-navigation-menu-item-validator.service.ts` — resolve the parent folder against both the optimistic maps and the to-be-created entities, mirroring the creation validator. - Integration test — sync an item, then in a second sync create a folder and move the item into it, asserting it succeeds in a single deploy. The change is generic and type-safe: all other update validators receive the new field and simply ignore it. `createdFlatEntityMaps` is `MetadataUniversalFlatEntityMaps`, matching the field's type. ## Test plan - [x] Added integration test `should move existing menu items into a folder created in the same sync` - [ ] CI green https://claude.ai/code/session_017pmBkho9Fh6Vjv8WA4m9YE --- _Generated by [Claude Code](https://claude.ai/code/session_017pmBkho9Fh6Vjv8WA4m9YE)_ Review in cubic --- ...kspace-entity-migration-builder.service.ts | 1 + ...flat-entity-update-validation-args.type.ts | 2 +- ...-navigation-menu-item-validator.service.ts | 14 +++- ...e-navigation-menu-item.integration-spec.ts | 70 +++++++++++++++++++ 4 files changed, 84 insertions(+), 3 deletions(-) diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/services/workspace-entity-migration-builder.service.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/services/workspace-entity-migration-builder.service.ts index fe3c167847..4d1c16da83 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/services/workspace-entity-migration-builder.service.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/services/workspace-entity-migration-builder.service.ts @@ -208,6 +208,7 @@ export abstract class WorkspaceEntityMigrationBuilderService< buildOptions, additionalCacheDataMaps, universalIdentifier: flatEntityToUpdateUniversalIdentifier, + remainingFlatEntityMapsToValidate: createdFlatEntityMaps, }); if (validationResult.status === 'fail') { diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/types/universal-flat-entity-update-validation-args.type.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/types/universal-flat-entity-update-validation-args.type.ts index d1cf1a5560..005da4ecd0 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/types/universal-flat-entity-update-validation-args.type.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/types/universal-flat-entity-update-validation-args.type.ts @@ -5,7 +5,7 @@ import { type UniversalFlatEntityValidationArgs } from 'src/engine/workspace-man export type FlatEntityUpdateValidationArgs = Omit< UniversalFlatEntityValidationArgs, - 'flatEntityToValidate' | 'remainingFlatEntityMapsToValidate' + 'flatEntityToValidate' > & { flatEntityUpdate: UniversalFlatEntityUpdate; universalIdentifier: string; diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-navigation-menu-item-validator.service.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-navigation-menu-item-validator.service.ts index 5972799eae..135119e99f 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-navigation-menu-item-validator.service.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-navigation-menu-item-validator.service.ts @@ -284,6 +284,7 @@ export class FlatNavigationMenuItemValidatorService { optimisticFlatEntityMapsAndRelatedFlatEntityMaps: { flatNavigationMenuItemMaps: optimisticFlatNavigationMenuItemMaps, }, + remainingFlatEntityMapsToValidate, }: FlatEntityUpdateValidationArgs< typeof ALL_METADATA_NAME.navigationMenuItem >): FailedFlatEntityValidation<'navigationMenuItem', 'update'> { @@ -352,12 +353,21 @@ export class FlatNavigationMenuItemValidatorService { const newFolderUniversalIdentifier = folderUniversalIdentifierUpdate; + const combinedFlatNavigationMenuItemMaps: MetadataUniversalFlatEntityMaps< + typeof ALL_METADATA_NAME.navigationMenuItem + > = { + byUniversalIdentifier: { + ...remainingFlatEntityMapsToValidate.byUniversalIdentifier, + ...optimisticFlatNavigationMenuItemMaps.byUniversalIdentifier, + }, + }; + const circularDependencyErrors = this.getCircularDependencyValidationErrors( { navigationMenuItemUniversalIdentifier: fromFlatNavigationMenuItem.universalIdentifier, folderUniversalIdentifier: newFolderUniversalIdentifier, - flatNavigationMenuItemMaps: optimisticFlatNavigationMenuItemMaps, + flatNavigationMenuItemMaps: combinedFlatNavigationMenuItemMaps, }, ); @@ -368,7 +378,7 @@ export class FlatNavigationMenuItemValidatorService { const referencedParentNavigationMenuItem = findFlatEntityByUniversalIdentifier({ universalIdentifier: newFolderUniversalIdentifier, - flatEntityMaps: optimisticFlatNavigationMenuItemMaps, + flatEntityMaps: combinedFlatNavigationMenuItemMaps, }); if (!isDefined(referencedParentNavigationMenuItem)) { diff --git a/packages/twenty-server/test/integration/metadata/suites/application/successful-manifest-update-navigation-menu-item.integration-spec.ts b/packages/twenty-server/test/integration/metadata/suites/application/successful-manifest-update-navigation-menu-item.integration-spec.ts index 51e32946c0..2a4eff55bd 100644 --- a/packages/twenty-server/test/integration/metadata/suites/application/successful-manifest-update-navigation-menu-item.integration-spec.ts +++ b/packages/twenty-server/test/integration/metadata/suites/application/successful-manifest-update-navigation-menu-item.integration-spec.ts @@ -167,6 +167,76 @@ describe('Manifest update - navigation menu items', () => { }); }, 60000); + it('should move existing menu items into a folder created in the same sync', async () => { + await syncApplication({ + manifest: buildManifest({ + navigationMenuItems: [ + { + universalIdentifier: TEST_CHILD_ID, + type: NavigationMenuItemType.LINK, + name: 'Child Link', + icon: 'IconLink', + position: 0, + link: 'https://example.com', + }, + ], + }), + expectToFail: false, + }); + + const itemsAfterFirstSync = await findAppNavigationMenuItems(); + + expect(itemsAfterFirstSync).toHaveLength(1); + expect(itemsAfterFirstSync[0]).toMatchObject({ + type: NavigationMenuItemType.LINK, + name: 'Child Link', + folderId: null, + }); + + await syncApplication({ + manifest: buildManifest({ + navigationMenuItems: [ + { + universalIdentifier: TEST_FOLDER_ID, + type: NavigationMenuItemType.FOLDER, + name: 'Test Folder', + icon: 'IconFolder', + position: 0, + }, + { + universalIdentifier: TEST_CHILD_ID, + type: NavigationMenuItemType.LINK, + name: 'Child Link', + icon: 'IconLink', + position: 0, + link: 'https://example.com', + folderUniversalIdentifier: TEST_FOLDER_ID, + }, + ], + }), + expectToFail: false, + }); + + const itemsAfterSecondSync = await findAppNavigationMenuItems(); + + expect(itemsAfterSecondSync).toHaveLength(2); + + const folder = itemsAfterSecondSync.find( + (item) => item.type === NavigationMenuItemType.FOLDER, + ); + const child = itemsAfterSecondSync.find( + (item) => item.type === NavigationMenuItemType.LINK, + ); + + expect(folder).toBeDefined(); + expect(child).toBeDefined(); + expect(child).toMatchObject({ + type: NavigationMenuItemType.LINK, + name: 'Child Link', + folderId: folder!.id, + }); + }, 60000); + it('should delete navigation menu items when removed from manifest on second sync', async () => { await syncApplication({ manifest: buildManifest({