Identify standard field do deploy until IS_WORKSPACE_CREATION_V2_ENABLED is enabled in prod#16981
Identify standard field do deploy until IS_WORKSPACE_CREATION_V2_ENABLED is enabled in prod#16981
IS_WORKSPACE_CREATION_V2_ENABLED is enabled in prod#16981Conversation
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.
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:13238 This environment will automatically shut down when the PR is closed or after 5 hours. |
There was a problem hiding this comment.
2 issues found across 10 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/twenty-server/src/engine/metadata-modules/field-metadata/dtos/field-metadata.dto.ts">
<violation number="1" location="packages/twenty-server/src/engine/metadata-modules/field-metadata/dtos/field-metadata.dto.ts:164">
P1: The `@Field` decorator is accidentally commented out. The TODO comment and the decorator are on the same line after `//`, which comments out the entire decorator. This will remove `applicationId` from the GraphQL schema. The TODO should be on a separate line.</violation>
</file>
<file name="packages/twenty-server/src/database/commands/upgrade-version-command/1-15/1-15-make-field-metadata-universal-identifier-and-application-id-not-nullable-migration.command.ts">
<violation number="1" location="packages/twenty-server/src/database/commands/upgrade-version-command/1-15/1-15-make-field-metadata-universal-identifier-and-application-id-not-nullable-migration.command.ts:14">
P1: Command name uses `upgrade:1-5:*` but the command lives in the `1-15/` upgrade set (and peers use `upgrade:1-15:*`). Likely typo → command won’t run when invoking the 1-15 upgrade commands.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
packages/twenty-server/src/engine/metadata-modules/field-metadata/dtos/field-metadata.dto.ts
Show resolved
Hide resolved
...ake-field-metadata-universal-identifier-and-application-id-not-nullable-migration.command.ts
Outdated
Show resolved
Hide resolved
...r/src/database/commands/upgrade-version-command/1-16/1-16-identify-field-metadata.command.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
2 issues found across 10 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/twenty-server/src/database/commands/upgrade-version-command/1-15/1-15-make-field-metadata-universal-identifier-and-application-id-not-nullable-migration.command.ts">
<violation number="1" location="packages/twenty-server/src/database/commands/upgrade-version-command/1-15/1-15-make-field-metadata-universal-identifier-and-application-id-not-nullable-migration.command.ts:36">
P2: Consider using `this.logger.error()` or `this.logger.warn()` instead of `this.logger.log()` for the rollback message. Transaction failures are error conditions that should be logged at an appropriate severity level for better observability and monitoring.</violation>
</file>
<file name="packages/twenty-server/src/database/commands/upgrade-version-command/1-15/1-15-identify-field-metadata.command.ts">
<violation number="1" location="packages/twenty-server/src/database/commands/upgrade-version-command/1-15/1-15-identify-field-metadata.command.ts:92">
P2: The "Successfully validated" log is emitted even when exceptions exist and the migration is about to abort. Move this log after the exceptions check (or change wording) to avoid misleading output during failures.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
...ake-field-metadata-universal-identifier-and-application-id-not-nullable-migration.command.ts
Outdated
Show resolved
Hide resolved
...r/src/database/commands/upgrade-version-command/1-16/1-16-identify-field-metadata.command.ts
Show resolved
Hide resolved
packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.entity.ts
Show resolved
Hide resolved
b486662 to
24d4684
Compare
...r/src/database/commands/upgrade-version-command/1-15/1-15-identify-field-metadata.command.ts
Fixed
Show fixed
Hide fixed
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.
84a53a1 to
56202bc
Compare
...ake-field-metadata-universal-identifier-and-application-id-not-nullable-migration.command.ts
Show resolved
Hide resolved
...r/src/database/commands/upgrade-version-command/1-15/1-15-identify-field-metadata.command.ts
Fixed
Show fixed
Hide fixed
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.
196e738 to
38bdae9
Compare
There was a problem hiding this comment.
1 issue found across 13 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/twenty-server/src/database/commands/upgrade-version-command/1-16/1-16-make-field-metadata-universal-identifier-and-application-id-not-nullable-migration.command.ts">
<violation number="1" location="packages/twenty-server/src/database/commands/upgrade-version-command/1-16/1-16-make-field-metadata-universal-identifier-and-application-id-not-nullable-migration.command.ts:47">
P1: Wrap `connect()`/`startTransaction()` inside the try/finally, log errors safely (don’t assume `error` is an `Error`), and propagate the failure (or at least prevent repeated attempts) so the upgrade doesn’t silently continue after a failed schema migration.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
...ake-field-metadata-universal-identifier-and-application-id-not-nullable-migration.command.ts
Show resolved
Hide resolved
...ake-field-metadata-universal-identifier-and-application-id-not-nullable-migration.command.ts
Show resolved
Hide resolved
1 similar comment
…BLED` is enabled in prod (#16981) fixes #16905 Do not merge until `IS_WORKSPACE_CREATION_V2_ENABLED` has been activated by default, and so sync metadata has been deprecated by doing so. As the sync metadata will attempt to insert `null` `applicationId` and `universalIdentifier` values while creating a workspace In this PR we're introducing a new `SyncableEntityRequired` which enforces the non nullable `applicationId` and `universalIdentifier` on extending entity In this PR we also migrate the field metadata entity to extend the required This command will search for workspace field metadata entities that aren't associated to an applicationId, dispatch them to either the workspace-custom `applicationId` or the twenty-standard `applicationId`. For the standard entities it will also set their universal identifier based on the `STANDARD_OBJECTS` const hashmap As the non nullable `applicationId` and `universalIdentifier`migration won't pass in the first we've been using the save point and upgrade command migration fallback pattern Tested the command on a prod extract locally Both `twenty-eng` and `twenty-for-twenty` have unexpected standard objects Please note that we will deprecate the `isCustom` and `standardId` col later in the future ```ts [Nest] 98971 - 01/01/2026, 3:18:00 PM LOG [IdentifyStandardEntitiesCommand] Successfully validated 600/600 field metadata update(s) for workspace 9870323e-22c3-4d14-9b7f-5bdc84f7d6ee (309 custom, 291 standard) [Nest] 98971 - 01/01/2026, 3:18:00 PM WARN [IdentifyStandardEntitiesCommand] Found 35 warning(s) while processing field metadata for workspace 9870323e-22c3-4d14-9b7f-5bdc84f7d6ee. These fields will become custom. ```
# Introduction Followup #16981 1/ Migration, applicationId and universalIdentifier are required on entity ( save point migration + upgrade command fallback pattern ) 2/ Backfill using previous standard ids
Introduction
fixes #16905
Do not merge until
IS_WORKSPACE_CREATION_V2_ENABLEDhas been activated by default, and so sync metadata has been deprecated by doing so. As the sync metadata will attempt to insertnullapplicationIdanduniversalIdentifiervalues while creating a workspaceIn this PR we're introducing a new
SyncableEntityRequiredwhich enforces the non nullableapplicationIdanduniversalIdentifieron extending entityIn this PR we also migrate the field metadata entity to extend the required
Identification upgrade command
This command will search for workspace field metadata entities that aren't associated to an applicationId, dispatch them to either the workspace-custom
applicationIdor the twenty-standardapplicationId. For the standard entities it will also set their universal identifier based on theSTANDARD_OBJECTSconst hashmapTypeorm migration
As the non nullable
applicationIdanduniversalIdentifiermigration won't pass in the first we've been using the save point and upgrade command migration fallback patternTests
Tested the command on a prod extract locally
Both
twenty-engandtwenty-for-twentyhave unexpected standard objectsPlease note that we will deprecate the
isCustomandstandardIdcol later in the futureTwenty-eng
Twenty for twenty
Just created workspace