From 95eba07f6ed5b83f8f84a4a4cd5c663d6c2e7448 Mon Sep 17 00:00:00 2001 From: eliasylonen Date: Tue, 8 Apr 2025 00:30:53 +0300 Subject: [PATCH] Share RICH_TEXT_V2 field value override between REST and GraphQL APIs (#10912) Fixes issue #10606. This PR makes `RICH_TEXT_V2` field behavior in REST API matche the current behavior in GraphQL API: Currently both `markdown` and `blocknote` fields must be included in the request, one of them can be `null`. The field with a `null` value will be filled by the converted value of the other field. In other words, this works: ``` curl http://localhost:3000/rest/notes \ --request POST \ --header 'Content-Type: application/json' \ --header 'Authorization: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIyMDIwMjAyMC0xYzI1LTRkMDItYmYyNS02YWVjY2Y3ZWE0MTkiLCJ0eXBlIjoiQVBJX0tFWSIsIndvcmtzcGFjZUlkIjoiMjAyMDIwMjAtMWMyNS00ZDAyLWJmMjUtNmFlY2NmN2VhNDE5IiwiaWF0IjoxNzQxODA1MzQyLCJleHAiOjQ4OTU0MDUzNDEsImp0aSI6ImZlMzU0NTBkLTlhMDMtNGE2ZS04ODVjLTBlNTU3M2Y3YTE0NiJ9.6_g8cwoSE7ZCX1Zzsw44gZIyBdLKNsnDqMOmm1bKik0' \ --data '{ "position": 1, "title": "a", "bodyV2": { "markdown": "test4\n\ntest3\n\n# test1\n", "blocknote": null }, "createdBy": { "source": "EMAIL" } }' ``` And this does not work: ``` curl http://localhost:3000/rest/notes \ --request POST \ --header 'Content-Type: application/json' \ --header 'Authorization: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIyMDIwMjAyMC0xYzI1LTRkMDItYmYyNS02YWVjY2Y3ZWE0MTkiLCJ0eXBlIjoiQVBJX0tFWSIsIndvcmtzcGFjZUlkIjoiMjAyMDIwMjAtMWMyNS00ZDAyLWJmMjUtNmFlY2NmN2VhNDE5IiwiaWF0IjoxNzQxODA1MzQyLCJleHAiOjQ4OTU0MDUzNDEsImp0aSI6ImZlMzU0NTBkLTlhMDMtNGE2ZS04ODVjLTBlNTU3M2Y3YTE0NiJ9.6_g8cwoSE7ZCX1Zzsw44gZIyBdLKNsnDqMOmm1bKik0' \ --data '{ "position": 1, "title": "", "body": "", "bodyV2": { "markdown": "test4\n\ntest3\n\n# test1\n" }, "createdBy": { "source": "EMAIL" } }' ``` --- It would be nice not to require the null value, maybe let's make that a separate PR? --- .../query-runner-args.factory.spec.ts | 2 + .../factories/query-runner-args.factory.ts | 127 ++-------- .../workspace-query-runner.module.ts | 2 + .../api/rest/core/rest-api-core-v2.service.ts | 28 ++- .../src/engine/api/rest/rest-api.module.ts | 2 + .../record-transformer.module.ts | 9 + .../record-input-transformer.service.ts | 235 ++++++++++++++++++ 7 files changed, 295 insertions(+), 110 deletions(-) create mode 100644 packages/twenty-server/src/engine/core-modules/record-transformer/record-transformer.module.ts create mode 100644 packages/twenty-server/src/engine/core-modules/record-transformer/services/record-input-transformer.service.ts 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 ce257fd59..b207a1cb1 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 @@ -10,6 +10,7 @@ import { RecordPositionService, RecordPositionServiceCreateArgs, } from 'src/engine/core-modules/record-position/services/record-position.service'; +import { RecordInputTransformerService } from 'src/engine/core-modules/record-transformer/services/record-input-transformer.service'; import { FieldMetadataMap } from 'src/engine/metadata-modules/types/field-metadata-map'; describe('QueryRunnerArgsFactory', () => { @@ -65,6 +66,7 @@ describe('QueryRunnerArgsFactory', () => { const module: TestingModule = await Test.createTestingModule({ providers: [ QueryRunnerArgsFactory, + RecordInputTransformerService, { provide: RecordPositionService, useValue: recordPositionService, 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 e807f2113..263813986 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 @@ -1,6 +1,5 @@ import { Injectable } from '@nestjs/common'; -import { ServerBlockNoteEditor } from '@blocknote/server-util'; import { FieldMetadataType } from 'twenty-shared/types'; import { @@ -21,13 +20,9 @@ import { } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; import { FieldMetadataInterface } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata.interface'; -import { lowercaseDomain } from 'src/engine/api/graphql/workspace-query-runner/utils/query-runner-links.util'; import { RecordPositionService } from 'src/engine/core-modules/record-position/services/record-position.service'; -import { - RichTextV2Metadata, - richTextV2ValueSchema, -} from 'src/engine/metadata-modules/field-metadata/composite-types/rich-text-v2.composite-type'; import { FieldMetadataMap } from 'src/engine/metadata-modules/types/field-metadata-map'; +import { RecordInputTransformerService } from 'src/engine/core-modules/record-transformer/services/record-input-transformer.service'; type ArgPositionBackfillInput = { argIndex?: number; @@ -36,7 +31,10 @@ type ArgPositionBackfillInput = { @Injectable() export class QueryRunnerArgsFactory { - constructor(private readonly recordPositionService: RecordPositionService) {} + constructor( + private readonly recordPositionService: RecordPositionService, + private readonly recordInputTransformerService: RecordInputTransformerService, + ) {} async create( args: ResolverArgs, @@ -101,7 +99,7 @@ export class QueryRunnerArgsFactory { case ResolverArgsType.UpdateMany: return { ...args, - filter: await this.overrideFilterByFieldMetadata( + filter: this.overrideFilterByFieldMetadata( (args as UpdateManyResolverArgs).filter, fieldMetadataMapByNameByName, ), @@ -118,7 +116,7 @@ export class QueryRunnerArgsFactory { case ResolverArgsType.FindOne: return { ...args, - filter: await this.overrideFilterByFieldMetadata( + filter: this.overrideFilterByFieldMetadata( (args as FindOneResolverArgs).filter, fieldMetadataMapByNameByName, ), @@ -126,7 +124,7 @@ export class QueryRunnerArgsFactory { case ResolverArgsType.FindMany: return { ...args, - filter: await this.overrideFilterByFieldMetadata( + filter: this.overrideFilterByFieldMetadata( (args as FindManyResolverArgs).filter, fieldMetadataMapByNameByName, ), @@ -205,93 +203,17 @@ export class QueryRunnerArgsFactory { return [key, newValue]; } case FieldMetadataType.NUMBER: - return [key, value === null ? null : Number(value)]; case FieldMetadataType.RICH_TEXT: - throw new Error( - 'Rich text is not supported, please use RICH_TEXT_V2 instead', - ); - case FieldMetadataType.RICH_TEXT_V2: { - const richTextV2Value = richTextV2ValueSchema.parse(value); - - const serverBlockNoteEditor = ServerBlockNoteEditor.create(); - - const convertedMarkdown = richTextV2Value.blocknote - ? await serverBlockNoteEditor.blocksToMarkdownLossy( - JSON.parse(richTextV2Value.blocknote), - ) - : null; - - const convertedBlocknote = richTextV2Value.markdown - ? JSON.stringify( - await serverBlockNoteEditor.tryParseMarkdownToBlocks( - richTextV2Value.markdown, - ), - ) - : null; - - const valueInBothFormats: RichTextV2Metadata = { - markdown: richTextV2Value.markdown || convertedMarkdown, - blocknote: richTextV2Value.blocknote || convertedBlocknote, - }; - - return [key, valueInBothFormats]; - } - case FieldMetadataType.LINKS: { - const newPrimaryLinkUrl = lowercaseDomain(value?.primaryLinkUrl); - - let secondaryLinks = value?.secondaryLinks; - - if (secondaryLinks) { - try { - const secondaryLinksArray = JSON.parse(secondaryLinks); - - secondaryLinks = JSON.stringify( - secondaryLinksArray.map((link) => { - return { - ...link, - url: lowercaseDomain(link.url), - }; - }), - ); - } catch { - /* empty */ - } - } - - return [ - key, - { - ...value, - primaryLinkUrl: newPrimaryLinkUrl, - secondaryLinks, - }, - ]; - } + case FieldMetadataType.RICH_TEXT_V2: + case FieldMetadataType.LINKS: case FieldMetadataType.EMAILS: { - let additionalEmails = value?.additionalEmails; - const primaryEmail = value?.primaryEmail - ? value.primaryEmail.toLowerCase() - : ''; + const transformedValue = + await this.recordInputTransformerService.transformFieldValue( + fieldMetadata.type, + value, + ); - if (additionalEmails) { - try { - const emailArray = JSON.parse(additionalEmails) as string[]; - - additionalEmails = JSON.stringify( - emailArray.map((email) => email.toLowerCase()), - ); - } catch { - /* empty */ - } - } - - return [ - key, - { - primaryEmail, - additionalEmails, - }, - ]; + return [key, transformedValue]; } default: return [key, value]; @@ -342,7 +264,7 @@ export class QueryRunnerArgsFactory { } else if (key === 'not') { acc[key] = overrideFilter(value); } else { - acc[key] = this.transformValueByType( + acc[key] = this.transformFilterValueByType( key, value, fieldMetadataMapByName, @@ -356,7 +278,7 @@ export class QueryRunnerArgsFactory { return overrideFilter(filter); } - private transformValueByType( + private transformFilterValueByType( key: string, value: any, fieldMetadataMapByName: FieldMetadataMap, @@ -367,8 +289,9 @@ export class QueryRunnerArgsFactory { return value; } + // Special handling for filter values, which have a specific structure switch (fieldMetadata.type) { - case 'NUMBER': { + case FieldMetadataType.NUMBER: { if (value?.is === 'NULL') { return value; } else { @@ -396,11 +319,9 @@ export class QueryRunnerArgsFactory { return value; } - switch (fieldMetadata.type) { - case FieldMetadataType.NUMBER: - return Number(value); - default: - return value; - } + return this.recordInputTransformerService.transformFieldValue( + fieldMetadata.type, + value, + ); } } diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/workspace-query-runner.module.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/workspace-query-runner.module.ts index abe786b41..277a1de8d 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/workspace-query-runner.module.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/workspace-query-runner.module.ts @@ -13,6 +13,7 @@ import { FileModule } from 'src/engine/core-modules/file/file.module'; import { RecordPositionModule } from 'src/engine/core-modules/record-position/record-position.module'; import { TelemetryModule } from 'src/engine/core-modules/telemetry/telemetry.module'; import { ObjectMetadataRepositoryModule } from 'src/engine/object-metadata-repository/object-metadata-repository.module'; +import { RecordTransformerModule } from 'src/engine/core-modules/record-transformer/record-transformer.module'; import { WorkspaceDataSourceModule } from 'src/engine/workspace-datasource/workspace-datasource.module'; import { WorkspaceMemberWorkspaceEntity } from 'src/modules/workspace-member/standard-objects/workspace-member.workspace-entity'; @@ -30,6 +31,7 @@ import { EntityEventsToDbListener } from './listeners/entity-events-to-db.listen TelemetryModule, FileModule, FeatureFlagModule, + RecordTransformerModule, RecordPositionModule, ], providers: [ diff --git a/packages/twenty-server/src/engine/api/rest/core/rest-api-core-v2.service.ts b/packages/twenty-server/src/engine/api/rest/core/rest-api-core-v2.service.ts index cf1b2cb6f..6ed4d0134 100644 --- a/packages/twenty-server/src/engine/api/rest/core/rest-api-core-v2.service.ts +++ b/packages/twenty-server/src/engine/api/rest/core/rest-api-core-v2.service.ts @@ -1,13 +1,14 @@ import { BadRequestException, Injectable } from '@nestjs/common'; -import { Request } from 'express'; import { capitalize } from 'twenty-shared/utils'; +import { Request } from 'express'; import { ObjectRecord } from 'src/engine/api/graphql/workspace-query-builder/interfaces/object-record.interface'; import { CoreQueryBuilderFactory } from 'src/engine/api/rest/core/query-builder/core-query-builder.factory'; import { parseCorePath } from 'src/engine/api/rest/core/query-builder/utils/path-parsers/parse-core-path.utils'; import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager'; +import { RecordInputTransformerService } from 'src/engine/core-modules/record-transformer/services/record-input-transformer.service'; import { ApiEventEmitterService } from 'src/engine/api/graphql/graphql-query-runner/services/api-event-emitter.service'; import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; @@ -16,7 +17,7 @@ export class RestApiCoreServiceV2 { constructor( private readonly coreQueryBuilderFactory: CoreQueryBuilderFactory, private readonly twentyORMGlobalManager: TwentyORMGlobalManager, - + private readonly recordInputTransformerService: RecordInputTransformerService, protected readonly apiEventEmitterService: ApiEventEmitterService, ) {} @@ -47,11 +48,15 @@ export class RestApiCoreServiceV2 { } async createOne(request: Request) { - const { body } = request; - const { objectMetadataNameSingular, objectMetadata, repository } = await this.getRepositoryAndMetadataOrFail(request); - const createdRecord = await repository.save(body); + + const overriddenBody = await this.recordInputTransformerService.process({ + recordInput: request.body, + objectMetadataMapItem: objectMetadata.objectMetadataMapItem, + }); + + const createdRecord = await repository.save(overriddenBody); this.apiEventEmitterService.emitCreateEvents( [createdRecord], @@ -80,9 +85,14 @@ export class RestApiCoreServiceV2 { where: { id: recordId }, }); + const overriddenBody = await this.recordInputTransformerService.process({ + recordInput: request.body, + objectMetadataMapItem: objectMetadata.objectMetadataMapItem, + }); + const updatedRecord = await repository.save({ ...recordToUpdate, - ...request.body, + ...overriddenBody, }); this.apiEventEmitterService.emitUpdateEvents( @@ -139,7 +149,11 @@ export class RestApiCoreServiceV2 { objectMetadataNameSingular, ); - return { objectMetadataNameSingular, objectMetadata, repository }; + return { + objectMetadataNameSingular, + objectMetadata, + repository, + }; } private getAuthContextFromRequest(request: Request): AuthContext { diff --git a/packages/twenty-server/src/engine/api/rest/rest-api.module.ts b/packages/twenty-server/src/engine/api/rest/rest-api.module.ts index 065b6417b..f6aab815b 100644 --- a/packages/twenty-server/src/engine/api/rest/rest-api.module.ts +++ b/packages/twenty-server/src/engine/api/rest/rest-api.module.ts @@ -14,6 +14,7 @@ import { RestApiMetadataController } from 'src/engine/api/rest/metadata/rest-api import { RestApiMetadataService } from 'src/engine/api/rest/metadata/rest-api-metadata.service'; import { RestApiService } from 'src/engine/api/rest/rest-api.service'; import { AuthModule } from 'src/engine/core-modules/auth/auth.module'; +import { RecordTransformerModule } from 'src/engine/core-modules/record-transformer/record-transformer.module'; import { TwentyORMModule } from 'src/engine/twenty-orm/twenty-orm.module'; import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/workspace-cache-storage.module'; import { ApiEventEmitterService } from 'src/engine/api/graphql/graphql-query-runner/services/api-event-emitter.service'; @@ -26,6 +27,7 @@ import { ApiEventEmitterService } from 'src/engine/api/graphql/graphql-query-run AuthModule, HttpModule, TwentyORMModule, + RecordTransformerModule, ], controllers: [ RestApiMetadataController, diff --git a/packages/twenty-server/src/engine/core-modules/record-transformer/record-transformer.module.ts b/packages/twenty-server/src/engine/core-modules/record-transformer/record-transformer.module.ts new file mode 100644 index 000000000..a0b354434 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/record-transformer/record-transformer.module.ts @@ -0,0 +1,9 @@ +import { Module } from '@nestjs/common'; + +import { RecordInputTransformerService } from './services/record-input-transformer.service'; + +@Module({ + providers: [RecordInputTransformerService], + exports: [RecordInputTransformerService], +}) +export class RecordTransformerModule {} diff --git a/packages/twenty-server/src/engine/core-modules/record-transformer/services/record-input-transformer.service.ts b/packages/twenty-server/src/engine/core-modules/record-transformer/services/record-input-transformer.service.ts new file mode 100644 index 000000000..378dcbeae --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/record-transformer/services/record-input-transformer.service.ts @@ -0,0 +1,235 @@ +import { Injectable } from '@nestjs/common'; + +import { FieldMetadataType } from 'twenty-shared/types'; +import { isDefined } from 'twenty-shared/utils'; +import { ServerBlockNoteEditor } from '@blocknote/server-util'; + +import { FieldMetadataInterface } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata.interface'; + +import { + RichTextV2Metadata, + richTextV2ValueSchema, +} from 'src/engine/metadata-modules/field-metadata/composite-types/rich-text-v2.composite-type'; +import { lowercaseDomain } from 'src/engine/api/graphql/workspace-query-runner/utils/query-runner-links.util'; +import { LinkMetadata } from 'src/engine/metadata-modules/field-metadata/composite-types/links.composite-type'; +import { ObjectMetadataItemWithFieldMaps } from 'src/engine/metadata-modules/types/object-metadata-item-with-field-maps'; +import { compositeTypeDefinitions } from 'src/engine/metadata-modules/field-metadata/composite-types'; + +@Injectable() +export class RecordInputTransformerService { + async process({ + recordInput, + objectMetadataMapItem, + }: { + recordInput: Record; + objectMetadataMapItem: ObjectMetadataItemWithFieldMaps; + }): Promise> { + if (!recordInput) { + return recordInput; + } + + const fieldMetadataByFieldName = objectMetadataMapItem.fields.reduce( + (acc, field) => { + acc[field.name] = field; + + return acc; + }, + {} as Record, + ); + + let transformedEntries = {}; + + for (const [key, value] of Object.entries(recordInput)) { + const fieldMetadata = fieldMetadataByFieldName[key]; + + if (!fieldMetadata) { + transformedEntries = { ...transformedEntries, [key]: value }; + continue; + } + + const transformedValue = this.parseSubFields( + fieldMetadata.type, + await this.transformFieldValue( + fieldMetadata.type, + this.stringifySubFields(fieldMetadata.type, value), + ), + ); + + transformedEntries = { ...transformedEntries, [key]: transformedValue }; + } + + return transformedEntries; + } + + async transformFieldValue( + fieldType: FieldMetadataType, + value: any, + ): Promise { + if (!isDefined(value)) { + return value; + } + + switch (fieldType) { + case FieldMetadataType.UUID: + return value || null; + case FieldMetadataType.NUMBER: + return value === null ? null : Number(value); + case FieldMetadataType.RICH_TEXT: + throw new Error( + 'Rich text is not supported, please use RICH_TEXT_V2 instead', + ); + case FieldMetadataType.RICH_TEXT_V2: + return this.transformRichTextV2Value(value); + case FieldMetadataType.LINKS: + return this.transformLinksValue(value); + case FieldMetadataType.EMAILS: + return this.transformEmailsValue(value); + default: + return value; + } + } + + private async transformRichTextV2Value( + richTextValue: any, + ): Promise { + const parsedValue = richTextV2ValueSchema.parse(richTextValue); + + const serverBlockNoteEditor = ServerBlockNoteEditor.create(); + + const convertedMarkdown = parsedValue.blocknote + ? await serverBlockNoteEditor.blocksToMarkdownLossy( + JSON.parse(parsedValue.blocknote), + ) + : null; + + const convertedBlocknote = parsedValue.markdown + ? JSON.stringify( + await serverBlockNoteEditor.tryParseMarkdownToBlocks( + parsedValue.markdown, + ), + ) + : null; + + return { + markdown: parsedValue.markdown || convertedMarkdown, + blocknote: parsedValue.blocknote || convertedBlocknote, + }; + } + + private transformLinksValue(value: any): any { + if (!value) { + return value; + } + + const newPrimaryLinkUrl = lowercaseDomain(value?.primaryLinkUrl); + + let secondaryLinks = value?.secondaryLinks; + + if (secondaryLinks) { + try { + const secondaryLinksArray = JSON.parse(secondaryLinks); + + secondaryLinks = JSON.stringify( + secondaryLinksArray.map((link: LinkMetadata) => { + return { + ...link, + url: lowercaseDomain(link.url), + }; + }), + ); + } catch { + /* empty */ + } + } + + return { + ...value, + primaryLinkUrl: newPrimaryLinkUrl, + secondaryLinks, + }; + } + + private transformEmailsValue(value: any): any { + if (!value) { + return value; + } + + let additionalEmails = value?.additionalEmails; + const primaryEmail = value?.primaryEmail + ? value.primaryEmail.toLowerCase() + : ''; + + if (additionalEmails) { + try { + const emailArray = JSON.parse(additionalEmails) as string[]; + + additionalEmails = JSON.stringify( + emailArray.map((email) => email.toLowerCase()), + ); + } catch { + /* empty */ + } + } + + return { + primaryEmail, + additionalEmails, + }; + } + + private stringifySubFields(fieldMetadataType: FieldMetadataType, value: any) { + const compositeType = compositeTypeDefinitions.get(fieldMetadataType); + + if (!compositeType) { + return value; + } + + return Object.entries(value).reduce( + (acc, [subFieldName, subFieldValue]) => { + const subFieldType = compositeType.properties.find( + (property) => property.name === subFieldName, + )?.type; + + if (subFieldType === FieldMetadataType.RAW_JSON) { + return { + ...acc, + [subFieldName]: subFieldValue + ? JSON.stringify(subFieldValue) + : subFieldValue, + }; + } + + return { ...acc, [subFieldName]: subFieldValue }; + }, + {}, + ); + } + + private parseSubFields(fieldMetadataType: FieldMetadataType, value: any) { + const compositeType = compositeTypeDefinitions.get(fieldMetadataType); + + if (!compositeType) { + return value; + } + + return Object.entries(value).reduce( + (acc, [subFieldName, subFieldValue]: [string, any]) => { + const subFieldType = compositeType.properties.find( + (property) => property.name === subFieldName, + )?.type; + + if (subFieldType === FieldMetadataType.RAW_JSON) { + return { + ...acc, + [subFieldName]: subFieldValue + ? JSON.parse(subFieldValue) + : subFieldValue, + }; + } + + return { ...acc, [subFieldName]: subFieldValue }; + }, + {}, + ); + } +}