From 968ad3bd31e409b581e5ce9a5589faca215b149e Mon Sep 17 00:00:00 2001 From: Paul Rastoin <45004772+prastoin@users.noreply.github.com> Date: Fri, 14 Feb 2025 15:43:09 +0100 Subject: [PATCH] [NITPICK] From args list to record args `RecordPositionFactory` (#10215) # Introduction Just some nitpicking while debugging a bug that wasn't one --- .../query-runner-args.factory.spec.ts | 29 ++++++++++++------- .../__tests__/record-position.factory.spec.ts | 18 ++++++++++-- .../factories/query-runner-args.factory.ts | 23 ++++++++------- .../factories/record-position.factory.ts | 16 ++++++---- .../record-position-backfill-service.ts | 10 +++---- 5 files changed, 61 insertions(+), 35 deletions(-) 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 fe6e8a44b..791d09696 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 @@ -6,7 +6,10 @@ import { WorkspaceQueryRunnerOptions } from 'src/engine/api/graphql/workspace-qu import { ResolverArgsType } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; import { QueryRunnerArgsFactory } from 'src/engine/api/graphql/workspace-query-runner/factories/query-runner-args.factory'; -import { RecordPositionFactory } from 'src/engine/api/graphql/workspace-query-runner/factories/record-position.factory'; +import { + RecordPositionFactory, + RecordPositionFactoryCreateArgs, +} from 'src/engine/api/graphql/workspace-query-runner/factories/record-position.factory'; import { FieldMetadataMap } from 'src/engine/metadata-modules/types/field-metadata-map'; describe('QueryRunnerArgsFactory', () => { @@ -104,12 +107,14 @@ describe('QueryRunnerArgsFactory', () => { ResolverArgsType.CreateMany, ); - expect(recordPositionFactory.create).toHaveBeenCalledWith( - 'last', - { isCustom: true, nameSingular: 'testNumber' }, + const expectedArgs: RecordPositionFactoryCreateArgs = { + value: 'last', + objectMetadata: { isCustom: true, nameSingular: 'testNumber' }, workspaceId, - 0, - ); + index: 0, + }; + + expect(recordPositionFactory.create).toHaveBeenCalledWith(expectedArgs); expect(result).toEqual({ id: 'uuid', data: [{ position: 2, testNumber: 1 }], @@ -128,12 +133,14 @@ describe('QueryRunnerArgsFactory', () => { ResolverArgsType.CreateMany, ); - expect(recordPositionFactory.create).toHaveBeenCalledWith( - 'first', - { isCustom: true, nameSingular: 'testNumber' }, + const expectedArgs: RecordPositionFactoryCreateArgs = { + value: 'first', + objectMetadata: { isCustom: true, nameSingular: 'testNumber' }, workspaceId, - 0, - ); + index: 0, + }; + + expect(recordPositionFactory.create).toHaveBeenCalledWith(expectedArgs); 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 88d8e4204..a6728b8fb 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 @@ -46,21 +46,33 @@ describe('RecordPositionFactory', () => { it('should return the value when value is a number', async () => { const value = 1; - const result = await factory.create(value, objectMetadata, workspaceId); + const result = await factory.create({ + value, + objectMetadata, + workspaceId, + }); expect(result).toEqual(value); }); it('should return the existing position -1 when value is first', async () => { const value = 'first'; - const result = await factory.create(value, objectMetadata, workspaceId); + const result = await factory.create({ + value, + objectMetadata, + workspaceId, + }); expect(result).toEqual(0); }); it('should return the existing position + 1 when value is last', async () => { const value = 'last'; - const result = await factory.create(value, objectMetadata, workspaceId); + const result = await factory.create({ + value, + objectMetadata, + workspaceId, + }); expect(result).toEqual(2); }); 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 2d3ff76f0..62edab3ce 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 @@ -177,6 +177,7 @@ export class QueryRunnerArgsFactory { return Promise.resolve({}); } + const workspaceId = options.authContext.workspace.id; let isFieldPositionPresent = false; const createArgByArgKeyPromises: Promise<[string, any]>[] = Object.entries( @@ -192,16 +193,16 @@ export class QueryRunnerArgsFactory { case FieldMetadataType.POSITION: { isFieldPositionPresent = true; - const newValue = await this.recordPositionFactory.create( + const newValue = await this.recordPositionFactory.create({ value, - { + workspaceId, + objectMetadata: { isCustom: options.objectMetadataItemWithFieldMaps.isCustom, nameSingular: options.objectMetadataItemWithFieldMaps.nameSingular, }, - options.authContext.workspace.id, - argPositionBackfillInput.argIndex, - ); + index: argPositionBackfillInput.argIndex, + }); return [key, newValue]; } @@ -248,16 +249,16 @@ export class QueryRunnerArgsFactory { ...newArgEntries, [ 'position', - await this.recordPositionFactory.create( - 'first', - { + await this.recordPositionFactory.create({ + value: 'first', + workspaceId, + objectMetadata: { isCustom: options.objectMetadataItemWithFieldMaps.isCustom, nameSingular: options.objectMetadataItemWithFieldMaps.nameSingular, }, - options.authContext.workspace.id, - argPositionBackfillInput.argIndex, - ), + index: argPositionBackfillInput.argIndex, + }), ], ]); } 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 5c68022c2..b3ae41d79 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 @@ -9,6 +9,12 @@ import { } from 'src/engine/api/graphql/workspace-query-builder/factories/record-position-query.factory'; import { WorkspaceDataSourceService } from 'src/engine/workspace-datasource/workspace-datasource.service'; +export type RecordPositionFactoryCreateArgs = { + value: number | 'first' | 'last'; + objectMetadata: { isCustom: boolean; nameSingular: string }; + workspaceId: string; + index?: number; +}; @Injectable() export class RecordPositionFactory { constructor( @@ -16,12 +22,12 @@ export class RecordPositionFactory { private readonly recordPositionQueryFactory: RecordPositionQueryFactory, ) {} - async create( - value: number | 'first' | 'last', - objectMetadata: { isCustom: boolean; nameSingular: string }, - workspaceId: string, + async create({ + objectMetadata, + value, + workspaceId, index = 0, - ): Promise { + }: RecordPositionFactoryCreateArgs): Promise { const dataSourceSchema = this.workspaceDataSourceService.getSchemaName(workspaceId); 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 fbb183e45..e0aef2b25 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 @@ -2,8 +2,8 @@ import { Injectable, Logger } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; import { isDefined } from 'class-validator'; -import { Repository } from 'typeorm'; import { FieldMetadataType } from 'twenty-shared'; +import { Repository } from 'typeorm'; import { RecordPositionQueryFactory, @@ -73,14 +73,14 @@ export class RecordPositionBackfillService { continue; } - const position = await this.recordPositionFactory.create( - 'last', - { + const position = await this.recordPositionFactory.create({ + objectMetadata: { isCustom: objectMetadata.isCustom, nameSingular: objectMetadata.nameSingular, }, + value: 'last', workspaceId, - ); + }); for ( let recordIndex = 0;