diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-builder/factories/__tests__/record-position-query.factory.spec.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-builder/factories/__tests__/record-position-query.factory.spec.ts index 455e51864..6bd4266e3 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-builder/factories/__tests__/record-position-query.factory.spec.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-query-builder/factories/__tests__/record-position-query.factory.spec.ts @@ -16,74 +16,66 @@ describe('RecordPositionQueryFactory', () => { }); describe('create', () => { - describe('createForGet', () => { - it('should return a string with the position when positionValue is first', async () => { - const positionValue = 'first'; - - const result = await factory.create( - RecordPositionQueryType.GET, - positionValue, - objectMetadataItem, - dataSourceSchema, - ); - - expect(result).toEqual( - `SELECT position FROM workspace_test."company" - WHERE "position" IS NOT NULL ORDER BY "position" ASC LIMIT 1`, - ); - }); - - it('should return a string with the position when positionValue is last', async () => { - const positionValue = 'last'; - - const result = await factory.create( - RecordPositionQueryType.GET, - positionValue, - objectMetadataItem, - dataSourceSchema, - ); - - expect(result).toEqual( - `SELECT position FROM workspace_test."company" - WHERE "position" IS NOT NULL ORDER BY "position" DESC LIMIT 1`, - ); - }); - }); - - it('should return a string with the position when positionValue is a number', async () => { + it('should return query and params for FIND_BY_POSITION', async () => { const positionValue = 1; - - try { - await factory.create( - RecordPositionQueryType.GET, - positionValue, - objectMetadataItem, - dataSourceSchema, - ); - } catch (error) { - expect(error.message).toEqual( - 'RecordPositionQueryType.GET requires positionValue to be a number', - ); - } - }); - }); - - describe('createForUpdate', () => { - it('should return a string when RecordPositionQueryType is UPDATE', async () => { - const positionValue = 1; - - const result = await factory.create( - RecordPositionQueryType.UPDATE, - positionValue, + const queryType = RecordPositionQueryType.FIND_BY_POSITION; + const [query, params] = await factory.create( + { positionValue, recordPositionQueryType: queryType }, objectMetadataItem, dataSourceSchema, ); - expect(result).toEqual( - `UPDATE workspace_test."company" + expect(query).toEqual( + `SELECT position FROM ${dataSourceSchema}."${objectMetadataItem.nameSingular}" + WHERE "position" = $1`, + ); + expect(params).toEqual([positionValue]); + }); + + it('should return query and params for FIND_MIN_POSITION', async () => { + const queryType = RecordPositionQueryType.FIND_MIN_POSITION; + const [query, params] = await factory.create( + { recordPositionQueryType: queryType }, + objectMetadataItem, + dataSourceSchema, + ); + + expect(query).toEqual( + `SELECT MIN(position) as position FROM ${dataSourceSchema}."${objectMetadataItem.nameSingular}"`, + ); + expect(params).toEqual([]); + }); + + it('should return query and params for FIND_MAX_POSITION', async () => { + const queryType = RecordPositionQueryType.FIND_MAX_POSITION; + const [query, params] = await factory.create( + { recordPositionQueryType: queryType }, + objectMetadataItem, + dataSourceSchema, + ); + + expect(query).toEqual( + `SELECT MAX(position) as position FROM ${dataSourceSchema}."${objectMetadataItem.nameSingular}"`, + ); + expect(params).toEqual([]); + }); + + it('should return query and params for UPDATE_POSITION', async () => { + const positionValue = 1; + const recordId = '1'; + const queryType = RecordPositionQueryType.UPDATE_POSITION; + const [query, params] = await factory.create( + { positionValue, recordId, recordPositionQueryType: queryType }, + objectMetadataItem, + dataSourceSchema, + ); + + expect(query).toEqual( + `UPDATE ${dataSourceSchema}."${objectMetadataItem.nameSingular}" SET "position" = $1 WHERE "id" = $2`, ); + expect(params).toEqual([positionValue, recordId]); }); }); }); diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-builder/factories/record-position-query.factory.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-builder/factories/record-position-query.factory.ts index a9201d426..dbd279395 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-builder/factories/record-position-query.factory.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-query-builder/factories/record-position-query.factory.ts @@ -1,54 +1,119 @@ import { Injectable } from '@nestjs/common'; +import { computeTableName } from 'src/engine/utils/compute-table-name.util'; + export enum RecordPositionQueryType { - GET = 'GET', - UPDATE = 'UPDATE', + FIND_MIN_POSITION = 'FIND_MIN_POSITION', + FIND_MAX_POSITION = 'FIND_MAX_POSITION', + FIND_BY_POSITION = 'FIND_BY_POSITION', + UPDATE_POSITION = 'UPDATE_POSITION', } +type FindByPositionQueryArgs = { + positionValue: number; + recordPositionQueryType: RecordPositionQueryType.FIND_BY_POSITION; +}; + +type FindMinPositionQueryArgs = { + recordPositionQueryType: RecordPositionQueryType.FIND_MIN_POSITION; +}; + +type FindMaxPositionQueryArgs = { + recordPositionQueryType: RecordPositionQueryType.FIND_MAX_POSITION; +}; + +type UpdatePositionQueryArgs = { + recordId: string; + positionValue: number; + recordPositionQueryType: RecordPositionQueryType.UPDATE_POSITION; +}; + +type RecordPositionQuery = string; + +type RecordPositionQueryParams = any[]; + +export type RecordPositionQueryArgs = + | FindByPositionQueryArgs + | FindMinPositionQueryArgs + | FindMaxPositionQueryArgs + | UpdatePositionQueryArgs; + @Injectable() export class RecordPositionQueryFactory { - async create( - recordPositionQueryType: RecordPositionQueryType, - positionValue: 'first' | 'last' | number, + create( + recordPositionQueryArgs: RecordPositionQueryArgs, objectMetadata: { isCustom: boolean; nameSingular: string }, dataSourceSchema: string, - ): Promise { - const name = - (objectMetadata.isCustom ? '_' : '') + objectMetadata.nameSingular; + ): [RecordPositionQuery, RecordPositionQueryParams] { + const name = computeTableName( + objectMetadata.nameSingular, + objectMetadata.isCustom, + ); - switch (recordPositionQueryType) { - case RecordPositionQueryType.GET: - if (typeof positionValue === 'number') { - throw new Error( - 'RecordPositionQueryType.GET requires positionValue to be a number', - ); - } - - return this.createForGet(positionValue, name, dataSourceSchema); - case RecordPositionQueryType.UPDATE: - return this.createForUpdate(name, dataSourceSchema); + switch (recordPositionQueryArgs.recordPositionQueryType) { + case RecordPositionQueryType.FIND_BY_POSITION: + return this.buildFindByPositionQuery( + recordPositionQueryArgs satisfies FindByPositionQueryArgs, + name, + dataSourceSchema, + ); + case RecordPositionQueryType.FIND_MIN_POSITION: + return this.buildFindMinPositionQuery(name, dataSourceSchema); + case RecordPositionQueryType.FIND_MAX_POSITION: + return this.buildFindMaxPositionQuery(name, dataSourceSchema); + case RecordPositionQueryType.UPDATE_POSITION: + return this.buildUpdatePositionQuery( + recordPositionQueryArgs satisfies UpdatePositionQueryArgs, + name, + dataSourceSchema, + ); default: throw new Error('Invalid RecordPositionQueryType'); } } - private async createForGet( - positionValue: 'first' | 'last', + private buildFindByPositionQuery( + { positionValue }: FindByPositionQueryArgs, name: string, dataSourceSchema: string, - ): Promise { - const orderByDirection = positionValue === 'first' ? 'ASC' : 'DESC'; - - return `SELECT position FROM ${dataSourceSchema}."${name}" - WHERE "position" IS NOT NULL ORDER BY "position" ${orderByDirection} LIMIT 1`; + ): [RecordPositionQuery, RecordPositionQueryParams] { + return [ + `SELECT position FROM ${dataSourceSchema}."${name}" + WHERE "position" = $1`, + [positionValue], + ]; } - private async createForUpdate( + private buildFindMaxPositionQuery( name: string, dataSourceSchema: string, - ): Promise { - return `UPDATE ${dataSourceSchema}."${name}" + ): [RecordPositionQuery, RecordPositionQueryParams] { + return [ + `SELECT MAX(position) as position FROM ${dataSourceSchema}."${name}"`, + [], + ]; + } + + private buildFindMinPositionQuery( + name: string, + dataSourceSchema: string, + ): [RecordPositionQuery, RecordPositionQueryParams] { + return [ + `SELECT MIN(position) as position FROM ${dataSourceSchema}."${name}"`, + [], + ]; + } + + private buildUpdatePositionQuery( + { recordId, positionValue }: UpdatePositionQueryArgs, + name: string, + dataSourceSchema: string, + ): [RecordPositionQuery, RecordPositionQueryParams] { + return [ + `UPDATE ${dataSourceSchema}."${name}" SET "position" = $1 - WHERE "id" = $2`; + WHERE "id" = $2`, + [positionValue, recordId], + ]; } } diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/__tests__/query-runner-args.factory.spec.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/__tests__/query-runner-args.factory.spec.ts index 317527e06..71e26b268 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/__tests__/query-runner-args.factory.spec.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/__tests__/query-runner-args.factory.spec.ts @@ -12,12 +12,14 @@ describe('QueryRunnerArgsFactory', () => { const recordPositionFactory = { create: jest.fn().mockResolvedValue(2), }; + const workspaceId = 'workspaceId'; const options = { fieldMetadataCollection: [ { name: 'position', type: FieldMetadataType.POSITION }, { name: 'testNumber', type: FieldMetadataType.NUMBER }, ] as FieldMetadataInterface[], objectMetadataItem: { isCustom: true, nameSingular: 'test' }, + workspaceId, } as WorkspaceQueryRunnerOptions; let factory: QueryRunnerArgsFactory; @@ -68,6 +70,36 @@ describe('QueryRunnerArgsFactory', () => { ResolverArgsType.CreateMany, ); + expect(recordPositionFactory.create).toHaveBeenCalledWith( + 'last', + { isCustom: true, nameSingular: 'test' }, + workspaceId, + 0, + ); + expect(result).toEqual({ + id: 'uuid', + data: [{ position: 2, testNumber: 1 }], + }); + }); + + it('createMany type should override position if not present', async () => { + const args = { + id: 'uuid', + data: [{ testNumber: '1' }], + }; + + const result = await factory.create( + args, + options, + ResolverArgsType.CreateMany, + ); + + expect(recordPositionFactory.create).toHaveBeenCalledWith( + 'first', + { isCustom: true, nameSingular: 'test' }, + workspaceId, + 0, + ); expect(result).toEqual({ id: 'uuid', data: [{ position: 2, testNumber: 1 }], diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/__tests__/record-position.factory.spec.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/__tests__/record-position.factory.spec.ts index 179637d6a..272309a01 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/__tests__/record-position.factory.spec.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/__tests__/record-position.factory.spec.ts @@ -9,14 +9,15 @@ describe('RecordPositionFactory', () => { create: jest.fn().mockResolvedValue('query'), }; - const workspaceDataSourceService = { - getSchemaName: jest.fn().mockReturnValue('schemaName'), - executeRawQuery: jest.fn().mockResolvedValue([{ position: 1 }]), - }; + let workspaceDataSourceService; let factory: RecordPositionFactory; beforeEach(async () => { + workspaceDataSourceService = { + getSchemaName: jest.fn().mockReturnValue('schemaName'), + executeRawQuery: jest.fn().mockResolvedValue([{ position: 1 }]), + }; const module: TestingModule = await Test.createTestingModule({ providers: [ RecordPositionFactory, @@ -44,10 +45,20 @@ describe('RecordPositionFactory', () => { it('should return the value when value is a number', async () => { const value = 1; + + workspaceDataSourceService.executeRawQuery.mockResolvedValue([]); + const result = await factory.create(value, objectMetadata, workspaceId); expect(result).toEqual(value); }); + it('should throw an error when position is not unique', async () => { + const value = 1; + + await expect( + factory.create(value, objectMetadata, workspaceId), + ).rejects.toThrow('Position is not unique'); + }); it('should return the existing position -1 when value is first', async () => { const value = 'first'; const result = await factory.create(value, objectMetadata, workspaceId); diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-runner-args.factory.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-runner-args.factory.ts index 8cbf35794..728d77ae3 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-runner-args.factory.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-runner-args.factory.ts @@ -16,6 +16,11 @@ import { FieldMetadataType } from 'src/engine/metadata-modules/field-metadata/fi import { RecordPositionFactory } from './record-position.factory'; +type ArgPositionBackfillInput = { + argIndex?: number; + shouldBackfillPosition: boolean; +}; + @Injectable() export class QueryRunnerArgsFactory { constructor(private readonly recordPositionFactory: RecordPositionFactory) {} @@ -39,8 +44,11 @@ export class QueryRunnerArgsFactory { return { ...args, data: await Promise.all( - (args as CreateManyResolverArgs).data.map((arg) => - this.overrideDataByFieldMetadata(arg, options, fieldMetadataMap), + (args as CreateManyResolverArgs).data.map((arg, index) => + this.overrideDataByFieldMetadata(arg, options, fieldMetadataMap, { + argIndex: index, + shouldBackfillPosition: true, + }), ), ), } satisfies CreateManyResolverArgs; @@ -73,6 +81,7 @@ export class QueryRunnerArgsFactory { (args as FindDuplicatesResolverArgs).data, options, fieldMetadataMap, + { shouldBackfillPosition: false }, ), }; default: @@ -84,11 +93,14 @@ export class QueryRunnerArgsFactory { data: Record | undefined, options: WorkspaceQueryRunnerOptions, fieldMetadataMap: Map, + argPositionBackfillInput: ArgPositionBackfillInput, ) { if (!data) { return; } + let isFieldPositionPresent = false; + const createArgPromiseByArgKey = Object.entries(data).map( async ([key, value]) => { const fieldMetadata = fieldMetadataMap.get(key); @@ -99,6 +111,8 @@ export class QueryRunnerArgsFactory { switch (fieldMetadata.type) { case FieldMetadataType.POSITION: + isFieldPositionPresent = true; + return [ key, await this.recordPositionFactory.create( @@ -108,6 +122,7 @@ export class QueryRunnerArgsFactory { nameSingular: options.objectMetadataItem.nameSingular, }, options.workspaceId, + argPositionBackfillInput.argIndex, ), ]; case FieldMetadataType.NUMBER: @@ -120,6 +135,27 @@ export class QueryRunnerArgsFactory { const newArgEntries = await Promise.all(createArgPromiseByArgKey); + if ( + !isFieldPositionPresent && + argPositionBackfillInput.shouldBackfillPosition + ) { + return Object.fromEntries([ + ...newArgEntries, + [ + 'position', + await this.recordPositionFactory.create( + 'first', + { + isCustom: options.objectMetadataItem.isCustom, + nameSingular: options.objectMetadataItem.nameSingular, + }, + options.workspaceId, + argPositionBackfillInput.argIndex, + ), + ], + ]); + } + return Object.fromEntries(newArgEntries); } diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/record-position.factory.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/record-position.factory.ts index 8aaf879a1..1b9cc5bb8 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/record-position.factory.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/record-position.factory.ts @@ -4,6 +4,7 @@ import { isDefined } from 'class-validator'; import { WorkspaceDataSourceService } from 'src/engine/workspace-datasource/workspace-datasource.service'; import { + RecordPositionQueryArgs, RecordPositionQueryFactory, RecordPositionQueryType, } from 'src/engine/api/graphql/workspace-query-builder/factories/record-position-query.factory'; @@ -19,40 +20,76 @@ export class RecordPositionFactory { value: number | 'first' | 'last', objectMetadata: { isCustom: boolean; nameSingular: string }, workspaceId: string, + index = 0, ): Promise { - if (typeof value === 'number') { - return value; - } - const dataSourceSchema = this.workspaceDataSourceService.getSchemaName(workspaceId); - const query = await this.recordPositionQueryFactory.create( - RecordPositionQueryType.GET, - value, + if (typeof value === 'number') { + const recordWithSamePosition = await this.findRecordPosition( + { + recordPositionQueryType: RecordPositionQueryType.FIND_BY_POSITION, + positionValue: value, + }, + objectMetadata, + dataSourceSchema, + workspaceId, + ); + + if (recordWithSamePosition) { + throw new Error('Position is not unique'); + } + + return value; + } + + if (value === 'first') { + const recordWithMinPosition = await this.findRecordPosition( + { + recordPositionQueryType: RecordPositionQueryType.FIND_MIN_POSITION, + }, + objectMetadata, + dataSourceSchema, + workspaceId, + ); + + return isDefined(recordWithMinPosition?.position) + ? recordWithMinPosition.position - index - 1 + : 1; + } + + const recordWithMaxPosition = await this.findRecordPosition( + { + recordPositionQueryType: RecordPositionQueryType.FIND_MAX_POSITION, + }, + objectMetadata, + dataSourceSchema, + workspaceId, + ); + + return isDefined(recordWithMaxPosition?.position) + ? recordWithMaxPosition.position + index + 1 + : 1; + } + + private async findRecordPosition( + recordPositionQueryArgs: RecordPositionQueryArgs, + objectMetadata: { isCustom: boolean; nameSingular: string }, + dataSourceSchema: string, + workspaceId: string, + ) { + const [query, params] = await this.recordPositionQueryFactory.create( + recordPositionQueryArgs, objectMetadata, dataSourceSchema, ); - // If the value was 'first', the first record will be the one with the lowest position - // If the value was 'last', the first record will be the one with the highest position const records = await this.workspaceDataSourceService.executeRawQuery( query, - [], + params, workspaceId, - undefined, ); - if ( - !isDefined(records) || - records.length === 0 || - !isDefined(records[0]?.position) - ) { - return 1; - } - - return value === 'first' - ? records[0].position - 1 - : records[0].position + 1; + return records?.[0]; } } diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/services/record-position-backfill-service.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/services/record-position-backfill-service.ts index 42242d086..1937e60a3 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/services/record-position-backfill-service.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/services/record-position-backfill-service.ts @@ -31,16 +31,19 @@ export class RecordPositionBackfillService { const dataSourceSchema = this.workspaceDataSourceService.getSchemaName(workspaceId); - const query = await this.recordPositionQueryFactory.create( - RecordPositionQueryType.UPDATE, - position, + const [query, params] = await this.recordPositionQueryFactory.create( + { + recordPositionQueryType: RecordPositionQueryType.UPDATE_POSITION, + recordId, + positionValue: position, + }, objectMetadata as ObjectMetadataInterface, dataSourceSchema, ); this.workspaceDataSourceService.executeRawQuery( query, - [position, recordId], + params, workspaceId, undefined, );