Skip to content

Remove viewGroup.fieldMetadataId#16571

Merged
charlesBochet merged 5 commits intomainfrom
tech--remove-field-metadata-id-on-view-group
Dec 15, 2025
Merged

Remove viewGroup.fieldMetadataId#16571
charlesBochet merged 5 commits intomainfrom
tech--remove-field-metadata-id-on-view-group

Conversation

@ijreilly
Copy link
Copy Markdown
Contributor

Final step of twentyhq/core-team-issues#1965

Removing viewGroup.fieldMetadataId.
It's already not used in FE anymore

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 20 files

Prompt for AI agents (all 1 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/typeorm/core/migrations/common/1765808791153-remove-field-metadata-id-in-view-group.ts">

<violation number="1" location="packages/twenty-server/src/database/typeorm/core/migrations/common/1765808791153-remove-field-metadata-id-in-view-group.ts:19">
P2: The `down` migration adds `fieldMetadataId` as `NOT NULL` without a default value. If any rows exist in `viewGroup` when rolling back, this will fail. Consider making the column nullable in the rollback, or adding a comment explaining the rollback limitation.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR


public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(
`ALTER TABLE "core"."viewGroup" ADD "fieldMetadataId" uuid NOT NULL`,
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The down migration adds fieldMetadataId as NOT NULL without a default value. If any rows exist in viewGroup when rolling back, this will fail. Consider making the column nullable in the rollback, or adding a comment explaining the rollback limitation.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-server/src/database/typeorm/core/migrations/common/1765808791153-remove-field-metadata-id-in-view-group.ts, line 19:

<comment>The `down` migration adds `fieldMetadataId` as `NOT NULL` without a default value. If any rows exist in `viewGroup` when rolling back, this will fail. Consider making the column nullable in the rollback, or adding a comment explaining the rollback limitation.</comment>

<file context>
@@ -0,0 +1,25 @@
+
+  public async down(queryRunner: QueryRunner): Promise&lt;void&gt; {
+    await queryRunner.query(
+      `ALTER TABLE &quot;core&quot;.&quot;viewGroup&quot; ADD &quot;fieldMetadataId&quot; uuid NOT NULL`,
+    );
+    await queryRunner.query(
</file context>

✅ Addressed in fb13a7d

Copy link
Copy Markdown
Member

@Weiko Weiko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines 78 to 88

const { flatViewMaps } =
await this.flatEntityMapsCacheService.getOrRecomputeManyOrAllFlatEntityMaps(
{
workspaceId,
flatMapsKeys: ['flatViewMaps'],
},
);

const flatViewGroupsToCreate = createViewGroupInputs.map(
(createViewGroupInput) => {
const mainGroupByFieldMetadataId =
flatViewMaps.byId[createViewGroupInput.viewId]
?.mainGroupByFieldMetadataId;

if (!isDefined(mainGroupByFieldMetadataId)) {
throw new ViewGroupException(
'The associated view is not a grouped view: mainGroupByFieldMetadataId is missing.',
ViewGroupExceptionCode.MISSING_MAIN_GROUP_BY_FIELD_METADATA_ID,
);
}

return fromCreateViewGroupInputToFlatViewGroupToCreate({
(createViewGroupInput) =>
fromCreateViewGroupInputToFlatViewGroupToCreate({
createViewGroupInput,
workspaceId,
workspaceCustomApplicationId: workspaceCustomFlatApplication.id,
mainGroupByFieldMetadataId,
});
},
}),
);

const validateAndBuildResult =

This comment was marked as outdated.

@charlesBochet charlesBochet merged commit a0b963e into main Dec 15, 2025
58 of 60 checks passed
@charlesBochet charlesBochet deleted the tech--remove-field-metadata-id-on-view-group branch December 15, 2025 16:58
@twenty-eng-sync
Copy link
Copy Markdown

Hey @ijreilly! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants