From c4a64467576fd8af68df400d0b558e639258c4c0 Mon Sep 17 00:00:00 2001 From: Etienne <45695613+etiennejouan@users.noreply.github.com> Date: Tue, 30 Jun 2026 11:41:26 +0200 Subject: [PATCH] fix(navigation-menu-item): reject PAGE_LAYOUT items that don't reference a STANDALONE_PAGE layout (#22343) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Issue A `PAGE_LAYOUT` navigation menu item can be created pointing at a page layout whose type is **not** `STANDALONE_PAGE` (e.g. a `DASHBOARD`). The sidebar always links such an item to `/page/`, but that route only renders `STANDALONE_PAGE` layouts, anything else is redirected to 404. Result: a silently broken sidebar link (cc: https://discord.com/channels/1130383047699738754/1519045990047285288). ## Root cause - `/page/:pageLayoutId` is standalone-only by design (route guard in `usePageChangeEffectNavigateLocation`, and `StandalonePageLayoutPage` hardcodes `layoutType: STANDALONE_PAGE`). Dashboards/record pages are reached elsewhere (record show page). - A `PAGE_LAYOUT` nav item unconditionally computes `/page/`. - No validation ensured the referenced layout is `STANDALONE_PAGE`: the migration/manifest validator only checked that `pageLayoutId` was present, the DB constraint only checked `NOT NULL`, and the runtime tool description even suggested pinning dashboards this way. So an app manifest pairing a `DASHBOARD` layout with a `PAGE_LAYOUT` nav item installed cleanly and produced a dead link. ## Fix (treat as invalid config — fail fast) - Cross-entity validation in `FlatNavigationMenuItemValidatorService` (both create and update): when `type === PAGE_LAYOUT`, resolve the referenced page layout from the optimistic page-layout maps and raise `INVALID_NAVIGATION_MENU_ITEM_INPUT` if its `type !== STANDALONE_PAGE`. Existence keeps being enforced by foreign-key resolution, so the type check only fires when the layout resolves. - Corrected the misleading `create_navigation_menu_item` tool description (no longer says "e.g. a dashboard"; states the target must be a `STANDALONE_PAGE`). - Added unit tests covering: `STANDALONE_PAGE` accepted; `DASHBOARD` rejected; `RECORD_PAGE` rejected; unresolved reference not flagged as a type error. ## Files changed - `flat-navigation-menu-item-validator.service.ts` — new `validatePageLayoutReference` + wired into create/update. - `create-navigation-menu-item.tool.ts` — tool description fix. - `__tests__/flat-navigation-menu-item-validator.service.spec.ts` — new tests (4 passing). ## Out of scope / follow-up - To open discussion, check https://github.com/twentyhq/twenty/pull/22255 Review in cubic --- ...layout-reference-cross-entity.util.spec.ts | 230 ++++++++++++++++++ ...page-layout-reference-cross-entity.util.ts | 128 ++++++++++ .../tools/create-navigation-menu-item.tool.ts | 2 +- ...ross-entity-transversal-validation.util.ts | 8 + 4 files changed, 367 insertions(+), 1 deletion(-) create mode 100644 packages/twenty-server/src/engine/metadata-modules/flat-navigation-menu-item/validators/utils/__tests__/validate-navigation-menu-item-page-layout-reference-cross-entity.util.spec.ts create mode 100644 packages/twenty-server/src/engine/metadata-modules/flat-navigation-menu-item/validators/utils/validate-navigation-menu-item-page-layout-reference-cross-entity.util.ts diff --git a/packages/twenty-server/src/engine/metadata-modules/flat-navigation-menu-item/validators/utils/__tests__/validate-navigation-menu-item-page-layout-reference-cross-entity.util.spec.ts b/packages/twenty-server/src/engine/metadata-modules/flat-navigation-menu-item/validators/utils/__tests__/validate-navigation-menu-item-page-layout-reference-cross-entity.util.spec.ts new file mode 100644 index 0000000000..c321765b6e --- /dev/null +++ b/packages/twenty-server/src/engine/metadata-modules/flat-navigation-menu-item/validators/utils/__tests__/validate-navigation-menu-item-page-layout-reference-cross-entity.util.spec.ts @@ -0,0 +1,230 @@ +import { NavigationMenuItemType } from 'twenty-shared/types'; + +import { createEmptyFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/constant/create-empty-flat-entity-maps.constant'; +import { validateNavigationMenuItemPageLayoutReferenceCrossEntity } from 'src/engine/metadata-modules/flat-navigation-menu-item/validators/utils/validate-navigation-menu-item-page-layout-reference-cross-entity.util'; +import { NavigationMenuItemExceptionCode } from 'src/engine/metadata-modules/navigation-menu-item/navigation-menu-item.exception'; +import { PageLayoutType } from 'src/engine/metadata-modules/page-layout/enums/page-layout-type.enum'; + +const NAVIGATION_MENU_ITEM_UNIVERSAL_IDENTIFIER = + '00000000-0000-0000-0000-000000000001'; +const PAGE_LAYOUT_UNIVERSAL_IDENTIFIER = '00000000-0000-0000-0000-0000000000aa'; +const MISSING_PAGE_LAYOUT_UNIVERSAL_IDENTIFIER = + '00000000-0000-0000-0000-0000000000bb'; + +const mapsFrom = (entities: { universalIdentifier: string }[]): any => { + const maps = createEmptyFlatEntityMaps() as any; + + for (const entity of entities) { + maps.byUniversalIdentifier[entity.universalIdentifier] = entity; + } + + return maps; +}; + +const pageLayout = ( + type: PageLayoutType, + universalIdentifier = PAGE_LAYOUT_UNIVERSAL_IDENTIFIER, +) => ({ + universalIdentifier, + type, +}); + +const navigationMenuItem = ( + pageLayoutUniversalIdentifier: string | null, + universalIdentifier = NAVIGATION_MENU_ITEM_UNIVERSAL_IDENTIFIER, +) => ({ + universalIdentifier, + type: NavigationMenuItemType.PAGE_LAYOUT, + pageLayoutUniversalIdentifier, +}); + +const emptyActions = () => ({ create: [], update: [], delete: [] }); + +const reportFrom = ({ + createdNavigationMenuItemUniversalIdentifiers = [], + updatedNavigationMenuItemUniversalIdentifiers = [], + createdPageLayoutUniversalIdentifiers = [], + updatedPageLayoutUniversalIdentifiers = [], +}: { + createdNavigationMenuItemUniversalIdentifiers?: string[]; + updatedNavigationMenuItemUniversalIdentifiers?: string[]; + createdPageLayoutUniversalIdentifiers?: string[]; + updatedPageLayoutUniversalIdentifiers?: string[]; +}): any => ({ + navigationMenuItem: { + ...emptyActions(), + create: createdNavigationMenuItemUniversalIdentifiers.map( + (universalIdentifier) => ({ flatEntity: { universalIdentifier } }), + ), + update: updatedNavigationMenuItemUniversalIdentifiers.map( + (universalIdentifier) => ({ universalIdentifier }), + ), + }, + pageLayout: { + ...emptyActions(), + create: createdPageLayoutUniversalIdentifiers.map( + (universalIdentifier) => ({ flatEntity: { universalIdentifier } }), + ), + update: updatedPageLayoutUniversalIdentifiers.map( + (universalIdentifier) => ({ + universalIdentifier, + }), + ), + }, +}); + +const run = ({ + navigationMenuItems, + pageLayouts, + report, +}: { + navigationMenuItems: { universalIdentifier: string }[]; + pageLayouts: { universalIdentifier: string }[]; + report: any; +}) => + validateNavigationMenuItemPageLayoutReferenceCrossEntity({ + optimisticUniversalFlatMaps: { + flatNavigationMenuItemMaps: mapsFrom(navigationMenuItems), + flatPageLayoutMaps: mapsFrom(pageLayouts), + }, + orchestratorActionsReport: report, + }); + +const errorCodes = (result: ReturnType) => + result.navigationMenuItem.flatMap((failed) => + failed.errors.map((error) => error.code), + ); + +describe('validateNavigationMenuItemPageLayoutReferenceCrossEntity', () => { + it('accepts a created PAGE_LAYOUT item referencing a same-migration STANDALONE_PAGE layout', () => { + const result = run({ + navigationMenuItems: [ + navigationMenuItem(PAGE_LAYOUT_UNIVERSAL_IDENTIFIER), + ], + pageLayouts: [pageLayout(PageLayoutType.STANDALONE_PAGE)], + report: reportFrom({ + createdNavigationMenuItemUniversalIdentifiers: [ + NAVIGATION_MENU_ITEM_UNIVERSAL_IDENTIFIER, + ], + createdPageLayoutUniversalIdentifiers: [ + PAGE_LAYOUT_UNIVERSAL_IDENTIFIER, + ], + }), + }); + + expect(errorCodes(result)).not.toContain( + NavigationMenuItemExceptionCode.INVALID_NAVIGATION_MENU_ITEM_INPUT, + ); + }); + + it('rejects a created PAGE_LAYOUT item referencing a same-migration DASHBOARD layout', () => { + const result = run({ + navigationMenuItems: [ + navigationMenuItem(PAGE_LAYOUT_UNIVERSAL_IDENTIFIER), + ], + pageLayouts: [pageLayout(PageLayoutType.DASHBOARD)], + report: reportFrom({ + createdNavigationMenuItemUniversalIdentifiers: [ + NAVIGATION_MENU_ITEM_UNIVERSAL_IDENTIFIER, + ], + createdPageLayoutUniversalIdentifiers: [ + PAGE_LAYOUT_UNIVERSAL_IDENTIFIER, + ], + }), + }); + + expect(errorCodes(result)).toContain( + NavigationMenuItemExceptionCode.INVALID_NAVIGATION_MENU_ITEM_INPUT, + ); + }); + + it('rejects a created PAGE_LAYOUT item referencing a same-migration RECORD_PAGE layout', () => { + const result = run({ + navigationMenuItems: [ + navigationMenuItem(PAGE_LAYOUT_UNIVERSAL_IDENTIFIER), + ], + pageLayouts: [pageLayout(PageLayoutType.RECORD_PAGE)], + report: reportFrom({ + createdNavigationMenuItemUniversalIdentifiers: [ + NAVIGATION_MENU_ITEM_UNIVERSAL_IDENTIFIER, + ], + createdPageLayoutUniversalIdentifiers: [ + PAGE_LAYOUT_UNIVERSAL_IDENTIFIER, + ], + }), + }); + + expect(errorCodes(result)).toContain( + NavigationMenuItemExceptionCode.INVALID_NAVIGATION_MENU_ITEM_INPUT, + ); + }); + + it('does not raise a type error when the referenced layout cannot be resolved', () => { + const result = run({ + navigationMenuItems: [ + navigationMenuItem(MISSING_PAGE_LAYOUT_UNIVERSAL_IDENTIFIER), + ], + pageLayouts: [pageLayout(PageLayoutType.STANDALONE_PAGE)], + report: reportFrom({ + createdNavigationMenuItemUniversalIdentifiers: [ + NAVIGATION_MENU_ITEM_UNIVERSAL_IDENTIFIER, + ], + }), + }); + + expect(errorCodes(result)).not.toContain( + NavigationMenuItemExceptionCode.INVALID_NAVIGATION_MENU_ITEM_INPUT, + ); + }); + + it('rejects an untouched item when its layout type is updated to a non-standalone type in the same migration', () => { + const result = run({ + navigationMenuItems: [ + navigationMenuItem(PAGE_LAYOUT_UNIVERSAL_IDENTIFIER), + ], + pageLayouts: [pageLayout(PageLayoutType.RECORD_PAGE)], + report: reportFrom({ + updatedPageLayoutUniversalIdentifiers: [ + PAGE_LAYOUT_UNIVERSAL_IDENTIFIER, + ], + }), + }); + + expect(errorCodes(result)).toContain( + NavigationMenuItemExceptionCode.INVALID_NAVIGATION_MENU_ITEM_INPUT, + ); + }); + + it('accepts an updated item whose layout is concurrently updated to STANDALONE_PAGE (no false positive)', () => { + const result = run({ + navigationMenuItems: [ + navigationMenuItem(PAGE_LAYOUT_UNIVERSAL_IDENTIFIER), + ], + pageLayouts: [pageLayout(PageLayoutType.STANDALONE_PAGE)], + report: reportFrom({ + updatedNavigationMenuItemUniversalIdentifiers: [ + NAVIGATION_MENU_ITEM_UNIVERSAL_IDENTIFIER, + ], + updatedPageLayoutUniversalIdentifiers: [ + PAGE_LAYOUT_UNIVERSAL_IDENTIFIER, + ], + }), + }); + + expect(errorCodes(result)).not.toContain( + NavigationMenuItemExceptionCode.INVALID_NAVIGATION_MENU_ITEM_INPUT, + ); + }); + + it('ignores items that are neither touched nor pointing to a touched layout', () => { + const result = run({ + navigationMenuItems: [ + navigationMenuItem(PAGE_LAYOUT_UNIVERSAL_IDENTIFIER), + ], + pageLayouts: [pageLayout(PageLayoutType.DASHBOARD)], + report: reportFrom({}), + }); + + expect(errorCodes(result)).toHaveLength(0); + }); +}); diff --git a/packages/twenty-server/src/engine/metadata-modules/flat-navigation-menu-item/validators/utils/validate-navigation-menu-item-page-layout-reference-cross-entity.util.ts b/packages/twenty-server/src/engine/metadata-modules/flat-navigation-menu-item/validators/utils/validate-navigation-menu-item-page-layout-reference-cross-entity.util.ts new file mode 100644 index 0000000000..2375b7a764 --- /dev/null +++ b/packages/twenty-server/src/engine/metadata-modules/flat-navigation-menu-item/validators/utils/validate-navigation-menu-item-page-layout-reference-cross-entity.util.ts @@ -0,0 +1,128 @@ +import { msg, t } from '@lingui/core/macro'; +import { NavigationMenuItemType } from 'twenty-shared/types'; +import { isDefined } from 'twenty-shared/utils'; + +import { findFlatEntityByUniversalIdentifier } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-universal-identifier.util'; +import { NavigationMenuItemExceptionCode } from 'src/engine/metadata-modules/navigation-menu-item/navigation-menu-item.exception'; +import { PageLayoutType } from 'src/engine/metadata-modules/page-layout/enums/page-layout-type.enum'; +import { + type OrchestratorActionsReport, + type OrchestratorFailureReport, +} from 'src/engine/workspace-manager/workspace-migration/types/workspace-migration-orchestrator.type'; +import { type AllUniversalFlatEntityMaps } from 'src/engine/workspace-manager/workspace-migration/universal-flat-entity/types/all-universal-flat-entity-maps.type'; +import { getEmptyFlatEntityValidationError } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-builder/builders/utils/get-flat-entity-validation-error.util'; + +export const validateNavigationMenuItemPageLayoutReferenceCrossEntity = ({ + optimisticUniversalFlatMaps, + orchestratorActionsReport, +}: { + optimisticUniversalFlatMaps: Pick< + AllUniversalFlatEntityMaps, + 'flatNavigationMenuItemMaps' | 'flatPageLayoutMaps' + >; + orchestratorActionsReport: Pick< + OrchestratorActionsReport, + 'navigationMenuItem' | 'pageLayout' + >; +}): Pick => { + const validationErrors: Pick< + OrchestratorFailureReport, + 'navigationMenuItem' + > = { + navigationMenuItem: [], + }; + + const createdNavigationMenuItemUniversalIdentifiers = new Set( + orchestratorActionsReport.navigationMenuItem.create.map( + (action) => action.flatEntity.universalIdentifier, + ), + ); + + const updatedNavigationMenuItemUniversalIdentifiers = new Set( + orchestratorActionsReport.navigationMenuItem.update.map( + (action) => action.universalIdentifier, + ), + ); + + const touchedPageLayoutUniversalIdentifiers = new Set([ + ...orchestratorActionsReport.pageLayout.create.map( + (action) => action.flatEntity.universalIdentifier, + ), + ...orchestratorActionsReport.pageLayout.update.map( + (action) => action.universalIdentifier, + ), + ]); + + const navigationMenuItemUniversalIdentifiersToValidate = new Set([ + ...createdNavigationMenuItemUniversalIdentifiers, + ...updatedNavigationMenuItemUniversalIdentifiers, + ]); + + if (touchedPageLayoutUniversalIdentifiers.size > 0) { + for (const navigationMenuItem of Object.values( + optimisticUniversalFlatMaps.flatNavigationMenuItemMaps + .byUniversalIdentifier, + )) { + if ( + isDefined(navigationMenuItem) && + isDefined(navigationMenuItem.pageLayoutUniversalIdentifier) && + touchedPageLayoutUniversalIdentifiers.has( + navigationMenuItem.pageLayoutUniversalIdentifier, + ) + ) { + navigationMenuItemUniversalIdentifiersToValidate.add( + navigationMenuItem.universalIdentifier, + ); + } + } + } + + for (const universalIdentifier of navigationMenuItemUniversalIdentifiersToValidate) { + const navigationMenuItem = findFlatEntityByUniversalIdentifier({ + universalIdentifier, + flatEntityMaps: optimisticUniversalFlatMaps.flatNavigationMenuItemMaps, + }); + + if ( + !isDefined(navigationMenuItem) || + navigationMenuItem.type !== NavigationMenuItemType.PAGE_LAYOUT || + !isDefined(navigationMenuItem.pageLayoutUniversalIdentifier) + ) { + continue; + } + + const referencedPageLayout = findFlatEntityByUniversalIdentifier({ + universalIdentifier: navigationMenuItem.pageLayoutUniversalIdentifier, + flatEntityMaps: optimisticUniversalFlatMaps.flatPageLayoutMaps, + }); + + if ( + !isDefined(referencedPageLayout) || + referencedPageLayout.type === PageLayoutType.STANDALONE_PAGE + ) { + continue; + } + + const failedValidation = getEmptyFlatEntityValidationError({ + flatEntityMinimalInformation: { + universalIdentifier: navigationMenuItem.universalIdentifier, + }, + metadataName: 'navigationMenuItem', + type: createdNavigationMenuItemUniversalIdentifiers.has( + universalIdentifier, + ) + ? 'create' + : 'update', + }); + + failedValidation.errors.push({ + code: NavigationMenuItemExceptionCode.INVALID_NAVIGATION_MENU_ITEM_INPUT, + message: t`PAGE_LAYOUT navigation menu item must reference a STANDALONE_PAGE page layout`, + userFriendlyMessage: msg`A page layout navigation menu item can only point to a standalone page`, + }); + + validationErrors.navigationMenuItem.push(failedValidation); + } + + return validationErrors; +}; diff --git a/packages/twenty-server/src/engine/metadata-modules/navigation-menu-item/tools/create-navigation-menu-item.tool.ts b/packages/twenty-server/src/engine/metadata-modules/navigation-menu-item/tools/create-navigation-menu-item.tool.ts index bf66496521..882d2112f8 100644 --- a/packages/twenty-server/src/engine/metadata-modules/navigation-menu-item/tools/create-navigation-menu-item.tool.ts +++ b/packages/twenty-server/src/engine/metadata-modules/navigation-menu-item/tools/create-navigation-menu-item.tool.ts @@ -150,7 +150,7 @@ Type chooses the variant: - OBJECT: pins an object's standard view (label auto-derived from the object's plural name; only pass 'name' if the user wants a custom label). - VIEW: pins a saved view (label auto-derived from the view's name; only pass 'name' for a custom label). - RECORD: pins a single record (label auto-derived from the record's identifier; only pass 'name' for a custom label). -- PAGE_LAYOUT: pins a page layout, e.g. a dashboard (name required — no auto-derivation). +- PAGE_LAYOUT: pins a standalone page layout (name required — no auto-derivation). The referenced page layout must be of type STANDALONE_PAGE; dashboards and record/index page layouts are not reachable this way. Note: creating a new custom object via create_object_metadata already auto-creates an OBJECT navigation menu item — do not double-create.`, inputSchema: createNavigationMenuItemSchema, diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/utils/cross-entity-transversal-validation.util.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/utils/cross-entity-transversal-validation.util.ts index 3f9934b197..a08109608f 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/utils/cross-entity-transversal-validation.util.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/utils/cross-entity-transversal-validation.util.ts @@ -1,3 +1,4 @@ +import { validateNavigationMenuItemPageLayoutReferenceCrossEntity } from 'src/engine/metadata-modules/flat-navigation-menu-item/validators/utils/validate-navigation-menu-item-page-layout-reference-cross-entity.util'; import { validateObjectMetadataCrossEntity } from 'src/engine/metadata-modules/flat-object-metadata/validators/utils/validate-object-metadata-cross-entity.util'; import { validatePermissionFlagNotInUseCrossEntity } from 'src/engine/metadata-modules/flat-permission-flag/validators/utils/validate-permission-flag-not-in-use-cross-entity.util'; import { validateViewFieldLabelIdentifierCrossEntity } from 'src/engine/metadata-modules/flat-view-field/validators/utils/validate-view-field-label-identifier-cross-entity.util'; @@ -39,9 +40,16 @@ export const crossEntityTransversalValidation = ({ orchestratorActionsReport.permissionFlag.delete, }); + const { navigationMenuItem } = + validateNavigationMenuItemPageLayoutReferenceCrossEntity({ + optimisticUniversalFlatMaps, + orchestratorActionsReport, + }); + crossEntityFailureReport.objectMetadata.push(...objectMetadata); crossEntityFailureReport.viewField.push(...viewField); crossEntityFailureReport.permissionFlag.push(...permissionFlag); + crossEntityFailureReport.navigationMenuItem.push(...navigationMenuItem); validateUniversalIdentifierCrossEntityUniquenessThroughReportMutation({ optimisticUniversalFlatMaps,