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>
This commit is contained in:
Naifer
2025-07-23 13:36:02 +01:00
committed by GitHub
parent 7ba8212972
commit dd5ae66449
16 changed files with 373 additions and 92 deletions
@@ -20,6 +20,7 @@ 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';
@@ -27,6 +28,7 @@ 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,
],
);
@@ -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);
});
});
@@ -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']);
@@ -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' }]);
});
});
@@ -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']);
});
});
@@ -1,40 +0,0 @@
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',
]);
});
});
@@ -0,0 +1,57 @@
import { getActivityAttachmentPathsAndName } from '@/activities/utils/getActivityAttachmentPathsAndName';
describe('getActivityAttachmentPathsAndName', () => {
it('should return the file paths and names from the activity blocknote', () => {
const activityBlocknote = JSON.stringify([
{ type: 'paragraph', props: { text: 'test' } },
{
type: 'image',
props: {
url: 'https://example.com/files/image/image.jpg?queryParam=value',
name: 'image',
},
},
{
type: 'file',
props: {
url: 'https://example.com/files/file/file.pdf?queryParam=value',
name: 'file',
},
},
{
type: 'video',
props: {
url: 'https://example.com/files/video/video.mp4?queryParam=value',
name: 'video',
},
},
{
type: 'audio',
props: {
url: 'https://example.com/files/audio/audio.mp3?queryParam=value',
name: 'audio',
},
},
]);
const res = getActivityAttachmentPathsAndName(activityBlocknote);
expect(res).toEqual([
{
path: 'https://example.com/files/image/image.jpg?queryParam=value',
name: 'image',
},
{
path: 'https://example.com/files/file/file.pdf?queryParam=value',
name: 'file',
},
{
path: 'https://example.com/files/video/video.mp4?queryParam=value',
name: 'video',
},
{
path: 'https://example.com/files/audio/audio.mp3?queryParam=value',
name: 'audio',
},
]);
});
});
@@ -8,14 +8,12 @@ describe('getActivityAttachmentPathsToRestore', () => {
type: 'paragraph',
},
]);
const oldActivityAttachments = [
{
id: '1',
fullPath: '/files/attachment/test.txt',
fullPath: 'https://example.com/files/images/test.txt',
},
] as Attachment[];
const attachmentPathsToRestore = getActivityAttachmentPathsToRestore(
newActivityBody,
oldActivityAttachments,
@@ -27,18 +25,18 @@ describe('getActivityAttachmentPathsToRestore', () => {
const newActivityBody = JSON.stringify([
{
type: 'file',
props: { url: '/files/attachment/test.txt' },
props: { url: 'https://example.com/files/images/test.txt' },
},
{
type: 'file',
props: { url: '/files/attachment/test2.txt' },
props: { url: 'https://example.com/files/images/test2.txt' },
},
]);
const oldActivityAttachments = [
{
id: '1',
fullPath: '/files/attachment/test.txt',
fullPath: 'https://example.com/files/images/test.txt',
},
] as Attachment[];
@@ -46,6 +44,8 @@ describe('getActivityAttachmentPathsToRestore', () => {
newActivityBody,
oldActivityAttachments,
);
expect(attachmentPathsToRestore).toEqual(['attachment/test2.txt']);
expect(attachmentPathsToRestore).toEqual([
'https://example.com/files/images/test2.txt',
]);
});
});
@@ -1,10 +1,18 @@
import { getAttachmentPath } from '@/activities/utils/getAttachmentPath';
describe('getAttachmentPath', () => {
it('should return the attachment path', () => {
it('should extract the correct path from a locally stored file URL with token', () => {
const token = 'dybszrrxvgrtefeidgybxzfxzr';
const res = getAttachmentPath(
'https://example.com/files/attachment/image.jpg?queryParam=value',
`https://server.com/files/attachment/${token}/image.jpg?queryParam=value`,
);
expect(res).toEqual('attachment/image.jpg');
expect(res).toEqual('https://server.com/files/attachment/image.jpg');
});
it('should extract the correct path from a regular file URL', () => {
const res = getAttachmentPath(
'https://exemple.com/files/images/image.jpg?queryParam=value',
);
expect(res).toEqual('https://exemple.com/files/images/image.jpg');
});
});
@@ -0,0 +1,14 @@
import { getAttachmentPath } from '@/activities/utils/getAttachmentPath';
export const compareUrls = (
firstAttachmentUrl: string,
secondAttachmentUrl: string,
): boolean => {
const urlA = new URL(firstAttachmentUrl);
const urlB = new URL(secondAttachmentUrl);
if (urlA.hostname !== urlB.hostname) return false;
return (
getAttachmentPath(firstAttachmentUrl) ===
getAttachmentPath(secondAttachmentUrl)
);
};
@@ -1,5 +1,5 @@
import { Attachment } from '@/activities/files/types/Attachment';
import { getAttachmentPath } from '@/activities/utils/getAttachmentPath';
import { compareUrls } from '@/activities/utils/compareUrls';
export const filterAttachmentsToRestore = (
attachmentPathsToRestore: string[],
@@ -7,8 +7,8 @@ export const filterAttachmentsToRestore = (
) => {
return softDeletedAttachments
.filter((attachment) =>
attachmentPathsToRestore.some(
(path) => getAttachmentPath(attachment.fullPath) === path,
attachmentPathsToRestore.some((path) =>
compareUrls(attachment.fullPath, path),
),
)
.map((attachment) => attachment.id);
@@ -0,0 +1,29 @@
import { Attachment } from '@/activities/files/types/Attachment';
import { compareUrls } from '@/activities/utils/compareUrls';
import {
AttachmentInfo,
getActivityAttachmentPathsAndName,
} from '@/activities/utils/getActivityAttachmentPathsAndName';
import { isDefined } from 'twenty-shared/utils';
export const getActivityAttachmentIdsAndNameToUpdate = (
newActivityBody: string,
oldActivityAttachments: Attachment[] = [],
) => {
const activityAttachmentsNameAndPaths =
getActivityAttachmentPathsAndName(newActivityBody);
if (activityAttachmentsNameAndPaths.length === 0) return [];
return activityAttachmentsNameAndPaths.reduce(
(acc: Partial<Attachment>[], activity: AttachmentInfo) => {
const foundActivity = oldActivityAttachments.find((attachment) =>
compareUrls(attachment.fullPath, activity.path),
);
if (isDefined(foundActivity) && foundActivity.name !== activity.name) {
acc.push({ id: foundActivity.id, name: activity.name });
}
return acc;
},
[],
);
};
@@ -1,15 +1,34 @@
import { Attachment } from '@/activities/files/types/Attachment';
import { compareUrls } from '@/activities/utils/compareUrls';
import { getActivityAttachmentPathsAndName } from '@/activities/utils/getActivityAttachmentPathsAndName';
export const getActivityAttachmentIdsToDelete = (
newActivityBody: string,
oldActivityAttachments: Attachment[] = [],
oldActivityBody: string,
) => {
if (oldActivityAttachments.length === 0) return [];
return oldActivityAttachments
const newActivityAttachmentPaths =
getActivityAttachmentPathsAndName(newActivityBody);
const oldActivityAttachmentPaths =
getActivityAttachmentPathsAndName(oldActivityBody);
const pathsToDelete = oldActivityAttachmentPaths
.filter(
(attachment) =>
!newActivityBody.includes(attachment.fullPath.split('?')[0]),
(oldActivity) =>
!newActivityAttachmentPaths.some(
(newActivity) => newActivity.path === oldActivity.path,
),
)
.map((activity) => activity.path);
return oldActivityAttachments
.filter((attachment) =>
pathsToDelete.some((pathToDelete) =>
compareUrls(attachment.fullPath, pathToDelete),
),
)
.map((attachment) => attachment.id);
};
@@ -1,17 +1,23 @@
import { getAttachmentPath } from '@/activities/utils/getAttachmentPath';
import { isNonEmptyString } from '@sniptt/guards';
export const getActivityAttachmentPaths = (
export type AttachmentInfo = {
path: string;
name: string;
};
export const getActivityAttachmentPathsAndName = (
stringifiedActivityBlocknote: string,
): string[] => {
): AttachmentInfo[] => {
const activityBlocknote = JSON.parse(stringifiedActivityBlocknote ?? '{}');
return activityBlocknote.reduce((acc: string[], block: any) => {
return activityBlocknote.reduce((acc: AttachmentInfo[], block: any) => {
if (
['image', 'file', 'video', 'audio'].includes(block.type) &&
isNonEmptyString(block.props.url)
) {
acc.push(getAttachmentPath(block.props.url));
acc.push({
path: block.props.url,
name: block.props.name,
});
}
return acc;
}, []);
@@ -1,17 +1,22 @@
import { Attachment } from '@/activities/files/types/Attachment';
import { getActivityAttachmentPaths } from '@/activities/utils/getActivityAttachmentPaths';
import { getAttachmentPath } from '@/activities/utils/getAttachmentPath';
import { compareUrls } from '@/activities/utils/compareUrls';
import { getActivityAttachmentPathsAndName } from '@/activities/utils/getActivityAttachmentPathsAndName';
export const getActivityAttachmentPathsToRestore = (
newActivityBody: string,
oldActivityAttachments: Attachment[],
) => {
const newActivityAttachmentPaths =
getActivityAttachmentPaths(newActivityBody);
getActivityAttachmentPathsAndName(newActivityBody);
return newActivityAttachmentPaths.filter((fullPath) =>
oldActivityAttachments.every(
(attachment) => getAttachmentPath(attachment.fullPath) !== fullPath,
),
);
const pathsToRestore = newActivityAttachmentPaths
.filter(
(newActivity) =>
!oldActivityAttachments.some((attachment) =>
compareUrls(newActivity.path, attachment.fullPath),
),
)
.map((activity) => activity.path);
return pathsToRestore;
};
@@ -1,3 +1,28 @@
export const getAttachmentPath = (attachmentFullPath: string) => {
return attachmentFullPath.split('/files/')[1].split('?')[0];
if (!attachmentFullPath.includes('/files/')) {
return attachmentFullPath?.split('?')[0];
}
const base = attachmentFullPath?.split('/files/')[0];
const rawPath = attachmentFullPath?.split('/files/')[1]?.split('?')[0];
if (!rawPath) {
throw new Error(`Invalid attachment path: ${attachmentFullPath}`);
}
if (!rawPath.startsWith('attachment/')) {
return attachmentFullPath?.split('?')[0];
}
const pathParts = rawPath.split('/');
if (pathParts.length < 2) {
throw new Error(
`Invalid attachment path structure: ${rawPath}. Path must have at least two segments.`,
);
}
const filename = pathParts.pop();
pathParts.pop();
return `${base}/files/${pathParts.join('/')}/${filename}`;
};