From e334551da934b55725ae6a39254eb3c9591a48ab Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Fri, 12 Jun 2026 10:50:09 +0200 Subject: [PATCH] (Fix) Upsert no longer rewrites `position` on existing records (#21375) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 | --- ...common-create-many-query-runner.service.ts | 56 ++++++++- .../common-create-one-query-runner.service.ts | 1 + .../suites/upsert/upsert.integration-spec.ts | 118 ++++++++++++++++++ 3 files changed, 173 insertions(+), 2 deletions(-) diff --git a/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/common-create-many-query-runner.service.ts b/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/common-create-many-query-runner.service.ts index a86c184dcb..b5b6955799 100644 --- a/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/common-create-many-query-runner.service.ts +++ b/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/common-create-many-query-runner.service.ts @@ -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, 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; flatObjectMetadata: FlatObjectMetadata; flatObjectMetadataMaps: FlatEntityMaps; flatFieldMetadataMaps: FlatEntityMaps; args: CommonExtendedInput; + workspaceId: string; }): Promise { 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; flatObjectMetadata: FlatObjectMetadata; @@ -231,6 +243,7 @@ export class CommonCreateManyQueryRunnerService extends CommonBaseQueryRunnerSer flatFieldMetadataMaps: FlatEntityMaps; args: CreateManyQueryArgs; selectedFieldsResult: CommonSelectedFieldsResult; + workspaceId: string; }): Promise { 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[]; + flatObjectMetadata: FlatObjectMetadata; + flatFieldMetadataMaps: FlatEntityMaps; + workspaceId: string; + }): Promise[]> { + 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, diff --git a/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-one-query-runner.service.ts b/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-one-query-runner.service.ts index b8bac82b00..b772629782 100644 --- a/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-one-query-runner.service.ts +++ b/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-one-query-runner.service.ts @@ -65,6 +65,7 @@ export class CommonCreateOneQueryRunnerService extends CommonBaseQueryRunnerServ flatObjectMetadata, flatFieldMetadataMaps, flatObjectMetadataMaps, + shouldBackfillPositionIfUndefined: !args.upsert, }); return { diff --git a/packages/twenty-server/test/integration/graphql/suites/upsert/upsert.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/upsert/upsert.integration-spec.ts index fdad0a7904..64c10acc2e 100644 --- a/packages/twenty-server/test/integration/graphql/suites/upsert/upsert.integration-spec.ts +++ b/packages/twenty-server/test/integration/graphql/suites/upsert/upsert.integration-spec.ts @@ -33,6 +33,20 @@ const deleteRecordsQuery = gql` } `; +const createRecordsWithPositionQuery = gql` + mutation CreateRecordsWithPosition( + $data: [TestRecordObjectCreateInput!]! + $upsert: Boolean + ) { + createTestRecordObjects(data: $data, upsert: $upsert) { + id + firstUniqueTestField + name + position + } + } +`; + describe('upsert (createMany with upsert:true)', () => { let createdObjectMetadataId = ''; @@ -256,4 +270,108 @@ describe('upsert (createMany with upsert:true)', () => { createdRecord.id, ); }); + + it('should not change the position of an existing record when upserting without a position', async () => { + const createResponse = await makeGraphqlAPIRequest({ + query: createRecordsWithPositionQuery, + variables: { + data: [ + { + firstUniqueTestField: 'positionTestField', + secondUniqueTestField: 'positionTestSecondField', + name: 'originalRecord', + }, + ], + upsert: false, + }, + }); + + const createdRecord = createResponse.body.data.createTestRecordObjects[0]; + const initialPosition = createdRecord.position; + + expect(typeof initialPosition).toBe('number'); + + const upsertResponse = await makeGraphqlAPIRequest({ + query: createRecordsWithPositionQuery, + variables: { + data: [ + { + firstUniqueTestField: 'positionTestField', + name: 'updatedRecord', + }, + ], + upsert: true, + }, + }); + + const upsertedRecord = upsertResponse.body.data.createTestRecordObjects[0]; + + expect(upsertedRecord.id).toEqual(createdRecord.id); + expect(upsertedRecord.name).toEqual('updatedRecord'); + expect(upsertedRecord.position).toEqual(initialPosition); + }); + + it('should auto-assign a position when an upsert creates a new record', async () => { + const upsertResponse = await makeGraphqlAPIRequest({ + query: createRecordsWithPositionQuery, + variables: { + data: [ + { + firstUniqueTestField: 'insertedViaUpsertField', + secondUniqueTestField: 'insertedViaUpsertSecondField', + name: 'insertedRecord', + }, + ], + upsert: true, + }, + }); + + const insertedRecord = upsertResponse.body.data.createTestRecordObjects[0]; + + expect(insertedRecord.id).toEqual(expect.any(String)); + expect(typeof insertedRecord.position).toBe('number'); + }); + + it('should update the position of an existing record when upserting with a position', async () => { + const createResponse = await makeGraphqlAPIRequest({ + query: createRecordsWithPositionQuery, + variables: { + data: [ + { + firstUniqueTestField: 'updatePositionTestField', + secondUniqueTestField: 'updatePositionTestSecondField', + name: 'originalRecord', + position: 1, + }, + ], + upsert: false, + }, + }); + + const createdRecord = createResponse.body.data.createTestRecordObjects[0]; + + expect(createdRecord.position).toEqual(1); + + const newPosition = 5; + + const upsertResponse = await makeGraphqlAPIRequest({ + query: createRecordsWithPositionQuery, + variables: { + data: [ + { + firstUniqueTestField: 'updatePositionTestField', + name: 'updatedRecord', + position: newPosition, + }, + ], + upsert: true, + }, + }); + + const upsertedRecord = upsertResponse.body.data.createTestRecordObjects[0]; + + expect(upsertedRecord.id).toEqual(createdRecord.id); + expect(upsertedRecord.name).toEqual('updatedRecord'); + expect(upsertedRecord.position).toEqual(newPosition); + }); });