From b52ef76376bb06958931788b26ccb8dd506960be Mon Sep 17 00:00:00 2001 From: martmull Date: Mon, 19 May 2025 12:46:03 +0200 Subject: [PATCH] 971 rest api bug sentry on filter parameters (#12088) - fix missing createBy injection in api createOne and createMany endpoints - add a command to fix null default value for createdBySource in production entities - tested on `1747159401197/` dump extract of production db without issue --- .../0-54-created-by-default-value.command.ts | 86 +++++++++++++++++++ .../0-54-upgrade-version-command.module.ts | 11 ++- .../upgrade.command.ts | 7 +- .../handlers/rest-api-create-many.handler.ts | 13 ++- .../handlers/rest-api-create-one.handler.ts | 13 ++- .../handlers/rest-api-delete-one.handler.ts | 4 +- .../rest-api-find-duplicates.handler.ts | 12 +-- .../handlers/rest-api-find-many.handler.ts | 12 +-- .../handlers/rest-api-find-one.handler.ts | 11 +-- .../handlers/rest-api-update-one.handler.ts | 4 +- .../core/interfaces/rest-api-base.handler.ts | 10 ++- .../api/rest/core/rest-api-core.module.ts | 2 + .../created-by.create-many.pre-query-hook.ts | 61 +++---------- .../created-by.create-one.pre-query-hook.ts | 63 ++++---------- ...eated-by-from-auth-context.service.spec.ts | 81 ++++++++++++----- .../created-by-from-auth-context.service.ts | 58 ++++++++++++- .../repository/workspace.repository.ts | 2 +- ...t-api-core-create-many.integration-spec.ts | 75 ++++++++++++++++ ...st-api-core-create-one.integration-spec.ts | 53 ++++++++++++ .../rest/utils/make-rest-api-request.util.ts | 2 +- 20 files changed, 418 insertions(+), 162 deletions(-) create mode 100644 packages/twenty-server/src/database/commands/upgrade-version-command/0-54/0-54-created-by-default-value.command.ts diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/0-54/0-54-created-by-default-value.command.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/0-54/0-54-created-by-default-value.command.ts new file mode 100644 index 000000000..1ba63539c --- /dev/null +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/0-54/0-54-created-by-default-value.command.ts @@ -0,0 +1,86 @@ +import { InjectRepository } from '@nestjs/typeorm'; + +import { Command } from 'nest-commander'; +import { Repository } from 'typeorm'; +import { FieldMetadataType } from 'twenty-shared/types'; + +import { + ActiveOrSuspendedWorkspacesMigrationCommandRunner, + RunOnWorkspaceArgs, +} from 'src/database/commands/command-runners/active-or-suspended-workspaces-migration.command-runner'; +import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; +import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager'; +import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; +import { ActorMetadata } from 'src/engine/metadata-modules/field-metadata/composite-types/actor.composite-type'; +import { generateDefaultValue } from 'src/engine/metadata-modules/field-metadata/utils/generate-default-value'; +import { WorkspaceDataSourceService } from 'src/engine/workspace-datasource/workspace-datasource.service'; +import { computeTableName } from 'src/engine/utils/compute-table-name.util'; + +@Command({ + name: 'upgrade:0-54:0-54-created-by-default-value', + description: 'Fix createdBy default value', +}) +export class FixCreatedByDefaultValueCommand extends ActiveOrSuspendedWorkspacesMigrationCommandRunner { + constructor( + @InjectRepository(Workspace, 'core') + protected readonly workspaceRepository: Repository, + protected readonly twentyORMGlobalManager: TwentyORMGlobalManager, + @InjectRepository(ObjectMetadataEntity, 'metadata') + private readonly objectMetadataRepository: Repository, + private readonly workspaceDataSourceService: WorkspaceDataSourceService, + ) { + super(workspaceRepository, twentyORMGlobalManager); + } + + override async runOnWorkspace({ + workspaceId, + dataSource, + }: RunOnWorkspaceArgs): Promise { + const objectsMetadataItems = await this.objectMetadataRepository.find({ + where: { workspaceId }, + relations: ['fields'], + }); + + for (const objectMetadataItem of objectsMetadataItems) { + const createdByFieldExists = objectMetadataItem.fields.some( + (field) => field.type === FieldMetadataType.ACTOR, + ); + + if (!createdByFieldExists) { + continue; + } + + const schemaName = + this.workspaceDataSourceService.getSchemaName(workspaceId); + + const tableName = computeTableName( + objectMetadataItem.nameSingular, + objectMetadataItem.isCustom, + ); + + const actualDefaultValue = ( + await dataSource.query(` + SELECT column_default FROM information_schema.columns + WHERE table_schema = '${schemaName}' + AND table_name = '${tableName}' + AND column_name = 'createdBySource'; + `) + )?.[0]?.column_default; + + if (actualDefaultValue !== null) { + continue; + } + + const createdByDefaultValues = generateDefaultValue( + FieldMetadataType.ACTOR, + ) as ActorMetadata; + + await dataSource.query(` + ALTER TABLE "${schemaName}"."${tableName}" + ALTER COLUMN "createdBySource" SET DEFAULT ${createdByDefaultValues.source}, + ALTER COLUMN "createdByName" SET DEFAULT ${createdByDefaultValues.name}, + ALTER COLUMN "createdByContext" SET DEFAULT '${JSON.stringify(createdByDefaultValues.context)}'; + `); + } + } +} diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/0-54/0-54-upgrade-version-command.module.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/0-54/0-54-upgrade-version-command.module.ts index 5dc89fd24..d9a015e7d 100644 --- a/packages/twenty-server/src/database/commands/upgrade-version-command/0-54/0-54-upgrade-version-command.module.ts +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/0-54/0-54-upgrade-version-command.module.ts @@ -8,6 +8,7 @@ import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadat import { WorkspaceMetadataVersionModule } from 'src/engine/metadata-modules/workspace-metadata-version/workspace-metadata-version.module'; import { WorkspaceDataSourceModule } from 'src/engine/workspace-datasource/workspace-datasource.module'; import { WorkspaceMigrationRunnerModule } from 'src/engine/workspace-manager/workspace-migration-runner/workspace-migration-runner.module'; +import { FixCreatedByDefaultValueCommand } from 'src/database/commands/upgrade-version-command/0-54/0-54-created-by-default-value.command'; @Module({ imports: [ @@ -20,7 +21,13 @@ import { WorkspaceMigrationRunnerModule } from 'src/engine/workspace-manager/wor WorkspaceMigrationRunnerModule, WorkspaceMetadataVersionModule, ], - providers: [FixStandardSelectFieldsPositionCommand], - exports: [FixStandardSelectFieldsPositionCommand], + providers: [ + FixStandardSelectFieldsPositionCommand, + FixCreatedByDefaultValueCommand, + ], + exports: [ + FixStandardSelectFieldsPositionCommand, + FixCreatedByDefaultValueCommand, + ], }) export class V0_54_UpgradeVersionCommandModule {} diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts index c7f2e48e7..490d5f5bb 100644 --- a/packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts @@ -28,6 +28,7 @@ import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twent import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager'; import { SyncWorkspaceMetadataCommand } from 'src/engine/workspace-manager/workspace-sync-metadata/commands/sync-workspace-metadata.command'; +import { FixCreatedByDefaultValueCommand } from 'src/database/commands/upgrade-version-command/0-54/0-54-created-by-default-value.command'; @Command({ name: 'upgrade', @@ -70,6 +71,7 @@ export class UpgradeCommand extends UpgradeCommandRunner { // 0.54 Commands protected readonly fixStandardSelectFieldsPositionCommand: FixStandardSelectFieldsPositionCommand, + protected readonly fixCreatedByDefaultValueCommand: FixCreatedByDefaultValueCommand, ) { super( workspaceRepository, @@ -127,7 +129,10 @@ export class UpgradeCommand extends UpgradeCommandRunner { }; const commands_054: VersionCommands = { - beforeSyncMetadata: [this.fixStandardSelectFieldsPositionCommand], + beforeSyncMetadata: [ + this.fixStandardSelectFieldsPositionCommand, + this.fixCreatedByDefaultValueCommand, + ], afterSyncMetadata: [], }; diff --git a/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-create-many.handler.ts b/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-create-many.handler.ts index ef8a96a7b..d42affa12 100644 --- a/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-create-many.handler.ts +++ b/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-create-many.handler.ts @@ -8,7 +8,7 @@ import { RestApiBaseHandler } from 'src/engine/api/rest/core/interfaces/rest-api @Injectable() export class RestApiCreateManyHandler extends RestApiBaseHandler { async handle(request: Request) { - const { objectMetadataNamePlural, objectMetadata, repository } = + const { objectMetadata, repository } = await this.getRepositoryAndMetadataOrFail(request); const body = request.body; @@ -45,7 +45,14 @@ export class RestApiCreateManyHandler extends RestApiBaseHandler { overriddenRecordsToCreate.push(overriddenBody); } - const createdRecords = await repository.save(overriddenRecordsToCreate); + const recordsToCreate = + await this.createdByFromAuthContextService.injectCreatedBy( + overriddenRecordsToCreate, + objectMetadata.objectMetadataMapItem.nameSingular, + this.getAuthContextFromRequest(request), + ); + + const createdRecords = await repository.save(recordsToCreate); this.apiEventEmitterService.emitCreateEvents( createdRecords, @@ -68,7 +75,7 @@ export class RestApiCreateManyHandler extends RestApiBaseHandler { return this.formatResult({ operation: 'create', - objectNamePlural: objectMetadataNamePlural, + objectNamePlural: objectMetadata.objectMetadataMapItem.namePlural, data: records, }); } diff --git a/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-create-one.handler.ts b/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-create-one.handler.ts index 0ad672d07..7c5a48de3 100644 --- a/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-create-one.handler.ts +++ b/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-create-one.handler.ts @@ -8,7 +8,7 @@ import { RestApiBaseHandler } from 'src/engine/api/rest/core/interfaces/rest-api @Injectable() export class RestApiCreateOneHandler extends RestApiBaseHandler { async handle(request: Request) { - const { objectMetadataNameSingular, objectMetadata, repository } = + const { objectMetadata, repository } = await this.getRepositoryAndMetadataOrFail(request); const overriddenBody = await this.recordInputTransformerService.process({ @@ -28,7 +28,14 @@ export class RestApiCreateOneHandler extends RestApiBaseHandler { throw new BadRequestException('Record already exists'); } - const createdRecord = await repository.save(overriddenBody); + const [recordToCreate] = + await this.createdByFromAuthContextService.injectCreatedBy( + [overriddenBody], + objectMetadata.objectMetadataMapItem.nameSingular, + this.getAuthContextFromRequest(request), + ); + + const createdRecord = await repository.save(recordToCreate); this.apiEventEmitterService.emitCreateEvents( [createdRecord], @@ -51,7 +58,7 @@ export class RestApiCreateOneHandler extends RestApiBaseHandler { return this.formatResult({ operation: 'create', - objectNameSingular: objectMetadataNameSingular, + objectNameSingular: objectMetadata.objectMetadataMapItem.nameSingular, data: record, }); } diff --git a/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-delete-one.handler.ts b/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-delete-one.handler.ts index f5ae29e8c..f49f01079 100644 --- a/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-delete-one.handler.ts +++ b/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-delete-one.handler.ts @@ -15,7 +15,7 @@ export class RestApiDeleteOneHandler extends RestApiBaseHandler { throw new BadRequestException('Record ID not found'); } - const { objectMetadataNameSingular, objectMetadata, repository } = + const { objectMetadata, repository } = await this.getRepositoryAndMetadataOrFail(request); const recordToDelete = await repository.findOneOrFail({ where: { id: recordId }, @@ -31,7 +31,7 @@ export class RestApiDeleteOneHandler extends RestApiBaseHandler { return this.formatResult({ operation: 'delete', - objectNameSingular: objectMetadataNameSingular, + objectNameSingular: objectMetadata.objectMetadataMapItem.nameSingular, data: { id: recordToDelete.id, }, diff --git a/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-find-duplicates.handler.ts b/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-find-duplicates.handler.ts index 864f3baf4..02556f335 100644 --- a/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-find-duplicates.handler.ts +++ b/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-find-duplicates.handler.ts @@ -18,12 +18,8 @@ export class RestApiFindDuplicatesHandler extends RestApiBaseHandler { async handle(request: Request) { this.validate(request); - const { - objectMetadataNameSingular, - repository, - objectMetadata, - objectMetadataItemWithFieldsMaps, - } = await this.getRepositoryAndMetadataOrFail(request); + const { repository, objectMetadata, objectMetadataItemWithFieldsMaps } = + await this.getRepositoryAndMetadataOrFail(request); const existingRecordsQueryBuilder = repository.createQueryBuilder( objectMetadataItemWithFieldsMaps.nameSingular, @@ -69,14 +65,14 @@ export class RestApiFindDuplicatesHandler extends RestApiBaseHandler { request, repository, objectMetadata, - objectMetadataNameSingular, objectMetadataItemWithFieldsMaps, extraFilters: duplicateCondition, }); const paginatedResult = this.formatPaginatedDuplicatesResult({ finalRecords: records, - objectMetadataNameSingular, + objectMetadataNameSingular: + objectMetadata.objectMetadataMapItem.nameSingular, isForwardPagination, hasMoreRecords, totalCount, diff --git a/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-find-many.handler.ts b/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-find-many.handler.ts index 2cb5acf96..d6e604eea 100644 --- a/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-find-many.handler.ts +++ b/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-find-many.handler.ts @@ -7,13 +7,8 @@ import { RestApiBaseHandler } from 'src/engine/api/rest/core/interfaces/rest-api @Injectable() export class RestApiFindManyHandler extends RestApiBaseHandler { async handle(request: Request) { - const { - objectMetadataNameSingular, - objectMetadataNamePlural, - repository, - objectMetadata, - objectMetadataItemWithFieldsMaps, - } = await this.getRepositoryAndMetadataOrFail(request); + const { repository, objectMetadata, objectMetadataItemWithFieldsMaps } = + await this.getRepositoryAndMetadataOrFail(request); const { records, @@ -26,13 +21,12 @@ export class RestApiFindManyHandler extends RestApiBaseHandler { request, repository, objectMetadata, - objectMetadataNameSingular, objectMetadataItemWithFieldsMaps, }); return this.formatPaginatedResult({ finalRecords: records, - objectMetadataNamePlural, + objectMetadataNamePlural: objectMetadata.objectMetadataMapItem.namePlural, isForwardPagination, hasMoreRecords, totalCount, diff --git a/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-find-one.handler.ts b/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-find-one.handler.ts index 0caaf86ab..ecceeab8d 100644 --- a/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-find-one.handler.ts +++ b/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-find-one.handler.ts @@ -18,19 +18,14 @@ export class RestApiFindOneHandler extends RestApiBaseHandler { ); } - const { - objectMetadataNameSingular, - repository, - objectMetadata, - objectMetadataItemWithFieldsMaps, - } = await this.getRepositoryAndMetadataOrFail(request); + const { repository, objectMetadata, objectMetadataItemWithFieldsMaps } = + await this.getRepositoryAndMetadataOrFail(request); const { records } = await this.findRecords({ request, recordId, repository, objectMetadata, - objectMetadataNameSingular, objectMetadataItemWithFieldsMaps, }); @@ -42,7 +37,7 @@ export class RestApiFindOneHandler extends RestApiBaseHandler { return this.formatResult({ operation: 'findOne', - objectNameSingular: objectMetadataNameSingular, + objectNameSingular: objectMetadata.objectMetadataMapItem.nameSingular, data: record, }); } diff --git a/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-update-one.handler.ts b/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-update-one.handler.ts index 5bc0d2a6d..b55e7d7b0 100644 --- a/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-update-one.handler.ts +++ b/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-update-one.handler.ts @@ -16,7 +16,7 @@ export class RestApiUpdateOneHandler extends RestApiBaseHandler { throw new BadRequestException('Record ID not found'); } - const { objectMetadataNameSingular, objectMetadata, repository } = + const { objectMetadata, repository } = await this.getRepositoryAndMetadataOrFail(request); const recordToUpdate = await repository.findOneOrFail({ @@ -56,7 +56,7 @@ export class RestApiUpdateOneHandler extends RestApiBaseHandler { return this.formatResult({ operation: 'update', - objectNameSingular: objectMetadataNameSingular, + objectNameSingular: objectMetadata.objectMetadataMapItem.nameSingular, data: record, }); } diff --git a/packages/twenty-server/src/engine/api/rest/core/interfaces/rest-api-base.handler.ts b/packages/twenty-server/src/engine/api/rest/core/interfaces/rest-api-base.handler.ts index f4fd15bf8..6f1ba1be5 100644 --- a/packages/twenty-server/src/engine/api/rest/core/interfaces/rest-api-base.handler.ts +++ b/packages/twenty-server/src/engine/api/rest/core/interfaces/rest-api-base.handler.ts @@ -32,6 +32,7 @@ import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global. import { formatResult as formatGetManyData } from 'src/engine/twenty-orm/utils/format-result.util'; import { encodeCursor } from 'src/engine/api/graphql/graphql-query-runner/utils/cursors.util'; import { computeCursorArgFilter } from 'src/engine/api/utils/compute-cursor-arg-filter.utils'; +import { CreatedByFromAuthContextService } from 'src/engine/core-modules/actor/services/created-by-from-auth-context.service'; export interface PageInfo { hasNextPage?: boolean; @@ -78,6 +79,8 @@ export abstract class RestApiBaseHandler { protected readonly workspacePermissionsCacheService: WorkspacePermissionsCacheService; @Inject() protected readonly apiEventEmitterService: ApiEventEmitterService; + @Inject() + protected readonly createdByFromAuthContextService: CreatedByFromAuthContextService; protected abstract handle( request: Request, @@ -136,8 +139,6 @@ export abstract class RestApiBaseHandler { ); return { - objectMetadataNameSingular, - objectMetadataNamePlural: objectMetadata.objectMetadataMapItem.namePlural, objectMetadata, repository, dataSource, @@ -279,7 +280,6 @@ export abstract class RestApiBaseHandler { recordId, repository, objectMetadata, - objectMetadataNameSingular, objectMetadataItemWithFieldsMaps, extraFilters, }: { @@ -290,12 +290,14 @@ export abstract class RestApiBaseHandler { objectMetadataMaps: ObjectMetadataMaps; objectMetadataMapItem: ObjectMetadataItemWithFieldMaps; }; - objectMetadataNameSingular: string; objectMetadataItemWithFieldsMaps: | ObjectMetadataItemWithFieldMaps | undefined; extraFilters?: Partial; }) { + const objectMetadataNameSingular = + objectMetadata.objectMetadataMapItem.nameSingular; + const qb = repository.createQueryBuilder(objectMetadataNameSingular); const inputs = this.getVariablesFactory.create( diff --git a/packages/twenty-server/src/engine/api/rest/core/rest-api-core.module.ts b/packages/twenty-server/src/engine/api/rest/core/rest-api-core.module.ts index 206bdccf3..c2950dad4 100644 --- a/packages/twenty-server/src/engine/api/rest/core/rest-api-core.module.ts +++ b/packages/twenty-server/src/engine/api/rest/core/rest-api-core.module.ts @@ -19,6 +19,7 @@ import { ApiEventEmitterService } from 'src/engine/api/graphql/graphql-query-run import { AuthModule } from 'src/engine/core-modules/auth/auth.module'; import { RestApiCreateManyHandler } from 'src/engine/api/rest/core/handlers/rest-api-create-many.handler'; import { RestApiFindDuplicatesHandler } from 'src/engine/api/rest/core/handlers/rest-api-find-duplicates.handler'; +import { ActorModule } from 'src/engine/core-modules/actor/actor.module'; const restApiCoreResolvers = [ RestApiDeleteOneHandler, @@ -39,6 +40,7 @@ const restApiCoreResolvers = [ TwentyORMModule, RecordTransformerModule, WorkspacePermissionsCacheModule, + ActorModule, ], controllers: [RestApiCoreController], providers: [ diff --git a/packages/twenty-server/src/engine/core-modules/actor/query-hooks/created-by.create-many.pre-query-hook.ts b/packages/twenty-server/src/engine/core-modules/actor/query-hooks/created-by.create-many.pre-query-hook.ts index 7ee488c60..156287fd9 100644 --- a/packages/twenty-server/src/engine/core-modules/actor/query-hooks/created-by.create-many.pre-query-hook.ts +++ b/packages/twenty-server/src/engine/core-modules/actor/query-hooks/created-by.create-many.pre-query-hook.ts @@ -1,7 +1,4 @@ -import { InjectRepository } from '@nestjs/typeorm'; - import { isDefined } from 'twenty-shared/utils'; -import { Repository } from 'typeorm'; import { WorkspacePreQueryHookInstance } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/interfaces/workspace-query-hook.interface'; import { CreateManyResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; @@ -11,34 +8,25 @@ import { GraphqlQueryRunnerExceptionCode, } from 'src/engine/api/graphql/graphql-query-runner/errors/graphql-query-runner.exception'; import { WorkspaceQueryHook } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/decorators/workspace-query-hook.decorator'; -import { CreatedByFromAuthContextService } from 'src/engine/core-modules/actor/services/created-by-from-auth-context.service'; +import { + CreatedByFromAuthContextService, + CreateInput, +} from 'src/engine/core-modules/actor/services/created-by-from-auth-context.service'; import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; -import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; -import { CustomWorkspaceEntity } from 'src/engine/twenty-orm/custom.workspace-entity'; - -type CustomWorkspaceItem = Omit< - CustomWorkspaceEntity, - 'createdAt' | 'updatedAt' -> & { - createdAt: string; - updatedAt: string; -}; @WorkspaceQueryHook(`*.createMany`) export class CreatedByCreateManyPreQueryHook implements WorkspacePreQueryHookInstance { constructor( - @InjectRepository(FieldMetadataEntity, 'metadata') - private readonly fieldMetadataRepository: Repository, private readonly createdByFromAuthContextService: CreatedByFromAuthContextService, ) {} async execute( authContext: AuthContext, objectName: string, - payload: CreateManyResolverArgs, - ): Promise> { + payload: CreateManyResolverArgs, + ): Promise> { if (!isDefined(payload.data)) { throw new GraphqlQueryRunnerException( 'Payload data is required', @@ -46,34 +34,13 @@ export class CreatedByCreateManyPreQueryHook ); } - // TODO: Once all objects have it, we can remove this check - const createdByFieldMetadata = await this.fieldMetadataRepository.findOne({ - where: { - object: { - nameSingular: objectName, - }, - name: 'createdBy', - workspaceId: authContext.workspace.id, - }, - }); - - if (!createdByFieldMetadata) { - return payload; - } - - const createdBy = - await this.createdByFromAuthContextService.buildCreatedBy(authContext); - - for (const datum of payload.data) { - // Front-end can fill the source field - if (createdBy && (!datum.createdBy || !datum.createdBy?.name)) { - datum.createdBy = { - ...createdBy, - source: datum.createdBy?.source ?? createdBy.source, - }; - } - } - - return payload; + return { + ...payload, + data: await this.createdByFromAuthContextService.injectCreatedBy( + payload.data, + objectName, + authContext, + ), + }; } } diff --git a/packages/twenty-server/src/engine/core-modules/actor/query-hooks/created-by.create-one.pre-query-hook.ts b/packages/twenty-server/src/engine/core-modules/actor/query-hooks/created-by.create-one.pre-query-hook.ts index 0441c6d62..92be164b6 100644 --- a/packages/twenty-server/src/engine/core-modules/actor/query-hooks/created-by.create-one.pre-query-hook.ts +++ b/packages/twenty-server/src/engine/core-modules/actor/query-hooks/created-by.create-one.pre-query-hook.ts @@ -1,7 +1,4 @@ -import { InjectRepository } from '@nestjs/typeorm'; - import { isDefined } from 'class-validator'; -import { Repository } from 'typeorm'; import { WorkspacePreQueryHookInstance } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/interfaces/workspace-query-hook.interface'; import { CreateOneResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; @@ -11,34 +8,25 @@ import { GraphqlQueryRunnerExceptionCode, } from 'src/engine/api/graphql/graphql-query-runner/errors/graphql-query-runner.exception'; import { WorkspaceQueryHook } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/decorators/workspace-query-hook.decorator'; -import { CreatedByFromAuthContextService } from 'src/engine/core-modules/actor/services/created-by-from-auth-context.service'; +import { + CreatedByFromAuthContextService, + CreateInput, +} from 'src/engine/core-modules/actor/services/created-by-from-auth-context.service'; import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; -import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; -import { CustomWorkspaceEntity } from 'src/engine/twenty-orm/custom.workspace-entity'; - -type CustomWorkspaceItem = Omit< - CustomWorkspaceEntity, - 'createdAt' | 'updatedAt' -> & { - createdAt: string; - updatedAt: string; -}; @WorkspaceQueryHook(`*.createOne`) export class CreatedByCreateOnePreQueryHook implements WorkspacePreQueryHookInstance { constructor( - @InjectRepository(FieldMetadataEntity, 'metadata') - private readonly fieldMetadataRepository: Repository, private readonly createdByFromAuthContextService: CreatedByFromAuthContextService, ) {} async execute( authContext: AuthContext, objectName: string, - payload: CreateOneResolverArgs, - ): Promise> { + payload: CreateOneResolverArgs, + ): Promise> { if (!isDefined(payload.data)) { throw new GraphqlQueryRunnerException( 'Payload data is required', @@ -46,35 +34,16 @@ export class CreatedByCreateOnePreQueryHook ); } - // TODO: Once all objects have it, we can remove this check - const createdByFieldMetadata = await this.fieldMetadataRepository.findOne({ - where: { - object: { - nameSingular: objectName, - }, - name: 'createdBy', - workspaceId: authContext.workspace.id, - }, - }); + const [recordToCreateData] = + await this.createdByFromAuthContextService.injectCreatedBy( + [payload.data], + objectName, + authContext, + ); - if (!createdByFieldMetadata) { - return payload; - } - - const createdBy = - await this.createdByFromAuthContextService.buildCreatedBy(authContext); - - // Front-end can fill the source field - if ( - createdBy && - (!payload.data.createdBy || !payload.data.createdBy?.name) - ) { - payload.data.createdBy = { - ...createdBy, - source: payload.data.createdBy?.source ?? createdBy.source, - }; - } - - return payload; + return { + ...payload, + data: recordToCreateData, + }; } } diff --git a/packages/twenty-server/src/engine/core-modules/actor/services/__tests__/created-by-from-auth-context.service.spec.ts b/packages/twenty-server/src/engine/core-modules/actor/services/__tests__/created-by-from-auth-context.service.spec.ts index e60a59227..9ae6f4015 100644 --- a/packages/twenty-server/src/engine/core-modules/actor/services/__tests__/created-by-from-auth-context.service.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/actor/services/__tests__/created-by-from-auth-context.service.spec.ts @@ -1,4 +1,5 @@ import { Test, TestingModule } from '@nestjs/testing'; +import { getRepositoryToken } from '@nestjs/typeorm'; import { CreatedByFromAuthContextService } from 'src/engine/core-modules/actor/services/created-by-from-auth-context.service'; import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; @@ -12,12 +13,16 @@ import { FullNameMetadata } from 'src/engine/metadata-modules/field-metadata/com import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager'; import { ApiKeyWorkspaceEntity } from 'src/modules/api-key/standard-objects/api-key.workspace-entity'; import { WorkspaceMemberWorkspaceEntity } from 'src/modules/workspace-member/standard-objects/workspace-member.workspace-entity'; +import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; type TestingAuthContext = Omit & { workspace: Partial; apiKey?: Partial; user?: Partial; }; + +type ExpectedResult = { createdBy: ActorMetadata }[]; + // TODO create util const fromFullNameMetadataToName = ({ firstName, @@ -45,6 +50,12 @@ describe('CreatedByFromAuthContextService', () => { provide: TwentyORMGlobalManager, useValue: twentyORMGlobalManager, }, + { + provide: getRepositoryToken(FieldMetadataEntity, 'metadata'), + useValue: { + findOne: jest.fn().mockResolvedValue(true), + }, + }, ], }).compile(); @@ -60,7 +71,7 @@ describe('CreatedByFromAuthContextService', () => { it('should be defined', () => { expect(service).toBeDefined(); }); - describe('buildCreatedBy', () => { + describe('injectCreatedBy', () => { it('should build metadata from workspaceMemberId and user when both are present', async () => { const authContext = { workspaceMemberId: '20202020-0b5c-4178-bed7-d371f6411eaa', @@ -72,14 +83,22 @@ describe('CreatedByFromAuthContextService', () => { workspace: { id: '20202020-bdec-497f-847a-1bb334fefe58' }, } as const satisfies TestingAuthContext; - const result = await service.buildCreatedBy(authContext as AuthContext); + const result = await service.injectCreatedBy( + [{}], + 'person', + authContext as AuthContext, + ); - expect(result).toEqual({ - context: {}, - name: fromFullNameMetadataToName(authContext.user), - workspaceMemberId: authContext.workspaceMemberId, - source: FieldActorSource.MANUAL, - }); + expect(result).toEqual([ + { + createdBy: { + context: {}, + name: fromFullNameMetadataToName(authContext.user), + workspaceMemberId: authContext.workspaceMemberId, + source: FieldActorSource.MANUAL, + }, + }, + ]); }); it('should build metadata from user when workspaceMemberId is missing', async () => { @@ -104,14 +123,22 @@ describe('CreatedByFromAuthContextService', () => { mockedWorkspaceMember, ); - const result = await service.buildCreatedBy(authContext as AuthContext); + const result = await service.injectCreatedBy( + [{}], + 'person', + authContext as AuthContext, + ); - expect(result).toEqual({ - context: {}, - name: fromFullNameMetadataToName(mockedWorkspaceMember.name), - workspaceMemberId: mockedWorkspaceMember.id, - source: FieldActorSource.MANUAL, - }); + expect(result).toEqual([ + { + createdBy: { + context: {}, + name: fromFullNameMetadataToName(mockedWorkspaceMember.name), + workspaceMemberId: mockedWorkspaceMember.id, + source: FieldActorSource.MANUAL, + }, + }, + ]); }); it('should build metadata from apiKey when only apiKey is present', async () => { @@ -123,14 +150,22 @@ describe('CreatedByFromAuthContextService', () => { workspace: { id: '20202020-bdec-497f-847a-1bb334fefe58' }, } as const satisfies TestingAuthContext; - const result = await service.buildCreatedBy(authContext as AuthContext); + const result = await service.injectCreatedBy( + [{}], + 'person', + authContext as AuthContext, + ); - expect(result).toEqual({ - source: FieldActorSource.API, - workspaceMemberId: null, - name: authContext.apiKey.name, - context: {}, - }); + expect(result).toEqual([ + { + createdBy: { + source: FieldActorSource.API, + workspaceMemberId: null, + name: authContext.apiKey.name, + context: {}, + }, + }, + ]); }); it('should throw error when no valid actor information is found', async () => { @@ -139,7 +174,7 @@ describe('CreatedByFromAuthContextService', () => { } as const satisfies TestingAuthContext; await expect( - service.buildCreatedBy(authContext as AuthContext), + service.injectCreatedBy([{}], 'person', authContext as AuthContext), ).rejects.toThrowErrorMatchingInlineSnapshot( `"Unable to build createdBy metadata - no valid actor information found in auth context"`, ); diff --git a/packages/twenty-server/src/engine/core-modules/actor/services/created-by-from-auth-context.service.ts b/packages/twenty-server/src/engine/core-modules/actor/services/created-by-from-auth-context.service.ts index d32bbb508..70ea218e6 100644 --- a/packages/twenty-server/src/engine/core-modules/actor/services/created-by-from-auth-context.service.ts +++ b/packages/twenty-server/src/engine/core-modules/actor/services/created-by-from-auth-context.service.ts @@ -1,6 +1,8 @@ import { Injectable, Logger } from '@nestjs/common'; +import { InjectRepository } from '@nestjs/typeorm'; import { isDefined } from 'twenty-shared/utils'; +import { Repository } from 'typeorm'; import { buildCreatedByFromApiKey } from 'src/engine/core-modules/actor/utils/build-created-by-from-api-key.util'; import { buildCreatedByFromFullNameMetadata } from 'src/engine/core-modules/actor/utils/build-created-by-from-full-name-metadata.util'; @@ -8,16 +10,70 @@ import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.typ import { ActorMetadata } from 'src/engine/metadata-modules/field-metadata/composite-types/actor.composite-type'; import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager'; import { WorkspaceMemberWorkspaceEntity } from 'src/modules/workspace-member/standard-objects/workspace-member.workspace-entity'; +import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export type CreateInput = Record; @Injectable() export class CreatedByFromAuthContextService { private readonly logger = new Logger(CreatedByFromAuthContextService.name); constructor( + @InjectRepository(FieldMetadataEntity, 'metadata') + private readonly fieldMetadataRepository: Repository, private readonly twentyORMGlobalManager: TwentyORMGlobalManager, ) {} - public async buildCreatedBy( + async injectCreatedBy( + records: CreateInput[], + objectMetadataNameSingular: string, + authContext: AuthContext, + ): Promise { + // TODO: Once all objects have it, we can remove this check + const createdByFieldMetadata = await this.fieldMetadataRepository.findOne({ + where: { + object: { + nameSingular: objectMetadataNameSingular, + }, + name: 'createdBy', + workspaceId: authContext.workspace.id, + }, + }); + + if (!createdByFieldMetadata) { + return records; + } + + const clonedRecords = structuredClone(records); + + const createdBy = await this.buildCreatedBy(authContext); + + if (Array.isArray(clonedRecords)) { + for (const datum of clonedRecords) { + this.injectCreatedByToRecord(createdBy, datum); + } + } else { + this.injectCreatedByToRecord(createdBy, clonedRecords); + } + + return clonedRecords; + } + + private injectCreatedByToRecord( + createdBy: ActorMetadata, + record: CreateInput, + ) { + // Front-end can fill the source field + if (createdBy && (!record.createdBy || !record.createdBy?.name)) { + record.createdBy = { + ...createdBy, + source: record.createdBy?.source ?? createdBy.source, + }; + } + } + + private async buildCreatedBy( authContext: AuthContext, ): Promise { const { workspace, workspaceMemberId, user, apiKey } = authContext; diff --git a/packages/twenty-server/src/engine/twenty-orm/repository/workspace.repository.ts b/packages/twenty-server/src/engine/twenty-orm/repository/workspace.repository.ts index 40c417ccc..e60a05889 100644 --- a/packages/twenty-server/src/engine/twenty-orm/repository/workspace.repository.ts +++ b/packages/twenty-server/src/engine/twenty-orm/repository/workspace.repository.ts @@ -226,7 +226,7 @@ export class WorkspaceRepository< const formattedEntityOrEntities = await this.formatData(entityOrEntities); let result: U | U[]; - // Needed becasuse save method has multiple signature, otherwise we will need to do a type assertion + // Needed because save method has multiple signature, otherwise we will need to do a type assertion if (Array.isArray(formattedEntityOrEntities)) { result = await manager.save( this.target, diff --git a/packages/twenty-server/test/integration/rest/suites/rest-api-core-create-many.integration-spec.ts b/packages/twenty-server/test/integration/rest/suites/rest-api-core-create-many.integration-spec.ts index 80bf970fa..0974e7e13 100644 --- a/packages/twenty-server/test/integration/rest/suites/rest-api-core-create-many.integration-spec.ts +++ b/packages/twenty-server/test/integration/rest/suites/rest-api-core-create-many.integration-spec.ts @@ -6,6 +6,9 @@ import { makeRestAPIRequest } from 'test/integration/rest/utils/make-rest-api-re import { deleteAllRecords } from 'test/integration/utils/delete-all-records'; import { TEST_COMPANY_1_ID } from 'test/integration/constants/test-company-ids.constants'; import { TEST_PRIMARY_LINK_URL } from 'test/integration/constants/test-primary-link-url.constant'; +import { TIM_ACCOUNT_ID } from 'test/integration/graphql/integration.constants'; + +import { FieldActorSource } from 'src/engine/metadata-modules/field-metadata/composite-types/actor.composite-type'; describe('Core REST API Create Many endpoint', () => { beforeEach(async () => { @@ -43,7 +46,79 @@ describe('Core REST API Create Many endpoint', () => { expect(createdPeople.length).toBe(2); expect(createdPeople[0].id).toBe(TEST_PERSON_1_ID); + expect(createdPeople[0].createdBy.source).toBe(FieldActorSource.API); + expect(createdPeople[0].createdBy.workspaceMemberId).toBe(null); + expect(createdPeople[1].id).toBe(TEST_PERSON_2_ID); + expect(createdPeople[1].createdBy.source).toBe(FieldActorSource.API); + expect(createdPeople[1].createdBy.workspaceMemberId).toBe(null); + }); + }); + + it('should create a new person with specific createdBy', async () => { + const requestBody = [ + { + id: TEST_PERSON_1_ID, + createdBy: { + source: FieldActorSource.EMAIL, + }, + }, + { + id: TEST_PERSON_2_ID, + createdBy: { + source: FieldActorSource.MANUAL, + }, + }, + ]; + + await makeRestAPIRequest({ + method: 'post', + path: `/batch/people`, + body: requestBody, + }) + .expect(201) + .expect((res) => { + const createdPeople = res.body.data.createPeople; + + expect(createdPeople[0].createdBy.source).toBe(FieldActorSource.EMAIL); + expect(createdPeople[0].createdBy.workspaceMemberId).toBe(null); + + expect(createdPeople[1].createdBy.source).toBe(FieldActorSource.MANUAL); + expect(createdPeople[1].createdBy.workspaceMemberId).toBe(null); + }); + }); + + it('should create many person with MANUAL createdBy if user identified', async () => { + const requestBody = [ + { + id: TEST_PERSON_1_ID, + }, + { + id: TEST_PERSON_2_ID, + }, + ]; + + await makeRestAPIRequest({ + method: 'post', + path: `/batch/people`, + body: requestBody, + bearer: ADMIN_ACCESS_TOKEN, + }) + .expect(201) + .expect((res) => { + const createdPeople = res.body.data.createPeople; + + expect(createdPeople.length).toBe(2); + + expect(createdPeople[0].createdBy.source).toBe(FieldActorSource.MANUAL); + expect(createdPeople[0].createdBy.workspaceMemberId).toBe( + TIM_ACCOUNT_ID, + ); + + expect(createdPeople[1].createdBy.source).toBe(FieldActorSource.MANUAL); + expect(createdPeople[1].createdBy.workspaceMemberId).toBe( + TIM_ACCOUNT_ID, + ); }); }); diff --git a/packages/twenty-server/test/integration/rest/suites/rest-api-core-create-one.integration-spec.ts b/packages/twenty-server/test/integration/rest/suites/rest-api-core-create-one.integration-spec.ts index 7e1488c25..84adc80f1 100644 --- a/packages/twenty-server/test/integration/rest/suites/rest-api-core-create-one.integration-spec.ts +++ b/packages/twenty-server/test/integration/rest/suites/rest-api-core-create-one.integration-spec.ts @@ -4,6 +4,9 @@ import { TEST_PRIMARY_LINK_URL } from 'test/integration/constants/test-primary-l import { makeRestAPIRequest } from 'test/integration/rest/utils/make-rest-api-request.util'; import { deleteAllRecords } from 'test/integration/utils/delete-all-records'; import { generateRecordName } from 'test/integration/utils/generate-record-name'; +import { TIM_ACCOUNT_ID } from 'test/integration/graphql/integration.constants'; + +import { FieldActorSource } from 'src/engine/metadata-modules/field-metadata/composite-types/actor.composite-type'; describe('Core REST API Create One endpoint', () => { beforeEach(async () => { @@ -39,6 +42,56 @@ describe('Core REST API Create One endpoint', () => { expect(createdPerson.id).toBe(TEST_PERSON_1_ID); expect(createdPerson.city).toBe(personCity); + expect(createdPerson.createdBy.source).toBe(FieldActorSource.API); + expect(createdPerson.createdBy.workspaceMemberId).toBe(null); + }); + }); + + it('should create a new person with specific createdBy', async () => { + const personCity = generateRecordName(TEST_PERSON_1_ID); + const requestBody = { + id: TEST_PERSON_1_ID, + city: personCity, + companyId: TEST_COMPANY_1_ID, + createdBy: { + source: FieldActorSource.EMAIL, + }, + }; + + await makeRestAPIRequest({ + method: 'post', + path: `/people`, + body: requestBody, + }) + .expect(201) + .expect((res) => { + const createdPerson = res.body.data.createPerson; + + expect(createdPerson.createdBy.source).toBe(FieldActorSource.EMAIL); + expect(createdPerson.createdBy.workspaceMemberId).toBe(null); + }); + }); + + it('should create a new person with MANUAL createdBy if user identified', async () => { + const personCity = generateRecordName(TEST_PERSON_1_ID); + const requestBody = { + id: TEST_PERSON_1_ID, + city: personCity, + companyId: TEST_COMPANY_1_ID, + }; + + await makeRestAPIRequest({ + method: 'post', + path: `/people`, + body: requestBody, + bearer: ADMIN_ACCESS_TOKEN, + }) + .expect(201) + .expect((res) => { + const createdPerson = res.body.data.createPerson; + + expect(createdPerson.createdBy.source).toBe(FieldActorSource.MANUAL); + expect(createdPerson.createdBy.workspaceMemberId).toBe(TIM_ACCOUNT_ID); }); }); diff --git a/packages/twenty-server/test/integration/rest/utils/make-rest-api-request.util.ts b/packages/twenty-server/test/integration/rest/utils/make-rest-api-request.util.ts index 5522adbe7..91d776998 100644 --- a/packages/twenty-server/test/integration/rest/utils/make-rest-api-request.util.ts +++ b/packages/twenty-server/test/integration/rest/utils/make-rest-api-request.util.ts @@ -12,7 +12,7 @@ interface RestAPIRequestParams { export const makeRestAPIRequest = ({ method, path, - bearer = ADMIN_ACCESS_TOKEN, + bearer = API_KEY_ACCESS_TOKEN, body = {}, }: RestAPIRequestParams) => { const client = request(`http://localhost:${APP_PORT}`);