Fix: deletion and name updates of attachments in body reflected in Files (#13169)

resolve #13168 , #8501 

This PR fixes an issue where the onChange trigger was deleting all
attachments by removing the JWT token from their paths (if it existed)
and comparing the new body with the old body to identify deleted
attachments. It ensures that only attachments actually removed from the
body get deleted, preventing unintended deletion of attachments not
added directly through the body. It also handles updating attachment
names in the body so changes are reflected in Files, with related tests
updated accordingly.


https://github.com/user-attachments/assets/8d824a24-b257-4794-942e-3b2dceb9907d

---------

Co-authored-by: Lucas Bordeau <bordeau.lucas@gmail.com>

Author: 2 people

packages/twenty-front/src/modules/activities/components/ActivityRichTextEditor.tsx

@@ -20,13 +20,15 @@ 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 { getActivityAttachmentIdsAndNameToUpdate } from '@/activities/utils/getActivityAttachmentIdsAndNameToUpdate';
 import { getActivityAttachmentIdsToDelete } from '@/activities/utils/getActivityAttachmentIdsToDelete';
 import { getActivityAttachmentPathsToRestore } from '@/activities/utils/getActivityAttachmentPathsToRestore';
 import { SIDE_PANEL_FOCUS_ID } from '@/command-menu/constants/SidePanelFocusId';
 import { useApolloCoreClient } from '@/object-metadata/hooks/useApolloCoreClient';
 import { useDeleteManyRecords } from '@/object-record/hooks/useDeleteManyRecords';
 import { useLazyFetchAllRecords } from '@/object-record/hooks/useLazyFetchAllRecords';
 import { useRestoreManyRecords } from '@/object-record/hooks/useRestoreManyRecords';
+import { useUpdateOneRecord } from '@/object-record/hooks/useUpdateOneRecord';
 import { useIsRecordReadOnly } from '@/object-record/record-field/hooks/useIsRecordReadOnly';
 import { isInlineCellInEditModeScopedState } from '@/object-record/record-inline-cell/states/isInlineCellInEditModeScopedState';
 import { useRecordShowContainerData } from '@/object-record/record-show/hooks/useRecordShowContainerData';
@@ -51,10 +53,6 @@ type ActivityRichTextEditorProps = {
     | CoreObjectNameSingular.Note;
 };
 
-type Activity = (Task | Note) & {
-  attachments: Attachment[];
-};
-
 export const ActivityRichTextEditor = ({
   activityId,
   activityObjectNameSingular,
@@ -101,7 +99,9 @@ export const ActivityRichTextEditor = ({
         },
       },
     });
-
+  const { updateOneRecord: updateOneAttachment } = useUpdateOneRecord({
+    objectNameSingular: CoreObjectNameSingular.Attachment,
+  });
   const { upsertActivity } = useUpsertActivity({
     activityObjectNameSingular: activityObjectNameSingular,
   });
@@ -177,7 +177,7 @@ export const ActivityRichTextEditor = ({
       async (newStringifiedBody: string) => {
         const oldActivity = snapshot
           .getLoadable(recordStoreFamilyState(activityId))
-          .getValue() as Activity;
+          .getValue();
 
         set(recordStoreFamilyState(activityId), (oldActivity) => {
           return {
@@ -209,7 +209,8 @@ export const ActivityRichTextEditor = ({
 
         const attachmentIdsToDelete = getActivityAttachmentIdsToDelete(
           newStringifiedBody,
-          oldActivity.attachments,
+          oldActivity?.attachments,
+          oldActivity?.bodyV2.blocknote,
         );
 
         if (attachmentIdsToDelete.length > 0) {
@@ -220,7 +221,7 @@ export const ActivityRichTextEditor = ({
 
         const attachmentPathsToRestore = getActivityAttachmentPathsToRestore(
           newStringifiedBody,
-          oldActivity.attachments,
+          oldActivity?.attachments,
         );
 
         if (attachmentPathsToRestore.length > 0) {
@@ -236,6 +237,19 @@ export const ActivityRichTextEditor = ({
             idsToRestore: attachmentIdsToRestore,
           });
         }
+        const attachmentsToUpdate = getActivityAttachmentIdsAndNameToUpdate(
+          newStringifiedBody,
+          oldActivity?.attachments,
+        );
+        if (attachmentsToUpdate.length > 0) {
+          for (const attachmentToUpdate of attachmentsToUpdate) {
+            if (!attachmentToUpdate.id) continue;
+            await updateOneAttachment({
+              idToUpdate: attachmentToUpdate.id,
+              updateOneRecordInput: { name: attachmentToUpdate.name },
+            });
+          }
+        }
       },
     [
       activityId,
@@ -245,6 +259,7 @@ export const ActivityRichTextEditor = ({
       deleteAttachments,
       restoreAttachments,
       findSoftDeletedAttachments,
+      updateOneAttachment,
     ],
   );
 

packages/twenty-front/src/modules/activities/utils/__tests__/compareUrls.test.ts

@@ -0,0 +1,33 @@
+import { compareUrls } from '@/activities/utils/compareUrls';
+
+describe('compareUrls', () => {
+  it('should return true if the two URLs are the same except for the token segment', () => {
+    const firstToken = 'ugudgugugxsqv';
+    const secondToken = 'yugguzvdhvyuzede';
+    const urlA = `https://exemple.com/files/attachment/${firstToken}/test.txt`;
+    const urlB = `https://exemple.com/files/attachment/${secondToken}/test.txt`;
+    const result = compareUrls(urlA, urlB);
+    expect(result).toEqual(true);
+  });
+
+  it('should return true if the two URLs are exactly the same', () => {
+    const urlA = `https://exemple.com/files/images/test.txt`;
+    const urlB = `https://exemple.com/files/images/test.txt`;
+    const result = compareUrls(urlA, urlB);
+    expect(result).toEqual(true);
+  });
+
+  it('should return false if filenames are different in the same domain', () => {
+    const urlA = `https://exemple.com/files/images/test1.txt`;
+    const urlB = `https://exemple.com/files/images/test.txt`;
+    const result = compareUrls(urlA, urlB);
+    expect(result).toEqual(false);
+  });
+
+  it('should return false if the domains are different', () => {
+    const urlA = `https://exemple1.com/files/images/test1.txt`;
+    const urlB = `https://exemple.com/files/images/test.txt`;
+    const result = compareUrls(urlA, urlB);
+    expect(result).toEqual(false);
+  });
+});

packages/twenty-front/src/modules/activities/utils/__tests__/filterAttachmentsToRestore.test.ts

@@ -6,7 +6,7 @@ describe('filterAttachmentsToRestore', () => {
     const softDeletedAttachments = [
       {
         id: '1',
-        fullPath: 'test.txt',
+        fullPath: 'https://exemple.com/test.txt',
       },
     ] as Attachment[];
     const attachmentIdsToRestore = filterAttachmentsToRestore(
@@ -18,7 +18,7 @@ describe('filterAttachmentsToRestore', () => {
 
   it('should not return any ids if there are no soft deleted attachments', () => {
     const attachmentIdsToRestore = filterAttachmentsToRestore(
-      ['/files/attachment/test.txt'],
+      ['https://exemple.com/files/attachment/test.txt'],
       [],
     );
     expect(attachmentIdsToRestore).toEqual([]);
@@ -28,15 +28,15 @@ describe('filterAttachmentsToRestore', () => {
     const softDeletedAttachments = [
       {
         id: '1',
-        fullPath: '/files/attachment/test.txt',
+        fullPath: 'https://exemple.com/files/images/test.txt',
       },
       {
         id: '2',
-        fullPath: '/files/attachment/test2.txt',
+        fullPath: 'https://exemple.com/files/images/test2.txt',
       },
     ] as Attachment[];
     const attachmentIdsToRestore = filterAttachmentsToRestore(
-      ['attachment/test.txt'],
+      ['https://exemple.com/files/images/test.txt'],
       softDeletedAttachments,
     );
     expect(attachmentIdsToRestore).toEqual(['1']);

packages/twenty-front/src/modules/activities/utils/__tests__/getActivityAttachmentIdsAndNameToUpdate.test.ts

@@ -0,0 +1,74 @@
+import { Attachment } from '@/activities/files/types/Attachment';
+import { getActivityAttachmentIdsAndNameToUpdate } from '@/activities/utils/getActivityAttachmentIdsAndNameToUpdate';
+
+describe('getActivityAttachmentIdsAndNameToUpdate', () => {
+  it('should return an empty array when attachment names are not changed in the body', () => {
+    const attachments = [
+      {
+        id: '1',
+        fullPath: 'https://exemple.com/files/images/test.txt',
+        name: 'image',
+      },
+      {
+        id: '2',
+        fullPath: 'https://exemple.com/files/images/test2.txt',
+        name: 'image1',
+      },
+    ] as Attachment[];
+
+    const activityBody = JSON.stringify([
+      {
+        type: 'file',
+        props: {
+          url: 'https://exemple.com/files/images/test.txt',
+          name: 'image',
+        },
+      },
+      {
+        type: 'file',
+        props: {
+          url: 'https://exemple.com/files/images/test2.txt',
+          name: 'image1',
+        },
+      },
+    ]);
+    const attachmentIdsAndNameToUpdate =
+      getActivityAttachmentIdsAndNameToUpdate(activityBody, attachments);
+    expect(attachmentIdsAndNameToUpdate).toEqual([]);
+  });
+
+  it('should return only the IDs and new names of attachments whose names were changed in the body', () => {
+    const attachments = [
+      {
+        id: '1',
+        fullPath: 'https://exemple.com/files/images/test.txt',
+        name: 'image',
+      },
+      {
+        id: '2',
+        fullPath: 'https://exemple.com/files/images/test2.txt',
+        name: 'image1',
+      },
+    ] as Attachment[];
+
+    const activityBody = JSON.stringify([
+      {
+        type: 'file',
+        props: {
+          url: 'https://exemple.com/files/images/test.txt',
+          name: 'image',
+        },
+      },
+      {
+        type: 'file',
+        props: {
+          url: 'https://exemple.com/files/images/test2.txt',
+          name: 'image4',
+        },
+      },
+    ]);
+    const attachmentIdsAndNameToUpdate =
+      getActivityAttachmentIdsAndNameToUpdate(activityBody, attachments);
+    expect(attachmentIdsAndNameToUpdate).toEqual([{ id: '2', name: 'image4' }]);
+  });
+});

packages/twenty-front/src/modules/activities/utils/__tests__/getActivityAttachmentIdsToDelete.test.ts

@@ -6,17 +6,37 @@ describe('getActivityAttachmentIdsToDelete', () => {
     const attachments = [
       {
         id: '1',
-        fullPath: '/files/attachment/test.txt',
+        fullPath: 'https://example.com/files/images/test.txt',
       },
       {
         id: '2',
-        fullPath: '/files/attachment/test2.txt',
+        fullPath: 'https://example.com/files/images/test2.txt',
       },
     ] as Attachment[];
-
+    const newActivityBody = JSON.stringify([
+      {
+        type: 'file',
+        props: { url: 'https://example.com/files/images/test.txt' },
+      },
+      {
+        type: 'file',
+        props: { url: 'https://example.com/files/images/test2.txt' },
+      },
+    ]);
+    const oldActivityBody = JSON.stringify([
+      {
+        type: 'file',
+        props: { url: 'https://example.com/files/images/test.txt' },
+      },
+      {
+        type: 'file',
+        props: { url: 'https://example.com/files/images/test2.txt' },
+      },
+    ]);
     const attachmentIdsToDelete = getActivityAttachmentIdsToDelete(
-      '/files/attachment/test2.txt /files/attachment/test.txt',
+      newActivityBody,
       attachments,
+      oldActivityBody,
     );
     expect(attachmentIdsToDelete).toEqual([]);
   });
@@ -25,18 +45,34 @@ describe('getActivityAttachmentIdsToDelete', () => {
     const attachments = [
       {
         id: '1',
-        fullPath: '/files/attachment/test.txt',
+        fullPath: 'https://example.com/files/images/test.txt',
       },
       {
         id: '2',
-        fullPath: '/files/attachment/test2.txt',
+        fullPath: 'https://example.com/files/images/test2.txt',
       },
     ] as Attachment[];
-
+    const newActivityBody = JSON.stringify([
+      {
+        type: 'file',
+        props: { url: 'https://example.com/files/images/test.txt' },
+      },
+    ]);
+    const oldActivityBody = JSON.stringify([
+      {
+        type: 'file',
+        props: { url: 'https://example.com/files/images/test.txt' },
+      },
+      {
+        type: 'file',
+        props: { url: 'https://example.com/files/images/test2.txt' },
+      },
+    ]);
     const attachmentIdsToDelete = getActivityAttachmentIdsToDelete(
-      '/files/attachment/test2.txt',
+      newActivityBody,
       attachments,
+      oldActivityBody,
     );
-    expect(attachmentIdsToDelete).toEqual(['1']);
+    expect(attachmentIdsToDelete).toEqual(['2']);
   });
 });