From 0ae62898a39ddeec0d4dafb4cb05861ff56ff3fa Mon Sep 17 00:00:00 2001 From: Charles Bochet Date: Wed, 18 Mar 2026 10:39:50 +0100 Subject: [PATCH] fix: restructure navigationMenuItem type migration for safe upgrade path (#18722) ## Summary - Restructures the `navigationMenuItem.type` column migration to follow the established 3-step safe upgrade pattern (used for webhooks, files, etc.) - The previous migration added `type` as `NOT NULL DEFAULT 'VIEW'` with a CHECK constraint in one step, which incorrectly assigned `VIEW` to all existing rows regardless of their actual type (folders, records, links, objects) - Now: (1) migration adds column as nullable, (2) upgrade command backfills correct type from existing columns (`viewId`, `targetRecordId`, `targetObjectMetadataId`, `link`) and cleans conflicting columns, (3) shared utility applies `NOT NULL` + `CHECK` constraint ### Changes **Modified:** - `1773681736596-add-type-to-navigation-menu-item.ts` -- adds column as nullable, no DEFAULT, no CHECK - `navigation-menu-item.entity.ts` -- removed `default: NavigationMenuItemType.VIEW` from column decorator - `upgrade.command.ts` / `1-19-upgrade-version-command.module.ts` -- wired new command **Created:** - `1773681736596-makeNavigationMenuItemTypeNotNull.util.ts` -- shared utility applying NOT NULL + CHECK constraint - `1773822077682-make-navigation-menu-item-type-not-null.ts` -- migration calling the util with savepoint (succeeds on fresh installs, swallows error on upgrades with NULL data) - `1-19-backfill-navigation-menu-item-type.command.ts` -- upgrade command that backfills type, cleans conflicting columns, then applies constraints via the shared utility ### Execution flow **Existing deployments (upgrade):** 1. TypeORM migration adds nullable `type` column, drops old CHECK 2. Savepoint migration fails gracefully (existing rows have NULL type) 3. 1-18 favorites migration creates items WITH correct type 4. 1-19 backfill command infers type for remaining NULL rows, cleans conflicting columns, applies NOT NULL + CHECK **Fresh installs:** 1. TypeORM migration adds nullable `type` column 2. Savepoint migration succeeds immediately (no data, constraints apply cleanly) ## Test plan - [x] `npx nx typecheck twenty-server` passes - [x] `npx nx lint:diff-with-main twenty-server` passes - [x] `npx nx test twenty-server` passes (477 suites, 4297 tests) - [ ] Verify fresh database setup with `npx nx database:reset twenty-server` applies both migrations and constraints correctly - [ ] Verify upgrade path: existing navigation menu items get correct type backfilled based on their columns Made with [Cursor](https://cursor.com) --- ...kfill-navigation-menu-item-type.command.ts | 117 ++++++++++++++++++ .../1-19-upgrade-version-command.module.ts | 3 + .../upgrade.command.ts | 6 +- ...736596-add-type-to-navigation-menu-item.ts | 16 +-- ...make-navigation-menu-item-type-not-null.ts | 49 ++++++++ ...-makeNavigationMenuItemTypeNotNull.util.ts | 19 +++ .../entities/navigation-menu-item.entity.ts | 3 +- 7 files changed, 195 insertions(+), 18 deletions(-) create mode 100644 packages/twenty-server/src/database/commands/upgrade-version-command/1-19/1-19-backfill-navigation-menu-item-type.command.ts create mode 100644 packages/twenty-server/src/database/typeorm/core/migrations/common/1773822077682-make-navigation-menu-item-type-not-null.ts create mode 100644 packages/twenty-server/src/database/typeorm/core/migrations/utils/1773681736596-makeNavigationMenuItemTypeNotNull.util.ts diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/1-19/1-19-backfill-navigation-menu-item-type.command.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/1-19/1-19-backfill-navigation-menu-item-type.command.ts new file mode 100644 index 0000000000..afdca4efbf --- /dev/null +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/1-19/1-19-backfill-navigation-menu-item-type.command.ts @@ -0,0 +1,117 @@ +import { InjectDataSource, InjectRepository } from '@nestjs/typeorm'; + +import { Command } from 'nest-commander'; +import { DataSource, Repository } from 'typeorm'; + +import { ActiveOrSuspendedWorkspacesMigrationCommandRunner } from 'src/database/commands/command-runners/active-or-suspended-workspaces-migration.command-runner'; +import { RunOnWorkspaceArgs } from 'src/database/commands/command-runners/workspaces-migration.command-runner'; +import { makeNavigationMenuItemTypeNotNullQueries } from 'src/database/typeorm/core/migrations/utils/1773681736596-makeNavigationMenuItemTypeNotNull.util'; +import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity'; +import { DataSourceService } from 'src/engine/metadata-modules/data-source/data-source.service'; +import { GlobalWorkspaceOrmManager } from 'src/engine/twenty-orm/global-workspace-datasource/global-workspace-orm.manager'; + +@Command({ + name: 'upgrade:1-19:backfill-navigation-menu-item-type', + description: + 'Backfill navigation menu item type based on existing columns, then apply NOT NULL and CHECK constraints', +}) +export class BackfillNavigationMenuItemTypeCommand extends ActiveOrSuspendedWorkspacesMigrationCommandRunner { + private hasRunOnce = false; + + constructor( + @InjectRepository(WorkspaceEntity) + protected readonly workspaceRepository: Repository, + protected readonly twentyORMGlobalManager: GlobalWorkspaceOrmManager, + protected readonly dataSourceService: DataSourceService, + @InjectDataSource() + private readonly coreDataSource: DataSource, + ) { + super(workspaceRepository, twentyORMGlobalManager, dataSourceService); + } + + override async runOnWorkspace({ + options, + }: RunOnWorkspaceArgs): Promise { + if (this.hasRunOnce) { + this.logger.warn( + 'Skipping has already been run once BackfillNavigationMenuItemTypeCommand', + ); + + return; + } + + if (options.dryRun) { + return; + } + + const queryRunner = this.coreDataSource.createQueryRunner(); + + await queryRunner.connect(); + await queryRunner.startTransaction(); + + try { + await this.backfillType(queryRunner); + await this.cleanConflictingColumns(queryRunner); + await makeNavigationMenuItemTypeNotNullQueries(queryRunner); + + await queryRunner.commitTransaction(); + this.logger.log('Successfully run BackfillNavigationMenuItemTypeCommand'); + this.hasRunOnce = true; + } catch (error) { + await queryRunner.rollbackTransaction(); + this.logger.error( + `Rolling back BackfillNavigationMenuItemTypeCommand: ${error.message}`, + ); + } finally { + await queryRunner.release(); + } + } + + private async backfillType( + queryRunner: ReturnType, + ): Promise { + await queryRunner.query( + `UPDATE "core"."navigationMenuItem" SET "type" = 'VIEW' WHERE "type" IS NULL AND "viewId" IS NOT NULL`, + ); + + await queryRunner.query( + `UPDATE "core"."navigationMenuItem" SET "type" = 'RECORD' WHERE "type" IS NULL AND "targetRecordId" IS NOT NULL AND "targetObjectMetadataId" IS NOT NULL`, + ); + + await queryRunner.query( + `UPDATE "core"."navigationMenuItem" SET "type" = 'OBJECT' WHERE "type" IS NULL AND "targetObjectMetadataId" IS NOT NULL AND "targetRecordId" IS NULL`, + ); + + await queryRunner.query( + `UPDATE "core"."navigationMenuItem" SET "type" = 'LINK' WHERE "type" IS NULL AND "link" IS NOT NULL`, + ); + + await queryRunner.query( + `UPDATE "core"."navigationMenuItem" SET "type" = 'FOLDER' WHERE "type" IS NULL`, + ); + } + + private async cleanConflictingColumns( + queryRunner: ReturnType, + ): Promise { + await queryRunner.query( + `UPDATE "core"."navigationMenuItem" SET "targetRecordId" = NULL, "targetObjectMetadataId" = NULL, "link" = NULL WHERE "type" = 'VIEW'`, + ); + + await queryRunner.query( + `UPDATE "core"."navigationMenuItem" SET "viewId" = NULL, "link" = NULL WHERE "type" = 'RECORD'`, + ); + + await queryRunner.query( + `UPDATE "core"."navigationMenuItem" SET "viewId" = NULL, "targetRecordId" = NULL, "link" = NULL WHERE "type" = 'OBJECT'`, + ); + + await queryRunner.query( + `UPDATE "core"."navigationMenuItem" SET "viewId" = NULL, "targetRecordId" = NULL, "targetObjectMetadataId" = NULL WHERE "type" = 'LINK'`, + ); + + await queryRunner.query( + `UPDATE "core"."navigationMenuItem" SET "viewId" = NULL, "targetRecordId" = NULL, "targetObjectMetadataId" = NULL, "link" = NULL WHERE "type" = 'FOLDER'`, + ); + } +} diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/1-19/1-19-upgrade-version-command.module.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/1-19/1-19-upgrade-version-command.module.ts index 5e51e2b97d..f440e67e64 100644 --- a/packages/twenty-server/src/database/commands/upgrade-version-command/1-19/1-19-upgrade-version-command.module.ts +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/1-19/1-19-upgrade-version-command.module.ts @@ -4,6 +4,7 @@ import { TypeOrmModule } from '@nestjs/typeorm'; import { AddMissingSystemFieldsToStandardObjectsCommand } from 'src/database/commands/upgrade-version-command/1-19/1-19-add-missing-system-fields-to-standard-objects.command'; import { BackfillMessageChannelMessageAssociationMessageFolderCommand } from 'src/database/commands/upgrade-version-command/1-19/1-19-backfill-message-channel-message-association-message-folder.command'; import { BackfillMissingStandardViewsCommand } from 'src/database/commands/upgrade-version-command/1-19/1-19-backfill-missing-standard-views.command'; +import { BackfillNavigationMenuItemTypeCommand } from 'src/database/commands/upgrade-version-command/1-19/1-19-backfill-navigation-menu-item-type.command'; import { BackfillSystemFieldsIsSystemCommand } from 'src/database/commands/upgrade-version-command/1-19/1-19-backfill-system-fields-is-system.command'; import { FixInvalidStandardUniversalIdentifiersCommand } from 'src/database/commands/upgrade-version-command/1-19/1-19-fix-invalid-standard-universal-identifiers.command'; import { SeedServerIdCommand } from 'src/database/commands/upgrade-version-command/1-19/1-19-seed-server-id.command'; @@ -34,6 +35,7 @@ import { WorkspaceMigrationModule } from 'src/engine/workspace-manager/workspace AddMissingSystemFieldsToStandardObjectsCommand, BackfillMessageChannelMessageAssociationMessageFolderCommand, BackfillMissingStandardViewsCommand, + BackfillNavigationMenuItemTypeCommand, FixInvalidStandardUniversalIdentifiersCommand, SeedServerIdCommand, ], @@ -42,6 +44,7 @@ import { WorkspaceMigrationModule } from 'src/engine/workspace-manager/workspace AddMissingSystemFieldsToStandardObjectsCommand, BackfillMessageChannelMessageAssociationMessageFolderCommand, BackfillMissingStandardViewsCommand, + BackfillNavigationMenuItemTypeCommand, FixInvalidStandardUniversalIdentifiersCommand, SeedServerIdCommand, ], diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts index 2d0a47ca3b..d71d1ffb44 100644 --- a/packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts @@ -29,9 +29,10 @@ import { MigrateWorkflowSendEmailAttachmentsCommand } from 'src/database/command import { MigrateWorkspacePicturesCommand } from 'src/database/commands/upgrade-version-command/1-18/1-18-migrate-workspace-pictures.command'; import { AddMissingSystemFieldsToStandardObjectsCommand } from 'src/database/commands/upgrade-version-command/1-19/1-19-add-missing-system-fields-to-standard-objects.command'; import { BackfillMessageChannelMessageAssociationMessageFolderCommand } from 'src/database/commands/upgrade-version-command/1-19/1-19-backfill-message-channel-message-association-message-folder.command'; +import { BackfillMissingStandardViewsCommand } from 'src/database/commands/upgrade-version-command/1-19/1-19-backfill-missing-standard-views.command'; +import { BackfillNavigationMenuItemTypeCommand } from 'src/database/commands/upgrade-version-command/1-19/1-19-backfill-navigation-menu-item-type.command'; import { BackfillSystemFieldsIsSystemCommand } from 'src/database/commands/upgrade-version-command/1-19/1-19-backfill-system-fields-is-system.command'; import { FixInvalidStandardUniversalIdentifiersCommand } from 'src/database/commands/upgrade-version-command/1-19/1-19-fix-invalid-standard-universal-identifiers.command'; -import { BackfillMissingStandardViewsCommand } from 'src/database/commands/upgrade-version-command/1-19/1-19-backfill-missing-standard-views.command'; import { SeedServerIdCommand } from 'src/database/commands/upgrade-version-command/1-19/1-19-seed-server-id.command'; import { BackfillCommandMenuItemsCommand } from 'src/database/commands/upgrade-version-command/1-20/1-20-backfill-command-menu-items.command'; import { BackfillPageLayoutsCommand } from 'src/database/commands/upgrade-version-command/1-20/1-20-backfill-page-layouts.command'; @@ -83,6 +84,7 @@ export class UpgradeCommand extends UpgradeCommandRunner { protected readonly addMissingSystemFieldsToStandardObjectsCommand: AddMissingSystemFieldsToStandardObjectsCommand, protected readonly backfillMessageChannelMessageAssociationMessageFolderCommand: BackfillMessageChannelMessageAssociationMessageFolderCommand, protected readonly backfillMissingStandardViewsCommand: BackfillMissingStandardViewsCommand, + protected readonly backfillNavigationMenuItemTypeCommand: BackfillNavigationMenuItemTypeCommand, protected readonly fixRoleAndAgentUniversalIdentifiersCommand: FixInvalidStandardUniversalIdentifiersCommand, protected readonly seedServerIdCommand: SeedServerIdCommand, @@ -133,10 +135,12 @@ export class UpgradeCommand extends UpgradeCommandRunner { this.addMissingSystemFieldsToStandardObjectsCommand, this.backfillMessageChannelMessageAssociationMessageFolderCommand, this.backfillMissingStandardViewsCommand, + this.backfillNavigationMenuItemTypeCommand, this.seedServerIdCommand, ]; const commands_1200: VersionCommands = [ + this.backfillNavigationMenuItemTypeCommand, this.migrateRichTextToTextCommand, this.backfillCommandMenuItemsCommand, this.backfillPageLayoutsCommand, diff --git a/packages/twenty-server/src/database/typeorm/core/migrations/common/1773681736596-add-type-to-navigation-menu-item.ts b/packages/twenty-server/src/database/typeorm/core/migrations/common/1773681736596-add-type-to-navigation-menu-item.ts index 020273d971..a08d46592c 100644 --- a/packages/twenty-server/src/database/typeorm/core/migrations/common/1773681736596-add-type-to-navigation-menu-item.ts +++ b/packages/twenty-server/src/database/typeorm/core/migrations/common/1773681736596-add-type-to-navigation-menu-item.ts @@ -11,29 +11,15 @@ export class AddTypeToNavigationMenuItem1773681736596 ); await queryRunner.query( - `ALTER TABLE "core"."navigationMenuItem" ADD "type" "core"."navigationMenuItem_type_enum" NOT NULL DEFAULT 'VIEW'`, + `ALTER TABLE "core"."navigationMenuItem" ADD "type" "core"."navigationMenuItem_type_enum"`, ); await queryRunner.query( `ALTER TABLE "core"."navigationMenuItem" DROP CONSTRAINT "CHK_navigation_menu_item_target_fields"`, ); - - await queryRunner.query( - `ALTER TABLE "core"."navigationMenuItem" ADD CONSTRAINT "CHK_navigation_menu_item_type_fields" CHECK ( - ("type" = 'FOLDER') - OR ("type" = 'OBJECT' AND "targetObjectMetadataId" IS NOT NULL) - OR ("type" = 'VIEW') - OR ("type" = 'RECORD' AND "targetRecordId" IS NOT NULL AND "targetObjectMetadataId" IS NOT NULL) - OR ("type" = 'LINK' AND "link" IS NOT NULL) - )`, - ); } public async down(queryRunner: QueryRunner): Promise { - await queryRunner.query( - `ALTER TABLE "core"."navigationMenuItem" DROP CONSTRAINT "CHK_navigation_menu_item_type_fields"`, - ); - await queryRunner.query( `ALTER TABLE "core"."navigationMenuItem" ADD CONSTRAINT "CHK_navigation_menu_item_target_fields" CHECK (("targetRecordId" IS NULL AND "targetObjectMetadataId" IS NULL) OR ("targetRecordId" IS NOT NULL AND "targetObjectMetadataId" IS NOT NULL))`, ); diff --git a/packages/twenty-server/src/database/typeorm/core/migrations/common/1773822077682-make-navigation-menu-item-type-not-null.ts b/packages/twenty-server/src/database/typeorm/core/migrations/common/1773822077682-make-navigation-menu-item-type-not-null.ts new file mode 100644 index 0000000000..a82b02bbae --- /dev/null +++ b/packages/twenty-server/src/database/typeorm/core/migrations/common/1773822077682-make-navigation-menu-item-type-not-null.ts @@ -0,0 +1,49 @@ +import { type MigrationInterface, type QueryRunner } from 'typeorm'; + +import { makeNavigationMenuItemTypeNotNullQueries } from 'src/database/typeorm/core/migrations/utils/1773681736596-makeNavigationMenuItemTypeNotNull.util'; + +export class MakeNavigationMenuItemTypeNotNull1773822077682 + implements MigrationInterface +{ + name = 'MakeNavigationMenuItemTypeNotNull1773822077682'; + + public async up(queryRunner: QueryRunner): Promise { + const savepointName = 'sp_make_navigation_menu_item_type_not_null'; + + try { + await queryRunner.query(`SAVEPOINT ${savepointName}`); + + await makeNavigationMenuItemTypeNotNullQueries(queryRunner); + + await queryRunner.query(`RELEASE SAVEPOINT ${savepointName}`); + } catch (e) { + try { + await queryRunner.query(`ROLLBACK TO SAVEPOINT ${savepointName}`); + await queryRunner.query(`RELEASE SAVEPOINT ${savepointName}`); + } catch (rollbackError) { + // oxlint-disable-next-line no-console + console.error( + 'Failed to rollback to savepoint in MakeNavigationMenuItemTypeNotNull1773822077682', + rollbackError, + ); + throw rollbackError; + } + + // oxlint-disable-next-line no-console + console.error( + 'Swallowing MakeNavigationMenuItemTypeNotNull1773822077682 error', + e, + ); + } + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query( + `ALTER TABLE "core"."navigationMenuItem" DROP CONSTRAINT "CHK_navigation_menu_item_type_fields"`, + ); + + await queryRunner.query( + `ALTER TABLE "core"."navigationMenuItem" ALTER COLUMN "type" DROP NOT NULL`, + ); + } +} diff --git a/packages/twenty-server/src/database/typeorm/core/migrations/utils/1773681736596-makeNavigationMenuItemTypeNotNull.util.ts b/packages/twenty-server/src/database/typeorm/core/migrations/utils/1773681736596-makeNavigationMenuItemTypeNotNull.util.ts new file mode 100644 index 0000000000..862581b008 --- /dev/null +++ b/packages/twenty-server/src/database/typeorm/core/migrations/utils/1773681736596-makeNavigationMenuItemTypeNotNull.util.ts @@ -0,0 +1,19 @@ +import { type QueryRunner } from 'typeorm'; + +export const makeNavigationMenuItemTypeNotNullQueries = async ( + queryRunner: QueryRunner, +): Promise => { + await queryRunner.query( + `ALTER TABLE "core"."navigationMenuItem" ALTER COLUMN "type" SET NOT NULL`, + ); + + await queryRunner.query( + `ALTER TABLE "core"."navigationMenuItem" ADD CONSTRAINT "CHK_navigation_menu_item_type_fields" CHECK ( + ("type" = 'FOLDER') + OR ("type" = 'OBJECT' AND "targetObjectMetadataId" IS NOT NULL) + OR ("type" = 'VIEW' AND "viewId" IS NOT NULL) + OR ("type" = 'RECORD' AND "targetRecordId" IS NOT NULL AND "targetObjectMetadataId" IS NOT NULL) + OR ("type" = 'LINK' AND "link" IS NOT NULL) + )`, + ); +}; diff --git a/packages/twenty-server/src/engine/metadata-modules/navigation-menu-item/entities/navigation-menu-item.entity.ts b/packages/twenty-server/src/engine/metadata-modules/navigation-menu-item/entities/navigation-menu-item.entity.ts index 9d37ad9e12..de9996137a 100644 --- a/packages/twenty-server/src/engine/metadata-modules/navigation-menu-item/entities/navigation-menu-item.entity.ts +++ b/packages/twenty-server/src/engine/metadata-modules/navigation-menu-item/entities/navigation-menu-item.entity.ts @@ -39,7 +39,7 @@ import { SyncableEntity } from 'src/engine/workspace-manager/types/syncable-enti 'CHK_navigation_menu_item_type_fields', `("type" = 'FOLDER') OR ("type" = 'OBJECT' AND "targetObjectMetadataId" IS NOT NULL) - OR ("type" = 'VIEW') + OR ("type" = 'VIEW' AND "viewId" IS NOT NULL) OR ("type" = 'RECORD' AND "targetRecordId" IS NOT NULL AND "targetObjectMetadataId" IS NOT NULL) OR ("type" = 'LINK' AND "link" IS NOT NULL)`, ) @@ -87,7 +87,6 @@ export class NavigationMenuItemEntity nullable: false, type: 'enum', enum: NavigationMenuItemType, - default: NavigationMenuItemType.VIEW, }) type: NavigationMenuItemType;