From c0a0214879c92d36bf22771cac153e784c3181f6 Mon Sep 17 00:00:00 2001 From: Etienne <45695613+etiennejouan@users.noreply.github.com> Date: Tue, 13 May 2025 19:18:38 +0200 Subject: [PATCH] delete attachment when file is removed from activity body (#11952) Using useEffect triggered at ActivityRichTextEditor unmount, to delete attachments only when note is closed (and not when file block is deleted during note update to keep command + z shortcut) closes : https://github.com/twentyhq/twenty/issues/11229 --- .../components/ActivityRichTextEditor.tsx | 77 ++++++++++++++++++- .../useReplaceActivityBlockEditorContent.ts | 2 +- .../filterAttachmentsToRestore.test.ts | 44 +++++++++++ .../getActivityAttachmentIdsToDelete.test.ts | 42 ++++++++++ .../getActivityAttachmentPaths.test.ts | 40 ++++++++++ ...etActivityAttachmentPathsToRestore.test.ts | 51 ++++++++++++ .../utils/__tests__/getAttachmentPath.test.ts | 10 +++ .../utils/filterAttachmentsToRestore.ts | 15 ++++ .../utils/getActivityAttachmentIdsToDelete.ts | 15 ++++ .../utils/getActivityAttachmentPaths.ts | 18 +++++ .../getActivityAttachmentPathsToRestore.ts | 17 ++++ .../activities/utils/getAttachmentPath.ts | 3 + .../record-input-transformer.service.ts | 26 ++++--- 13 files changed, 347 insertions(+), 13 deletions(-) create mode 100644 packages/twenty-front/src/modules/activities/utils/__tests__/filterAttachmentsToRestore.test.ts create mode 100644 packages/twenty-front/src/modules/activities/utils/__tests__/getActivityAttachmentIdsToDelete.test.ts create mode 100644 packages/twenty-front/src/modules/activities/utils/__tests__/getActivityAttachmentPaths.test.ts create mode 100644 packages/twenty-front/src/modules/activities/utils/__tests__/getActivityAttachmentPathsToRestore.test.ts create mode 100644 packages/twenty-front/src/modules/activities/utils/__tests__/getAttachmentPath.test.ts create mode 100644 packages/twenty-front/src/modules/activities/utils/filterAttachmentsToRestore.ts create mode 100644 packages/twenty-front/src/modules/activities/utils/getActivityAttachmentIdsToDelete.ts create mode 100644 packages/twenty-front/src/modules/activities/utils/getActivityAttachmentPaths.ts create mode 100644 packages/twenty-front/src/modules/activities/utils/getActivityAttachmentPathsToRestore.ts create mode 100644 packages/twenty-front/src/modules/activities/utils/getAttachmentPath.ts diff --git a/packages/twenty-front/src/modules/activities/components/ActivityRichTextEditor.tsx b/packages/twenty-front/src/modules/activities/components/ActivityRichTextEditor.tsx index 1b8f7dfbd..8f598891a 100644 --- a/packages/twenty-front/src/modules/activities/components/ActivityRichTextEditor.tsx +++ b/packages/twenty-front/src/modules/activities/components/ActivityRichTextEditor.tsx @@ -20,9 +20,16 @@ import { useDebouncedCallback } from 'use-debounce'; import { BLOCK_SCHEMA } from '@/activities/blocks/constants/Schema'; import { ActivityRichTextEditorChangeOnActivityIdEffect } from '@/activities/components/ActivityRichTextEditorChangeOnActivityIdEffect'; +import { Attachment } from '@/activities/files/types/Attachment'; import { Note } from '@/activities/types/Note'; import { Task } from '@/activities/types/Task'; +import { filterAttachmentsToRestore } from '@/activities/utils/filterAttachmentsToRestore'; +import { getActivityAttachmentIdsToDelete } from '@/activities/utils/getActivityAttachmentIdsToDelete'; +import { getActivityAttachmentPathsToRestore } from '@/activities/utils/getActivityAttachmentPathsToRestore'; import { CommandMenuHotkeyScope } from '@/command-menu/types/CommandMenuHotkeyScope'; +import { useDeleteManyRecords } from '@/object-record/hooks/useDeleteManyRecords'; +import { useLazyFetchAllRecords } from '@/object-record/hooks/useLazyFetchAllRecords'; +import { useRestoreManyRecords } from '@/object-record/hooks/useRestoreManyRecords'; import { useIsRecordReadOnly } from '@/object-record/record-field/hooks/useIsRecordReadOnly'; import { BlockEditor } from '@/ui/input/editor/components/BlockEditor'; import { PartialBlock } from '@blocknote/core'; @@ -40,6 +47,10 @@ type ActivityRichTextEditorProps = { | CoreObjectNameSingular.Note; }; +type Activity = (Task | Note) & { + attachments: Attachment[]; +}; + export const ActivityRichTextEditor = ({ activityId, activityObjectNameSingular, @@ -61,6 +72,24 @@ export const ActivityRichTextEditor = ({ isRecordReadOnly, }); + const { deleteManyRecords: deleteAttachments } = useDeleteManyRecords({ + objectNameSingular: CoreObjectNameSingular.Attachment, + }); + + const { restoreManyRecords: restoreAttachments } = useRestoreManyRecords({ + objectNameSingular: CoreObjectNameSingular.Attachment, + }); + + const { fetchAllRecords: findSoftDeletedAttachments } = + useLazyFetchAllRecords({ + objectNameSingular: CoreObjectNameSingular.Attachment, + filter: { + deletedAt: { + is: 'NOT_NULL', + }, + }, + }); + const { goBackToPreviousHotkeyScope, setHotkeyScopeAndMemorizePreviousScope, @@ -137,8 +166,12 @@ export const ActivityRichTextEditor = ({ ); const handleBodyChange = useRecoilCallback( - ({ set }) => - (newStringifiedBody: string) => { + ({ set, snapshot }) => + async (newStringifiedBody: string) => { + const oldActivity = snapshot + .getLoadable(recordStoreFamilyState(activityId)) + .getValue() as Activity; + set(recordStoreFamilyState(activityId), (oldActivity) => { return { ...oldActivity, @@ -166,8 +199,46 @@ export const ActivityRichTextEditor = ({ }); handlePersistBody(newStringifiedBody); + + const attachmentIdsToDelete = getActivityAttachmentIdsToDelete( + newStringifiedBody, + oldActivity.attachments, + ); + + if (attachmentIdsToDelete.length > 0) { + await deleteAttachments({ + recordIdsToDelete: attachmentIdsToDelete, + }); + } + + const attachmentPathsToRestore = getActivityAttachmentPathsToRestore( + newStringifiedBody, + oldActivity.attachments, + ); + + if (attachmentPathsToRestore.length > 0) { + const softDeletedAttachments = + (await findSoftDeletedAttachments()) as Attachment[]; + + const attachmentIdsToRestore = filterAttachmentsToRestore( + attachmentPathsToRestore, + softDeletedAttachments, + ); + + await restoreAttachments({ + idsToRestore: attachmentIdsToRestore, + }); + } }, - [activityId, cache, objectMetadataItemActivity, handlePersistBody], + [ + activityId, + cache, + objectMetadataItemActivity, + handlePersistBody, + deleteAttachments, + restoreAttachments, + findSoftDeletedAttachments, + ], ); const handleBodyChangeDebounced = useDebouncedCallback(handleBodyChange, 500); diff --git a/packages/twenty-front/src/modules/activities/hooks/useReplaceActivityBlockEditorContent.ts b/packages/twenty-front/src/modules/activities/hooks/useReplaceActivityBlockEditorContent.ts index cdf35a838..ae1e1ba69 100644 --- a/packages/twenty-front/src/modules/activities/hooks/useReplaceActivityBlockEditorContent.ts +++ b/packages/twenty-front/src/modules/activities/hooks/useReplaceActivityBlockEditorContent.ts @@ -2,8 +2,8 @@ import { BLOCK_SCHEMA } from '@/activities/blocks/constants/Schema'; import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState'; import { isNonEmptyString } from '@sniptt/guards'; import { useRecoilCallback } from 'recoil'; -import { isDeeplyEqual } from '~/utils/isDeeplyEqual'; import { isDefined } from 'twenty-shared/utils'; +import { isDeeplyEqual } from '~/utils/isDeeplyEqual'; export const useReplaceActivityBlockEditorContent = ( editor: typeof BLOCK_SCHEMA.BlockNoteEditor, diff --git a/packages/twenty-front/src/modules/activities/utils/__tests__/filterAttachmentsToRestore.test.ts b/packages/twenty-front/src/modules/activities/utils/__tests__/filterAttachmentsToRestore.test.ts new file mode 100644 index 000000000..9b4b07f2f --- /dev/null +++ b/packages/twenty-front/src/modules/activities/utils/__tests__/filterAttachmentsToRestore.test.ts @@ -0,0 +1,44 @@ +import { Attachment } from '@/activities/files/types/Attachment'; +import { filterAttachmentsToRestore } from '../filterAttachmentsToRestore'; + +describe('filterAttachmentsToRestore', () => { + it('should not return any ids if there are no attachment paths to restore', () => { + const softDeletedAttachments = [ + { + id: '1', + fullPath: 'test.txt', + }, + ] as Attachment[]; + const attachmentIdsToRestore = filterAttachmentsToRestore( + [], + softDeletedAttachments, + ); + expect(attachmentIdsToRestore).toEqual([]); + }); + + it('should not return any ids if there are no soft deleted attachments', () => { + const attachmentIdsToRestore = filterAttachmentsToRestore( + ['/files/attachment/test.txt'], + [], + ); + expect(attachmentIdsToRestore).toEqual([]); + }); + + it('should return the ids of the soft deleted attachments that are present in the attachment paths to restore', () => { + const softDeletedAttachments = [ + { + id: '1', + fullPath: '/files/attachment/test.txt', + }, + { + id: '2', + fullPath: '/files/attachment/test2.txt', + }, + ] as Attachment[]; + const attachmentIdsToRestore = filterAttachmentsToRestore( + ['attachment/test.txt'], + softDeletedAttachments, + ); + expect(attachmentIdsToRestore).toEqual(['1']); + }); +}); diff --git a/packages/twenty-front/src/modules/activities/utils/__tests__/getActivityAttachmentIdsToDelete.test.ts b/packages/twenty-front/src/modules/activities/utils/__tests__/getActivityAttachmentIdsToDelete.test.ts new file mode 100644 index 000000000..853241137 --- /dev/null +++ b/packages/twenty-front/src/modules/activities/utils/__tests__/getActivityAttachmentIdsToDelete.test.ts @@ -0,0 +1,42 @@ +import { Attachment } from '@/activities/files/types/Attachment'; +import { getActivityAttachmentIdsToDelete } from '@/activities/utils/getActivityAttachmentIdsToDelete'; + +describe('getActivityAttachmentIdsToDelete', () => { + it('should not return any ids if attachment are present in the body', () => { + const attachments = [ + { + id: '1', + fullPath: '/files/attachment/test.txt', + }, + { + id: '2', + fullPath: '/files/attachment/test2.txt', + }, + ] as Attachment[]; + + const attachmentIdsToDelete = getActivityAttachmentIdsToDelete( + '/files/attachment/test2.txt /files/attachment/test.txt', + attachments, + ); + expect(attachmentIdsToDelete).toEqual([]); + }); + + it('should return the ids of the attachments that are not present in the body', () => { + const attachments = [ + { + id: '1', + fullPath: '/files/attachment/test.txt', + }, + { + id: '2', + fullPath: '/files/attachment/test2.txt', + }, + ] as Attachment[]; + + const attachmentIdsToDelete = getActivityAttachmentIdsToDelete( + '/files/attachment/test2.txt', + attachments, + ); + expect(attachmentIdsToDelete).toEqual(['1']); + }); +}); diff --git a/packages/twenty-front/src/modules/activities/utils/__tests__/getActivityAttachmentPaths.test.ts b/packages/twenty-front/src/modules/activities/utils/__tests__/getActivityAttachmentPaths.test.ts new file mode 100644 index 000000000..0a32150ee --- /dev/null +++ b/packages/twenty-front/src/modules/activities/utils/__tests__/getActivityAttachmentPaths.test.ts @@ -0,0 +1,40 @@ +import { getActivityAttachmentPaths } from '@/activities/utils/getActivityAttachmentPaths'; + +describe('getActivityAttachmentPaths', () => { + it('should return the file paths from the activity blocknote', () => { + const activityBlocknote = JSON.stringify([ + { type: 'paragraph', props: { text: 'test' } }, + { + type: 'image', + props: { + url: 'https://example.com/files/attachment/image.jpg?queryParam=value', + }, + }, + { + type: 'file', + props: { + url: 'https://example.com/files/attachment/file.pdf?queryParam=value', + }, + }, + { + type: 'video', + props: { + url: 'https://example.com/files/attachment/video.mp4?queryParam=value', + }, + }, + { + type: 'audio', + props: { + url: 'https://example.com/files/attachment/audio.mp3?queryParam=value', + }, + }, + ]); + const res = getActivityAttachmentPaths(activityBlocknote); + expect(res).toEqual([ + 'attachment/image.jpg', + 'attachment/file.pdf', + 'attachment/video.mp4', + 'attachment/audio.mp3', + ]); + }); +}); diff --git a/packages/twenty-front/src/modules/activities/utils/__tests__/getActivityAttachmentPathsToRestore.test.ts b/packages/twenty-front/src/modules/activities/utils/__tests__/getActivityAttachmentPathsToRestore.test.ts new file mode 100644 index 000000000..717e5322e --- /dev/null +++ b/packages/twenty-front/src/modules/activities/utils/__tests__/getActivityAttachmentPathsToRestore.test.ts @@ -0,0 +1,51 @@ +import { Attachment } from '@/activities/files/types/Attachment'; +import { getActivityAttachmentPathsToRestore } from '@/activities/utils/getActivityAttachmentPathsToRestore'; + +describe('getActivityAttachmentPathsToRestore', () => { + it('should not return any attachment paths to restore if there are no paths in body', () => { + const newActivityBody = JSON.stringify([ + { + type: 'paragraph', + }, + ]); + + const oldActivityAttachments = [ + { + id: '1', + fullPath: '/files/attachment/test.txt', + }, + ] as Attachment[]; + + const attachmentPathsToRestore = getActivityAttachmentPathsToRestore( + newActivityBody, + oldActivityAttachments, + ); + expect(attachmentPathsToRestore).toEqual([]); + }); + + it('should return the attachment paths to restore if paths in body are not present in attachments', () => { + const newActivityBody = JSON.stringify([ + { + type: 'file', + props: { url: '/files/attachment/test.txt' }, + }, + { + type: 'file', + props: { url: '/files/attachment/test2.txt' }, + }, + ]); + + const oldActivityAttachments = [ + { + id: '1', + fullPath: '/files/attachment/test.txt', + }, + ] as Attachment[]; + + const attachmentPathsToRestore = getActivityAttachmentPathsToRestore( + newActivityBody, + oldActivityAttachments, + ); + expect(attachmentPathsToRestore).toEqual(['attachment/test2.txt']); + }); +}); diff --git a/packages/twenty-front/src/modules/activities/utils/__tests__/getAttachmentPath.test.ts b/packages/twenty-front/src/modules/activities/utils/__tests__/getAttachmentPath.test.ts new file mode 100644 index 000000000..6047b442c --- /dev/null +++ b/packages/twenty-front/src/modules/activities/utils/__tests__/getAttachmentPath.test.ts @@ -0,0 +1,10 @@ +import { getAttachmentPath } from '@/activities/utils/getAttachmentPath'; + +describe('getAttachmentPath', () => { + it('should return the attachment path', () => { + const res = getAttachmentPath( + 'https://example.com/files/attachment/image.jpg?queryParam=value', + ); + expect(res).toEqual('attachment/image.jpg'); + }); +}); diff --git a/packages/twenty-front/src/modules/activities/utils/filterAttachmentsToRestore.ts b/packages/twenty-front/src/modules/activities/utils/filterAttachmentsToRestore.ts new file mode 100644 index 000000000..cc8e2ce30 --- /dev/null +++ b/packages/twenty-front/src/modules/activities/utils/filterAttachmentsToRestore.ts @@ -0,0 +1,15 @@ +import { Attachment } from '@/activities/files/types/Attachment'; +import { getAttachmentPath } from '@/activities/utils/getAttachmentPath'; + +export const filterAttachmentsToRestore = ( + attachmentPathsToRestore: string[], + softDeletedAttachments: Attachment[], +) => { + return softDeletedAttachments + .filter((attachment) => + attachmentPathsToRestore.some( + (path) => getAttachmentPath(attachment.fullPath) === path, + ), + ) + .map((attachment) => attachment.id); +}; diff --git a/packages/twenty-front/src/modules/activities/utils/getActivityAttachmentIdsToDelete.ts b/packages/twenty-front/src/modules/activities/utils/getActivityAttachmentIdsToDelete.ts new file mode 100644 index 000000000..e439b2327 --- /dev/null +++ b/packages/twenty-front/src/modules/activities/utils/getActivityAttachmentIdsToDelete.ts @@ -0,0 +1,15 @@ +import { Attachment } from '@/activities/files/types/Attachment'; + +export const getActivityAttachmentIdsToDelete = ( + newActivityBody: string, + oldActivityAttachments: Attachment[], +) => { + if (oldActivityAttachments.length === 0) return []; + + return oldActivityAttachments + .filter( + (attachment) => + !newActivityBody.includes(attachment.fullPath.split('?')[0]), + ) + .map((attachment) => attachment.id); +}; diff --git a/packages/twenty-front/src/modules/activities/utils/getActivityAttachmentPaths.ts b/packages/twenty-front/src/modules/activities/utils/getActivityAttachmentPaths.ts new file mode 100644 index 000000000..5af59a735 --- /dev/null +++ b/packages/twenty-front/src/modules/activities/utils/getActivityAttachmentPaths.ts @@ -0,0 +1,18 @@ +import { getAttachmentPath } from '@/activities/utils/getAttachmentPath'; +import { isNonEmptyString } from '@sniptt/guards'; + +export const getActivityAttachmentPaths = ( + stringifiedActivityBlocknote: string, +): string[] => { + const activityBlocknote = JSON.parse(stringifiedActivityBlocknote ?? '{}'); + + return activityBlocknote.reduce((acc: string[], block: any) => { + if ( + ['image', 'file', 'video', 'audio'].includes(block.type) && + isNonEmptyString(block.props.url) + ) { + acc.push(getAttachmentPath(block.props.url)); + } + return acc; + }, []); +}; diff --git a/packages/twenty-front/src/modules/activities/utils/getActivityAttachmentPathsToRestore.ts b/packages/twenty-front/src/modules/activities/utils/getActivityAttachmentPathsToRestore.ts new file mode 100644 index 000000000..365ad8e1c --- /dev/null +++ b/packages/twenty-front/src/modules/activities/utils/getActivityAttachmentPathsToRestore.ts @@ -0,0 +1,17 @@ +import { Attachment } from '@/activities/files/types/Attachment'; +import { getActivityAttachmentPaths } from '@/activities/utils/getActivityAttachmentPaths'; +import { getAttachmentPath } from '@/activities/utils/getAttachmentPath'; + +export const getActivityAttachmentPathsToRestore = ( + newActivityBody: string, + oldActivityAttachments: Attachment[], +) => { + const newActivityAttachmentPaths = + getActivityAttachmentPaths(newActivityBody); + + return newActivityAttachmentPaths.filter((fullPath) => + oldActivityAttachments.every( + (attachment) => getAttachmentPath(attachment.fullPath) !== fullPath, + ), + ); +}; diff --git a/packages/twenty-front/src/modules/activities/utils/getAttachmentPath.ts b/packages/twenty-front/src/modules/activities/utils/getAttachmentPath.ts new file mode 100644 index 000000000..8aaac26e2 --- /dev/null +++ b/packages/twenty-front/src/modules/activities/utils/getAttachmentPath.ts @@ -0,0 +1,3 @@ +export const getAttachmentPath = (attachmentFullPath: string) => { + return attachmentFullPath.split('/files/')[1].split('?')[0]; +}; 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 index 378dcbeae..30d6d9d66 100644 --- 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 @@ -1,19 +1,19 @@ import { Injectable } from '@nestjs/common'; +import { ServerBlockNoteEditor } from '@blocknote/server-util'; 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 { lowercaseDomain } from 'src/engine/api/graphql/workspace-query-runner/utils/query-runner-links.util'; +import { compositeTypeDefinitions } from 'src/engine/metadata-modules/field-metadata/composite-types'; +import { LinkMetadata } from 'src/engine/metadata-modules/field-metadata/composite-types/links.composite-type'; 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 { @@ -96,11 +96,19 @@ export class RecordInputTransformerService { const serverBlockNoteEditor = ServerBlockNoteEditor.create(); - const convertedMarkdown = parsedValue.blocknote - ? await serverBlockNoteEditor.blocksToMarkdownLossy( - JSON.parse(parsedValue.blocknote), - ) - : null; + // Patch: Handle cases where blocknote to markdown conversion fails for certain block types (custom/code blocks) + // Todo : This may be resolved once the server-utils library is updated with proper conversion support - #947 + let convertedMarkdown: string | null = null; + + try { + convertedMarkdown = parsedValue.blocknote + ? await serverBlockNoteEditor.blocksToMarkdownLossy( + JSON.parse(parsedValue.blocknote), + ) + : null; + } catch { + convertedMarkdown = parsedValue.blocknote; + } const convertedBlocknote = parsedValue.markdown ? JSON.stringify(