chore: remove 1-19 upgrade commands and navigation menu item feature flags#19083
chore: remove 1-19 upgrade commands and navigation menu item feature flags#19083charlesBochet merged 8 commits intomainfrom
Conversation
…flags Removes the entire 1-19 upgrade version command directory and its references from the upgrade module and command runner. Also removes IS_NAVIGATION_MENU_ITEM_ENABLED and IS_NAVIGATION_MENU_ITEM_EDITING_ENABLED feature flags from enum, seeds, generated schemas, and test mocks. Made-with: Cursor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Pull request overview
Removes legacy upgrade scaffolding for version 1.19 and retires two navigation-menu-related feature flags now that navigation menu items are always enabled.
Changes:
- Deleted the entire
upgrade-version-command/1-19command set and removed it from the upgrade command/module wiring. - Removed
IS_NAVIGATION_MENU_ITEM_ENABLEDandIS_NAVIGATION_MENU_ITEM_EDITING_ENABLEDfrom the shared feature-flag enum and all related seed/test/generated artifacts. - Regenerated client/front GraphQL schema types to reflect the feature flag removal.
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/twenty-shared/src/types/FeatureFlagKey.ts | Removes the two navigation menu item feature flags from the shared enum. |
| packages/twenty-server/src/engine/workspace-manager/dev-seeder/core/utils/seed-feature-flags.util.ts | Stops seeding the removed feature flags. |
| packages/twenty-server/src/engine/twenty-orm/entity-manager/workspace-entity-manager.spec.ts | Updates test mock feature flag map to match the enum changes. |
| packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts | Removes 1.19 command wiring; keeps only 1.20 command set in allCommands. |
| packages/twenty-server/src/database/commands/upgrade-version-command/upgrade-version-command.module.ts | Drops the Nest module import for the removed 1.19 command module. |
| packages/twenty-server/src/database/commands/upgrade-version-command/1-19/1-19-upgrade-version-command.module.ts | Deleted: 1.19 upgrade command Nest module. |
| packages/twenty-server/src/database/commands/upgrade-version-command/1-19/1-19-seed-server-id.command.ts | Deleted: 1.19 upgrade command implementation. |
| packages/twenty-server/src/database/commands/upgrade-version-command/1-19/1-19-fix-invalid-standard-universal-identifiers.command.ts | Deleted: 1.19 upgrade command implementation. |
| packages/twenty-server/src/database/commands/upgrade-version-command/1-19/1-19-backfill-system-fields-is-system.command.ts | Deleted: 1.19 upgrade command implementation. |
| packages/twenty-server/src/database/commands/upgrade-version-command/1-19/1-19-backfill-missing-standard-views.command.ts | Deleted: 1.19 upgrade command implementation. |
| packages/twenty-server/src/database/commands/upgrade-version-command/1-19/1-19-backfill-message-channel-message-association-message-folder.command.ts | Deleted: 1.19 upgrade command implementation. |
| packages/twenty-server/src/database/commands/upgrade-version-command/1-19/1-19-add-missing-system-fields-to-standard-objects.command.ts | Deleted: 1.19 upgrade command implementation. |
| packages/twenty-front/src/generated-metadata/graphql.ts | Removes the two flags from the generated front-end GraphQL enum. |
| packages/twenty-client-sdk/src/metadata/generated/schema.ts | Updates generated SDK types/constants to remove the two flags. |
| packages/twenty-client-sdk/src/metadata/generated/schema.graphql | Updates generated SDK GraphQL schema enum to remove the two flags. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -125,7 +101,6 @@ export class UpgradeCommand extends UpgradeCommandRunner { | |||
| ]; | |||
|
|
|||
| this.allCommands = { | |||
There was a problem hiding this comment.
allCommands is typed as AllCommands = Record<UpgradeCommandVersion, VersionCommands>, and UpgradeCommandVersion currently includes '1.19.0' (see UPGRADE_COMMAND_SUPPORTED_VERSIONS). With only '1.20.0' provided here, this should fail type-checking and can also break runtime lookups if APP_VERSION is ever set to 1.19.x. Either include a '1.19.0' entry (e.g. empty commands) or relax AllCommands / UpgradeCommandVersion typing so only the current version key is required (keeping '1.19.0' in the supported versions list if it’s still needed to compute the previous version).
| this.allCommands = { | |
| this.allCommands = { | |
| '1.19.0': [], |
📊 API Changes ReportGraphQL Schema ChangesGraphQL Schema Changes[error] Error: Unable to read JSON file: /home/runner/work/twenty/twenty/main-schema-introspection.json: Not valid JSON content GraphQL Metadata Schema ChangesGraphQL Metadata Schema Changes[error] Error: Unable to read JSON file: /home/runner/work/twenty/twenty/main-metadata-schema-introspection.json: Not valid JSON content REST API Analysis ErrorError OutputREST Metadata API Analysis ErrorError Output |
Made-with: Cursor
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:61172 This environment will automatically shut down after 5 hours. |
The upgrade runner needs the previous version entry to validate workspace versions before upgrading to 1.20.0. Made-with: Cursor
Three issues caused flaky server-integration-test failures: 1. teardown-test.ts did not await destroy()/close(), so Redis and Postgres connections closed while async event handlers were still running. 2. CacheLockService.withLock had an unprotected releaseLock in its finally block — if Redis was already closed, the error became an unhandled rejection crashing the process. 3. WorkflowRunEnqueueWorkspaceService had the same unprotected releaseWorkflowEnqueueLock in its finally block. Made-with: Cursor
The metricsService.incrementCounter call inside the catch block also uses Redis. When Redis is shutting down, this creates a second unhandled rejection that crashes the process. Made-with: Cursor
…ises MetricsService.incrementCounter and batchIncrementCounter both called metricsCacheService.updateCounter (which uses Redis) without await, creating floating promises. When Redis closes during shutdown, these become unhandled rejections that crash the process. Made-with: Cursor
|
|
||
| this.allCommands = { | ||
| '1.19.0': commands_1190, | ||
| '1.19.0': [], |
| return await fn(); | ||
| } finally { | ||
| await this.cacheStorageService.releaseLock(key); | ||
| try { |
There was a problem hiding this comment.
Suggesting a fireAndForget
function fireAndForget(
promise: Promise<unknown>,
onError?: (error: unknown) => void,
): void {
promise.catch((error) => {
onError?.(error);
});
}(which is not exactly what you have, fireAndForget should NOT await the result so maybe the name is not correct)
|
Hey @charlesBochet! After you've done the QA of your Pull Request, you can mark it as done here. Thank you! |
Summary
1-19upgrade version command directory (7 files, ~1500 lines) and removes all references from the upgrade module and command runner.IS_NAVIGATION_MENU_ITEM_ENABLEDandIS_NAVIGATION_MENU_ITEM_EDITING_ENABLEDfeature flags from the enum, seed data, generated schema files, and test mocks. These flags had no remaining feature-gated code — navigation menu items are now always enabled.