From bb037e13dd5e25f1c539c4da2bf2f131ed0eb727 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Malfait?= Date: Tue, 10 Feb 2026 18:07:49 +0100 Subject: [PATCH] Fix blocknote.map crash with generic field-level RICH_TEXT_V2 handler (#17834) ## Summary Fixes #17667 - **Root cause**: `ActivityQueryResultGetterHandler` called `JSON.parse()` on the `blocknote` field and assumed the result was always an array. When the stored value was valid JSON but not an array (e.g., `"{}"`), `blocknote.map()` crashed with `blocknote.map is not a function`, breaking the entire notes page. - **Fix**: Replaced the object-level `ActivityQueryResultGetterHandler` (hardcoded for `note`/`task` only) with a generic field-level `RichTextV2FieldQueryResultGetterHandler` that safely parses blocknote JSON with `Array.isArray` validation and gracefully skips malformed values instead of crashing. - **Bonus**: The new handler works for **all** objects with `RICH_TEXT_V2` fields (not just `note`/`task`), following the same pattern as the existing `FilesFieldQueryResultGetterHandler`. ## Changes | File | Change | |------|--------| | `rich-text-v2-field-query-result-getter.handler.ts` | New field-level handler with safe blocknote parsing | | `common-result-getters.service.ts` | Register new handler, remove `note`/`task` object handlers | | `activity-query-result-getter.handler.ts` | Deleted (replaced by field-level handler) | | `rich-text-v2-field-query-result-getter.handler.spec.ts` | 9 tests covering all edge cases | ## Test plan - [x] Unit tests pass (9 tests covering: null blocknote, non-string blocknote, invalid JSON, non-array JSON like `"{}"`, no images, external URLs, internal URLs, multiple fields) - [x] Lint passes (`lint:diff-with-main`) - [x] Typecheck passes Made with [Cursor](https://cursor.com) --------- Co-authored-by: Cursor --- .../common-result-getters.service.ts | 8 +- ...-field-query-result-getter.handler.spec.ts | 224 ++++++++++++++++++ ...xt-v2-field-query-result-getter.handler.ts | 120 ++++++++++ ...tivity-query-result-getter.handler.spec.ts | 185 --------------- .../activity-query-result-getter.handler.ts | 89 ------- 5 files changed, 349 insertions(+), 277 deletions(-) create mode 100644 packages/twenty-server/src/engine/api/common/common-result-getters/handlers/field-handlers/__tests__/rich-text-v2-field-query-result-getter.handler.spec.ts create mode 100644 packages/twenty-server/src/engine/api/common/common-result-getters/handlers/field-handlers/rich-text-v2-field-query-result-getter.handler.ts delete mode 100644 packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/__tests__/activity-query-result-getter.handler.spec.ts delete mode 100644 packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/activity-query-result-getter.handler.ts diff --git a/packages/twenty-server/src/engine/api/common/common-result-getters/common-result-getters.service.ts b/packages/twenty-server/src/engine/api/common/common-result-getters/common-result-getters.service.ts index 478237d396f..7754ac6635f 100644 --- a/packages/twenty-server/src/engine/api/common/common-result-getters/common-result-getters.service.ts +++ b/packages/twenty-server/src/engine/api/common/common-result-getters/common-result-getters.service.ts @@ -11,7 +11,7 @@ import { type QueryResultFieldValue } from 'src/engine/api/graphql/workspace-que import { type QueryResultGetterHandlerInterface } from 'src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/interfaces/query-result-getter-handler.interface'; import { FilesFieldQueryResultGetterHandler } from 'src/engine/api/common/common-result-getters/handlers/field-handlers/files-field-query-result-getter.handler'; -import { ActivityQueryResultGetterHandler } from 'src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/activity-query-result-getter.handler'; +import { RichTextV2FieldQueryResultGetterHandler } from 'src/engine/api/common/common-result-getters/handlers/field-handlers/rich-text-v2-field-query-result-getter.handler'; import { AttachmentQueryResultGetterHandler } from 'src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/attachment-query-result-getter.handler'; import { PersonQueryResultGetterHandler } from 'src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/person-query-result-getter.handler'; import { WorkspaceMemberQueryResultGetterHandler } from 'src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/workspace-member-query-result-getter.handler'; @@ -55,8 +55,6 @@ export class CommonResultGettersService { 'workspaceMember', new WorkspaceMemberQueryResultGetterHandler(this.fileService), ], - ['note', new ActivityQueryResultGetterHandler(this.fileService)], - ['task', new ActivityQueryResultGetterHandler(this.fileService)], ]); } @@ -69,6 +67,10 @@ export class CommonResultGettersService { FieldMetadataType.FILES, new FilesFieldQueryResultGetterHandler(this.filesFieldService), ], + [ + FieldMetadataType.RICH_TEXT_V2, + new RichTextV2FieldQueryResultGetterHandler(this.fileService), + ], ]); } diff --git a/packages/twenty-server/src/engine/api/common/common-result-getters/handlers/field-handlers/__tests__/rich-text-v2-field-query-result-getter.handler.spec.ts b/packages/twenty-server/src/engine/api/common/common-result-getters/handlers/field-handlers/__tests__/rich-text-v2-field-query-result-getter.handler.spec.ts new file mode 100644 index 00000000000..dbd067804f4 --- /dev/null +++ b/packages/twenty-server/src/engine/api/common/common-result-getters/handlers/field-handlers/__tests__/rich-text-v2-field-query-result-getter.handler.spec.ts @@ -0,0 +1,224 @@ +import { FieldMetadataType, type ObjectRecord } from 'twenty-shared/types'; + +import { RichTextV2FieldQueryResultGetterHandler } from 'src/engine/api/common/common-result-getters/handlers/field-handlers/rich-text-v2-field-query-result-getter.handler'; +import { type FileService } from 'src/engine/core-modules/file/services/file.service'; +import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata.type'; + +const baseRecord: ObjectRecord = { + id: '1', + createdAt: '2021-01-01', + updatedAt: '2021-01-01', + deletedAt: null, +}; + +const richTextFieldMetadata = [ + { + type: FieldMetadataType.RICH_TEXT_V2, + name: 'bodyV2', + }, +] as FlatFieldMetadata[]; + +const mockFileService = { + signFileUrl: jest.fn().mockReturnValue('signed-path'), +} as unknown as FileService; + +describe('RichTextV2FieldQueryResultGetterHandler', () => { + let handler: RichTextV2FieldQueryResultGetterHandler; + + beforeEach(() => { + process.env.SERVER_URL = 'https://my-domain.twenty.com'; + handler = new RichTextV2FieldQueryResultGetterHandler(mockFileService); + }); + + afterEach(() => { + jest.clearAllMocks(); + delete process.env.SERVER_URL; + }); + + describe('should return record unchanged', () => { + it('when no RICH_TEXT_V2 field metadata is present', async () => { + const record = { + ...baseRecord, + bodyV2: { blocknote: '[]', markdown: null }, + }; + + const result = await handler.handle(record, 'ws-1', []); + + expect(result).toEqual(record); + }); + + it('when blocknote is null', async () => { + const record = { + ...baseRecord, + bodyV2: { blocknote: null, markdown: null }, + }; + + const result = await handler.handle( + record, + 'ws-1', + richTextFieldMetadata, + ); + + expect(result).toEqual(record); + }); + + it('when blocknote is not a string', async () => { + const record = { + ...baseRecord, + bodyV2: { blocknote: 123, markdown: null }, + }; + + const result = await handler.handle( + record, + 'ws-1', + richTextFieldMetadata, + ); + + expect(result).toEqual(record); + }); + + it('when blocknote is an invalid JSON string', async () => { + const record = { + ...baseRecord, + bodyV2: { blocknote: 'not-json', markdown: null }, + }; + + const result = await handler.handle( + record, + 'ws-1', + richTextFieldMetadata, + ); + + expect(result).toEqual(record); + }); + + it('when blocknote parses to a non-array value', async () => { + const record = { + ...baseRecord, + bodyV2: { blocknote: '{}', markdown: null }, + }; + + const result = await handler.handle( + record, + 'ws-1', + richTextFieldMetadata, + ); + + expect(result).toEqual(record); + }); + + it('when blocknote has no image blocks', async () => { + const record = { + ...baseRecord, + bodyV2: { + markdown: null, + blocknote: JSON.stringify([ + { type: 'paragraph', text: 'Hello, world!' }, + ]), + }, + }; + + const result = await handler.handle( + record, + 'ws-1', + richTextFieldMetadata, + ); + + expect(result).toEqual(record); + }); + + it('when image block has external URL', async () => { + const record = { + ...baseRecord, + bodyV2: { + markdown: null, + blocknote: JSON.stringify([ + { + type: 'image', + props: { url: 'https://external.com/image.jpg' }, + }, + ]), + }, + }; + + const result = await handler.handle( + record, + 'ws-1', + richTextFieldMetadata, + ); + + expect(result).toEqual(record); + }); + }); + + describe('should sign internal image URLs', () => { + it('when image block has an internal attachment URL', async () => { + const imageBlock = { + type: 'image', + props: { + name: 'photo.jpg', + url: 'https://my-domain.twenty.com/files/attachment/some-token/photo.jpg', + caption: '', + }, + children: [], + }; + + const record = { + ...baseRecord, + bodyV2: { + markdown: null, + blocknote: JSON.stringify([imageBlock]), + }, + }; + + const result = await handler.handle( + record, + 'ws-1', + richTextFieldMetadata, + ); + + expect(result).toEqual({ + ...baseRecord, + bodyV2: { + markdown: null, + blocknote: JSON.stringify([ + { + ...imageBlock, + props: { + ...imageBlock.props, + url: 'https://my-domain.twenty.com/files/signed-path', + }, + }, + ]), + }, + }); + }); + }); + + describe('should handle multiple RICH_TEXT_V2 fields', () => { + it('when record has multiple rich text fields', async () => { + const multiFieldMetadata = [ + { type: FieldMetadataType.RICH_TEXT_V2, name: 'bodyV2' }, + { type: FieldMetadataType.RICH_TEXT_V2, name: 'description' }, + ] as FlatFieldMetadata[]; + + const record = { + ...baseRecord, + bodyV2: { + markdown: null, + blocknote: JSON.stringify([{ type: 'paragraph', text: 'Hello' }]), + }, + description: { + markdown: null, + blocknote: '{}', + }, + }; + + const result = await handler.handle(record, 'ws-1', multiFieldMetadata); + + // bodyV2 should be unchanged (no images), description should be + // unchanged (non-array blocknote) + expect(result).toEqual(record); + }); + }); +}); diff --git a/packages/twenty-server/src/engine/api/common/common-result-getters/handlers/field-handlers/rich-text-v2-field-query-result-getter.handler.ts b/packages/twenty-server/src/engine/api/common/common-result-getters/handlers/field-handlers/rich-text-v2-field-query-result-getter.handler.ts new file mode 100644 index 00000000000..dc177e9917e --- /dev/null +++ b/packages/twenty-server/src/engine/api/common/common-result-getters/handlers/field-handlers/rich-text-v2-field-query-result-getter.handler.ts @@ -0,0 +1,120 @@ +import { FieldMetadataType, type ObjectRecord } from 'twenty-shared/types'; +import { isDefined } from 'twenty-shared/utils'; + +import { type QueryResultGetterHandlerInterface } from 'src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/interfaces/query-result-getter-handler.interface'; + +import { type FileService } from 'src/engine/core-modules/file/services/file.service'; +import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata.type'; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type RichTextBlock = Record; + +const signBlocknoteImageUrls = ( + blocknoteBlocks: RichTextBlock[], + workspaceId: string, + fileService: FileService, +): RichTextBlock[] => { + return blocknoteBlocks.map((block: RichTextBlock) => { + if (block.type !== 'image' || !block.props?.url) { + return block; + } + + let url: URL; + + try { + url = new URL(block.props.url); + } catch { + return block; + } + + const pathname = url.pathname; + const isLinkExternal = !pathname.startsWith('/files/attachment/'); + + if (isLinkExternal) { + return block; + } + + const fileName = pathname.match(/files\/attachment\/(?:.+)\/(.+)$/)?.[1]; + + if (!isDefined(fileName)) { + return block; + } + + const signedPath = fileService.signFileUrl({ + url: `attachment/${fileName}`, + workspaceId, + }); + + return { + ...block, + props: { + ...block.props, + url: `${process.env.SERVER_URL}/files/${signedPath}`, + }, + }; + }); +}; + +const parseBlocknoteJsonSafely = ( + blocknoteJson: string, +): RichTextBlock[] | null => { + try { + const parsed = JSON.parse(blocknoteJson); + + if (!Array.isArray(parsed)) { + return null; + } + + return parsed; + } catch { + return null; + } +}; + +export class RichTextV2FieldQueryResultGetterHandler + implements QueryResultGetterHandlerInterface +{ + constructor(private readonly fileService: FileService) {} + + async handle( + record: ObjectRecord, + workspaceId: string, + flatFieldMetadata: FlatFieldMetadata[], + ): Promise { + const richTextV2Fields = flatFieldMetadata.filter( + (field) => field.type === FieldMetadataType.RICH_TEXT_V2, + ); + + if (richTextV2Fields.length === 0) { + return record; + } + + for (const field of richTextV2Fields) { + const fieldValue = record[field.name]; + const blocknoteJson = fieldValue?.blocknote; + + if (!blocknoteJson || typeof blocknoteJson !== 'string') { + continue; + } + + const blocknoteBlocks = parseBlocknoteJsonSafely(blocknoteJson); + + if (!isDefined(blocknoteBlocks)) { + continue; + } + + const signedBlocks = signBlocknoteImageUrls( + blocknoteBlocks, + workspaceId, + this.fileService, + ); + + record[field.name] = { + ...fieldValue, + blocknote: JSON.stringify(signedBlocks), + }; + } + + return record; + } +} diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/__tests__/activity-query-result-getter.handler.spec.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/__tests__/activity-query-result-getter.handler.spec.ts deleted file mode 100644 index adc590d680c..00000000000 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/__tests__/activity-query-result-getter.handler.spec.ts +++ /dev/null @@ -1,185 +0,0 @@ -import { Test, type TestingModule } from '@nestjs/testing'; - -import { FieldActorSource } from 'twenty-shared/types'; - -import { ActivityQueryResultGetterHandler } from 'src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/activity-query-result-getter.handler'; -import { FileService } from 'src/engine/core-modules/file/services/file.service'; -import { type NoteWorkspaceEntity } from 'src/modules/note/standard-objects/note.workspace-entity'; - -const baseNote = { - id: '1', - bodyV2: { - markdown: null, - blocknote: null, - }, - position: 1, - title: 'Test', - createdBy: { - name: 'Test', - source: FieldActorSource.MANUAL, - workspaceMemberId: '1', - context: {}, - }, - updatedBy: { - name: 'Test', - source: FieldActorSource.MANUAL, - workspaceMemberId: '1', - context: {}, - }, - createdAt: '2021-01-01', - updatedAt: '2021-01-01', - noteTargets: [], - attachments: [], - timelineActivities: [], - favorites: [], - searchVector: '', - deletedAt: null, -} satisfies NoteWorkspaceEntity; - -const baseTask = { - ...baseNote, - type: 'task', -}; - -describe('ActivityQueryResultGetterHandler', () => { - let handler: ActivityQueryResultGetterHandler; - - beforeEach(async () => { - process.env.SERVER_URL = 'https://my-domain.twenty.com'; - const module: TestingModule = await Test.createTestingModule({ - providers: [ - ActivityQueryResultGetterHandler, - { - provide: FileService, - useValue: { - signFileUrl: jest.fn().mockReturnValue('signed-path'), - }, - }, - ], - }).compile(); - - handler = module.get( - ActivityQueryResultGetterHandler, - ); - }); - - afterEach(() => { - jest.clearAllMocks(); - delete process.env.SERVER_URL; - }); - - describe('should do nothing', () => { - it('when activity is a note and no image is found', async () => { - const note = { - ...baseNote, - bodyV2: { - markdown: null, - blocknote: JSON.stringify([ - { type: 'paragraph', text: 'Hello, world!' }, - ]), - }, - }; - - const result = await handler.handle(note, '1'); - - expect(result).toEqual(note); - }); - - it('when activity is a note and link is external', async () => { - const note = { - ...baseNote, - bodyV2: { - markdown: null, - blocknote: JSON.stringify([ - { - id: 'c6a5f700-5e56-480d-90a9-7f295216370e', - type: 'image', - props: { - backgroundColor: 'default', - textAlignment: 'left', - name: '20240529_123208.jpg', - url: 'http://external-content.com/image.jpg', - caption: '', - showPreview: true, - }, - children: [], - }, - { - id: 'e2454736-51c1-4e61-a02d-71f0890bdda7', - type: 'paragraph', - props: { - textColor: 'default', - backgroundColor: 'default', - textAlignment: 'left', - }, - content: [], - children: [], - }, - ]), - }, - }; - - const result = await handler.handle(note, '1'); - - expect(result).toEqual(note); - }); - - it('when activity is a task and no image is found', async () => { - const task = { - ...baseTask, - bodyV2: { - markdown: null, - blocknote: null, - }, - }; - - const result = await handler.handle(task, '1'); - - expect(result).toEqual(task); - }); - }); - - describe('should update token in file link', () => { - it('when file link is in the body', async () => { - const imageBlock = { - id: 'c6a5f700-5e56-480d-90a9-7f295216370e', - type: 'image', - props: { - backgroundColor: 'default', - textAlignment: 'left', - name: '20240529_123208.jpg', - url: 'https://my-domain.twenty.com/files/attachment/eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJmaWxlbmFtZSI6ImU0NWNiNDhhLTM2MmYtNGU4Zi1iOTEzLWM5MmI1ZTNlMGFhNi5qcGciLCJ3b3Jrc3BhY2VJZCI6IjIwMjAyMDIwLTFjMjUtNGQwMi1iZjI1LTZhZWNjZjdlYTQxOSIsInN1YiI6IjIwMjAyMDIwLTFjMjUtNGQwMi1iZjI1LTZhZWNjZjdlYTQxOSIsInR5cGUiOiJGSUxFIiwiaWF0IjoxNzUwNDI4NDQ1LCJleHAiOjE3NTA1MTQ4NDV9.qTN1b9IcmZvfVAqt1UlfJ_nn3GwIAEp7G9IoPtRJDxk/e45cb48a-362f-4e8f-b913-c92b5e3e0aa6.jpg', - caption: '', - showPreview: true, - }, - children: [], - }; - - const note = { - ...baseNote, - bodyV2: { - markdown: null, - blocknote: JSON.stringify([imageBlock]), - }, - }; - - const result = await handler.handle(note, '1'); - - expect(result).toEqual({ - ...note, - bodyV2: { - markdown: null, - blocknote: JSON.stringify([ - { - ...imageBlock, - props: { - ...imageBlock.props, - url: 'https://my-domain.twenty.com/files/signed-path', - }, - }, - ]), - }, - }); - }); - }); -}); diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/activity-query-result-getter.handler.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/activity-query-result-getter.handler.ts deleted file mode 100644 index 5e93b678c69..00000000000 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/activity-query-result-getter.handler.ts +++ /dev/null @@ -1,89 +0,0 @@ -import { Injectable } from '@nestjs/common'; - -import { type QueryResultGetterHandlerInterface } from 'src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/interfaces/query-result-getter-handler.interface'; - -import { FileService } from 'src/engine/core-modules/file/services/file.service'; -import { type NoteWorkspaceEntity } from 'src/modules/note/standard-objects/note.workspace-entity'; -import { type TaskWorkspaceEntity } from 'src/modules/task/standard-objects/task.workspace-entity'; - -// eslint-disable-next-line @typescript-eslint/no-explicit-any -type RichTextBlock = Record; - -type RichTextBody = RichTextBlock[]; - -@Injectable() -export class ActivityQueryResultGetterHandler - implements QueryResultGetterHandlerInterface -{ - constructor(private readonly fileService: FileService) {} - - async handle( - activity: TaskWorkspaceEntity | NoteWorkspaceEntity, - workspaceId: string, - ): Promise { - const blocknoteJson = activity.bodyV2?.blocknote; - - if (!activity.id || !blocknoteJson) { - return activity; - } - - let blocknote: RichTextBody = []; - - try { - blocknote = JSON.parse(blocknoteJson); - } catch { - blocknote = []; - // TODO: Remove this once we have removed the old rich text - // eslint-disable-next-line no-console - console.warn( - `Failed to parse body for activity ${activity.id} in workspace ${workspaceId}, for rich text version 'v2'`, - ); - // eslint-disable-next-line no-console - console.warn(blocknoteJson); - } - - const blocknoteWithSignedPayload = await Promise.all( - blocknote.map(async (block: RichTextBlock) => { - if (block.type !== 'image' || !block.props.url) { - return block; - } - - const imageProps = block.props; - const url = new URL(imageProps.url); - - const pathname = url.pathname; - - const isLinkExternal = !pathname.startsWith('/files/attachment/'); - - if (isLinkExternal) { - return block; - } - - const fileName = pathname.match( - /files\/attachment\/(?:.+)\/(.+)$/, - )?.[1]; - - const signedPath = this.fileService.signFileUrl({ - url: `attachment/${fileName}`, - workspaceId, - }); - - return { - ...block, - props: { - ...imageProps, - url: `${process.env.SERVER_URL}/files/${signedPath}`, - }, - }; - }), - ); - - return { - ...activity, - bodyV2: { - blocknote: JSON.stringify(blocknoteWithSignedPayload), - markdown: activity.bodyV2?.markdown ?? null, - }, - }; - } -}