From d1bcfd6c477689162ea0f810f8fd868dc523c20a Mon Sep 17 00:00:00 2001 From: Francis Terrero Date: Thu, 10 Sep 2026 23:33:29 -0400 Subject: [PATCH 01/20] refactor: extract name collision move actions out of the container Move the keep/replace handling for moved items into nameCollision.actions behind resolveMoveCollision, with unit tests. No behaviour change. --- .../NameCollisionContainer.tsx | 78 +++++----------- .../nameCollision.actions.test.ts | 89 +++++++++++++++++++ .../nameCollision.actions.ts | 86 ++++++++++++++++++ 3 files changed, 195 insertions(+), 58 deletions(-) create mode 100644 src/app/drive/components/NameCollisionDialog/nameCollision.actions.test.ts create mode 100644 src/app/drive/components/NameCollisionDialog/nameCollision.actions.ts diff --git a/src/app/drive/components/NameCollisionDialog/NameCollisionContainer.tsx b/src/app/drive/components/NameCollisionDialog/NameCollisionContainer.tsx index f20ebff415..e56f952d33 100644 --- a/src/app/drive/components/NameCollisionDialog/NameCollisionContainer.tsx +++ b/src/app/drive/components/NameCollisionDialog/NameCollisionContainer.tsx @@ -3,7 +3,6 @@ import NameCollisionDialog, { OnSubmitPressed } from '.'; import { moveItemsToTrash } from 'views/Trash/services'; import { RootState } from 'app/store'; import { useAppDispatch, useAppSelector } from 'app/store/hooks'; -import { storageActions } from 'app/store/slices/storage'; import storageThunks from 'app/store/slices/storage/storage.thunks'; import { fetchSortedFolderContentThunk } from 'app/store/slices/storage/storage.thunks/fetchSortedFolderContentThunk'; import { uiActions } from 'app/store/slices/ui'; @@ -15,12 +14,8 @@ import replaceFileService from 'views/Drive/services/replaceFile.service'; import { Network, getEnvironmentConfig } from 'app/drive/services/network.service'; import { fileVersionsActions, fileVersionsSelectors } from 'app/store/slices/fileVersions'; import { isVersioningExtensionAllowed } from 'views/Drive/components/VersionHistory/utils'; -import { checkFolderDuplicated } from 'app/store/slices/storage/folderUtils/checkFolderDuplicated'; -import { getUniqueFolderName } from 'app/store/slices/storage/folderUtils/getUniqueFolderName'; -import { getUniqueFilename } from 'app/store/slices/storage/fileUtils/getUniqueFilename'; -import { checkDuplicatedFiles } from 'app/store/slices/storage/fileUtils/checkDuplicatedFiles'; import { CollisionGroup } from 'app/store/slices/storage/storage.model'; -import { MoveItemPayload } from 'app/store/slices/storage/storage.thunks/moveItemsThunk'; +import { NameCollisionContext, resolveMoveCollision } from './nameCollision.actions'; const NameCollisionContainer: FC = () => { const dispatch = useAppDispatch(); @@ -37,47 +32,12 @@ const NameCollisionContainer: FC = () => { const maxUploadFileSize = useAppSelector(fileVersionsSelectors.getMaxFileSizeLimit); const isVersioningEnabled = limits?.versioning?.enabled ?? false; + const context: NameCollisionContext = { dispatch, selectedWorkspace, maxUploadFileSize, isVersioningEnabled }; + const closeDialog = () => { dispatch(uiActions.setIsNameCollisionDialogOpen({ open: false, info: undefined })); }; - const replaceAndMoveItem = async (group: CollisionGroup) => { - await moveItemsToTrash(group.existingItems); - await dispatch( - storageThunks.moveItemsThunk({ - items: group.duplicatedItems as DriveItemData[], - destinationFolderId: group.destinationUuid, - }), - ); - }; - - const keepAndMoveItem = async (group: CollisionGroup) => { - for (const item of group.duplicatedItems as DriveItemData[]) { - let itemParsed: MoveItemPayload; - - if (item.isFolder) { - const { duplicatedFoldersResponse } = await checkFolderDuplicated([item], group.destinationUuid); - const finalName = await getUniqueFolderName( - item.plainName ?? item.name, - duplicatedFoldersResponse as DriveItemData[], - group.destinationUuid, - ); - itemParsed = { ...item, name: finalName, plain_name: finalName, newItemName: finalName }; - } else { - const { duplicatedFilesResponse } = await checkDuplicatedFiles([item], group.destinationUuid); - const finalName = await getUniqueFilename(item.name, item.type, duplicatedFilesResponse, group.destinationUuid); - itemParsed = { ...item, name: finalName, plainName: finalName, plain_name: finalName, newItemName: finalName }; - } - - await dispatch( - storageThunks.moveItemsThunk({ - items: [itemParsed], - destinationFolderId: group.destinationUuid, - }), - ); - } - }; - const uploadFileAndGetFileId = async (file: File, itemToReplace: DriveItemData) => { const { bridgeUser, bridgePass, encryptionKey, bucketId } = await getEnvironmentConfig(!!selectedWorkspace); const network = new Network(bridgeUser, bridgePass, encryptionKey); @@ -156,21 +116,23 @@ const NameCollisionContainer: FC = () => { const triggerSelectedOptionsOnSubmit = async ({ operationType, operation }: OnSubmitPressed) => { for (const group of collisionGroups) { - switch (operationType + operation) { - case 'move' + 'keep': - await keepAndMoveItem(group); - dispatch(storageActions.popItemsToDelete(group.duplicatedItems as DriveItemData[])); - break; - case 'move' + 'replace': - await replaceAndMoveItem(group); - dispatch(storageActions.popItemsToDelete(group.duplicatedItems as DriveItemData[])); - break; - case 'upload' + 'keep': - await keepAndUploadItem(group); - break; - case 'upload' + 'replace': - await replaceAndUploadItem(group); - break; + if (operationType === 'move') { + await resolveMoveCollision( + { + operation, + items: group.duplicatedItems as DriveItemData[], + existingItems: group.existingItems, + destinationUuid: group.destinationUuid, + }, + context, + ); + continue; + } + + if (operation === 'keep') { + await keepAndUploadItem(group); + } else { + await replaceAndUploadItem(group); } } closeDialog(); diff --git a/src/app/drive/components/NameCollisionDialog/nameCollision.actions.test.ts b/src/app/drive/components/NameCollisionDialog/nameCollision.actions.test.ts new file mode 100644 index 0000000000..11f8c41758 --- /dev/null +++ b/src/app/drive/components/NameCollisionDialog/nameCollision.actions.test.ts @@ -0,0 +1,89 @@ +import { beforeEach, describe, expect, test, vi } from 'vitest'; +import { getDriveItemData } from 'testUtils/fixtures/drive.fixtures'; +import { NameCollisionContext, ResolveMoveCollisionParams, resolveMoveCollision } from './nameCollision.actions'; + +const mocks = vi.hoisted(() => ({ + moveItemsToTrash: vi.fn(), + moveItemsThunk: vi.fn(), + popItemsToDelete: vi.fn(), + checkDuplicatedFiles: vi.fn(), + getUniqueFilename: vi.fn(), + checkFolderDuplicated: vi.fn(), + getUniqueFolderName: vi.fn(), +})); + +vi.mock('views/Trash/services', () => ({ moveItemsToTrash: mocks.moveItemsToTrash })); +vi.mock('app/store/slices/storage/storage.thunks', () => ({ default: { moveItemsThunk: mocks.moveItemsThunk } })); +vi.mock('app/store/slices/storage', () => ({ storageActions: { popItemsToDelete: mocks.popItemsToDelete } })); +vi.mock('app/store/slices/storage/fileUtils/checkDuplicatedFiles', () => ({ + checkDuplicatedFiles: mocks.checkDuplicatedFiles, +})); +vi.mock('app/store/slices/storage/fileUtils/getUniqueFilename', () => ({ getUniqueFilename: mocks.getUniqueFilename })); +vi.mock('app/store/slices/storage/folderUtils/checkFolderDuplicated', () => ({ + checkFolderDuplicated: mocks.checkFolderDuplicated, +})); +vi.mock('app/store/slices/storage/folderUtils/getUniqueFolderName', () => ({ + getUniqueFolderName: mocks.getUniqueFolderName, +})); + +const DESTINATION = 'destination-uuid'; + +const getContext = (): NameCollisionContext => ({ + dispatch: vi.fn() as unknown as NameCollisionContext['dispatch'], + selectedWorkspace: null, + maxUploadFileSize: 5000, + isVersioningEnabled: false, +}); + +const resolve = (params: Omit) => + resolveMoveCollision({ ...params, destinationUuid: DESTINATION }, getContext()); + +/** + * Mocks are reset by hand because the browser test project does not do it between tests. + */ +beforeEach(() => { + vi.resetAllMocks(); + mocks.checkDuplicatedFiles.mockResolvedValue({ duplicatedFilesResponse: ['file-dup'] }); + mocks.getUniqueFilename.mockResolvedValue('report (1)'); + mocks.checkFolderDuplicated.mockResolvedValue({ duplicatedFoldersResponse: ['folder-dup'] }); + mocks.getUniqueFolderName.mockResolvedValue('Photos (1)'); +}); + +describe('resolveMoveCollision', () => { + test('when keeping both, then each item is moved under a unique name and leaves the pending deletion list', async () => { + const file = getDriveItemData({ plainName: 'report', name: 'report', type: 'pdf', isFolder: false }); + const folder = getDriveItemData({ plainName: 'Photos', name: 'Photos', isFolder: true }); + + await resolve({ operation: 'keep', items: [file, folder], existingItems: [] }); + + expect(mocks.getUniqueFilename).toHaveBeenCalledWith('report', 'pdf', ['file-dup'], DESTINATION); + expect(mocks.getUniqueFolderName).toHaveBeenCalledWith('Photos', ['folder-dup'], DESTINATION); + expect(mocks.moveItemsThunk).toHaveBeenNthCalledWith(1, { + items: [ + { ...file, name: 'report (1)', plainName: 'report (1)', plain_name: 'report (1)', newItemName: 'report (1)' }, + ], + destinationFolderId: DESTINATION, + }); + expect(mocks.moveItemsThunk).toHaveBeenNthCalledWith(2, { + items: [{ ...folder, name: 'Photos (1)', plain_name: 'Photos (1)', newItemName: 'Photos (1)' }], + destinationFolderId: DESTINATION, + }); + expect(mocks.moveItemsToTrash).not.toHaveBeenCalled(); + expect(mocks.popItemsToDelete).toHaveBeenCalledWith([file, folder]); + }); + + test('when replacing, then existing items are trashed before moving and moved items leave the pending deletion list', async () => { + const item = getDriveItemData({ uuid: 'new' }); + const existing = getDriveItemData({ uuid: 'existing' }); + const callOrder: string[] = []; + mocks.moveItemsToTrash.mockImplementation(async () => callOrder.push('trash')); + mocks.moveItemsThunk.mockImplementation(() => callOrder.push('move')); + + await resolve({ operation: 'replace', items: [item], existingItems: [existing] }); + + expect(mocks.moveItemsToTrash).toHaveBeenCalledWith([existing]); + expect(mocks.moveItemsThunk).toHaveBeenCalledWith({ items: [item], destinationFolderId: DESTINATION }); + expect(callOrder).toEqual(['trash', 'move']); + expect(mocks.popItemsToDelete).toHaveBeenCalledWith([item]); + }); +}); diff --git a/src/app/drive/components/NameCollisionDialog/nameCollision.actions.ts b/src/app/drive/components/NameCollisionDialog/nameCollision.actions.ts new file mode 100644 index 0000000000..e4cdd27cb9 --- /dev/null +++ b/src/app/drive/components/NameCollisionDialog/nameCollision.actions.ts @@ -0,0 +1,86 @@ +import { WorkspaceData } from '@internxt/sdk/dist/workspaces'; +import { DriveItemData } from 'app/drive/types'; +import { AppDispatch } from 'app/store'; +import { storageActions } from 'app/store/slices/storage'; +import { checkDuplicatedFiles } from 'app/store/slices/storage/fileUtils/checkDuplicatedFiles'; +import { getUniqueFilename } from 'app/store/slices/storage/fileUtils/getUniqueFilename'; +import { checkFolderDuplicated } from 'app/store/slices/storage/folderUtils/checkFolderDuplicated'; +import { getUniqueFolderName } from 'app/store/slices/storage/folderUtils/getUniqueFolderName'; +import storageThunks from 'app/store/slices/storage/storage.thunks'; +import { MoveItemPayload } from 'app/store/slices/storage/storage.thunks/moveItemsThunk'; +import { moveItemsToTrash } from 'views/Trash/services'; + +export type CollisionOperation = 'keep' | 'replace'; + +export interface NameCollisionContext { + dispatch: AppDispatch; + selectedWorkspace: WorkspaceData | null; + maxUploadFileSize: number; + isVersioningEnabled: boolean; +} + +export interface ResolveMoveCollisionParams { + operation: CollisionOperation; + items: DriveItemData[]; + existingItems: DriveItemData[]; + destinationUuid: string; +} + +const getUniqueNameMovePayload = async (item: DriveItemData, destinationUuid: string): Promise => { + if (item.isFolder) { + const { duplicatedFoldersResponse } = await checkFolderDuplicated([item], destinationUuid); + const finalName = await getUniqueFolderName( + item.plainName ?? item.name, + duplicatedFoldersResponse as DriveItemData[], + destinationUuid, + ); + return { ...item, name: finalName, plain_name: finalName, newItemName: finalName }; + } + + const { duplicatedFilesResponse } = await checkDuplicatedFiles([item], destinationUuid); + const finalName = await getUniqueFilename(item.name, item.type, duplicatedFilesResponse, destinationUuid); + return { ...item, name: finalName, plainName: finalName, plain_name: finalName, newItemName: finalName }; +}; + +/** + * Moves each item to the destination under a name that does not collide with anything there. + */ +const keepAndMoveItems = async ( + items: DriveItemData[], + destinationUuid: string, + { dispatch }: NameCollisionContext, +) => { + for (const item of items) { + const itemParsed = await getUniqueNameMovePayload(item, destinationUuid); + await dispatch(storageThunks.moveItemsThunk({ items: [itemParsed], destinationFolderId: destinationUuid })); + } +}; + +/** + * Trashes the colliding drive items and then moves the incoming ones into their place. + */ +const replaceAndMoveItems = async ( + items: DriveItemData[], + existingItems: DriveItemData[], + destinationUuid: string, + { dispatch }: NameCollisionContext, +) => { + await moveItemsToTrash(existingItems); + await dispatch(storageThunks.moveItemsThunk({ items, destinationFolderId: destinationUuid })); +}; + +/** + * Applies the chosen resolution to items that collide while being moved, then removes them + * from the pending-deletion list. + */ +export const resolveMoveCollision = async ( + { operation, items, existingItems, destinationUuid }: ResolveMoveCollisionParams, + context: NameCollisionContext, +): Promise => { + if (operation === 'keep') { + await keepAndMoveItems(items, destinationUuid, context); + } else { + await replaceAndMoveItems(items, existingItems, destinationUuid, context); + } + context.dispatch(storageActions.popItemsToDelete(items)); +}; From 6f45d6038123424c0822aa2780d0c642cf0160e8 Mon Sep 17 00:00:00 2001 From: Francis Terrero Date: Mon, 14 Sep 2026 22:47:52 -0400 Subject: [PATCH 02/20] refactor: update mocks and dispatch actions in name collision tests --- .../nameCollision.actions.test.ts | 43 +++++++++++++------ 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/src/app/drive/components/NameCollisionDialog/nameCollision.actions.test.ts b/src/app/drive/components/NameCollisionDialog/nameCollision.actions.test.ts index 11f8c41758..1b9c034696 100644 --- a/src/app/drive/components/NameCollisionDialog/nameCollision.actions.test.ts +++ b/src/app/drive/components/NameCollisionDialog/nameCollision.actions.test.ts @@ -3,6 +3,7 @@ import { getDriveItemData } from 'testUtils/fixtures/drive.fixtures'; import { NameCollisionContext, ResolveMoveCollisionParams, resolveMoveCollision } from './nameCollision.actions'; const mocks = vi.hoisted(() => ({ + dispatch: vi.fn(), moveItemsToTrash: vi.fn(), moveItemsThunk: vi.fn(), popItemsToDelete: vi.fn(), @@ -28,8 +29,11 @@ vi.mock('app/store/slices/storage/folderUtils/getUniqueFolderName', () => ({ const DESTINATION = 'destination-uuid'; +const asMoveAction = (payload: unknown) => ({ type: 'move', payload }); +const asPopAction = (payload: unknown) => ({ type: 'pop', payload }); + const getContext = (): NameCollisionContext => ({ - dispatch: vi.fn() as unknown as NameCollisionContext['dispatch'], + dispatch: mocks.dispatch as unknown as NameCollisionContext['dispatch'], selectedWorkspace: null, maxUploadFileSize: 5000, isVersioningEnabled: false, @@ -47,29 +51,35 @@ beforeEach(() => { mocks.getUniqueFilename.mockResolvedValue('report (1)'); mocks.checkFolderDuplicated.mockResolvedValue({ duplicatedFoldersResponse: ['folder-dup'] }); mocks.getUniqueFolderName.mockResolvedValue('Photos (1)'); + mocks.moveItemsThunk.mockImplementation(asMoveAction); + mocks.popItemsToDelete.mockImplementation(asPopAction); }); describe('resolveMoveCollision', () => { test('when keeping both, then each item is moved under a unique name and leaves the pending deletion list', async () => { const file = getDriveItemData({ plainName: 'report', name: 'report', type: 'pdf', isFolder: false }); const folder = getDriveItemData({ plainName: 'Photos', name: 'Photos', isFolder: true }); - - await resolve({ operation: 'keep', items: [file, folder], existingItems: [] }); - - expect(mocks.getUniqueFilename).toHaveBeenCalledWith('report', 'pdf', ['file-dup'], DESTINATION); - expect(mocks.getUniqueFolderName).toHaveBeenCalledWith('Photos', ['folder-dup'], DESTINATION); - expect(mocks.moveItemsThunk).toHaveBeenNthCalledWith(1, { + const renamedFileMove = { items: [ { ...file, name: 'report (1)', plainName: 'report (1)', plain_name: 'report (1)', newItemName: 'report (1)' }, ], destinationFolderId: DESTINATION, - }); - expect(mocks.moveItemsThunk).toHaveBeenNthCalledWith(2, { + }; + const renamedFolderMove = { items: [{ ...folder, name: 'Photos (1)', plain_name: 'Photos (1)', newItemName: 'Photos (1)' }], destinationFolderId: DESTINATION, - }); + }; + + await resolve({ operation: 'keep', items: [file, folder], existingItems: [] }); + + expect(mocks.getUniqueFilename).toHaveBeenCalledWith('report', 'pdf', ['file-dup'], DESTINATION); + expect(mocks.getUniqueFolderName).toHaveBeenCalledWith('Photos', ['folder-dup'], DESTINATION); expect(mocks.moveItemsToTrash).not.toHaveBeenCalled(); - expect(mocks.popItemsToDelete).toHaveBeenCalledWith([file, folder]); + expect(mocks.dispatch.mock.calls).toEqual([ + [asMoveAction(renamedFileMove)], + [asMoveAction(renamedFolderMove)], + [asPopAction([file, folder])], + ]); }); test('when replacing, then existing items are trashed before moving and moved items leave the pending deletion list', async () => { @@ -77,13 +87,18 @@ describe('resolveMoveCollision', () => { const existing = getDriveItemData({ uuid: 'existing' }); const callOrder: string[] = []; mocks.moveItemsToTrash.mockImplementation(async () => callOrder.push('trash')); - mocks.moveItemsThunk.mockImplementation(() => callOrder.push('move')); + mocks.moveItemsThunk.mockImplementation((payload: unknown) => { + callOrder.push('move'); + return asMoveAction(payload); + }); await resolve({ operation: 'replace', items: [item], existingItems: [existing] }); expect(mocks.moveItemsToTrash).toHaveBeenCalledWith([existing]); - expect(mocks.moveItemsThunk).toHaveBeenCalledWith({ items: [item], destinationFolderId: DESTINATION }); expect(callOrder).toEqual(['trash', 'move']); - expect(mocks.popItemsToDelete).toHaveBeenCalledWith([item]); + expect(mocks.dispatch.mock.calls).toEqual([ + [asMoveAction({ items: [item], destinationFolderId: DESTINATION })], + [asPopAction([item])], + ]); }); }); From dbd2971259265e1a980854da428632e863233e46 Mon Sep 17 00:00:00 2001 From: Francis Terrero Date: Thu, 10 Sep 2026 23:37:45 -0400 Subject: [PATCH 03/20] fix: match name collisions by name and type and batch their resolution Duplicated items were matched to their existing counterpart by array index, which could make Replace trash the wrong file. They are now paired by name and type (or name only for folders), and each resolution goes through the array-based primitives (moveItemsToTrash, moveItemsThunk, upload managers) so batching, concurrency limits and per-item error handling are inherited instead of reimplemented per item. --- .../nameCollision.actions.test.ts | 137 +++++++++----- .../nameCollision.actions.ts | 167 +++++++++++------- .../nameCollision.utils.test.ts | 71 ++++++++ .../nameCollision.utils.ts | 39 ++++ 4 files changed, 310 insertions(+), 104 deletions(-) create mode 100644 src/app/drive/components/NameCollisionDialog/nameCollision.utils.test.ts diff --git a/src/app/drive/components/NameCollisionDialog/nameCollision.actions.test.ts b/src/app/drive/components/NameCollisionDialog/nameCollision.actions.test.ts index 72ef91a22e..b79c540871 100644 --- a/src/app/drive/components/NameCollisionDialog/nameCollision.actions.test.ts +++ b/src/app/drive/components/NameCollisionDialog/nameCollision.actions.test.ts @@ -100,35 +100,45 @@ beforeEach(() => { }); describe('resolveCollision', () => { - test('when moving with keep, then each item is moved under a unique name and leaves the pending deletion list', async () => { + test.each<[ResolveCollisionParams['operationType'], ResolveCollisionParams['operation'], unknown[][]]>([ + ['move', 'keep', [[asPopAction([])]]], + ['move', 'replace', [[asPopAction([])]]], + ['upload', 'keep', []], + ['upload', 'replace', []], + ])( + 'when %s + %s gets no items, then nothing is moved, trashed or uploaded', + async (operationType, operation, expectedDispatchCalls) => { + await resolve({ operationType, operation, items: [], existingItems: [] }); + + expect(mocks.moveItemsToTrash).not.toHaveBeenCalled(); + expect(mocks.uploadFoldersWithTracking).not.toHaveBeenCalled(); + expect(mocks.dispatch.mock.calls).toEqual(expectedDispatchCalls); + }, + ); + + test('when moving with keep, then items get a unique name and leave the pending deletion list', async () => { const file = getDriveItemData({ plainName: 'report', name: 'report', type: 'pdf', isFolder: false }); const folder = getDriveItemData({ plainName: 'Photos', name: 'Photos', isFolder: true }); - const renamedFileMove = { + const renamedMove = { items: [ { ...file, name: 'report (1)', plainName: 'report (1)', plain_name: 'report (1)', newItemName: 'report (1)' }, + { ...folder, name: 'Photos (1)', plain_name: 'Photos (1)', newItemName: 'Photos (1)' }, ], destinationFolderId: DESTINATION, }; - const renamedFolderMove = { - items: [{ ...folder, name: 'Photos (1)', plain_name: 'Photos (1)', newItemName: 'Photos (1)' }], - destinationFolderId: DESTINATION, - }; await resolve({ operationType: 'move', operation: 'keep', items: [file, folder], existingItems: [] }); expect(mocks.getUniqueFilename).toHaveBeenCalledWith('report', 'pdf', ['file-dup'], DESTINATION); expect(mocks.getUniqueFolderName).toHaveBeenCalledWith('Photos', ['folder-dup'], DESTINATION); expect(mocks.moveItemsToTrash).not.toHaveBeenCalled(); - expect(mocks.dispatch.mock.calls).toEqual([ - [asMoveAction(renamedFileMove)], - [asMoveAction(renamedFolderMove)], - [asPopAction([file, folder])], - ]); + expect(mocks.dispatch.mock.calls).toEqual([[asMoveAction(renamedMove)], [asPopAction([file, folder])]]); }); - test('when moving with replace, then existing items are trashed before moving and moved items leave the pending deletion list', async () => { - const item = getDriveItemData({ uuid: 'new' }); - const existing = getDriveItemData({ uuid: 'existing' }); + test('when moving with replace, then matched existing items are trashed before moving and every moved item leaves the pending deletion list', async () => { + const matched = getDriveItemData({ uuid: 'matched', plainName: 'report', type: 'pdf' }); + const unmatched = getDriveItemData({ uuid: 'unmatched', plainName: 'other', type: 'pdf' }); + const existing = getDriveItemData({ uuid: 'existing', plainName: 'report', type: 'pdf' }); const callOrder: string[] = []; mocks.moveItemsToTrash.mockImplementation(async () => callOrder.push('trash')); mocks.moveItemsThunk.mockImplementation((payload: unknown) => { @@ -136,13 +146,18 @@ describe('resolveCollision', () => { return asMoveAction(payload); }); - await resolve({ operationType: 'move', operation: 'replace', items: [item], existingItems: [existing] }); + await resolve({ + operationType: 'move', + operation: 'replace', + items: [matched, unmatched], + existingItems: [existing], + }); expect(mocks.moveItemsToTrash).toHaveBeenCalledWith([existing]); expect(callOrder).toEqual(['trash', 'move']); expect(mocks.dispatch.mock.calls).toEqual([ - [asMoveAction({ items: [item], destinationFolderId: DESTINATION })], - [asPopAction([item])], + [asMoveAction({ items: [matched], destinationFolderId: DESTINATION })], + [asPopAction([matched, unmatched])], ]); }); @@ -160,56 +175,67 @@ describe('resolveCollision', () => { maxUploadFileSize: 123, }); expect(mocks.dispatch.mock.calls).toEqual([ - [asUploadAction({ files: [file], parentFolderId: DESTINATION, options: undefined })], - [asRefreshAction(DESTINATION)], + [asUploadAction({ files: [file], parentFolderId: DESTINATION, options: { disableDuplicatedNamesCheck: false } })], [asRefreshAction(DESTINATION)], ]); expect(mocks.popItemsToDelete).not.toHaveBeenCalled(); }); - test('when uploading with replace and versioning is off, then each existing item is trashed and the new one is uploaded without the duplicates check', async () => { - const file = new File(['content'], 'report.pdf'); - const root = getRoot(); - const existingFile = getDriveItemData({ uuid: 'existing-file', type: 'pdf' }); - const existingFolder = getDriveItemData({ uuid: 'existing-folder', isFolder: true }); + test('when uploading with replace and versioning is off, then only matched existing items are trashed and re-uploaded without the duplicates check', async () => { + const matched = new File(['content'], 'report.pdf'); + const unmatched = new File(['content'], 'other.txt'); + const existing = getDriveItemData({ uuid: 'existing', plainName: 'report', type: 'pdf' }); await resolve({ operationType: 'upload', operation: 'replace', - items: [file, root], - existingItems: [existingFile, existingFolder], + items: [matched, unmatched], + existingItems: [existing], }); - expect(mocks.moveItemsToTrash).toHaveBeenNthCalledWith(1, [existingFile]); - expect(mocks.moveItemsToTrash).toHaveBeenNthCalledWith(2, [existingFolder]); - expect(mocks.uploadFoldersWithTracking).toHaveBeenCalledWith( - expect.objectContaining({ payload: [{ root: { ...root }, currentFolderId: DESTINATION }] }), - ); + expect(mocks.moveItemsToTrash).toHaveBeenCalledWith([existing]); + expect(mocks.uploadFoldersWithTracking).not.toHaveBeenCalled(); expect(mocks.networkUploadFile).not.toHaveBeenCalled(); expect(mocks.dispatch.mock.calls).toEqual([ - [asUploadAction({ files: [file], parentFolderId: DESTINATION, options: { disableDuplicatedNamesCheck: true } })], - [asRefreshAction(DESTINATION)], + [ + asUploadAction({ + files: [matched], + parentFolderId: DESTINATION, + options: { disableDuplicatedNamesCheck: true }, + }), + ], [asRefreshAction(DESTINATION)], ]); }); - test('when uploading with replace and versioning is on, then allowed extensions become a new version while the rest are trashed and re-uploaded', async () => { + test('when uploading with replace and versioning is on, then allowed extensions become a new version while folders and other files are trashed and re-uploaded', async () => { const context = getContext({ isVersioningEnabled: true, selectedWorkspace: { id: 'ws' } as never }); const pdf = new File(['content'], 'report.pdf'); const image = new File(['content'], 'photo.png'); - const existingPdf = getDriveItemData({ uuid: 'existing-pdf', type: 'pdf' }); - const existingImage = getDriveItemData({ uuid: 'existing-png', type: 'png' }); + const root = getRoot(); + const existingPdf = getDriveItemData({ uuid: 'existing-pdf', plainName: 'report', type: 'pdf' }); + const existingImage = getDriveItemData({ uuid: 'existing-png', plainName: 'photo', type: 'png' }); + const existingFolder = getDriveItemData({ + uuid: 'existing-folder', + plainName: 'Photos', + isFolder: true, + type: 'pdf', + }); await resolve( { operationType: 'upload', operation: 'replace', - items: [pdf, image], - existingItems: [existingPdf, existingImage], + items: [pdf, image, root], + existingItems: [existingPdf, existingImage, existingFolder], }, context, ); + expect(mocks.moveItemsToTrash).toHaveBeenCalledWith([existingImage, existingFolder]); + expect(mocks.uploadFoldersWithTracking).toHaveBeenCalledWith( + expect.objectContaining({ payload: [{ root: { ...root }, currentFolderId: DESTINATION }] }), + ); expect(mocks.getEnvironmentConfig).toHaveBeenCalledWith(true); expect(mocks.networkUploadFile).toHaveBeenCalledWith( 'b', @@ -217,11 +243,42 @@ describe('resolveCollision', () => { { taskId: expect.stringMatching(/^replace-existing-pdf-\d+$/) }, ); expect(mocks.replaceFile).toHaveBeenCalledWith('existing-pdf', { fileId: 'new-file-id', size: pdf.size }); - expect(mocks.moveItemsToTrash).toHaveBeenCalledWith([existingImage]); expect(mocks.dispatch.mock.calls).toEqual([ + [asUploadAction(expect.objectContaining({ files: [image], options: { disableDuplicatedNamesCheck: true } }))], [asInvalidateCacheAction('existing-pdf')], [asRefreshAction(DESTINATION)], - [asUploadAction(expect.objectContaining({ files: [image] }))], + ]); + }); + + test('when several files are versioned, then they are replaced one at a time', async () => { + let inFlight = 0; + let maxInFlight = 0; + mocks.replaceFile.mockImplementation(async () => { + inFlight += 1; + maxInFlight = Math.max(maxInFlight, inFlight); + await Promise.resolve(); + inFlight -= 1; + }); + + await resolve( + { + operationType: 'upload', + operation: 'replace', + items: [new File(['a'], 'a.pdf'), new File(['b'], 'b.pdf')], + existingItems: [ + getDriveItemData({ uuid: 'a', plainName: 'a', type: 'pdf' }), + getDriveItemData({ uuid: 'b', plainName: 'b', type: 'pdf' }), + ], + }, + getContext({ isVersioningEnabled: true }), + ); + + expect(mocks.replaceFile).toHaveBeenCalledTimes(2); + expect(maxInFlight).toBe(1); + expect(mocks.moveItemsToTrash).not.toHaveBeenCalled(); + expect(mocks.dispatch.mock.calls).toEqual([ + [asInvalidateCacheAction('a')], + [asInvalidateCacheAction('b')], [asRefreshAction(DESTINATION)], ]); }); diff --git a/src/app/drive/components/NameCollisionDialog/nameCollision.actions.ts b/src/app/drive/components/NameCollisionDialog/nameCollision.actions.ts index 22ec21ca35..0edb613c13 100644 --- a/src/app/drive/components/NameCollisionDialog/nameCollision.actions.ts +++ b/src/app/drive/components/NameCollisionDialog/nameCollision.actions.ts @@ -16,7 +16,7 @@ import { IRoot } from 'app/store/slices/storage/types'; import { isVersioningExtensionAllowed } from 'views/Drive/components/VersionHistory/utils'; import replaceFileService from 'views/Drive/services/replaceFile.service'; import { moveItemsToTrash } from 'views/Trash/services'; -import { CollisionItem, CollisionPair, isFolderUpload } from './nameCollision.utils'; +import { CollisionItem, CollisionPair, getCollisionPairs, isFolderUpload } from './nameCollision.utils'; export type CollisionOperationType = 'move' | 'upload'; export type CollisionOperation = 'keep' | 'replace'; @@ -58,30 +58,36 @@ const getUniqueNameMovePayload = async (item: DriveItemData, destinationUuid: st }; /** - * Moves each item to the destination under a name that does not collide with anything there. + * Moves the items to the destination under a name that does not collide with anything there. */ const keepAndMoveItems = async ( items: DriveItemData[], destinationUuid: string, { dispatch }: NameCollisionContext, -) => { - for (const item of items) { - const itemParsed = await getUniqueNameMovePayload(item, destinationUuid); - await dispatch(storageThunks.moveItemsThunk({ items: [itemParsed], destinationFolderId: destinationUuid })); - } +): Promise => { + if (items.length === 0) return; + + const itemsParsed = await Promise.all(items.map((item) => getUniqueNameMovePayload(item, destinationUuid))); + await dispatch(storageThunks.moveItemsThunk({ items: itemsParsed, destinationFolderId: destinationUuid })); }; /** * Trashes the colliding drive items and then moves the incoming ones into their place. */ const replaceAndMoveItems = async ( - items: DriveItemData[], - existingItems: DriveItemData[], + pairs: CollisionPair[], destinationUuid: string, { dispatch }: NameCollisionContext, -) => { - await moveItemsToTrash(existingItems); - await dispatch(storageThunks.moveItemsThunk({ items, destinationFolderId: destinationUuid })); +): Promise => { + if (pairs.length === 0) return; + + await moveItemsToTrash(pairs.map((pair) => pair.existing)); + await dispatch( + storageThunks.moveItemsThunk({ + items: pairs.map((pair) => pair.item), + destinationFolderId: destinationUuid, + }), + ); }; /** @@ -95,7 +101,7 @@ const resolveMoveCollision = async ( if (operation === 'keep') { await keepAndMoveItems(items, destinationUuid, context); } else { - await replaceAndMoveItems(items, existingItems, destinationUuid, context); + await replaceAndMoveItems(getCollisionPairs(items, existingItems), destinationUuid, context); } context.dispatch(storageActions.popItemsToDelete(items)); }; @@ -122,84 +128,119 @@ const replaceFileVersion = async (file: File, itemToReplace: DriveItemData, cont context.dispatch(fileVersionsActions.invalidateCache(itemToReplace.uuid)); }; -const uploadFolder = async ( - root: IRoot, +const uploadFiles = async ( + files: File[], + destinationUuid: string, + { dispatch }: NameCollisionContext, + shouldSkipDuplicatesCheck = false, +) => { + if (files.length === 0) return; + + await dispatch( + storageThunks.uploadItemsThunk({ + files, + parentFolderId: destinationUuid, + options: { disableDuplicatedNamesCheck: shouldSkipDuplicatesCheck }, + }), + ); +}; + +const uploadFolders = async ( + folders: IRoot[], destinationUuid: string, { dispatch, selectedWorkspace, maxUploadFileSize }: NameCollisionContext, -) => - uploadFoldersWithTracking({ - payload: [{ root: { ...root }, currentFolderId: destinationUuid }], +) => { + if (folders.length === 0) return; + + await uploadFoldersWithTracking({ + payload: folders.map((root) => ({ root: { ...root }, currentFolderId: destinationUuid })), selectedWorkspace, dispatch, maxUploadFileSize, }); +}; -const uploadFile = async ( - file: File, +const uploadItems = async ( + items: (IRoot | File)[], destinationUuid: string, - { dispatch }: NameCollisionContext, + context: NameCollisionContext, shouldSkipDuplicatesCheck = false, -) => - dispatch( - storageThunks.uploadItemsThunk({ - files: [file], - parentFolderId: destinationUuid, - options: shouldSkipDuplicatesCheck ? { disableDuplicatedNamesCheck: true } : undefined, - }), - ); +) => { + const folders = items.filter(isFolderUpload); + const files = items.filter((item): item is File => !isFolderUpload(item)); + + await uploadFolders(folders, destinationUuid, context); + await uploadFiles(files, destinationUuid, context, shouldSkipDuplicatesCheck); +}; -const canReplaceVersion = (pair: CollisionPair, { isVersioningEnabled }: NameCollisionContext) => +const isVersionedFilePair = (pair: CollisionPair, { isVersioningEnabled }: NameCollisionContext) => !isFolderUpload(pair.item) && isVersioningEnabled && isVersioningExtensionAllowed(pair.existing); -const trashAndUpload = async ( - pair: CollisionPair, +/** + * Versioned files are replaced one at a time because that upload bypasses the upload queue. + */ +const replaceFileVersions = async (pairs: CollisionPair[], context: NameCollisionContext) => { + for (const pair of pairs) { + await replaceFileVersion(pair.item as File, pair.existing, context); + } +}; + +const trashAndUploadItems = async ( + pairs: CollisionPair[], destinationUuid: string, context: NameCollisionContext, ) => { - await moveItemsToTrash([pair.existing]); - if (isFolderUpload(pair.item)) { - await uploadFolder(pair.item, destinationUuid, context); - } else { - await uploadFile(pair.item, destinationUuid, context, true); - } + if (pairs.length === 0) return; + + await moveItemsToTrash(pairs.map((pair) => pair.existing)); + await uploadItems( + pairs.map((pair) => pair.item), + destinationUuid, + context, + true, + ); }; /** - * Replaces each colliding drive item with the uploaded one. Files whose extension supports + * Replaces the colliding drive items with the uploaded ones. Files whose extension supports * versioning become a new version of the existing file; everything else is trashed and re-uploaded. */ const replaceAndUploadItems = async ( pairs: CollisionPair[], destinationUuid: string, context: NameCollisionContext, -) => { - for (const pair of pairs) { - if (canReplaceVersion(pair, context)) { - await replaceFileVersion(pair.item as File, pair.existing, context); - } else { - await trashAndUpload(pair, destinationUuid, context); - } - context.dispatch(fetchSortedFolderContentThunk(destinationUuid)); - } +): Promise => { + if (pairs.length === 0) return; + + await trashAndUploadItems( + pairs.filter((pair) => !isVersionedFilePair(pair, context)), + destinationUuid, + context, + ); + await replaceFileVersions( + pairs.filter((pair) => isVersionedFilePair(pair, context)), + context, + ); + + context.dispatch(fetchSortedFolderContentThunk(destinationUuid)); }; /** - * Uploads each item next to the existing one, letting the upload flow pick a unique name. + * Uploads the items next to the existing ones, letting the upload flow pick a unique name. */ -const keepAndUploadItems = async (items: (IRoot | File)[], destinationUuid: string, context: NameCollisionContext) => { - for (const item of items) { - if (isFolderUpload(item)) { - await uploadFolder(item, destinationUuid, context); - } else { - await uploadFile(item, destinationUuid, context); - } - context.dispatch(fetchSortedFolderContentThunk(destinationUuid)); - } +const keepAndUploadItems = async ( + items: (IRoot | File)[], + destinationUuid: string, + context: NameCollisionContext, +): Promise => { + if (items.length === 0) return; + + await uploadItems(items, destinationUuid, context); + context.dispatch(fetchSortedFolderContentThunk(destinationUuid)); }; /** - * Applies the chosen resolution to items that collide while being uploaded. Existing items are - * matched to the uploaded ones by position. + * Applies the chosen resolution to items that collide while being uploaded. */ const resolveUploadCollision = async ( { operation, items, existingItems, destinationUuid }: ResolveUploadCollisionParams, @@ -207,11 +248,9 @@ const resolveUploadCollision = async ( ) => { if (operation === 'keep') { await keepAndUploadItems(items, destinationUuid, context); - return; + } else { + await replaceAndUploadItems(getCollisionPairs(items, existingItems), destinationUuid, context); } - - const pairs = items.map((item, index) => ({ item, existing: existingItems[index] })); - await replaceAndUploadItems(pairs, destinationUuid, context); }; /** diff --git a/src/app/drive/components/NameCollisionDialog/nameCollision.utils.test.ts b/src/app/drive/components/NameCollisionDialog/nameCollision.utils.test.ts new file mode 100644 index 0000000000..ce5640d47b --- /dev/null +++ b/src/app/drive/components/NameCollisionDialog/nameCollision.utils.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, test } from 'vitest'; +import { getDriveItemData } from 'testUtils/fixtures/drive.fixtures'; +import { IRoot } from 'app/store/slices/storage/types'; +import { CollisionItem, findExistingItemFor, getCollisionPairs, isFolderUpload } from './nameCollision.utils'; + +const getRoot = (name = 'Photos'): IRoot => ({ + name, + folderId: null, + childrenFiles: [], + childrenFolders: [], + fullPathEdited: `/${name}`, +}); + +const existingFile = getDriveItemData({ uuid: 'file', plainName: 'report', type: 'pdf', isFolder: false }); +const existingFolder = getDriveItemData({ uuid: 'folder', plainName: 'report', type: null as never, isFolder: true }); +const existingNamedByName = getDriveItemData({ + uuid: 'legacy', + name: 'README', + plainName: undefined, + type: null as never, +}); +const existingItems = [existingFile, existingFolder, existingNamedByName]; + +describe('isFolderUpload', () => { + test('when the item has an edited full path, then it is an uploaded folder', () => { + expect(isFolderUpload(getRoot())).toBe(true); + expect(isFolderUpload(new File([''], 'notes.txt'))).toBe(false); + expect(isFolderUpload(getDriveItemData({ isFolder: true }))).toBe(false); + }); +}); + +describe('findExistingItemFor', () => { + test.each<[string, CollisionItem, ReturnType | undefined]>([ + ['an uploaded folder with the same name', getRoot('report'), existingFile], + ['an uploaded file with the same name and extension', new File([''], 'report.pdf'), existingFile], + ['an uploaded file with the same name but another extension', new File([''], 'report.docx'), undefined], + [ + 'an uploaded file without extension against a legacy item named by name', + new File([''], 'README'), + existingNamedByName, + ], + ['a moved file with the same name and type', getDriveItemData({ plainName: 'report', type: 'pdf' }), existingFile], + [ + 'a moved file with the same name but another type', + getDriveItemData({ plainName: 'report', type: 'docx' }), + undefined, + ], + [ + 'a moved folder with the same name regardless of type', + getDriveItemData({ plainName: 'report', type: 'x', isFolder: true }), + existingFolder, + ], + [ + 'a moved item without plainName', + getDriveItemData({ name: 'README', plainName: undefined, type: null as never }), + existingNamedByName, + ], + ])('when given %s, then the matching item is returned', (_, item, expected) => { + expect(findExistingItemFor(item, existingItems)).toBe(expected); + }); +}); + +describe('getCollisionPairs', () => { + test('when only some items collide, then only those are paired with their existing item', () => { + const matched = new File([''], 'report.pdf'); + + expect(getCollisionPairs([matched, new File([''], 'unknown.txt')], existingItems)).toEqual([ + { item: matched, existing: existingFile }, + ]); + }); +}); diff --git a/src/app/drive/components/NameCollisionDialog/nameCollision.utils.ts b/src/app/drive/components/NameCollisionDialog/nameCollision.utils.ts index 23e36a3bc2..36310d963f 100644 --- a/src/app/drive/components/NameCollisionDialog/nameCollision.utils.ts +++ b/src/app/drive/components/NameCollisionDialog/nameCollision.utils.ts @@ -1,3 +1,4 @@ +import { items as itemUtils } from '@internxt/lib'; import { DriveItemData } from 'app/drive/types'; import { IRoot } from 'app/store/slices/storage/types'; @@ -6,3 +7,41 @@ export type CollisionItem = File | IRoot | DriveItemData; export type CollisionPair = { item: T; existing: DriveItemData }; export const isFolderUpload = (item: CollisionItem): item is IRoot => !!(item as IRoot).fullPathEdited; + +const matchesName = (existing: DriveItemData, name: string) => (existing.plainName ?? existing.name) === name; + +const matchesType = (existing: DriveItemData, type?: string | null) => (existing.type ?? null) === (type ?? null); + +/** + * Finds the drive item that collides with the given item. Uploaded folders match by name only, + * uploaded files by name and extension, and moved items by kind, name and (for files) extension. + */ +export const findExistingItemFor = (item: CollisionItem, existingItems: DriveItemData[]): DriveItemData | undefined => { + if (isFolderUpload(item)) { + return existingItems.find((existing) => matchesName(existing, item.name)); + } + + if (item instanceof File) { + const { filename, extension } = itemUtils.getFilenameAndExt(item.name); + return existingItems.find((existing) => matchesName(existing, filename) && matchesType(existing, extension)); + } + + return existingItems.find( + (existing) => + !!existing.isFolder === !!item.isFolder && + matchesName(existing, item.plainName ?? item.name) && + (item.isFolder || matchesType(existing, item.type)), + ); +}; + +/** + * Pairs each item with the drive item it collides with, dropping items that have no match. + */ +export const getCollisionPairs = ( + items: T[], + existingItems: DriveItemData[], +): CollisionPair[] => + items.flatMap((item) => { + const existing = findExistingItemFor(item, existingItems); + return existing ? [{ item, existing }] : []; + }); From 71f6dcc2be6c83cda69bf4ede960eeea4486536b Mon Sep 17 00:00:00 2001 From: Francis Terrero Date: Fri, 11 Sep 2026 00:41:41 -0400 Subject: [PATCH 04/20] fix: tag colliding existing items with isFolder so folders and files never cross-match The duplicate checks return raw API items that carry no isFolder, so the name matcher could never pair a moved folder with its existing folder and could pair an uploaded folder with a file of the same name. Existing items are now tagged when the collision groups are built, and the matcher only pairs folders with folders and files with files. --- .../nameCollision.utils.test.ts | 3 ++- .../nameCollision.utils.ts | 11 +++++---- .../storage.thunks/renameItemsThunk.test.ts | 17 ++++++++------ .../storage.thunks/renameItemsThunk.ts | 23 ++++++++++++------- 4 files changed, 34 insertions(+), 20 deletions(-) diff --git a/src/app/drive/components/NameCollisionDialog/nameCollision.utils.test.ts b/src/app/drive/components/NameCollisionDialog/nameCollision.utils.test.ts index ce5640d47b..75d246655d 100644 --- a/src/app/drive/components/NameCollisionDialog/nameCollision.utils.test.ts +++ b/src/app/drive/components/NameCollisionDialog/nameCollision.utils.test.ts @@ -31,7 +31,8 @@ describe('isFolderUpload', () => { describe('findExistingItemFor', () => { test.each<[string, CollisionItem, ReturnType | undefined]>([ - ['an uploaded folder with the same name', getRoot('report'), existingFile], + ['an uploaded folder with the same name', getRoot('report'), existingFolder], + ['an uploaded folder whose name only matches a file', getRoot('README'), undefined], ['an uploaded file with the same name and extension', new File([''], 'report.pdf'), existingFile], ['an uploaded file with the same name but another extension', new File([''], 'report.docx'), undefined], [ diff --git a/src/app/drive/components/NameCollisionDialog/nameCollision.utils.ts b/src/app/drive/components/NameCollisionDialog/nameCollision.utils.ts index 36310d963f..5e5b06f36a 100644 --- a/src/app/drive/components/NameCollisionDialog/nameCollision.utils.ts +++ b/src/app/drive/components/NameCollisionDialog/nameCollision.utils.ts @@ -13,17 +13,20 @@ const matchesName = (existing: DriveItemData, name: string) => (existing.plainNa const matchesType = (existing: DriveItemData, type?: string | null) => (existing.type ?? null) === (type ?? null); /** - * Finds the drive item that collides with the given item. Uploaded folders match by name only, - * uploaded files by name and extension, and moved items by kind, name and (for files) extension. + * Finds the drive item that collides with the given item. Folders only match folders and files + * only match files; uploaded folders match by name, uploaded files by name and extension, and + * moved items by name and (for files) extension. */ export const findExistingItemFor = (item: CollisionItem, existingItems: DriveItemData[]): DriveItemData | undefined => { if (isFolderUpload(item)) { - return existingItems.find((existing) => matchesName(existing, item.name)); + return existingItems.find((existing) => existing.isFolder && matchesName(existing, item.name)); } if (item instanceof File) { const { filename, extension } = itemUtils.getFilenameAndExt(item.name); - return existingItems.find((existing) => matchesName(existing, filename) && matchesType(existing, extension)); + return existingItems.find( + (existing) => !existing.isFolder && matchesName(existing, filename) && matchesType(existing, extension), + ); } return existingItems.find( diff --git a/src/app/store/slices/storage/storage.thunks/renameItemsThunk.test.ts b/src/app/store/slices/storage/storage.thunks/renameItemsThunk.test.ts index 8e491991cb..115d5a73ae 100644 --- a/src/app/store/slices/storage/storage.thunks/renameItemsThunk.test.ts +++ b/src/app/store/slices/storage/storage.thunks/renameItemsThunk.test.ts @@ -89,9 +89,9 @@ describe('Rename items - Thunk', () => { test('When items collide in the destination, then they appear in duplicated items and existing items', async () => { const movingFile = getDriveItemData({ isFolder: false, uuid: 'file-moving' }); - const existingFile = getDriveItemData({ isFolder: false, uuid: 'file-existing' }); + const existingFile = getDriveItemData({ uuid: 'file-existing' }); const movingFolder = getDriveItemData({ isFolder: true, uuid: 'folder-moving' }); - const existingFolder = getDriveItemData({ isFolder: true, uuid: 'folder-existing' }); + const existingFolder = getDriveItemData({ uuid: 'folder-existing' }); mockCheckDuplicatedFiles.mockResolvedValue({ filesWithDuplicates: [movingFile], duplicatedFilesResponse: [existingFile], @@ -107,8 +107,10 @@ describe('Rename items - Thunk', () => { expect(result[0].duplicatedItems).toContain(movingFile); expect(result[0].duplicatedItems).toContain(movingFolder); - expect(result[0].existingItems).toContain(existingFile); - expect(result[0].existingItems).toContain(existingFolder); + expect(result[0].existingItems).toEqual([ + { ...existingFile, isFolder: false }, + { ...existingFolder, isFolder: true }, + ]); expect(result[0].unrepeatedItems).toHaveLength(0); }); @@ -176,7 +178,7 @@ describe('Rename items - Thunk', () => { const result = await handleRepeatedUploadingFiles([file], 'dest-uuid'); expect(result.repeatedItems).toContain(file); - expect(result.existingItems).toContain(file); + expect(result.existingItems).toEqual([{ ...file, isFolder: false }]); expect(result.unrepeatedItems).toHaveLength(0); }); @@ -214,16 +216,17 @@ describe('Rename items - Thunk', () => { describe('Handling repeated folders', () => { test('When a folder has a duplicate in the destination, it is returned as a repeated item', async () => { const folder = getDriveItemData({ isFolder: true }); + const existingFolder = getDriveItemData({ uuid: 'existing-folder' }); mockCheckFolderDuplicated.mockResolvedValue({ foldersWithDuplicates: [folder], - duplicatedFoldersResponse: [folder], + duplicatedFoldersResponse: [existingFolder], foldersWithoutDuplicates: [], }); const result = await handleRepeatedUploadingFolders([folder], 'dest-uuid'); expect(result.repeatedItems).toContain(folder); - expect(result.existingItems).toContain(folder); + expect(result.existingItems).toEqual([{ ...existingFolder, isFolder: true }]); expect(result.unrepeatedItems).toHaveLength(0); }); diff --git a/src/app/store/slices/storage/storage.thunks/renameItemsThunk.ts b/src/app/store/slices/storage/storage.thunks/renameItemsThunk.ts index dc4a905d7a..28a8670c15 100644 --- a/src/app/store/slices/storage/storage.thunks/renameItemsThunk.ts +++ b/src/app/store/slices/storage/storage.thunks/renameItemsThunk.ts @@ -17,6 +17,13 @@ import { getUniqueFolderName } from '../folderUtils/getUniqueFolderName'; import { CollisionGroup, StorageState } from '../storage.model'; import { IRoot } from '../types'; +/** + * The duplicate checks return raw API items without isFolder, so existing items are tagged + * here to let consumers tell colliding files and folders apart. + */ +const asExistingItems = (items: (DriveFileData | DriveFolderData)[], isFolder: boolean): DriveItemData[] => + items.map((item) => ({ ...item, isFolder })) as DriveItemData[]; + export const getCollisionGroups = async ( groups: { destinationUuid: string; items: DriveItemData[] }[], ): Promise => { @@ -39,8 +46,8 @@ export const getCollisionGroups = async ( ...(foldersResult.foldersWithDuplicates as DriveItemData[]), ]; const existingItems = [ - ...(filesResult.duplicatedFilesResponse as DriveItemData[]), - ...(foldersResult.duplicatedFoldersResponse as DriveItemData[]), + ...asExistingItems(filesResult.duplicatedFilesResponse, false), + ...asExistingItems(foldersResult.duplicatedFoldersResponse, true), ]; const unrepeatedItems = [ ...(filesResult.filesWithoutDuplicates as DriveItemData[]), @@ -62,7 +69,7 @@ export const handleRepeatedUploadingFiles = async ( destinationFolderUuid: string, ): Promise<{ repeatedItems: (DriveFileData | File)[]; - existingItems: DriveFileData[]; + existingItems: DriveItemData[]; unrepeatedItems: (DriveFileData | File)[]; }> => { const batchs = getFilesByBatchs(files); @@ -73,13 +80,13 @@ export const handleRepeatedUploadingFiles = async ( return results.reduce( (acc, cur) => { acc.repeatedItems.push(...cur.filesWithDuplicates); - acc.existingItems.push(...cur.duplicatedFilesResponse); + acc.existingItems.push(...asExistingItems(cur.duplicatedFilesResponse, false)); acc.unrepeatedItems.push(...cur.filesWithoutDuplicates); return acc; }, { repeatedItems: [] as (DriveFileData | File)[], - existingItems: [] as DriveFileData[], + existingItems: [] as DriveItemData[], unrepeatedItems: [] as (DriveFileData | File)[], }, ); @@ -90,7 +97,7 @@ export const handleRepeatedUploadingFolders = async ( destinationFolderUuid: string, ): Promise<{ repeatedItems: (DriveFolderData | IRoot)[]; - existingItems: DriveFolderData[]; + existingItems: DriveItemData[]; unrepeatedItems: (DriveFolderData | IRoot)[]; }> => { const batchs = getFilesByBatchs(folders as (IRoot | DriveFolderData)[]); @@ -101,13 +108,13 @@ export const handleRepeatedUploadingFolders = async ( return results.reduce( (acc, cur) => { acc.repeatedItems.push(...cur.foldersWithDuplicates); - acc.existingItems.push(...cur.duplicatedFoldersResponse); + acc.existingItems.push(...asExistingItems(cur.duplicatedFoldersResponse, true)); acc.unrepeatedItems.push(...cur.foldersWithoutDuplicates); return acc; }, { repeatedItems: [] as (DriveFolderData | IRoot)[], - existingItems: [] as DriveFolderData[], + existingItems: [] as DriveItemData[], unrepeatedItems: [] as (DriveFolderData | IRoot)[], }, ); From 5a3ca29e4042cfc27b3f6e33f375429bef4ec19e Mon Sep 17 00:00:00 2001 From: Francis Terrero Date: Sun, 20 Sep 2026 18:55:27 -0400 Subject: [PATCH 05/20] fix: rename asExistingItems to addIsFolderField for clarity and consistency in handling existing items --- .../storage/storage.thunks/renameItemsThunk.ts | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/app/store/slices/storage/storage.thunks/renameItemsThunk.ts b/src/app/store/slices/storage/storage.thunks/renameItemsThunk.ts index 28a8670c15..e7092fb811 100644 --- a/src/app/store/slices/storage/storage.thunks/renameItemsThunk.ts +++ b/src/app/store/slices/storage/storage.thunks/renameItemsThunk.ts @@ -17,11 +17,7 @@ import { getUniqueFolderName } from '../folderUtils/getUniqueFolderName'; import { CollisionGroup, StorageState } from '../storage.model'; import { IRoot } from '../types'; -/** - * The duplicate checks return raw API items without isFolder, so existing items are tagged - * here to let consumers tell colliding files and folders apart. - */ -const asExistingItems = (items: (DriveFileData | DriveFolderData)[], isFolder: boolean): DriveItemData[] => +const addIsFolderField = (items: (DriveFileData | DriveFolderData)[], isFolder: boolean): DriveItemData[] => items.map((item) => ({ ...item, isFolder })) as DriveItemData[]; export const getCollisionGroups = async ( @@ -46,8 +42,8 @@ export const getCollisionGroups = async ( ...(foldersResult.foldersWithDuplicates as DriveItemData[]), ]; const existingItems = [ - ...asExistingItems(filesResult.duplicatedFilesResponse, false), - ...asExistingItems(foldersResult.duplicatedFoldersResponse, true), + ...addIsFolderField(filesResult.duplicatedFilesResponse, false), + ...addIsFolderField(foldersResult.duplicatedFoldersResponse, true), ]; const unrepeatedItems = [ ...(filesResult.filesWithoutDuplicates as DriveItemData[]), @@ -80,7 +76,7 @@ export const handleRepeatedUploadingFiles = async ( return results.reduce( (acc, cur) => { acc.repeatedItems.push(...cur.filesWithDuplicates); - acc.existingItems.push(...asExistingItems(cur.duplicatedFilesResponse, false)); + acc.existingItems.push(...addIsFolderField(cur.duplicatedFilesResponse, false)); acc.unrepeatedItems.push(...cur.filesWithoutDuplicates); return acc; }, @@ -108,7 +104,7 @@ export const handleRepeatedUploadingFolders = async ( return results.reduce( (acc, cur) => { acc.repeatedItems.push(...cur.foldersWithDuplicates); - acc.existingItems.push(...asExistingItems(cur.duplicatedFoldersResponse, true)); + acc.existingItems.push(...addIsFolderField(cur.duplicatedFoldersResponse, true)); acc.unrepeatedItems.push(...cur.foldersWithoutDuplicates); return acc; }, From a73a26b693e86475b297f7f3c3abefd9a1af0d7b Mon Sep 17 00:00:00 2001 From: Francis Terrero Date: Thu, 10 Sep 2026 23:34:39 -0400 Subject: [PATCH 06/20] refactor: extract name collision upload actions out of the container Move the keep/replace handling for uploaded files and folders, including versioned replacement, into nameCollision.actions. resolveCollision now covers both move and upload collisions, so the container only reads store state and delegates. No behaviour change. --- .../NameCollisionContainer.tsx | 117 ++------------ .../nameCollision.actions.test.ts | 144 +++++++++++++++-- .../nameCollision.actions.ts | 151 +++++++++++++++++- .../nameCollision.utils.ts | 8 + 4 files changed, 301 insertions(+), 119 deletions(-) create mode 100644 src/app/drive/components/NameCollisionDialog/nameCollision.utils.ts diff --git a/src/app/drive/components/NameCollisionDialog/NameCollisionContainer.tsx b/src/app/drive/components/NameCollisionDialog/NameCollisionContainer.tsx index e56f952d33..30cd66efc7 100644 --- a/src/app/drive/components/NameCollisionDialog/NameCollisionContainer.tsx +++ b/src/app/drive/components/NameCollisionDialog/NameCollisionContainer.tsx @@ -1,21 +1,12 @@ import { FC, useMemo } from 'react'; import NameCollisionDialog, { OnSubmitPressed } from '.'; -import { moveItemsToTrash } from 'views/Trash/services'; import { RootState } from 'app/store'; import { useAppDispatch, useAppSelector } from 'app/store/hooks'; -import storageThunks from 'app/store/slices/storage/storage.thunks'; -import { fetchSortedFolderContentThunk } from 'app/store/slices/storage/storage.thunks/fetchSortedFolderContentThunk'; import { uiActions } from 'app/store/slices/ui'; -import { DriveItemData } from 'app/drive/types'; import { IRoot } from 'app/store/slices/storage/types'; import workspacesSelectors from 'app/store/slices/workspaces/workspaces.selectors'; -import { uploadFoldersWithTracking } from 'app/drive/services/folder.service/uploadFoldersWithTracking'; -import replaceFileService from 'views/Drive/services/replaceFile.service'; -import { Network, getEnvironmentConfig } from 'app/drive/services/network.service'; -import { fileVersionsActions, fileVersionsSelectors } from 'app/store/slices/fileVersions'; -import { isVersioningExtensionAllowed } from 'views/Drive/components/VersionHistory/utils'; -import { CollisionGroup } from 'app/store/slices/storage/storage.model'; -import { NameCollisionContext, resolveMoveCollision } from './nameCollision.actions'; +import { fileVersionsSelectors } from 'app/store/slices/fileVersions'; +import { NameCollisionContext, resolveCollision } from './nameCollision.actions'; const NameCollisionContainer: FC = () => { const dispatch = useAppDispatch(); @@ -38,102 +29,18 @@ const NameCollisionContainer: FC = () => { dispatch(uiActions.setIsNameCollisionDialogOpen({ open: false, info: undefined })); }; - const uploadFileAndGetFileId = async (file: File, itemToReplace: DriveItemData) => { - const { bridgeUser, bridgePass, encryptionKey, bucketId } = await getEnvironmentConfig(!!selectedWorkspace); - const network = new Network(bridgeUser, bridgePass, encryptionKey); - const taskId = `replace-${itemToReplace.uuid}-${Date.now()}`; - const [uploadPromise] = network.uploadFile( - bucketId, - { filecontent: file, filesize: file.size, progressCallback: () => {} }, - { taskId }, - ); - return uploadPromise; - }; - - const replaceFileVersion = async (file: File, itemToReplace: DriveItemData) => { - const newFileId = await uploadFileAndGetFileId(file, itemToReplace); - await replaceFileService.replaceFile(itemToReplace.uuid, { fileId: newFileId, size: file.size }); - dispatch(fileVersionsActions.invalidateCache(itemToReplace.uuid)); - }; - - const replaceAndUploadItem = async (group: CollisionGroup) => { - const itemsToUpload = group.duplicatedItems as (IRoot | File)[]; - const itemsToReplace = group.existingItems; - - for (let i = 0; i < itemsToUpload.length; i++) { - const itemToUpload = itemsToUpload[i]; - const itemToReplace = itemsToReplace[i]; - - if ((itemToUpload as IRoot).fullPathEdited) { - await moveItemsToTrash([itemToReplace]); - await uploadFoldersWithTracking({ - payload: [{ root: { ...(itemToUpload as IRoot) }, currentFolderId: group.destinationUuid }], - selectedWorkspace, - dispatch, - maxUploadFileSize, - }); - } else { - const file = itemToUpload as File; - const canReplaceVersion = isVersioningEnabled && isVersioningExtensionAllowed(itemToReplace); - if (canReplaceVersion) { - await replaceFileVersion(file, itemToReplace); - } else { - await moveItemsToTrash([itemToReplace]); - await dispatch( - storageThunks.uploadItemsThunk({ - files: [file], - parentFolderId: group.destinationUuid, - options: { disableDuplicatedNamesCheck: true }, - }), - ); - } - } - - dispatch(fetchSortedFolderContentThunk(group.destinationUuid)); - } - }; - - const keepAndUploadItem = async (group: CollisionGroup) => { - for (const itemToUpload of group.duplicatedItems as (IRoot | File)[]) { - if ((itemToUpload as IRoot).fullPathEdited) { - await uploadFoldersWithTracking({ - payload: [{ root: { ...(itemToUpload as IRoot) }, currentFolderId: group.destinationUuid }], - selectedWorkspace, - dispatch, - maxUploadFileSize, - }); - } else { - await dispatch( - storageThunks.uploadItemsThunk({ - files: [itemToUpload as File], - parentFolderId: group.destinationUuid, - }), - ); - } - dispatch(fetchSortedFolderContentThunk(group.destinationUuid)); - } - }; - const triggerSelectedOptionsOnSubmit = async ({ operationType, operation }: OnSubmitPressed) => { for (const group of collisionGroups) { - if (operationType === 'move') { - await resolveMoveCollision( - { - operation, - items: group.duplicatedItems as DriveItemData[], - existingItems: group.existingItems, - destinationUuid: group.destinationUuid, - }, - context, - ); - continue; - } - - if (operation === 'keep') { - await keepAndUploadItem(group); - } else { - await replaceAndUploadItem(group); - } + await resolveCollision( + { + operationType, + operation, + items: group.duplicatedItems, + existingItems: group.existingItems, + destinationUuid: group.destinationUuid, + }, + context, + ); } closeDialog(); }; diff --git a/src/app/drive/components/NameCollisionDialog/nameCollision.actions.test.ts b/src/app/drive/components/NameCollisionDialog/nameCollision.actions.test.ts index 1b9c034696..72ef91a22e 100644 --- a/src/app/drive/components/NameCollisionDialog/nameCollision.actions.test.ts +++ b/src/app/drive/components/NameCollisionDialog/nameCollision.actions.test.ts @@ -1,21 +1,35 @@ import { beforeEach, describe, expect, test, vi } from 'vitest'; import { getDriveItemData } from 'testUtils/fixtures/drive.fixtures'; -import { NameCollisionContext, ResolveMoveCollisionParams, resolveMoveCollision } from './nameCollision.actions'; +import { IRoot } from 'app/store/slices/storage/types'; +import { NameCollisionContext, ResolveCollisionParams, resolveCollision } from './nameCollision.actions'; const mocks = vi.hoisted(() => ({ dispatch: vi.fn(), moveItemsToTrash: vi.fn(), moveItemsThunk: vi.fn(), + uploadItemsThunk: vi.fn(), + fetchSortedFolderContentThunk: vi.fn(), popItemsToDelete: vi.fn(), + invalidateCache: vi.fn(), checkDuplicatedFiles: vi.fn(), getUniqueFilename: vi.fn(), checkFolderDuplicated: vi.fn(), getUniqueFolderName: vi.fn(), + uploadFoldersWithTracking: vi.fn(), + getEnvironmentConfig: vi.fn(), + networkUploadFile: vi.fn(), + replaceFile: vi.fn(), })); vi.mock('views/Trash/services', () => ({ moveItemsToTrash: mocks.moveItemsToTrash })); -vi.mock('app/store/slices/storage/storage.thunks', () => ({ default: { moveItemsThunk: mocks.moveItemsThunk } })); +vi.mock('app/store/slices/storage/storage.thunks', () => ({ + default: { moveItemsThunk: mocks.moveItemsThunk, uploadItemsThunk: mocks.uploadItemsThunk }, +})); +vi.mock('app/store/slices/storage/storage.thunks/fetchSortedFolderContentThunk', () => ({ + fetchSortedFolderContentThunk: mocks.fetchSortedFolderContentThunk, +})); vi.mock('app/store/slices/storage', () => ({ storageActions: { popItemsToDelete: mocks.popItemsToDelete } })); +vi.mock('app/store/slices/fileVersions', () => ({ fileVersionsActions: { invalidateCache: mocks.invalidateCache } })); vi.mock('app/store/slices/storage/fileUtils/checkDuplicatedFiles', () => ({ checkDuplicatedFiles: mocks.checkDuplicatedFiles, })); @@ -26,21 +40,46 @@ vi.mock('app/store/slices/storage/folderUtils/checkFolderDuplicated', () => ({ vi.mock('app/store/slices/storage/folderUtils/getUniqueFolderName', () => ({ getUniqueFolderName: mocks.getUniqueFolderName, })); +vi.mock('app/drive/services/folder.service/uploadFoldersWithTracking', () => ({ + uploadFoldersWithTracking: mocks.uploadFoldersWithTracking, +})); +vi.mock('app/drive/services/network.service', () => ({ + getEnvironmentConfig: mocks.getEnvironmentConfig, + Network: class { + uploadFile = mocks.networkUploadFile; + }, +})); +vi.mock('views/Drive/services/replaceFile.service', () => ({ default: { replaceFile: mocks.replaceFile } })); +vi.mock('views/Drive/components/VersionHistory/utils', () => ({ + isVersioningExtensionAllowed: (item?: { type?: string }) => item?.type === 'pdf', +})); const DESTINATION = 'destination-uuid'; const asMoveAction = (payload: unknown) => ({ type: 'move', payload }); const asPopAction = (payload: unknown) => ({ type: 'pop', payload }); +const asUploadAction = (payload: unknown) => ({ type: 'upload', payload }); +const asRefreshAction = (payload: unknown) => ({ type: 'refresh', payload }); +const asInvalidateCacheAction = (payload: unknown) => ({ type: 'invalidateCache', payload }); + +const getRoot = (name = 'Photos'): IRoot => ({ + name, + folderId: null, + childrenFiles: [], + childrenFolders: [], + fullPathEdited: `/${name}`, +}); -const getContext = (): NameCollisionContext => ({ +const getContext = (overrides: Partial = {}): NameCollisionContext => ({ dispatch: mocks.dispatch as unknown as NameCollisionContext['dispatch'], selectedWorkspace: null, maxUploadFileSize: 5000, isVersioningEnabled: false, + ...overrides, }); -const resolve = (params: Omit) => - resolveMoveCollision({ ...params, destinationUuid: DESTINATION }, getContext()); +const resolve = (params: Omit, context = getContext()) => + resolveCollision({ ...params, destinationUuid: DESTINATION }, context); /** * Mocks are reset by hand because the browser test project does not do it between tests. @@ -53,10 +92,15 @@ beforeEach(() => { mocks.getUniqueFolderName.mockResolvedValue('Photos (1)'); mocks.moveItemsThunk.mockImplementation(asMoveAction); mocks.popItemsToDelete.mockImplementation(asPopAction); + mocks.uploadItemsThunk.mockImplementation(asUploadAction); + mocks.fetchSortedFolderContentThunk.mockImplementation(asRefreshAction); + mocks.invalidateCache.mockImplementation(asInvalidateCacheAction); + mocks.getEnvironmentConfig.mockResolvedValue({ bridgeUser: 'u', bridgePass: 'p', encryptionKey: 'k', bucketId: 'b' }); + mocks.networkUploadFile.mockReturnValue([Promise.resolve('new-file-id'), undefined]); }); -describe('resolveMoveCollision', () => { - test('when keeping both, then each item is moved under a unique name and leaves the pending deletion list', async () => { +describe('resolveCollision', () => { + test('when moving with keep, then each item is moved under a unique name and leaves the pending deletion list', async () => { const file = getDriveItemData({ plainName: 'report', name: 'report', type: 'pdf', isFolder: false }); const folder = getDriveItemData({ plainName: 'Photos', name: 'Photos', isFolder: true }); const renamedFileMove = { @@ -70,7 +114,7 @@ describe('resolveMoveCollision', () => { destinationFolderId: DESTINATION, }; - await resolve({ operation: 'keep', items: [file, folder], existingItems: [] }); + await resolve({ operationType: 'move', operation: 'keep', items: [file, folder], existingItems: [] }); expect(mocks.getUniqueFilename).toHaveBeenCalledWith('report', 'pdf', ['file-dup'], DESTINATION); expect(mocks.getUniqueFolderName).toHaveBeenCalledWith('Photos', ['folder-dup'], DESTINATION); @@ -82,7 +126,7 @@ describe('resolveMoveCollision', () => { ]); }); - test('when replacing, then existing items are trashed before moving and moved items leave the pending deletion list', async () => { + test('when moving with replace, then existing items are trashed before moving and moved items leave the pending deletion list', async () => { const item = getDriveItemData({ uuid: 'new' }); const existing = getDriveItemData({ uuid: 'existing' }); const callOrder: string[] = []; @@ -92,7 +136,7 @@ describe('resolveMoveCollision', () => { return asMoveAction(payload); }); - await resolve({ operation: 'replace', items: [item], existingItems: [existing] }); + await resolve({ operationType: 'move', operation: 'replace', items: [item], existingItems: [existing] }); expect(mocks.moveItemsToTrash).toHaveBeenCalledWith([existing]); expect(callOrder).toEqual(['trash', 'move']); @@ -101,4 +145,84 @@ describe('resolveMoveCollision', () => { [asPopAction([item])], ]); }); + + test('when uploading with keep, then folders upload with tracking, files upload with the duplicates check, and the folder is refreshed', async () => { + const context = getContext({ maxUploadFileSize: 123, selectedWorkspace: { id: 'ws' } as never }); + const file = new File(['content'], 'report.pdf'); + const root = getRoot(); + + await resolve({ operationType: 'upload', operation: 'keep', items: [file, root], existingItems: [] }, context); + + expect(mocks.uploadFoldersWithTracking).toHaveBeenCalledWith({ + payload: [{ root: { ...root }, currentFolderId: DESTINATION }], + selectedWorkspace: { id: 'ws' }, + dispatch: context.dispatch, + maxUploadFileSize: 123, + }); + expect(mocks.dispatch.mock.calls).toEqual([ + [asUploadAction({ files: [file], parentFolderId: DESTINATION, options: undefined })], + [asRefreshAction(DESTINATION)], + [asRefreshAction(DESTINATION)], + ]); + expect(mocks.popItemsToDelete).not.toHaveBeenCalled(); + }); + + test('when uploading with replace and versioning is off, then each existing item is trashed and the new one is uploaded without the duplicates check', async () => { + const file = new File(['content'], 'report.pdf'); + const root = getRoot(); + const existingFile = getDriveItemData({ uuid: 'existing-file', type: 'pdf' }); + const existingFolder = getDriveItemData({ uuid: 'existing-folder', isFolder: true }); + + await resolve({ + operationType: 'upload', + operation: 'replace', + items: [file, root], + existingItems: [existingFile, existingFolder], + }); + + expect(mocks.moveItemsToTrash).toHaveBeenNthCalledWith(1, [existingFile]); + expect(mocks.moveItemsToTrash).toHaveBeenNthCalledWith(2, [existingFolder]); + expect(mocks.uploadFoldersWithTracking).toHaveBeenCalledWith( + expect.objectContaining({ payload: [{ root: { ...root }, currentFolderId: DESTINATION }] }), + ); + expect(mocks.networkUploadFile).not.toHaveBeenCalled(); + expect(mocks.dispatch.mock.calls).toEqual([ + [asUploadAction({ files: [file], parentFolderId: DESTINATION, options: { disableDuplicatedNamesCheck: true } })], + [asRefreshAction(DESTINATION)], + [asRefreshAction(DESTINATION)], + ]); + }); + + test('when uploading with replace and versioning is on, then allowed extensions become a new version while the rest are trashed and re-uploaded', async () => { + const context = getContext({ isVersioningEnabled: true, selectedWorkspace: { id: 'ws' } as never }); + const pdf = new File(['content'], 'report.pdf'); + const image = new File(['content'], 'photo.png'); + const existingPdf = getDriveItemData({ uuid: 'existing-pdf', type: 'pdf' }); + const existingImage = getDriveItemData({ uuid: 'existing-png', type: 'png' }); + + await resolve( + { + operationType: 'upload', + operation: 'replace', + items: [pdf, image], + existingItems: [existingPdf, existingImage], + }, + context, + ); + + expect(mocks.getEnvironmentConfig).toHaveBeenCalledWith(true); + expect(mocks.networkUploadFile).toHaveBeenCalledWith( + 'b', + expect.objectContaining({ filecontent: pdf, filesize: pdf.size }), + { taskId: expect.stringMatching(/^replace-existing-pdf-\d+$/) }, + ); + expect(mocks.replaceFile).toHaveBeenCalledWith('existing-pdf', { fileId: 'new-file-id', size: pdf.size }); + expect(mocks.moveItemsToTrash).toHaveBeenCalledWith([existingImage]); + expect(mocks.dispatch.mock.calls).toEqual([ + [asInvalidateCacheAction('existing-pdf')], + [asRefreshAction(DESTINATION)], + [asUploadAction(expect.objectContaining({ files: [image] }))], + [asRefreshAction(DESTINATION)], + ]); + }); }); diff --git a/src/app/drive/components/NameCollisionDialog/nameCollision.actions.ts b/src/app/drive/components/NameCollisionDialog/nameCollision.actions.ts index e4cdd27cb9..22ec21ca35 100644 --- a/src/app/drive/components/NameCollisionDialog/nameCollision.actions.ts +++ b/src/app/drive/components/NameCollisionDialog/nameCollision.actions.ts @@ -1,15 +1,24 @@ import { WorkspaceData } from '@internxt/sdk/dist/workspaces'; +import { uploadFoldersWithTracking } from 'app/drive/services/folder.service/uploadFoldersWithTracking'; +import { Network, getEnvironmentConfig } from 'app/drive/services/network.service'; import { DriveItemData } from 'app/drive/types'; import { AppDispatch } from 'app/store'; +import { fileVersionsActions } from 'app/store/slices/fileVersions'; import { storageActions } from 'app/store/slices/storage'; import { checkDuplicatedFiles } from 'app/store/slices/storage/fileUtils/checkDuplicatedFiles'; import { getUniqueFilename } from 'app/store/slices/storage/fileUtils/getUniqueFilename'; import { checkFolderDuplicated } from 'app/store/slices/storage/folderUtils/checkFolderDuplicated'; import { getUniqueFolderName } from 'app/store/slices/storage/folderUtils/getUniqueFolderName'; import storageThunks from 'app/store/slices/storage/storage.thunks'; +import { fetchSortedFolderContentThunk } from 'app/store/slices/storage/storage.thunks/fetchSortedFolderContentThunk'; import { MoveItemPayload } from 'app/store/slices/storage/storage.thunks/moveItemsThunk'; +import { IRoot } from 'app/store/slices/storage/types'; +import { isVersioningExtensionAllowed } from 'views/Drive/components/VersionHistory/utils'; +import replaceFileService from 'views/Drive/services/replaceFile.service'; import { moveItemsToTrash } from 'views/Trash/services'; +import { CollisionItem, CollisionPair, isFolderUpload } from './nameCollision.utils'; +export type CollisionOperationType = 'move' | 'upload'; export type CollisionOperation = 'keep' | 'replace'; export interface NameCollisionContext { @@ -19,13 +28,19 @@ export interface NameCollisionContext { isVersioningEnabled: boolean; } -export interface ResolveMoveCollisionParams { +export interface ResolveCollisionParams { + operationType: CollisionOperationType; operation: CollisionOperation; - items: DriveItemData[]; + items: CollisionItem[]; existingItems: DriveItemData[]; destinationUuid: string; } +type ResolveMoveCollisionParams = Omit & { items: DriveItemData[] }; +type ResolveUploadCollisionParams = Omit & { + items: (IRoot | File)[]; +}; + const getUniqueNameMovePayload = async (item: DriveItemData, destinationUuid: string): Promise => { if (item.isFolder) { const { duplicatedFoldersResponse } = await checkFolderDuplicated([item], destinationUuid); @@ -73,10 +88,10 @@ const replaceAndMoveItems = async ( * Applies the chosen resolution to items that collide while being moved, then removes them * from the pending-deletion list. */ -export const resolveMoveCollision = async ( +const resolveMoveCollision = async ( { operation, items, existingItems, destinationUuid }: ResolveMoveCollisionParams, context: NameCollisionContext, -): Promise => { +) => { if (operation === 'keep') { await keepAndMoveItems(items, destinationUuid, context); } else { @@ -84,3 +99,131 @@ export const resolveMoveCollision = async ( } context.dispatch(storageActions.popItemsToDelete(items)); }; + +const uploadFileAndGetFileId = async ( + file: File, + itemToReplace: DriveItemData, + { selectedWorkspace }: NameCollisionContext, +): Promise => { + const { bridgeUser, bridgePass, encryptionKey, bucketId } = await getEnvironmentConfig(!!selectedWorkspace); + const network = new Network(bridgeUser, bridgePass, encryptionKey); + const taskId = `replace-${itemToReplace.uuid}-${Date.now()}`; + const [uploadPromise] = network.uploadFile( + bucketId, + { filecontent: file, filesize: file.size, progressCallback: () => {} }, + { taskId }, + ); + return uploadPromise; +}; + +const replaceFileVersion = async (file: File, itemToReplace: DriveItemData, context: NameCollisionContext) => { + const newFileId = await uploadFileAndGetFileId(file, itemToReplace, context); + await replaceFileService.replaceFile(itemToReplace.uuid, { fileId: newFileId, size: file.size }); + context.dispatch(fileVersionsActions.invalidateCache(itemToReplace.uuid)); +}; + +const uploadFolder = async ( + root: IRoot, + destinationUuid: string, + { dispatch, selectedWorkspace, maxUploadFileSize }: NameCollisionContext, +) => + uploadFoldersWithTracking({ + payload: [{ root: { ...root }, currentFolderId: destinationUuid }], + selectedWorkspace, + dispatch, + maxUploadFileSize, + }); + +const uploadFile = async ( + file: File, + destinationUuid: string, + { dispatch }: NameCollisionContext, + shouldSkipDuplicatesCheck = false, +) => + dispatch( + storageThunks.uploadItemsThunk({ + files: [file], + parentFolderId: destinationUuid, + options: shouldSkipDuplicatesCheck ? { disableDuplicatedNamesCheck: true } : undefined, + }), + ); + +const canReplaceVersion = (pair: CollisionPair, { isVersioningEnabled }: NameCollisionContext) => + !isFolderUpload(pair.item) && isVersioningEnabled && isVersioningExtensionAllowed(pair.existing); + +const trashAndUpload = async ( + pair: CollisionPair, + destinationUuid: string, + context: NameCollisionContext, +) => { + await moveItemsToTrash([pair.existing]); + if (isFolderUpload(pair.item)) { + await uploadFolder(pair.item, destinationUuid, context); + } else { + await uploadFile(pair.item, destinationUuid, context, true); + } +}; + +/** + * Replaces each colliding drive item with the uploaded one. Files whose extension supports + * versioning become a new version of the existing file; everything else is trashed and re-uploaded. + */ +const replaceAndUploadItems = async ( + pairs: CollisionPair[], + destinationUuid: string, + context: NameCollisionContext, +) => { + for (const pair of pairs) { + if (canReplaceVersion(pair, context)) { + await replaceFileVersion(pair.item as File, pair.existing, context); + } else { + await trashAndUpload(pair, destinationUuid, context); + } + context.dispatch(fetchSortedFolderContentThunk(destinationUuid)); + } +}; + +/** + * Uploads each item next to the existing one, letting the upload flow pick a unique name. + */ +const keepAndUploadItems = async (items: (IRoot | File)[], destinationUuid: string, context: NameCollisionContext) => { + for (const item of items) { + if (isFolderUpload(item)) { + await uploadFolder(item, destinationUuid, context); + } else { + await uploadFile(item, destinationUuid, context); + } + context.dispatch(fetchSortedFolderContentThunk(destinationUuid)); + } +}; + +/** + * Applies the chosen resolution to items that collide while being uploaded. Existing items are + * matched to the uploaded ones by position. + */ +const resolveUploadCollision = async ( + { operation, items, existingItems, destinationUuid }: ResolveUploadCollisionParams, + context: NameCollisionContext, +) => { + if (operation === 'keep') { + await keepAndUploadItems(items, destinationUuid, context); + return; + } + + const pairs = items.map((item, index) => ({ item, existing: existingItems[index] })); + await replaceAndUploadItems(pairs, destinationUuid, context); +}; + +/** + * Applies the chosen collision resolution to the given items. + */ +export const resolveCollision = async ( + { operationType, items, ...params }: ResolveCollisionParams, + context: NameCollisionContext, +): Promise => { + if (operationType === 'move') { + await resolveMoveCollision({ ...params, items: items as DriveItemData[] }, context); + } else { + await resolveUploadCollision({ ...params, items: items as (IRoot | File)[] }, context); + } +}; diff --git a/src/app/drive/components/NameCollisionDialog/nameCollision.utils.ts b/src/app/drive/components/NameCollisionDialog/nameCollision.utils.ts new file mode 100644 index 0000000000..23e36a3bc2 --- /dev/null +++ b/src/app/drive/components/NameCollisionDialog/nameCollision.utils.ts @@ -0,0 +1,8 @@ +import { DriveItemData } from 'app/drive/types'; +import { IRoot } from 'app/store/slices/storage/types'; + +export type CollisionItem = File | IRoot | DriveItemData; + +export type CollisionPair = { item: T; existing: DriveItemData }; + +export const isFolderUpload = (item: CollisionItem): item is IRoot => !!(item as IRoot).fullPathEdited; From 4b0a43e24710d6d43a8a2d0e95ff15e081b72a7c Mon Sep 17 00:00:00 2001 From: Francis Terrero Date: Fri, 11 Sep 2026 01:30:25 -0400 Subject: [PATCH 07/20] test: add e2e coverage for the skip option in name collisions Covers single and multiple duplicates, per-item and apply-to-all skipping, non-conflicting files still uploading, and Replace trashing the matching file, with auth, bootstrap and Drive endpoints mocked so the specs run without a backend. Queued uploads are asserted through the task panel because Playwright cannot intercept the bridge in Firefox. --- .../components/TaskLogger/TaskLogger.tsx | 1 + test/e2e/tests/helper/authRouteMocks.ts | 14 ++- test/e2e/tests/helper/driveRouteMocks.ts | 86 ++++++++++++-- test/e2e/tests/helper/mockedDrive.ts | 31 ++++- test/e2e/tests/helper/staticData.ts | 7 ++ test/e2e/tests/pages/drivePage.ts | 8 ++ .../tests/pages/nameCollisionDialogPage.ts | 59 ++++++++++ ...DRIVE-internxt-name-collision-skip.spec.ts | 109 ++++++++++++++++++ test/e2e/tests/specs/internxt-login.spec.ts | 47 +------- 9 files changed, 306 insertions(+), 56 deletions(-) create mode 100644 test/e2e/tests/pages/nameCollisionDialogPage.ts create mode 100644 test/e2e/tests/specs/DRIVE-internxt-name-collision-skip.spec.ts diff --git a/src/app/tasks/components/TaskLogger/TaskLogger.tsx b/src/app/tasks/components/TaskLogger/TaskLogger.tsx index 33c3440ce2..5615b055b8 100644 --- a/src/app/tasks/components/TaskLogger/TaskLogger.tsx +++ b/src/app/tasks/components/TaskLogger/TaskLogger.tsx @@ -100,6 +100,7 @@ const TaskLogger = (): JSX.Element => { return (
json: { hasKeys: true, sKey: LOGIN_SALT_KEY, tfa: false, hasKyberKeys: true, hasEccKeys: true }, }); -const mockAccessCall = (route: Route) => - route.fulfill({ +const mockAccessCall = (route: Route, request: Request) => { + const { email } = request.postDataJSON(); + if (email === INVALID_EMAIL) { + return route.fulfill({ status: HTTP_BAD_REQUEST, json: { message: staticData.wrongLoginWarning } }); + } + + return route.fulfill({ json: { user: loggedUser.user, token: loggedUser.token, @@ -25,6 +32,7 @@ const mockAccessCall = (route: Route) => userTeam: loggedUser.userTeam, }, }); +}; const mockRefreshUserCall = (route: Route) => route.fulfill({ json: { user: loggedUser.user, newToken: loggedUser.newToken } }); diff --git a/test/e2e/tests/helper/driveRouteMocks.ts b/test/e2e/tests/helper/driveRouteMocks.ts index 69a739a024..8f83ecd59e 100644 --- a/test/e2e/tests/helper/driveRouteMocks.ts +++ b/test/e2e/tests/helper/driveRouteMocks.ts @@ -13,6 +13,7 @@ const BRIDGE_FILE_INFO_ID_PATTERN = /\/files\/([^/]+)\/info/; const FILES_LISTING_PATH = '/files/'; const START_UPLOAD_PATH = '/files/start'; const FINISH_UPLOAD_PATH = '/files/finish'; +const FILES_EXISTENCE_PATH = '/files/existence'; const HTTP_METHOD = { get: 'GET', post: 'POST', put: 'PUT' }; const HTTP_NOT_FOUND = 404; @@ -22,6 +23,7 @@ const BRIDGE_FILE_VERSION = 2; const BRIDGE_FILE_ID_BYTES = 12; const FIRST_UPLOADED_FILE_ID = 5000; const FIRST_THUMBNAIL_ID = 9000; +const EXISTING_FILE_SIZE = 1024; const ITEM_TIMESTAMP = '2026-08-01T10:00:00.000Z'; const ITEM_STATUS = 'EXISTS'; @@ -68,12 +70,21 @@ type FinishUploadRequest = { hmac?: { type: string; value: string }; }; +type ExistenceCheckRequest = { files: { plainName: string; type: string }[] }; +export type TrashRequest = { items: { uuid: string; type: string }[] }; + +/** + * Uploads only reach the bridge, and so `fileEntries`, in Chromium: Playwright cannot route + * the bridge CORS preflight in Firefox. + */ type RecordedRequests = { fileEntries: FileEntryRequest[]; thumbnailEntries: ThumbnailEntryRequest[]; downloadedFileIds: string[]; + trash: TrashRequest[]; }; export type MockedDriveOptions = { + files?: ExistingFile[]; declaredSizes?: Record; }; @@ -90,12 +101,16 @@ const buildThumbnail = (id: number, fileId: number, entry: ThumbnailEntryRequest }); type StoredThumbnail = ReturnType; -const buildFile = (id: number, { fileId, type, size, plainName, bucket, folderUuid }: FileEntryRequest) => { +const buildFile = ( + id: number, + { fileId, type, size, plainName, bucket, folderUuid }: FileEntryRequest, + uuid: string = randomUUID(), +) => { const thumbnails: StoredThumbnail[] = []; return { id, - uuid: randomUUID(), + uuid, fileId, name: plainName, plainName, @@ -112,23 +127,58 @@ const buildFile = (id: number, { fileId, type, size, plainName, bucket, folderUu }; type StoredFile = ReturnType; +export const buildExistingFile = (id: number, plainName: string, type: string) => + buildFile( + id, + { + fileId: `existing-bridge-file-${id}`, + type, + size: EXISTING_FILE_SIZE, + plainName, + bucket: loggedUser.user.bucket, + folderUuid: loggedUser.user.rootFolderId, + }, + `existing-file-uuid-${id}`, + ); +export type ExistingFile = StoredFile; + +/** + * Trashing removes files, so follow-up listings and existence checks see the new state. + */ class InMemoryDrive { - private readonly files: StoredFile[] = []; + private files: StoredFile[]; + private readonly declaredSizes: Record; + private uploadedFilesCount = 0; private thumbnailsCount = 0; - constructor(private readonly declaredSizes: Record = {}) {} + constructor({ files = [], declaredSizes = {} }: MockedDriveOptions) { + this.files = [...files]; + this.declaredSizes = declaredSizes; + } filesIn(folderUuid: string) { return this.files.filter((file) => file.folderUuid === folderUuid); } + existingFilesIn(folderUuid: string, candidates: ExistenceCheckRequest['files']) { + return this.filesIn(folderUuid).filter((existing) => + candidates.some(({ plainName, type }) => plainName === existing.plainName && type === existing.type), + ); + } + + trash(uuids: string[]) { + const trashed = new Set(uuids); + this.files = this.files.filter((file) => !trashed.has(file.uuid)); + } + findFile(uuid: string) { return this.files.find((file) => file.uuid === uuid); } addFile(entry: FileEntryRequest) { const size = this.declaredSizes[entry.plainName] ?? entry.size; - const file = buildFile(FIRST_UPLOADED_FILE_ID + this.files.length, { ...entry, size }); + const file = buildFile(FIRST_UPLOADED_FILE_ID + this.uploadedFilesCount, { ...entry, size }); + this.uploadedFilesCount += 1; this.files.push(file); return file; } @@ -210,6 +260,15 @@ const mockAppBootstrapCalls = async (page: Page) => { } }; +const fulfillExistenceCheck = (route: Route, request: Request, drive: InMemoryDrive) => { + const url = request.url(); + if (!url.endsWith(FILES_EXISTENCE_PATH)) return route.fulfill({ json: NO_DUPLICATES }); + + const { files } = request.postDataJSON() as ExistenceCheckRequest; + const existentFiles = drive.existingFilesIn(firstCapture(FOLDER_CONTENT_UUID_PATTERN, url), files); + return route.fulfill({ json: { existentFiles } }); +}; + const mockFolderContentRoutes = async (page: Page, drive: InMemoryDrive) => { const folderContentUrl = `${BASE_API_URL}/folders/content/**`; @@ -219,7 +278,9 @@ const mockFolderContentRoutes = async (page: Page, drive: InMemoryDrive) => { const files = drive.filesIn(firstCapture(FOLDER_CONTENT_UUID_PATTERN, url)); return route.fulfill({ json: isFilesListing ? { files } : { folders: [] } }); }); - await mockRouteForMethod(page, folderContentUrl, HTTP_METHOD.post, (route) => route.fulfill({ json: NO_DUPLICATES })); + await mockRouteForMethod(page, folderContentUrl, HTTP_METHOD.post, (route, request) => + fulfillExistenceCheck(route, request, drive), + ); }; const mockFileEntryRoutes = async (page: Page, drive: InMemoryDrive, requests: RecordedRequests) => { @@ -240,6 +301,14 @@ const mockFileEntryRoutes = async (page: Page, drive: InMemoryDrive, requests: R ); }; +const mockTrashRoute = (page: Page, drive: InMemoryDrive, requests: RecordedRequests) => + mockRouteForMethod(page, `${BASE_API_URL}/storage/trash/add`, HTTP_METHOD.post, (route, request) => { + const trashRequest = request.postDataJSON() as TrashRequest; + drive.trash(trashRequest.items.map((item) => item.uuid)); + requests.trash.push(trashRequest); + return route.fulfill({ json: {} }); + }); + const fulfillBridgeCall = (route: Route, request: Request, bridge: InMemoryBridge, requests: RecordedRequests) => { const url = request.url(); if (url.includes(START_UPLOAD_PATH)) return route.fulfill({ json: { uploads: [bridge.startUpload()] } }); @@ -274,13 +343,14 @@ const mockBridgeStorageRoutes = async (page: Page, requests: RecordedRequests) = }; export const mockDriveRoutes = async (page: Page, options: MockedDriveOptions = {}): Promise => { - const drive = new InMemoryDrive(options.declaredSizes); - const requests: RecordedRequests = { fileEntries: [], thumbnailEntries: [], downloadedFileIds: [] }; + const drive = new InMemoryDrive(options); + const requests: RecordedRequests = { fileEntries: [], thumbnailEntries: [], downloadedFileIds: [], trash: [] }; await mockAppBootstrapCalls(page); await mockAuthRoutes(page); await mockFolderContentRoutes(page, drive); await mockFileEntryRoutes(page, drive, requests); + await mockTrashRoute(page, drive, requests); await mockBridgeStorageRoutes(page, requests); return requests; diff --git a/test/e2e/tests/helper/mockedDrive.ts b/test/e2e/tests/helper/mockedDrive.ts index 971a3afdf8..d3d0aba49c 100644 --- a/test/e2e/tests/helper/mockedDrive.ts +++ b/test/e2e/tests/helper/mockedDrive.ts @@ -1,11 +1,36 @@ -import { Page } from '@playwright/test'; -import { DrivePage } from '../pages/drivePage'; +import { expect, Page } from '@playwright/test'; +import { DrivePage, UploadFile } from '../pages/drivePage'; +import { NameCollisionDialogPage } from '../pages/nameCollisionDialogPage'; import { logInThroughUI } from './authRouteMocks'; import { MockedDriveOptions, mockDriveRoutes } from './driveRouteMocks'; +const FIRST_FILE_LISTED_TIMEOUT = 10000; +const MIME_TYPES_BY_EXTENSION: Record = { + txt: 'text/plain', + pdf: 'application/pdf', +}; + +export const buildUploadFile = (name: string): UploadFile => ({ + name, + mimeType: MIME_TYPES_BY_EXTENSION[name.split('.').pop() ?? ''] ?? 'application/octet-stream', + buffer: Buffer.from(`content of ${name}`), +}); + +/** + * Mocks the API around the given Drive, logs in through the UI and, when the Drive has + * files, waits until the first one is listed. + */ export const openMockedDrive = async (page: Page, options: MockedDriveOptions = {}) => { const requests = await mockDriveRoutes(page, options); await logInThroughUI(page); - return { drivePage: new DrivePage(page), requests }; + const drivePage = new DrivePage(page); + const [firstFile] = options.files ?? []; + if (firstFile) { + await expect(drivePage.fileRow(`${firstFile.plainName}.${firstFile.type}`)).toBeVisible({ + timeout: FIRST_FILE_LISTED_TIMEOUT, + }); + } + + return { drivePage, collisionDialog: new NameCollisionDialogPage(page), requests }; }; diff --git a/test/e2e/tests/helper/staticData.ts b/test/e2e/tests/helper/staticData.ts index 250f04610b..c481c8c24a 100644 --- a/test/e2e/tests/helper/staticData.ts +++ b/test/e2e/tests/helper/staticData.ts @@ -47,4 +47,11 @@ export const staticData = { loadingPreviewText: 'Loading preview', downloadButtonText: 'Download', blobUrlPattern: /^blob:/, + + //NAME COLLISION DIALOG + collisionDialogTitle: 'Item already exists', + collisionReplaceOption: 'Replace current item', + collisionKeepBothOption: 'Keep both', + collisionSkipOption: 'Skip this item', + collisionApplyToAll: 'Apply this action to all duplicates', }; diff --git a/test/e2e/tests/pages/drivePage.ts b/test/e2e/tests/pages/drivePage.ts index f8d22c8c11..9398c0239e 100644 --- a/test/e2e/tests/pages/drivePage.ts +++ b/test/e2e/tests/pages/drivePage.ts @@ -201,6 +201,14 @@ export class DrivePage { await expect(thumbnail).toHaveAttribute('src', staticData.blobUrlPattern); } + fileRow(fileName: string) { + return this.page.locator(`[title="${fileName}"]`); + } + + taskItem(itemName: string) { + return this.page.locator(`[data-test="task-logger"] [title="${itemName}"]`); + } + private fileListElement(fileName: string, element: FileListElement) { return this.page.locator(`[data-test="file-list-file-${fileName}-${element}"]`); } diff --git a/test/e2e/tests/pages/nameCollisionDialogPage.ts b/test/e2e/tests/pages/nameCollisionDialogPage.ts new file mode 100644 index 0000000000..884cc4cbaf --- /dev/null +++ b/test/e2e/tests/pages/nameCollisionDialogPage.ts @@ -0,0 +1,59 @@ +import { expect, Locator, Page } from '@playwright/test'; +import { staticData } from '../helper/staticData'; + +export class NameCollisionDialogPage { + private page: Page; + private dialog: Locator; + private title: Locator; + private options: Locator; + private applyToAllLabel: Locator; + private applyToAllCheckbox: Locator; + private submitButton: Locator; + + constructor(page: Page) { + this.page = page; + this.dialog = this.page.getByRole('dialog'); + this.title = this.dialog.getByText(staticData.collisionDialogTitle); + this.options = this.dialog.getByRole('radio'); + this.applyToAllLabel = this.dialog.getByText(staticData.collisionApplyToAll); + this.applyToAllCheckbox = this.dialog.locator('#apply-to-all'); + this.submitButton = this.dialog.getByRole('button', { name: /^(Upload|Move)$/ }); + } + + async expectOpenFor(itemName: string) { + await expect(this.title).toBeVisible({ timeout: 10000 }); + await expect(this.dialog.getByText(`${itemName} already exists in this location`)).toBeVisible(); + } + + async expectClosed() { + await expect(this.title).toBeHidden({ timeout: 10000 }); + } + + async expectOptions(optionNames: string[]) { + await expect(this.options).toHaveText(optionNames); + } + + async expectApplyToAllVisible(isVisible: boolean) { + await expect(this.applyToAllLabel).toBeVisible({ visible: isVisible }); + } + + async selectOption(optionName: string) { + await this.dialog.getByRole('radio', { name: optionName }).click(); + } + + async checkApplyToAllByClickingLabel() { + await this.applyToAllLabel.click(); + await expect(this.applyToAllCheckbox).toBeChecked(); + } + + async submit() { + await expect(this.submitButton).toBeVisible(); + await this.submitButton.click(); + } + + async resolve(itemName: string, optionName: string) { + await this.expectOpenFor(itemName); + await this.selectOption(optionName); + await this.submit(); + } +} diff --git a/test/e2e/tests/specs/DRIVE-internxt-name-collision-skip.spec.ts b/test/e2e/tests/specs/DRIVE-internxt-name-collision-skip.spec.ts new file mode 100644 index 0000000000..ec5e451a1e --- /dev/null +++ b/test/e2e/tests/specs/DRIVE-internxt-name-collision-skip.spec.ts @@ -0,0 +1,109 @@ +import { expect, test } from '@playwright/test'; +import { buildExistingFile } from '../helper/driveRouteMocks'; +import { buildUploadFile, openMockedDrive } from '../helper/mockedDrive'; +import { staticData } from '../helper/staticData'; + +const existingReport = buildExistingFile(1, 'report', 'txt'); +const existingInvoice = buildExistingFile(2, 'invoice', 'pdf'); + +const duplicatedReport = buildUploadFile('report.txt'); +const duplicatedInvoice = buildUploadFile('invoice.pdf'); +const newFile = buildUploadFile('brand-new.txt'); + +const allOptions = [ + staticData.collisionReplaceOption, + staticData.collisionKeepBothOption, + staticData.collisionSkipOption, +]; + +test.describe('Internxt name collision skip option', () => { + test.use({ storageState: { cookies: [], origins: [] } }); + + let drive: Awaited>; + + test.beforeEach('Logging in with existing files in Drive', async ({ page }) => { + drive = await openMockedDrive(page, { files: [existingReport, existingInvoice] }); + }); + + test('TC1: Validate that skipping a single duplicated file keeps the existing file and uploads nothing', async () => { + const { drivePage, collisionDialog, requests } = drive; + + await drivePage.uploadFiles([duplicatedReport]); + + await collisionDialog.expectOpenFor('report.txt'); + await collisionDialog.expectOptions(allOptions); + await collisionDialog.expectApplyToAllVisible(false); + + await collisionDialog.selectOption(staticData.collisionSkipOption); + await collisionDialog.submit(); + + await collisionDialog.expectClosed(); + await expect(drivePage.fileRow('report.txt')).toHaveCount(1); + expect(requests.trash).toHaveLength(0); + expect(requests.fileEntries).toHaveLength(0); + }); + + test('TC2: Validate that only the non-conflicting files are uploaded when the duplicated one is skipped', async () => { + const { drivePage, collisionDialog, requests } = drive; + + await drivePage.uploadFiles([duplicatedReport, newFile]); + + await collisionDialog.expectOpenFor('report.txt'); + await expect(drivePage.taskItem('brand-new.txt')).toBeVisible({ timeout: 10000 }); + + await collisionDialog.selectOption(staticData.collisionSkipOption); + await collisionDialog.submit(); + + await collisionDialog.expectClosed(); + expect(requests.trash).toHaveLength(0); + }); + + test('TC3: Validate that duplicated files are resolved one by one when "apply to all" is not checked', async () => { + const { drivePage, collisionDialog, requests } = drive; + + await drivePage.uploadFiles([duplicatedReport, duplicatedInvoice]); + + await collisionDialog.expectOpenFor('report.txt'); + await collisionDialog.expectApplyToAllVisible(true); + await collisionDialog.selectOption(staticData.collisionSkipOption); + await collisionDialog.submit(); + + await collisionDialog.expectOpenFor('invoice.pdf'); + await collisionDialog.expectApplyToAllVisible(false); + await collisionDialog.selectOption(staticData.collisionSkipOption); + await collisionDialog.submit(); + + await collisionDialog.expectClosed(); + expect(requests.trash).toHaveLength(0); + expect(requests.fileEntries).toHaveLength(0); + }); + + test('TC4: Validate that "apply to all" skips every duplicated file at once and closes the dialog', async () => { + const { drivePage, collisionDialog, requests } = drive; + + await drivePage.uploadFiles([duplicatedReport, duplicatedInvoice]); + + await collisionDialog.expectOpenFor('report.txt'); + await collisionDialog.checkApplyToAllByClickingLabel(); + await collisionDialog.selectOption(staticData.collisionSkipOption); + await collisionDialog.submit(); + + await collisionDialog.expectClosed(); + await expect(drivePage.fileRow('report.txt')).toHaveCount(1); + await expect(drivePage.fileRow('invoice.pdf')).toHaveCount(1); + expect(requests.trash).toHaveLength(0); + expect(requests.fileEntries).toHaveLength(0); + }); + + test('TC5: Validate that replacing a duplicated file sends its matching existing file to trash', async () => { + const { drivePage, collisionDialog, requests } = drive; + + await drivePage.uploadFiles([duplicatedInvoice]); + await collisionDialog.resolve('invoice.pdf', staticData.collisionReplaceOption); + + await expect + .poll(() => requests.trash, { timeout: 10000 }) + .toEqual([{ items: [{ uuid: existingInvoice.uuid, type: 'file' }] }]); + await collisionDialog.expectClosed(); + }); +}); diff --git a/test/e2e/tests/specs/internxt-login.spec.ts b/test/e2e/tests/specs/internxt-login.spec.ts index 21ec5fbfa8..a8f50a8e02 100644 --- a/test/e2e/tests/specs/internxt-login.spec.ts +++ b/test/e2e/tests/specs/internxt-login.spec.ts @@ -1,54 +1,17 @@ -import { expect, Request, Route, test } from '@playwright/test'; -import { getLoggedUser, getUserCredentials } from '../helper/getUser'; +import { expect, test } from '@playwright/test'; +import { INVALID_EMAIL, mockAuthRoutes } from '../helper/authRouteMocks'; +import { getUserCredentials } from '../helper/getUser'; import { staticData } from '../helper/staticData'; import { LoginPage } from '../pages/loginPage'; -const BASE_API_URL = process.env.REACT_APP_DRIVE_NEW_API_URL; const credentialsFile = getUserCredentials(); -const user = getLoggedUser(); -const invalidEmail = 'invalid@internxt.com'; - -const mockLoginCall = async (route: Route, request: Request) => { - await route.fulfill({ - status: 200, - contentType: 'application/json', - body: JSON.stringify({ - hasKeys: true, - sKey: '53616c7465645f5f2aa5386bc0b15f6f69a733acdd46a6551dc004f6c1cb6352390535de3ec17e9b96da7de984e5d27e79ad04a88a2cc8c6315f03dc0b0d174c', - tfa: false, - hasKyberKeys: true, - hasEccKeys: true, - }), - }); -}; - -const mockAccessCall = async (route: Route, request: Request) => { - const { email } = request.postDataJSON(); - - if (invalidEmail === email) { - return route.fulfill({ - status: 400, - body: JSON.stringify({ message: 'Wrong login credentials' }), - }); - } - - await route.fulfill({ - status: 200, - body: JSON.stringify({ - user: user.user, - token: user.token, - newToken: user.newToken, - userTeam: user.userTeam, - }), - }); -}; +const invalidEmail = INVALID_EMAIL; test.describe('internxt login', async () => { test.use({ storageState: { cookies: [], origins: [] } }); test.beforeEach('Visiting Internxt', async ({ page }) => { - await page.route(`${BASE_API_URL}/auth/login`, mockLoginCall); - await page.route(`${BASE_API_URL}/auth/login/access`, mockAccessCall); + await mockAuthRoutes(page); await page.goto('/'); await expect(page).toHaveURL('http://localhost:3000/login'); From c947fa07ecf08410ce2b4bb86c91669a7e49a97d Mon Sep 17 00:00:00 2001 From: Francis Terrero Date: Thu, 10 Sep 2026 23:38:34 -0400 Subject: [PATCH 08/20] feat: skip item option in name collision Adds a Skip option to the name-collision dialog, next to Replace and Keep both, for both upload and move collisions. Duplicates are resolved one at a time, and a new "Apply this action to all duplicates" checkbox handles them all at once, closing the dialog immediately and running the work in the background. The renameModal i18n block is renamed to alreadyExistsModal (the dialog does not rename anything), the description text is operation-neutral and the apply-to-all label toggles the checkbox. --- .../NameCollisionContainer.tsx | 67 ++++++++++++--- .../components/NameCollisionDialog/index.tsx | 82 +++++++++++++------ .../nameCollision.actions.test.ts | 8 +- .../nameCollision.actions.ts | 11 ++- .../nameCollision.utils.test.ts | 53 +++++++++++- .../nameCollision.utils.ts | 26 ++++++ src/app/i18n/locales/de.json | 10 +-- src/app/i18n/locales/en.json | 10 +-- src/app/i18n/locales/es.json | 10 +-- src/app/i18n/locales/fr.json | 10 +-- src/app/i18n/locales/it.json | 10 +-- src/app/i18n/locales/ru.json | 10 +-- src/app/i18n/locales/tw.json | 10 +-- src/app/i18n/locales/zh.json | 10 +-- 14 files changed, 243 insertions(+), 84 deletions(-) diff --git a/src/app/drive/components/NameCollisionDialog/NameCollisionContainer.tsx b/src/app/drive/components/NameCollisionDialog/NameCollisionContainer.tsx index 30cd66efc7..70aea771d7 100644 --- a/src/app/drive/components/NameCollisionDialog/NameCollisionContainer.tsx +++ b/src/app/drive/components/NameCollisionDialog/NameCollisionContainer.tsx @@ -7,6 +7,7 @@ import { IRoot } from 'app/store/slices/storage/types'; import workspacesSelectors from 'app/store/slices/workspaces/workspaces.selectors'; import { fileVersionsSelectors } from 'app/store/slices/fileVersions'; import { NameCollisionContext, resolveCollision } from './nameCollision.actions'; +import { findExistingItemFor, findPendingGroupIndex, getRemainingGroups } from './nameCollision.utils'; const NameCollisionContainer: FC = () => { const dispatch = useAppDispatch(); @@ -17,6 +18,7 @@ const NameCollisionContainer: FC = () => { const operationType = collisionDialogInfo?.operation; const newItems = useMemo(() => collisionGroups.flatMap((g) => g.duplicatedItems), [collisionGroups]); const existingItems = useMemo(() => collisionGroups.flatMap((g) => g.existingItems), [collisionGroups]); + const remainingItemsCount = existingItems.length; const selectedWorkspace = useAppSelector(workspacesSelectors.getSelectedWorkspace); const limits = useAppSelector(fileVersionsSelectors.getLimits); @@ -29,20 +31,60 @@ const NameCollisionContainer: FC = () => { dispatch(uiActions.setIsNameCollisionDialogOpen({ open: false, info: undefined })); }; - const triggerSelectedOptionsOnSubmit = async ({ operationType, operation }: OnSubmitPressed) => { - for (const group of collisionGroups) { - await resolveCollision( - { - operationType, - operation, - items: group.duplicatedItems, - existingItems: group.existingItems, - destinationUuid: group.destinationUuid, - }, - context, + const triggerSelectedOptionsOnSubmit = async ({ operationType, operation, applyToAll }: OnSubmitPressed) => { + if (applyToAll) { + closeDialog(); + await Promise.all( + collisionGroups.map((group) => + resolveCollision( + { + operationType, + operation, + items: group.duplicatedItems, + existingItems: group.existingItems, + destinationUuid: group.destinationUuid, + }, + context, + ), + ), ); + return; + } + + const groupIndex = findPendingGroupIndex(collisionGroups); + const hasPendingGroup = groupIndex !== -1; + if (!hasPendingGroup) { + closeDialog(); + return; + } + + const group = collisionGroups[groupIndex]; + const itemToUpload = group.duplicatedItems[0]; + const itemToReplace = findExistingItemFor(itemToUpload, group.existingItems); + + await resolveCollision( + { + operationType, + operation, + items: [itemToUpload], + existingItems: group.existingItems, + destinationUuid: group.destinationUuid, + }, + context, + ); + + const remainingGroups = getRemainingGroups(collisionGroups, groupIndex, itemToReplace); + const hasRemainingGroups = remainingGroups.length > 0; + if (hasRemainingGroups) { + dispatch( + uiActions.setIsNameCollisionDialogOpen({ + open: true, + info: { groups: remainingGroups, operation: operationType }, + }), + ); + } else { + closeDialog(); } - closeDialog(); }; if (!collisionDialogInfo) return null; @@ -56,6 +98,7 @@ const NameCollisionContainer: FC = () => { onSubmitButtonPressed={triggerSelectedOptionsOnSubmit} onCloseDialog={closeDialog} operationType={operationType as 'move' | 'upload'} + remainingItemsCount={remainingItemsCount} /> ); }; diff --git a/src/app/drive/components/NameCollisionDialog/index.tsx b/src/app/drive/components/NameCollisionDialog/index.tsx index 7db0c245bb..f6419a0d9d 100644 --- a/src/app/drive/components/NameCollisionDialog/index.tsx +++ b/src/app/drive/components/NameCollisionDialog/index.tsx @@ -1,7 +1,7 @@ import { FC, useEffect, useMemo, useState } from 'react'; import { Label, Radio, RadioGroup } from '@headlessui/react'; -import { Button, Modal } from '@internxt/ui'; +import { Button, Checkbox, Modal } from '@internxt/ui'; import { DriveItemData } from 'app/drive/types'; import { useTranslationContext } from 'app/i18n/provider/TranslationProvider'; import { IRoot } from 'app/store/slices/storage/types'; @@ -13,9 +13,10 @@ export const OPERATION_TYPE = { export type OnSubmitPressed = { operationType: 'move' | 'upload'; - operation: 'keep' | 'replace'; + operation: 'keep' | 'replace' | 'skip'; itemsToUpload: (File | IRoot | DriveItemData)[]; itemsToReplace: (DriveItemData | IRoot)[]; + applyToAll: boolean; }; export interface NameCollisionDialogProps { @@ -23,9 +24,16 @@ export interface NameCollisionDialogProps { isOpen: boolean; driveItems: (DriveItemData | IRoot)[]; newItems: (File | IRoot)[]; + remainingItemsCount: number; onCloseDialog(): void; onCancelButtonPressed(): void; - onSubmitButtonPressed({ operationType, operation, itemsToUpload, itemsToReplace }: OnSubmitPressed): Promise; + onSubmitButtonPressed({ + operationType, + operation, + itemsToUpload, + itemsToReplace, + applyToAll, + }: OnSubmitPressed): Promise; } const NameCollisionDialog: FC = ({ @@ -33,35 +41,41 @@ const NameCollisionDialog: FC = ({ operationType, newItems, driveItems, + remainingItemsCount, onCancelButtonPressed, onCloseDialog, onSubmitButtonPressed, }: NameCollisionDialogProps) => { const { translate } = useTranslationContext(); - const options = [ - { - operation: 'replace' as const, - name: translate('modals.renameModal.replaceItem'), - }, - { - operation: 'keep' as const, - name: translate('modals.renameModal.keepBoth'), - }, - ]; + const options = useMemo( + () => [ + { + operation: 'replace' as const, + name: translate('modals.alreadyExistsModal.replaceItem'), + }, + { + operation: 'keep' as const, + name: translate('modals.alreadyExistsModal.keepBoth'), + }, + { + operation: 'skip' as const, + name: translate('modals.alreadyExistsModal.skipItem'), + }, + ], + [translate], + ); const [isLoading, setIsLoading] = useState(false); const [selectedOption, setSelectedOption] = useState(options[0]); + const [applyToAll, setApplyToAll] = useState(false); + + const title = translate('modals.alreadyExistsModal.title'); + const description = translate('modals.alreadyExistsModal.description', { itemName: newItems?.[0]?.name }); - const title = - newItems?.length > 1 ? translate('modals.renameModal.titleMultipleItems') : translate('modals.renameModal.title'); - const description = - newItems?.length > 1 - ? translate('modals.renameModal.multipleDescription') - : translate('modals.renameModal.description', { itemName: newItems?.[0]?.name }); const primaryButtonText = useMemo( () => operationType === OPERATION_TYPE.MOVE - ? translate('modals.renameModal.move') - : translate('modals.renameModal.upload'), + ? translate('modals.alreadyExistsModal.move') + : translate('modals.alreadyExistsModal.upload'), [operationType], ); @@ -69,13 +83,15 @@ const NameCollisionDialog: FC = ({ if (isOpen) { setIsLoading(false); setSelectedOption(options[0]); + setApplyToAll(false); } - }, [isOpen]); + }, [isOpen, options]); const onClose = (): void => { onCloseDialog(); onCancelButtonPressed(); setSelectedOption(options[0]); + setApplyToAll(false); setIsLoading(false); }; @@ -88,9 +104,10 @@ const NameCollisionDialog: FC = ({ operation: selectedOption.operation, itemsToUpload: newItems, itemsToReplace: driveItems, + applyToAll, }); - onClose(); + setIsLoading(false); }; return ( @@ -100,7 +117,7 @@ const NameCollisionDialog: FC = ({

{description}

- +
{options.map((option) => ( @@ -109,12 +126,14 @@ const NameCollisionDialog: FC = ({ className={`flex h-5 w-5 flex-col items-center justify-center rounded-full ${ option.operation === selectedOption.operation ? 'bg-primary active:bg-primary-dark' - : 'border border-gray-40 bg-white group-hover:border-gray-50' + : 'border border-gray-40 bg-white dark:bg-transparent group-hover:border-gray-50' }`} >
@@ -128,6 +147,17 @@ const NameCollisionDialog: FC = ({
+ {remainingItemsCount > 1 && ( +
+ setApplyToAll((prev) => !prev)} /> + +
+ )}