(Fix) Upsert no longer rewrites position on existing records (#21375)

## Fix: upsert no longer rewrites `position` on existing records ### Problem `createX(..., upsert: true)` resets the `position` of records that resolve to an **update**, even when the payload doesn't include a `position`. The create-many/upsert runner backfills `position` (to `"first"`) in `computeArgs` over the **whole batch**, before records are split into insert vs update. So existing rows get a freshly recomputed `position` written on every upsert. For callers that re-upsert their full dataset on a schedule (e.g. a daily sync), this rewrites `position` for every record on each run and drifts the values steadily negative — and it floods audit/event logs with position churn. The dedicated `updateOne`/`updateMany` runners already pass `shouldBackfillPositionIfUndefined: false`; the upsert path did not. ### Fix Only backfill `position` for records that are actually inserted: - `computeArgs` now passes `shouldBackfillPositionIfUndefined: !args.upsert` in both the create-many and create-one runners, so undefined positions are left untouched on upsert. - `performUpsertOperation` backfills `"first"` positions for `recordsToInsert` only, **after** categorization, via `RecordPositionService`. Explicit `position` values (`"first"`, `"last"`, or a number) in the payload are still honored. Plain (non-upsert) create behavior is unchanged. ### Behavior | Scenario | Before | After | |---|---|---| | Upsert updates existing row, no `position` sent | `position` rewritten | `position` untouched | | Upsert inserts new row, no `position` sent | gets `"first"` | gets `"first"` (unchanged) | | Explicit `position` on upsert | applied | applied | | Plain create | unchanged | unchanged |
2026-06-12 10:50:09 +02:00
parent a0e3c43234
commit e334551da9
3 changed files with 173 additions and 2 deletions
@@ -7,8 +7,8 @@ import { isDefined } from 'twenty-shared/utils';
 import { FindOptionsRelations, In, InsertResult, ObjectLiteral } from 'typeorm';

 import { CommonBaseQueryRunnerService } from 'src/engine/api/common/common-query-runners/common-base-query-runner.service';
-import { PartialObjectRecordWithId } from 'src/engine/api/common/common-query-runners/common-create-many-query-runner/types/partial-object-record-with-id.type';
 import { type ConflictingFieldGroup } from 'src/engine/api/common/common-query-runners/common-create-many-query-runner/types/conflicting-field-group.type';
+import { PartialObjectRecordWithId } from 'src/engine/api/common/common-query-runners/common-create-many-query-runner/types/partial-object-record-with-id.type';
 import { buildWhereConditions } from 'src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/build-where-conditions.util';
 import { categorizeRecords } from 'src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/categorize-records.util';
 import { getConflictingFields } from 'src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/get-conflicting-fields.util';
@@ -31,6 +31,7 @@ import { buildColumnsToSelect } from 'src/engine/api/graphql/graphql-query-runne
 import { assertIsValidUuid } from 'src/engine/api/graphql/workspace-query-runner/utils/assert-is-valid-uuid.util';
 import { getAllSelectableColumnNames } from 'src/engine/api/utils/get-all-selectable-column-names.utils';
 import { WorkspaceAuthContext } from 'src/engine/core-modules/auth/types/workspace-auth-context.type';
+import { RecordPositionService } from 'src/engine/core-modules/record-position/services/record-position.service';
 import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-maps.type';
 import { findFlatEntityByIdInFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-id-in-flat-entity-maps.util';
 import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata.type';
@@ -47,6 +48,11 @@ export class CommonCreateManyQueryRunnerService extends CommonBaseQueryRunnerSer
  ObjectRecord[]
 > {
  protected readonly operationName = CommonQueryNames.CREATE_MANY;
+
+  constructor(private readonly recordPositionService: RecordPositionService) {
+    super();
+  }
+
  async run(
    args: CommonExtendedInput<CreateManyQueryArgs>,
    queryRunnerContext: CommonExtendedQueryRunnerContext,
@@ -77,6 +83,7 @@ export class CommonCreateManyQueryRunnerService extends CommonBaseQueryRunnerSer
      flatObjectMetadataMaps,
      flatFieldMetadataMaps,
      args,
+      workspaceId: authContext.workspace.id,
    });

    const upsertedRecords = await this.fetchUpsertedRecords({
@@ -161,6 +168,7 @@ export class CommonCreateManyQueryRunnerService extends CommonBaseQueryRunnerSer
        flatObjectMetadata,
        flatFieldMetadataMaps,
        flatObjectMetadataMaps,
+        shouldBackfillPositionIfUndefined: !args.upsert,
      }),
    };
  }
@@ -186,12 +194,14 @@ export class CommonCreateManyQueryRunnerService extends CommonBaseQueryRunnerSer
    flatObjectMetadataMaps,
    flatFieldMetadataMaps,
    args,
+    workspaceId,
  }: {
    repository: WorkspaceRepository<ObjectLiteral>;
    flatObjectMetadata: FlatObjectMetadata;
    flatObjectMetadataMaps: FlatEntityMaps<FlatObjectMetadata>;
    flatFieldMetadataMaps: FlatEntityMaps<FlatFieldMetadata>;
    args: CommonExtendedInput<CreateManyQueryArgs>;
+    workspaceId: string;
  }): Promise<InsertResult> {
    const { selectedFieldsResult } = args;

@@ -214,6 +224,7 @@ export class CommonCreateManyQueryRunnerService extends CommonBaseQueryRunnerSer
      flatFieldMetadataMaps,
      args,
      selectedFieldsResult,
+      workspaceId,
    });
  }

@@ -224,6 +235,7 @@ export class CommonCreateManyQueryRunnerService extends CommonBaseQueryRunnerSer
    flatFieldMetadataMaps,
    args,
    selectedFieldsResult,
+    workspaceId,
  }: {
    repository: WorkspaceRepository<ObjectLiteral>;
    flatObjectMetadata: FlatObjectMetadata;
@@ -231,6 +243,7 @@ export class CommonCreateManyQueryRunnerService extends CommonBaseQueryRunnerSer
    flatFieldMetadataMaps: FlatEntityMaps<FlatFieldMetadata>;
    args: CreateManyQueryArgs;
    selectedFieldsResult: CommonSelectedFieldsResult;
+    workspaceId: string;
  }): Promise<InsertResult> {
    const conflictingFieldGroups = getConflictingFields(
      flatObjectMetadata,
@@ -250,6 +263,13 @@ export class CommonCreateManyQueryRunnerService extends CommonBaseQueryRunnerSer
      existingRecords,
    );

+    const recordsToInsertWithPosition = await this.backfillPositionForInserts({
+      recordsToInsert,
+      flatObjectMetadata,
+      flatFieldMetadataMaps,
+      workspaceId,
+    });
+
    const result: InsertResult = {
      identifiers: [],
      generatedMaps: [],
@@ -276,7 +296,7 @@ export class CommonCreateManyQueryRunnerService extends CommonBaseQueryRunnerSer
    }

    await this.processRecordsToInsert({
-      recordsToInsert,
+      recordsToInsert: recordsToInsertWithPosition,
      repository,
      result,
      columnsToReturn,
@@ -285,6 +305,38 @@ export class CommonCreateManyQueryRunnerService extends CommonBaseQueryRunnerSer
    return result;
  }

+  private async backfillPositionForInserts({
+    recordsToInsert,
+    flatObjectMetadata,
+    flatFieldMetadataMaps,
+    workspaceId,
+  }: {
+    recordsToInsert: Partial<ObjectRecord>[];
+    flatObjectMetadata: FlatObjectMetadata;
+    flatFieldMetadataMaps: FlatEntityMaps<FlatFieldMetadata>;
+    workspaceId: string;
+  }): Promise<Partial<ObjectRecord>[]> {
+    if (recordsToInsert.length === 0) {
+      return recordsToInsert;
+    }
+
+    const { fieldIdByName } = buildFieldMapsFromFlatObjectMetadata(
+      flatFieldMetadataMaps,
+      flatObjectMetadata,
+    );
+
+    return this.recordPositionService.overridePositionOnRecords({
+      partialRecordInputs: recordsToInsert,
+      workspaceId,
+      objectMetadata: {
+        isCustom: flatObjectMetadata.isCustom ?? false,
+        nameSingular: flatObjectMetadata.nameSingular,
+        fieldIdByName,
+      },
+      shouldBackfillPositionIfUndefined: true,
+    });
+  }
+
  private async findExistingRecords({
    repository,
    flatObjectMetadata,
@@ -65,6 +65,7 @@ export class CommonCreateOneQueryRunnerService extends CommonBaseQueryRunnerServ
      flatObjectMetadata,
      flatFieldMetadataMaps,
      flatObjectMetadataMaps,
+      shouldBackfillPositionIfUndefined: !args.upsert,
    });

    return {