Skip to content

feat: add atomic upsertFieldsWidget mutation to replace multiple view field group/field API calls#18137

Merged
Weiko merged 8 commits intomainfrom
copilot/migrate-upsert-fields-widget
Feb 23, 2026
Merged

feat: add atomic upsertFieldsWidget mutation to replace multiple view field group/field API calls#18137
Weiko merged 8 commits intomainfrom
copilot/migrate-upsert-fields-widget

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 20, 2026

The useSaveFieldsWidgetGroups hook was making multiple sequential API calls per widget (create groups, delete groups, update groups, update fields) — non-atomic, chatty, and with heavy diff computation on the frontend.

Backend

  • New input DTOs: UpsertFieldsWidgetInput / UpsertFieldsWidgetGroupInput / UpsertFieldsWidgetFieldInput — caller passes the full desired state of groups + fields for a widget
  • FieldsWidgetUpsertService: looks up widget → resolves viewId, diffs against existing flat entity maps, builds optimistic group maps so newly-created groups can be referenced by field updates in the same pass, then runs creates/updates/deletes in a single validateBuildAndRunWorkspaceMigration call
  • upsertFieldsWidget mutation added to ViewFieldGroupResolver; new FIELDS_WIDGET_NOT_FOUND exception code added and wired into the GraphQL exception filter

Frontend

  • New UPSERT_FIELDS_WIDGET GQL document
  • useSaveFieldsWidgetGroups replaces all per-operation calls with a single mutation per widget, passing the full draft state — diff logic moves entirely to the backend
mutation UpsertFieldsWidget($input: UpsertFieldsWidgetInput!) {
  upsertFieldsWidget(input: $input) {
    id
    name
    position
    isVisible
    viewId
    viewFields { id isVisible position ... }
  }
}

Widget IDs are collected from both draft and persisted state so widgets whose draft groups were cleared still trigger deletion of their server-side groups.

Original prompt

Start implementation

The user has attached the following file paths as relevant context:

  • CLAUDE.md
[Chronological Review: The conversation began with the user requesting a review of how updates on views are stored in the frontend of record page layouts. The user expressed a desire to migrate heavy computations and multiple requests from the frontend to the backend. They proposed creating a custom upsertFieldsWidget mutation to handle updates atomically. The conversation has focused on this migration strategy and its implementation.]

[Intent Mapping: The user explicitly requested to "check how we store updates on views in the frontend of record page layouts" and indicated a need to "migrate this task to the backend" by creating a custom mutation for atomic updates.]

[Technical Inventory: The conversation involves the following technical concepts:

  • Frontend and backend architecture for handling page layout updates.
  • Custom GraphQL mutation (upsertFieldsWidget) for atomic updates.
  • The file in focus is page-layout-update.service.ts, which is likely responsible for handling updates related to page layouts in the backend.]

[Code Archaeology: The specific file being worked on is /Users/devessier/Web/twenty/packages/twenty-server/src/engine/metadata-modules/page-layout/services/page-layout-update.service.ts. The user is likely looking to modify or enhance this service to accommodate the new mutation for handling updates.]

[Progress Assessment: The user has identified the need for a backend migration strategy but has not yet implemented any changes. The focus is on planning the transition from frontend to backend processing.]

[Context Validation: All critical information for continuation is captured, including the user's intent to migrate computations to the backend and the specific file being modified.]

[Recent Commands Analysis: The last agent commands involved checking the current file context and the user's request to summarize the conversation. No specific tool results were generated as the focus was on summarizing the conversation history. The agent was actively working on summarizing the conversation when the token budget was exceeded, which was triggered by the user's request for a summary.]

1. Conversation Overview: - Primary Objectives: The user requested to "check how we store updates on views in the frontend of record page layouts" and expressed a desire to "migrate this task to the backend" by creating a custom upsertFieldsWidget mutation for atomic updates. - Session Context: The conversation has focused on the need to shift heavy computations from the frontend to the backend, with an emphasis on implementing a new mutation to streamline updates. - User Intent Evolution: The user's needs have remained consistent, focusing on backend migration for efficiency and atomicity in updates.
  1. Technical Foundation:
  • Core Technology: The conversation involves a backend service likely built with Node.js and TypeScript, as indicated by the file path.
  • Framework/Library: The use of GraphQL for handling mutations is implied through the mention of a custom mutation.
  • Architectural Pattern: The user is looking to implement a service-oriented architecture where the backend handles complex computations instead of the frontend.
  • Environment Detail: The current working file is located in a server package of the twenty repository.
  1. Codebase Status:
  • File Name: page-layout-update.service.ts
  • Purpose: This file is responsible for managing updates related to page layouts in the backend.
  • Current State: The user is considering modifications to implement a new mutation for handling updates.
  • Key Code Segments: Specific functions or classes have not been detailed yet, as the focus is on planning changes.
  • Dependencies: This service likely interacts with other components in the metadata-modules related to page layouts.
  1. Problem Resolution:
  • Issues Encountered: The current challenge is the inefficiency of handling updates in the frontend.
  • Solutions Implemented: The proposed solution is to create a custom mutation to handle updates atomically in the backend.
  • Debugging Context: No ongoing troubleshooting efforts have been mentioned yet.
  • Lessons Learned: The need for backend processing to improve performance has been highlighted.
  1. Progress Tracking:
  • Completed Tasks: No tasks have been completed yet; the user is in the planning phase.
  • Partially Complete Work: The user is preparing to implement a new mutation for backend updates.
  • Validated Outcomes: No features have been confirmed working through testing at this stage.
  1. Active Work State:
  • Current Focus: The user is focused on modifying the page-layout-update.service.ts to implement the new mutation.
  • Recent Context: The last few exchanges involved discussing the migration of update tasks from the frontend to the backend.
  • Working Code: No specific code snippets have been modified yet, as the co...

Created from VS Code.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits February 20, 2026 15:54
…ld updates

Co-authored-by: Devessier <29370468+Devessier@users.noreply.github.com>
…e safety

Co-authored-by: Devessier <29370468+Devessier@users.noreply.github.com>
Copilot AI changed the title [WIP] Implement custom upsertFieldsWidget mutation for views feat: add atomic upsertFieldsWidget mutation to replace multiple view field group/field API calls Feb 20, 2026
Copilot AI requested a review from Devessier February 20, 2026 16:00
@Devessier Devessier marked this pull request as ready for review February 20, 2026 18:28
Copilot AI review requested due to automatic review settings February 20, 2026 18:28
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR migrates the fields widget update logic from multiple sequential, non-atomic API calls on the frontend to a single atomic backend mutation. The change improves reliability by ensuring all group and field updates succeed or fail together, and reduces network overhead by consolidating multiple API calls into one.

Changes:

  • Added a new upsertFieldsWidget GraphQL mutation that atomically updates all groups and fields for a widget in a single transaction
  • Implemented FieldsWidgetUpsertService backend service to handle diffing, optimistic group resolution, and atomic persistence via workspace migrations
  • Refactored useSaveFieldsWidgetGroups hook to replace multiple API calls (create/update/delete groups, update fields) with a single mutation call

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/twenty-server/src/engine/metadata-modules/view-field-group/services/fields-widget-upsert.service.ts New service implementing atomic upsert logic with diff computation, optimistic group map building, and workspace migration orchestration
packages/twenty-server/src/engine/metadata-modules/view-field-group/resolvers/view-field-group.resolver.ts Added upsertFieldsWidget mutation resolver that delegates to FieldsWidgetUpsertService
packages/twenty-server/src/engine/metadata-modules/view-field-group/dtos/inputs/upsert-fields-widget.input.ts Input DTO accepting widgetId and full desired state of groups
packages/twenty-server/src/engine/metadata-modules/view-field-group/dtos/inputs/upsert-fields-widget-group.input.ts Input DTO for group with nested fields array
packages/twenty-server/src/engine/metadata-modules/view-field-group/dtos/inputs/upsert-fields-widget-field.input.ts Input DTO for field properties (viewFieldId, isVisible, position)
packages/twenty-server/src/engine/metadata-modules/view-field-group/exceptions/view-field-group.exception.ts Added FIELDS_WIDGET_NOT_FOUND exception code
packages/twenty-server/src/engine/metadata-modules/view/utils/view-graphql-api-exception-handler.util.ts Wired new exception code into GraphQL exception filter
packages/twenty-server/src/engine/metadata-modules/view-field-group/view-field-group.module.ts Registered FieldsWidgetUpsertService provider
packages/twenty-front/src/modules/page-layout/hooks/useSaveFieldsWidgetGroups.ts Replaced sequential API calls and frontend diff logic with single upsertFieldsWidget mutation call
packages/twenty-front/src/modules/page-layout/graphql/mutations/upsertFieldsWidget.ts GraphQL mutation document using ViewFieldGroupFragment

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 69 to 100
for (const widgetId of widgetIds) {
const draftGroups = allDraftGroups[widgetId] ?? [];
const persistedGroups = allPersistedGroups[widgetId] ?? [];

if (draftGroups.length === 0 && persistedGroups.length === 0) {
continue;
}

const viewId = getViewIdForWidget(widgetId);

if (!isDefined(viewId)) {
continue;
}

const { createdGroups, deletedGroups, updatedGroups } =
computeFieldsWidgetGroupDiff(persistedGroups, draftGroups);

if (createdGroups.length > 0) {
await performViewFieldGroupAPICreate({
inputs: createdGroups.map((group) => ({
id: group.id,
name: group.name,
position: group.position,
isVisible: group.isVisible,
viewId,
})),
});
}

if (deletedGroups.length > 0) {
for (const group of deletedGroups) {
await performViewFieldGroupAPIDelete([
{ input: { id: group.id } },
]);
}
}

if (updatedGroups.length > 0) {
const updates = updatedGroups.map((group) => ({

await upsertFieldsWidgetMutation({
variables: {
input: {
id: group.id,
update: {
widgetId,
groups: draftGroups.map((group) => ({
id: group.id,
name: group.name,
position: group.position,
isVisible: group.isVisible,
},
fields: group.fields.flatMap((field) => {
if (!isDefined(field.viewFieldId)) {
return [];
}

return [
{
viewFieldId: field.viewFieldId,
isVisible: field.isVisible,
position: field.position,
},
];
}),
})),
},
}));

await performViewFieldGroupAPIUpdate(updates);
}

const fieldUpdates = computeFieldsWidgetFieldDiff(
persistedGroups,
draftGroups,
);

if (fieldUpdates.length > 0) {
const viewFieldUpdateInputs = fieldUpdates.map(
({ viewFieldId, ...updates }) => ({
input: {
id: viewFieldId,
update: updates,
},
}),
);

await performViewFieldAPIUpdate(viewFieldUpdateInputs);
}
},
});
}

set(fieldsWidgetGroupsPersistedState, allDraftGroups);
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The mutation loop lacks error handling. If a mutation fails partway through, the persisted state is still updated on line 100, potentially causing inconsistency between frontend state and backend state. Consider wrapping the loop in try-catch and only updating persisted state if all mutations succeed, or handle partial failures appropriately.

Copilot uses AI. Check for mistakes.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Feb 20, 2026

Greptile Summary

This PR successfully migrates field widget group/field updates from multiple sequential frontend API calls to a single atomic backend mutation (upsertFieldsWidget).

Key changes:

  • Backend now handles full diff computation between desired state and existing entities
  • Optimistic group maps enable newly-created groups to be referenced by field updates in the same transaction
  • Frontend sends complete widget state instead of computing diffs
  • Widget IDs collected from both draft and persisted state ensure deletions work correctly

Migration benefits:

  • Reduces network chattiness (multiple calls → single mutation per widget)
  • Atomic transaction guarantees consistency
  • Moves complex diff logic to backend where it belongs
  • Eliminates race conditions from sequential updates

Confidence Score: 4/5

  • This PR is safe to merge with thorough testing of the atomic mutation behavior
  • The implementation is well-structured with proper validation, exception handling, and atomic transactions. The optimistic group map pattern correctly handles cross-references. However, missing tests and potential edge cases around concurrent updates or orphaned view fields reduce confidence slightly.
  • Pay close attention to fields-widget-upsert.service.ts for the diff computation logic and ensure comprehensive testing of edge cases

Important Files Changed

Filename Overview
packages/twenty-server/src/engine/metadata-modules/view-field-group/services/fields-widget-upsert.service.ts New service implementing atomic upsert logic for fields widgets with optimistic group map building and comprehensive diff computation
packages/twenty-front/src/modules/page-layout/hooks/useSaveFieldsWidgetGroups.ts Refactored to use single atomic mutation per widget instead of multiple sequential API calls, includes widget IDs from both draft and persisted state
packages/twenty-server/src/engine/metadata-modules/view-field-group/resolvers/view-field-group.resolver.ts Added upsertFieldsWidget mutation handler that delegates to FieldsWidgetUpsertService

Sequence Diagram

sequenceDiagram
    participant Frontend
    participant GraphQL
    participant FieldsWidgetUpsertService
    participant MigrationService
    participant Cache
    participant DB

    Frontend->>GraphQL: upsertFieldsWidget(widgetId, groups[])
    GraphQL->>FieldsWidgetUpsertService: upsertFieldsWidget()
    
    FieldsWidgetUpsertService->>Cache: getOrRecomputeManyOrAllFlatEntityMaps()
    Cache-->>FieldsWidgetUpsertService: flatPageLayoutWidgetMaps, flatViewFieldGroupMaps, flatViewFieldMaps, flatViewMaps
    
    FieldsWidgetUpsertService->>FieldsWidgetUpsertService: findWidget & validate
    FieldsWidgetUpsertService->>FieldsWidgetUpsertService: compute groups to create/update/delete
    FieldsWidgetUpsertService->>FieldsWidgetUpsertService: build optimistic group maps
    FieldsWidgetUpsertService->>FieldsWidgetUpsertService: compute view fields to update
    
    FieldsWidgetUpsertService->>MigrationService: validateBuildAndRunWorkspaceMigration()
    MigrationService->>DB: atomic transaction (create/update/delete groups & fields)
    DB-->>MigrationService: success
    MigrationService-->>FieldsWidgetUpsertService: result
    
    FieldsWidgetUpsertService->>Cache: getOrRecomputeManyOrAllFlatEntityMaps()
    Cache-->>FieldsWidgetUpsertService: recomputed flatViewFieldGroupMaps
    
    FieldsWidgetUpsertService-->>GraphQL: ViewFieldGroupDTO[]
    GraphQL-->>Frontend: updated groups with view fields
Loading

Last reviewed commit: 6ff7a5b

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.

10 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +166 to +215
const viewFieldsToUpdate = existingViewFields.flatMap((existingField) => {
const inputGroup = inputGroups.find((g) =>
g.fields.some((f) => f.viewFieldId === existingField.id),
);

if (!isDefined(inputGroup)) {
return [];
}

const inputField = inputGroup.fields.find(
(f) => f.viewFieldId === existingField.id,
);

if (!isDefined(inputField)) {
return [];
}

const newViewFieldGroupId = inputGroup.id;

const hasChanged =
existingField.isVisible !== inputField.isVisible ||
existingField.position !== inputField.position ||
existingField.viewFieldGroupId !== newViewFieldGroupId;

if (!hasChanged) {
return [];
}

const { viewFieldGroupUniversalIdentifier } =
resolveEntityRelationUniversalIdentifiers({
metadataName: 'viewField',
foreignKeyValues: {
viewFieldGroupId: newViewFieldGroupId,
},
flatEntityMaps: {
flatViewFieldGroupMaps: optimisticFlatViewFieldGroupMaps,
},
});

return [
{
...existingField,
isVisible: inputField.isVisible,
position: inputField.position,
viewFieldGroupId: newViewFieldGroupId,
viewFieldGroupUniversalIdentifier,
updatedAt: now,
},
];
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Check that this logic handles orphaned fields correctly. If a viewField exists but isn't referenced in any input group's fields array, the flatMap returns empty array and skips it. This means orphaned fields won't be deleted or reassigned - verify this is the intended behavior.

Comment on lines +81 to +93
fields: group.fields.flatMap((field) => {
if (!isDefined(field.viewFieldId)) {
return [];
}

return [
{
viewFieldId: field.viewFieldId,
isVisible: field.isVisible,
position: field.position,
},
];
}),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Using flatMap with isDefined check correctly filters out fields without viewFieldId, preventing undefined values from being sent to the backend

Comment on lines +154 to +164
// Build optimistic maps so that newly created groups can be resolved when
// computing viewFieldGroupUniversalIdentifier for view field updates.
const optimisticFlatViewFieldGroupMaps: FlatViewFieldGroupMaps =
groupsToCreate.reduce(
(maps, group) =>
addFlatEntityToFlatEntityMapsOrThrow({
flatEntity: group,
flatEntityMaps: maps,
}),
flatViewFieldGroupMaps,
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Optimistic group map building is essential here. Newly created groups are added to optimisticFlatViewFieldGroupMaps so that when computing viewFieldGroupUniversalIdentifier for field updates (lines 194-203), the resolveEntityRelationUniversalIdentifiers can find groups created in the same transaction.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Feb 20, 2026

Additional Comments (1)

packages/twenty-front/src/modules/page-layout/hooks/useSaveFieldsWidgetGroups.ts
Including widgetIds from both draft and persisted state correctly ensures widgets deleted in draft (empty draftGroups) still trigger server-side cleanup. This prevents orphaned groups on the backend.

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 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. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/twenty-server/src/engine/metadata-modules/view-field-group/services/fields-widget-upsert.service.ts">

<violation number="1" location="packages/twenty-server/src/engine/metadata-modules/view-field-group/services/fields-widget-upsert.service.ts:148">
P2: Avoid relying on deletedAt for core view field groups/fields. Use the workspace migration delete operations (or another hard-delete path) instead of soft-deleting groups/fields via deletedAt.

(Based on your team's feedback about avoiding deletedAt for core entities.) [FEEDBACK_USED]</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Feb 20, 2026

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:21727

This environment will automatically shut down when the PR is closed or after 5 hours.

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.

Much better but I think we need to address the fact that fields widgets don't always have groups. Maybe confirm with product @Devessier

@ValidateNested({ each: true })
@Type(() => UpsertFieldsWidgetGroupInput)
@Field(() => [UpsertFieldsWidgetGroupInput])
groups: UpsertFieldsWidgetGroupInput[];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what about fields widget associated with views that don't have groups? I believe this is optional

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.

Left more comments

const draftGroups = allDraftGroups[widgetId] ?? [];
const persistedGroups = allPersistedGroups[widgetId] ?? [];

if (draftGroups.length === 0 && persistedGroups.length === 0) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I can't find this logic anymore. Just want to make sure that we are not calling mutations for unchanged groups 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you click on the Save button of a dashboard, it triggers a mutation even if you changed nothing. The behavior is similar for record page layouts. I think we can go this way.

if (!inputGroupIds.has(existingGroup.id)) {
groupsToUpdate.push({
...existingGroup,
deletedAt: now,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we want to get rid of soft-deletion for those tables. It would simplify some edge cases as well. Let's put the group to flatEntityToDelete instead

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.

3 issues found across 11 files (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/twenty-server/test/integration/metadata/suites/view-field-group/upsert-fields-widget.integration-spec.ts">

<violation number="1" location="packages/twenty-server/test/integration/metadata/suites/view-field-group/upsert-fields-widget.integration-spec.ts:117">
P3: Add a precondition so the test fails with a clear assertion if fewer than two fields exist; otherwise `twoFields[1]` can be undefined and crash the test before it exercises the mutation.</violation>
</file>

<file name="packages/twenty-server/src/engine/metadata-modules/view-field-group/dtos/inputs/upsert-fields-widget.input.ts">

<violation number="1" location="packages/twenty-server/src/engine/metadata-modules/view-field-group/dtos/inputs/upsert-fields-widget.input.ts:33">
P2: The new `groups`/`fields` inputs are documented as mutually exclusive, but there is no validation to enforce that only one is provided (or that at least one is present). This allows invalid payloads to pass validation.</violation>
</file>

<file name="packages/twenty-server/src/engine/metadata-modules/view-field-group/services/fields-widget-upsert.service.ts">

<violation number="1" location="packages/twenty-server/src/engine/metadata-modules/view-field-group/services/fields-widget-upsert.service.ts:337">
P1: Fields not included in `inputFields` will keep a stale `viewFieldGroupId` after all groups are deleted, creating dangling references. When a field is not in the input but has a non-null `viewFieldGroupId`, it should still have its group reference nullified since all groups are being deleted.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment on lines +337 to +339
if (!isDefined(inputField)) {
return [];
}
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 23, 2026

Choose a reason for hiding this comment

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

P1: Fields not included in inputFields will keep a stale viewFieldGroupId after all groups are deleted, creating dangling references. When a field is not in the input but has a non-null viewFieldGroupId, it should still have its group reference nullified since all groups are being deleted.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-server/src/engine/metadata-modules/view-field-group/services/fields-widget-upsert.service.ts, line 337:

<comment>Fields not included in `inputFields` will keep a stale `viewFieldGroupId` after all groups are deleted, creating dangling references. When a field is not in the input but has a non-null `viewFieldGroupId`, it should still have its group reference nullified since all groups are being deleted.</comment>

<file context>
@@ -242,19 +310,82 @@ export class FieldsWidgetUpsertService {
+        (f) => f.viewFieldId === existingField.id,
+      );
+
+      if (!isDefined(inputField)) {
+        return [];
+      }
</file context>
Suggested change
if (!isDefined(inputField)) {
return [];
}
if (!isDefined(inputField)) {
if (existingField.viewFieldGroupId !== null) {
return [
{
...existingField,
viewFieldGroupId: null,
viewFieldGroupUniversalIdentifier: null,
updatedAt: now,
},
];
}
return [];
}
Fix with Cubic

description:
'The groups (with nested fields) to upsert. Mutually exclusive with "fields".',
})
groups?: UpsertFieldsWidgetGroupInput[];
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 23, 2026

Choose a reason for hiding this comment

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

P2: The new groups/fields inputs are documented as mutually exclusive, but there is no validation to enforce that only one is provided (or that at least one is present). This allows invalid payloads to pass validation.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-server/src/engine/metadata-modules/view-field-group/dtos/inputs/upsert-fields-widget.input.ts, line 33:

<comment>The new `groups`/`fields` inputs are documented as mutually exclusive, but there is no validation to enforce that only one is provided (or that at least one is present). This allows invalid payloads to pass validation.</comment>

<file context>
@@ -16,8 +22,23 @@ export class UpsertFieldsWidgetInput {
+    description:
+      'The groups (with nested fields) to upsert. Mutually exclusive with "fields".',
+  })
+  groups?: UpsertFieldsWidgetGroupInput[];
+
+  @IsOptional()
</file context>
Fix with Cubic

const groupToDeleteId = uuidv4();
const groupToKeepId = uuidv4();

const twoFields = testSetup.viewFields.slice(0, 2);
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 23, 2026

Choose a reason for hiding this comment

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

P3: Add a precondition so the test fails with a clear assertion if fewer than two fields exist; otherwise twoFields[1] can be undefined and crash the test before it exercises the mutation.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-server/test/integration/metadata/suites/view-field-group/upsert-fields-widget.integration-spec.ts, line 117:

<comment>Add a precondition so the test fails with a clear assertion if fewer than two fields exist; otherwise `twoFields[1]` can be undefined and crash the test before it exercises the mutation.</comment>

<file context>
@@ -0,0 +1,504 @@
+      const groupToDeleteId = uuidv4();
+      const groupToKeepId = uuidv4();
+
+      const twoFields = testSetup.viewFields.slice(0, 2);
+
+      await upsertFieldsWidget({
</file context>
Fix with Cubic

Comment on lines +235 to +244
const viewFieldsToUpdate = existingViewFields.flatMap((existingField) => {
const inputGroup = inputGroups.find((g) =>
g.fields.some((f) => f.viewFieldId === existingField.id),
);

if (!isDefined(inputGroup)) {
return [];
}

const inputField = inputGroup.fields.find(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: When a field's group is deleted, the field's viewFieldGroupId is not updated in the cache if the field isn't re-assigned to a new group, causing cache inconsistency.
Severity: MEDIUM

Suggested Fix

Adjust the logic in upsertFieldsWidgetWithGroups to identify fields that belonged to a deleted group and are not part of any new group. For these fields, explicitly create an update operation to set their viewFieldGroupId to null, ensuring the change is reflected in the application cache.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location:
packages/twenty-server/src/engine/metadata-modules/view-field-group/services/fields-widget-upsert.service.ts#L235-L244

Potential issue: In the `upsertFieldsWidgetWithGroups` function, if a field belongs to a
group that is deleted (by not being present in the input), and that field is not
re-assigned to another group in the input, it is skipped during the update process. The
database correctly sets the field's `viewFieldGroupId` to `NULL` due to the `ON DELETE
SET NULL` constraint. However, the application's migration logic does not track this
change, causing the in-memory cache to retain a stale `viewFieldGroupId` until the next
full cache refresh. This leads to a temporary cache-database inconsistency.

Did we get this right? 👍 / 👎 to inform future reviews.

@Weiko Weiko merged commit 9a0295f into main Feb 23, 2026
46 of 55 checks passed
@Weiko Weiko deleted the copilot/migrate-upsert-fields-widget branch February 23, 2026 18:25
@twenty-eng-sync
Copy link
Copy Markdown

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

@github-actions
Copy link
Copy Markdown
Contributor

Fails
🚫

node failed.

Log

Details
�[31mError: �[39m RequestError [HttpError]: Validation Failed: {"message":"The listed users cannot be searched either because the users do not exist or you do not have permission to view the users.","resource":"Search","field":"q","code":"invalid"} - https://docs.github.com/v3/search/
    at /home/runner/work/twenty/twenty/node_modules/�[4m@octokit/request�[24m/dist-node/index.js:125:21
�[90m    at process.processTicksAndRejections (node:internal/process/task_queues:103:5)�[39m {
  status: �[33m422�[39m,
  response: {
    url: �[32m'https://api.github.com/search/issues?q=is%3Apr%20author%3ACopilot%20is%3Aclosed%20repo%3Atwentyhq%2Ftwenty&per_page=2&page=1'�[39m,
    status: �[33m422�[39m,
    headers: {
      �[32m'access-control-allow-origin'�[39m: �[32m'*'�[39m,
      �[32m'access-control-expose-headers'�[39m: �[32m'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset'�[39m,
      �[32m'cache-control'�[39m: �[32m'no-cache'�[39m,
      �[32m'content-length'�[39m: �[32m'300'�[39m,
      �[32m'content-security-policy'�[39m: �[32m"default-src 'none'"�[39m,
      �[32m'content-type'�[39m: �[32m'application/json; charset=utf-8'�[39m,
      date: �[32m'Mon, 23 Feb 2026 18:27:24 GMT'�[39m,
      �[32m'referrer-policy'�[39m: �[32m'origin-when-cross-origin, strict-origin-when-cross-origin'�[39m,
      server: �[32m'github.com'�[39m,
      �[32m'strict-transport-security'�[39m: �[32m'max-age=31536000; includeSubdomains; preload'�[39m,
      vary: �[32m'Accept, Authorization, Cookie, X-GitHub-OTP,Accept-Encoding, Accept, X-Requested-With'�[39m,
      �[32m'x-accepted-github-permissions'�[39m: �[32m'allows_permissionless_access=true'�[39m,
      �[32m'x-content-type-options'�[39m: �[32m'nosniff'�[39m,
      �[32m'x-frame-options'�[39m: �[32m'deny'�[39m,
      �[32m'x-github-api-version-selected'�[39m: �[32m'2022-11-28'�[39m,
      �[32m'x-github-media-type'�[39m: �[32m'github.v3; format=json'�[39m,
      �[32m'x-github-request-id'�[39m: �[32m'E97E:364F1E:286702F:B17A6E6:699C9C0C'�[39m,
      �[32m'x-ratelimit-limit'�[39m: �[32m'30'�[39m,
      �[32m'x-ratelimit-remaining'�[39m: �[32m'29'�[39m,
      �[32m'x-ratelimit-reset'�[39m: �[32m'1771871304'�[39m,
      �[32m'x-ratelimit-resource'�[39m: �[32m'search'�[39m,
      �[32m'x-ratelimit-used'�[39m: �[32m'1'�[39m,
      �[32m'x-xss-protection'�[39m: �[32m'0'�[39m
    },
    data: {
      message: �[32m'Validation Failed'�[39m,
      errors: �[36m[Array]�[39m,
      documentation_url: �[32m'https://docs.github.com/v3/search/'�[39m,
      status: �[32m'422'�[39m
    }
  },
  request: {
    method: �[32m'GET'�[39m,
    url: �[32m'https://api.github.com/search/issues?q=is%3Apr%20author%3ACopilot%20is%3Aclosed%20repo%3Atwentyhq%2Ftwenty&per_page=2&page=1'�[39m,
    headers: {
      accept: �[32m'application/vnd.github.v3+json'�[39m,
      �[32m'user-agent'�[39m: �[32m'octokit-rest.js/20.1.2 octokit-core.js/5.2.2 Node.js/24'�[39m,
      authorization: �[32m'token [REDACTED]'�[39m
    },
    request: { hook: �[36m[Function: bound bound register]�[39m }
  }
}
danger-results://tmp/danger-results-a908f5f8.json

Generated by 🚫 dangerJS against c556e1e

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.

4 participants