From c82c60b44803ac51ffbcd39e5caa071cc5091f67 Mon Sep 17 00:00:00 2001 From: Thomas Trompette Date: Mon, 11 Mar 2024 16:18:15 +0100 Subject: [PATCH] Build arg setter for position (#4396) * Build arg setter for position * Build separated query factory + rename existing * Sort record by position in front * Add tests * Set first for type board --------- Co-authored-by: Thomas Trompette --- .../internal/useSetRecordBoardRecordIds.ts | 19 +++ .../components/RecordBoardColumnNewButton.tsx | 1 + .../RecordBoardColumnNewOpportunityButton.tsx | 1 + .../pages/object-record/RecordIndexPage.tsx | 2 +- .../record-position-query.factory.spec.ts | 48 +++++++ .../factories/factories.ts | 2 + .../record-position-query.factory.ts | 21 ++++ .../workspace-query-builder.module.ts | 7 +- .../query-runner-args.factory.spec.ts | 92 ++++++++++++++ .../workspace-query-runner/factories/index.ts | 3 + .../factories/query-runner-args.factory.ts | 119 ++++++++++++++++++ .../workspace-query-runner.module.ts | 3 +- .../workspace-query-runner.service.ts | 9 +- 13 files changed, 323 insertions(+), 4 deletions(-) create mode 100644 packages/twenty-server/src/workspace/workspace-query-builder/factories/__tests__/record-position-query.factory.spec.ts create mode 100644 packages/twenty-server/src/workspace/workspace-query-builder/factories/record-position-query.factory.ts create mode 100644 packages/twenty-server/src/workspace/workspace-query-runner/factories/__tests__/query-runner-args.factory.spec.ts create mode 100644 packages/twenty-server/src/workspace/workspace-query-runner/factories/index.ts create mode 100644 packages/twenty-server/src/workspace/workspace-query-runner/factories/query-runner-args.factory.ts diff --git a/packages/twenty-front/src/modules/object-record/record-board/hooks/internal/useSetRecordBoardRecordIds.ts b/packages/twenty-front/src/modules/object-record/record-board/hooks/internal/useSetRecordBoardRecordIds.ts index db2811af0..03db29158 100644 --- a/packages/twenty-front/src/modules/object-record/record-board/hooks/internal/useSetRecordBoardRecordIds.ts +++ b/packages/twenty-front/src/modules/object-record/record-board/hooks/internal/useSetRecordBoardRecordIds.ts @@ -28,6 +28,7 @@ export const useSetRecordBoardRecordIds = (recordBoardId?: string) => { const columnRecordIds = records .filter((record) => record.stage === column?.value) + .sort(sortRecordsByPosition) .map((record) => record.id); if (!isDeeplyEqual(existingColumnRecordIds, columnRecordIds)) { @@ -43,3 +44,21 @@ export const useSetRecordBoardRecordIds = (recordBoardId?: string) => { setRecordIds, }; }; + +const sortRecordsByPosition = ( + record1: ObjectRecord, + record2: ObjectRecord, +) => { + if ( + typeof record1.position == 'number' && + typeof record2.position == 'number' + ) { + return record1.position - record2.position; + } else if (record1.position === 'first' || record2.position === 'last') { + return -1; + } else if (record2.position === 'first' || record1.position === 'last') { + return 1; + } else { + return 0; + } +}; diff --git a/packages/twenty-front/src/modules/object-record/record-board/record-board-column/components/RecordBoardColumnNewButton.tsx b/packages/twenty-front/src/modules/object-record/record-board/record-board-column/components/RecordBoardColumnNewButton.tsx index deac52c76..98275ce7a 100644 --- a/packages/twenty-front/src/modules/object-record/record-board/record-board-column/components/RecordBoardColumnNewButton.tsx +++ b/packages/twenty-front/src/modules/object-record/record-board/record-board-column/components/RecordBoardColumnNewButton.tsx @@ -32,6 +32,7 @@ export const RecordBoardColumnNewButton = () => { const onNewClick = () => { createOneRecord({ [selectFieldMetadataItem.name]: columnDefinition.value, + position: 'last', }); }; diff --git a/packages/twenty-front/src/modules/object-record/record-board/record-board-column/components/RecordBoardColumnNewOpportunityButton.tsx b/packages/twenty-front/src/modules/object-record/record-board/record-board-column/components/RecordBoardColumnNewOpportunityButton.tsx index 9cac6da4f..101f72e24 100644 --- a/packages/twenty-front/src/modules/object-record/record-board/record-board-column/components/RecordBoardColumnNewOpportunityButton.tsx +++ b/packages/twenty-front/src/modules/object-record/record-board/record-board-column/components/RecordBoardColumnNewOpportunityButton.tsx @@ -52,6 +52,7 @@ export const RecordBoardColumnNewOpportunityButton = () => { createOneRecord({ name: company.name, companyId: company.id, + position: 'last', [selectFieldMetadataItem.name]: columnDefinition.value, }); }; diff --git a/packages/twenty-front/src/pages/object-record/RecordIndexPage.tsx b/packages/twenty-front/src/pages/object-record/RecordIndexPage.tsx index 61f719d43..286ca0728 100644 --- a/packages/twenty-front/src/pages/object-record/RecordIndexPage.tsx +++ b/packages/twenty-front/src/pages/object-record/RecordIndexPage.tsx @@ -37,7 +37,7 @@ export const RecordIndexPage = () => { const handleAddButtonClick = async () => { await createOneObject?.({ - position: 0, + position: 'first', }); setSelectedTableCellEditMode(0, 0); diff --git a/packages/twenty-server/src/workspace/workspace-query-builder/factories/__tests__/record-position-query.factory.spec.ts b/packages/twenty-server/src/workspace/workspace-query-builder/factories/__tests__/record-position-query.factory.spec.ts new file mode 100644 index 000000000..80430dd6d --- /dev/null +++ b/packages/twenty-server/src/workspace/workspace-query-builder/factories/__tests__/record-position-query.factory.spec.ts @@ -0,0 +1,48 @@ +import { ObjectMetadataInterface } from 'src/metadata/field-metadata/interfaces/object-metadata.interface'; + +import { RecordPositionQueryFactory } from 'src/workspace/workspace-query-builder/factories/record-position-query.factory'; + +describe('RecordPositionQueryFactory', () => { + const objectMetadataItem = { + isCustom: false, + nameSingular: 'company', + } as ObjectMetadataInterface; + const dataSourceSchema = 'workspace_test'; + const factory: RecordPositionQueryFactory = new RecordPositionQueryFactory(); + + it('should be defined', () => { + expect(factory).toBeDefined(); + }); + + describe('create', () => { + it('should return a string with the position when positionValue is first', async () => { + const positionValue = 'first'; + + const result = await factory.create( + 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( + positionValue, + objectMetadataItem, + dataSourceSchema, + ); + + expect(result).toEqual( + `SELECT position FROM workspace_test."company" + WHERE "position" IS NOT NULL ORDER BY "position" DESC LIMIT 1`, + ); + }); + }); +}); diff --git a/packages/twenty-server/src/workspace/workspace-query-builder/factories/factories.ts b/packages/twenty-server/src/workspace/workspace-query-builder/factories/factories.ts index 0a876fe94..dd1ec66d8 100644 --- a/packages/twenty-server/src/workspace/workspace-query-builder/factories/factories.ts +++ b/packages/twenty-server/src/workspace/workspace-query-builder/factories/factories.ts @@ -11,6 +11,7 @@ import { UpdateOneQueryFactory } from './update-one-query.factory'; import { UpdateManyQueryFactory } from './update-many-query.factory'; import { DeleteManyQueryFactory } from './delete-many-query.factory'; import { FindDuplicatesQueryFactory } from './find-duplicates-query.factory'; +import { RecordPositionQueryFactory } from './record-position-query.factory'; export const workspaceQueryBuilderFactories = [ ArgsAliasFactory, @@ -23,6 +24,7 @@ export const workspaceQueryBuilderFactories = [ FindManyQueryFactory, FindOneQueryFactory, FindDuplicatesQueryFactory, + RecordPositionQueryFactory, UpdateOneQueryFactory, UpdateManyQueryFactory, DeleteManyQueryFactory, diff --git a/packages/twenty-server/src/workspace/workspace-query-builder/factories/record-position-query.factory.ts b/packages/twenty-server/src/workspace/workspace-query-builder/factories/record-position-query.factory.ts new file mode 100644 index 000000000..d6b55a134 --- /dev/null +++ b/packages/twenty-server/src/workspace/workspace-query-builder/factories/record-position-query.factory.ts @@ -0,0 +1,21 @@ +import { Injectable } from '@nestjs/common'; + +import { ObjectMetadataInterface } from 'src/metadata/field-metadata/interfaces/object-metadata.interface'; + +@Injectable() +export class RecordPositionQueryFactory { + async create( + positionValue: 'first' | 'last', + objectMetadataItem: ObjectMetadataInterface, + dataSourceSchema: string, + ): Promise { + const orderByDirection = positionValue === 'first' ? 'ASC' : 'DESC'; + + const name = + (objectMetadataItem.isCustom ? '_' : '') + + objectMetadataItem.nameSingular; + + return `SELECT position FROM ${dataSourceSchema}."${name}" + WHERE "position" IS NOT NULL ORDER BY "position" ${orderByDirection} LIMIT 1`; + } +} diff --git a/packages/twenty-server/src/workspace/workspace-query-builder/workspace-query-builder.module.ts b/packages/twenty-server/src/workspace/workspace-query-builder/workspace-query-builder.module.ts index 367d95b49..b4ead5ae6 100644 --- a/packages/twenty-server/src/workspace/workspace-query-builder/workspace-query-builder.module.ts +++ b/packages/twenty-server/src/workspace/workspace-query-builder/workspace-query-builder.module.ts @@ -2,6 +2,7 @@ import { Module } from '@nestjs/common'; import { ObjectMetadataModule } from 'src/metadata/object-metadata/object-metadata.module'; import { FieldsStringFactory } from 'src/workspace/workspace-query-builder/factories/fields-string.factory'; +import { RecordPositionQueryFactory } from 'src/workspace/workspace-query-builder/factories/record-position-query.factory'; import { WorkspaceQueryBuilderFactory } from './workspace-query-builder.factory'; @@ -10,6 +11,10 @@ import { workspaceQueryBuilderFactories } from './factories/factories'; @Module({ imports: [ObjectMetadataModule], providers: [...workspaceQueryBuilderFactories, WorkspaceQueryBuilderFactory], - exports: [WorkspaceQueryBuilderFactory, FieldsStringFactory], + exports: [ + WorkspaceQueryBuilderFactory, + FieldsStringFactory, + RecordPositionQueryFactory, + ], }) export class WorkspaceQueryBuilderModule {} diff --git a/packages/twenty-server/src/workspace/workspace-query-runner/factories/__tests__/query-runner-args.factory.spec.ts b/packages/twenty-server/src/workspace/workspace-query-runner/factories/__tests__/query-runner-args.factory.spec.ts new file mode 100644 index 000000000..a1028092b --- /dev/null +++ b/packages/twenty-server/src/workspace/workspace-query-runner/factories/__tests__/query-runner-args.factory.spec.ts @@ -0,0 +1,92 @@ +import { Test, TestingModule } from '@nestjs/testing'; + +import { WorkspaceQueryRunnerOptions } from 'src/workspace/workspace-query-runner/interfaces/query-runner-option.interface'; +import { FieldMetadataInterface } from 'src/metadata/field-metadata/interfaces/field-metadata.interface'; + +import { WorkspaceDataSourceService } from 'src/workspace/workspace-datasource/workspace-datasource.service'; +import { RecordPositionQueryFactory } from 'src/workspace/workspace-query-builder/factories/record-position-query.factory'; +import { QueryRunnerArgsFactory } from 'src/workspace/workspace-query-runner/factories/query-runner-args.factory'; +import { FieldMetadataType } from 'src/metadata/field-metadata/field-metadata.entity'; + +describe('QueryRunnerArgsFactory', () => { + const workspaceDataSourceService = { + getSchemaName: jest.fn().mockResolvedValue('test schema'), + executeRawQuery: jest.fn(), + }; + const recordPositionQueryFactory = { + create: jest.fn().mockResolvedValue('test query'), + }; + const options = { + fieldMetadataCollection: [ + { name: 'position', type: FieldMetadataType.POSITION }, + ] as FieldMetadataInterface[], + } as WorkspaceQueryRunnerOptions; + + let factory: QueryRunnerArgsFactory; + + beforeEach(async () => { + jest.resetAllMocks(); + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + QueryRunnerArgsFactory, + { + provide: RecordPositionQueryFactory, + useValue: { + create: recordPositionQueryFactory.create, + }, + }, + { + provide: WorkspaceDataSourceService, + useValue: workspaceDataSourceService, + }, + ], + }).compile(); + + factory = module.get(QueryRunnerArgsFactory); + }); + + it('should be defined', () => { + expect(factory).toBeDefined(); + }); + + describe('create', () => { + it('should return the args when empty', async () => { + const args = {}; + const result = await factory.create(args, options); + + expect(result).toEqual(args); + }); + + it('should override position when of type string', async () => { + const args = { + position: 'first', + }; + + workspaceDataSourceService.executeRawQuery.mockResolvedValue([ + { position: 2 }, + ]); + + const result = await factory.create(args, options); + + expect(result).toEqual({ + position: 1, // Calculates 2 / 2 + }); + }); + + it('should override args when of type array', async () => { + const args = [{ id: 1 }, { position: 'last' }]; + + workspaceDataSourceService.executeRawQuery.mockResolvedValue([ + { position: 1 }, + ]); + + const result = await factory.create(args, options); + + expect(result).toEqual([ + { id: 1 }, + { position: 2 }, // Calculates 1 + 1 + ]); + }); + }); +}); diff --git a/packages/twenty-server/src/workspace/workspace-query-runner/factories/index.ts b/packages/twenty-server/src/workspace/workspace-query-runner/factories/index.ts new file mode 100644 index 000000000..fcff782e0 --- /dev/null +++ b/packages/twenty-server/src/workspace/workspace-query-runner/factories/index.ts @@ -0,0 +1,3 @@ +import { QueryRunnerArgsFactory } from 'src/workspace/workspace-query-runner/factories/query-runner-args.factory'; + +export const workspaceQueryRunnerFactories = [QueryRunnerArgsFactory]; diff --git a/packages/twenty-server/src/workspace/workspace-query-runner/factories/query-runner-args.factory.ts b/packages/twenty-server/src/workspace/workspace-query-runner/factories/query-runner-args.factory.ts new file mode 100644 index 000000000..6da315df0 --- /dev/null +++ b/packages/twenty-server/src/workspace/workspace-query-runner/factories/query-runner-args.factory.ts @@ -0,0 +1,119 @@ +import { Injectable } from '@nestjs/common'; + +import { FieldMetadataInterface } from 'src/metadata/field-metadata/interfaces/field-metadata.interface'; +import { WorkspaceQueryRunnerOptions } from 'src/workspace/workspace-query-runner/interfaces/query-runner-option.interface'; +import { ObjectMetadataInterface } from 'src/metadata/field-metadata/interfaces/object-metadata.interface'; + +import { WorkspaceDataSourceService } from 'src/workspace/workspace-datasource/workspace-datasource.service'; +import { FieldMetadataType } from 'src/metadata/field-metadata/field-metadata.entity'; +import { RecordPositionQueryFactory } from 'src/workspace/workspace-query-builder/factories/record-position-query.factory'; + +@Injectable() +export class QueryRunnerArgsFactory { + constructor( + private readonly workspaceDataSourceService: WorkspaceDataSourceService, + private readonly recordPositionQueryFactory: RecordPositionQueryFactory, + ) {} + + async create( + args: Record, + options: WorkspaceQueryRunnerOptions, + ) { + const fieldMetadataCollection = options.fieldMetadataCollection; + + const fieldMetadataMap = new Map( + fieldMetadataCollection.map((fieldMetadata) => [ + fieldMetadata.name, + fieldMetadata, + ]), + ); + + return this.createArgsRecursive(args, options, fieldMetadataMap); + } + + private async createArgsRecursive( + args: Record, + options: WorkspaceQueryRunnerOptions, + fieldMetadataMap: Map, + ) { + // If it's not an object, we don't need to do anything + if (typeof args !== 'object' || args === null) { + return args; + } + + // If it's an array, we need to map all items + if (Array.isArray(args)) { + return Promise.all( + args.map((arg) => + this.createArgsRecursive(arg, options, fieldMetadataMap), + ), + ); + } + + const createArgPromisesByArgKey = Object.entries(args).map( + async ([key, value]) => { + const fieldMetadata = fieldMetadataMap.get(key); + + if (!fieldMetadata) { + return [ + key, + await this.createArgsRecursive(value, options, fieldMetadataMap), + ]; + } + + switch (fieldMetadata.type) { + case FieldMetadataType.POSITION: + return [ + key, + await this.buildPositionValue( + value, + options.objectMetadataItem, + options.workspaceId, + ), + ]; + default: + return [ + key, + await this.createArgsRecursive(value, options, fieldMetadataMap), + ]; + } + }, + ); + + const newArgEntries = await Promise.all(createArgPromisesByArgKey); + + return Object.fromEntries(newArgEntries); + } + + private async buildPositionValue( + value: number | 'first' | 'last', + objectMetadataItem: ObjectMetadataInterface, + workspaceId: string, + ) { + if (typeof value === 'number') { + return value; + } + + const dataSourceSchema = + this.workspaceDataSourceService.getSchemaName(workspaceId); + + const query = await this.recordPositionQueryFactory.create( + value, + objectMetadataItem, + dataSourceSchema, + ); + + const records = await this.workspaceDataSourceService.executeRawQuery( + query, + [], + workspaceId, + undefined, + ); + + return ( + (value === 'first' + ? records[0]?.position / 2 + : records[0]?.position + 1) || 1 + ); + } +} diff --git a/packages/twenty-server/src/workspace/workspace-query-runner/workspace-query-runner.module.ts b/packages/twenty-server/src/workspace/workspace-query-runner/workspace-query-runner.module.ts index 0202cb4b7..c31f854c0 100644 --- a/packages/twenty-server/src/workspace/workspace-query-runner/workspace-query-runner.module.ts +++ b/packages/twenty-server/src/workspace/workspace-query-runner/workspace-query-runner.module.ts @@ -3,6 +3,7 @@ import { Module } from '@nestjs/common'; import { WorkspaceQueryBuilderModule } from 'src/workspace/workspace-query-builder/workspace-query-builder.module'; import { WorkspaceDataSourceModule } from 'src/workspace/workspace-datasource/workspace-datasource.module'; import { WorkspacePreQueryHookModule } from 'src/workspace/workspace-query-runner/workspace-pre-query-hook/workspace-pre-query-hook.module'; +import { workspaceQueryRunnerFactories } from 'src/workspace/workspace-query-runner/factories'; import { WorkspaceQueryRunnerService } from './workspace-query-runner.service'; @@ -12,7 +13,7 @@ import { WorkspaceQueryRunnerService } from './workspace-query-runner.service'; WorkspaceDataSourceModule, WorkspacePreQueryHookModule, ], - providers: [WorkspaceQueryRunnerService], + providers: [WorkspaceQueryRunnerService, ...workspaceQueryRunnerFactories], exports: [WorkspaceQueryRunnerService], }) export class WorkspaceQueryRunnerModule {} diff --git a/packages/twenty-server/src/workspace/workspace-query-runner/workspace-query-runner.service.ts b/packages/twenty-server/src/workspace/workspace-query-runner/workspace-query-runner.service.ts index 3ae9a304a..42de864a3 100644 --- a/packages/twenty-server/src/workspace/workspace-query-runner/workspace-query-runner.service.ts +++ b/packages/twenty-server/src/workspace/workspace-query-runner/workspace-query-runner.service.ts @@ -44,6 +44,7 @@ import { ObjectRecordUpdateEvent } from 'src/integrations/event-emitter/types/ob import { WorkspacePreQueryHookService } from 'src/workspace/workspace-query-runner/workspace-pre-query-hook/workspace-pre-query-hook.service'; import { EnvironmentService } from 'src/integrations/environment/environment.service'; import { NotFoundError } from 'src/filters/utils/graphql-errors.util'; +import { QueryRunnerArgsFactory } from 'src/workspace/workspace-query-runner/factories/query-runner-args.factory'; import { WorkspaceQueryRunnerOptions } from './interfaces/query-runner-option.interface'; import { @@ -59,6 +60,7 @@ export class WorkspaceQueryRunnerService { constructor( private readonly workspaceQueryBuilderFactory: WorkspaceQueryBuilderFactory, private readonly workspaceDataSourceService: WorkspaceDataSourceService, + private readonly queryRunnerArgsFactory: QueryRunnerArgsFactory, @Inject(MessageQueue.webhookQueue) private readonly messageQueueService: MessageQueueService, private readonly eventEmitter: EventEmitter2, @@ -213,11 +215,16 @@ export class WorkspaceQueryRunnerService { options: WorkspaceQueryRunnerOptions, ): Promise { const { workspaceId, objectMetadataItem } = options; - const query = await this.workspaceQueryBuilderFactory.createMany( + const computedArgs = await this.queryRunnerArgsFactory.create( args, options, ); + const query = await this.workspaceQueryBuilderFactory.createMany( + computedArgs, + options, + ); + const result = await this.execute(query, workspaceId); const parsedResults = this.parseResult>(