Repository navigation
[PB-6531]: match name collisions by name and type and batch their resolution - #2148
Merged
terrerox merged 19 commits intoOct 7, 2026
Conversation
This was referenced Sep 11, 2026
Deploying drive-web with
|
| Latest commit: |
afbfe83
|
| Status: | ⚡️ Build in progress... |
terrerox
added this pull request to stack #2149
September 11, 2026 04:06
terrerox
removed this pull request from stack #2149
September 11, 2026 05:41
terrerox
added this pull request to stack #2151
September 11, 2026 05:44
terrerox
force-pushed
the
refactor/extract-name-collision-upload-actions
branch
from
September 15, 2026 03:33
34f9b46 to
af37153
Compare
terrerox
force-pushed
the
fix/name-collision-matching-and-batching
branch
from
September 15, 2026 03:33
7091a30 to
bce942f
Compare
CandelR
reviewed
Sep 18, 2026
| * 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[] => |
Collaborator
There was a problem hiding this comment.
nit: asExistingItems reads like a cast but it's actually adding isFolder field, maybe addIsFolderField or similar would be more honest about what it does
Comment on lines
+63
to
+71
| 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<void> => { | ||
| if (items.length === 0) return; | ||
|
|
||
| const itemsParsed = await Promise.all(items.map((item) => getUniqueNameMovePayload(item, destinationUuid))); | ||
| await dispatch(storageThunks.moveItemsThunk({ items: itemsParsed, destinationFolderId: destinationUuid })); |
Collaborator
There was a problem hiding this comment.
maybe would be fine to check if some items have same name+extension, because I think that from trash could go items with same name and extension to same folder and will produce errors
Contributor
Author
4 of 7 tasks
CandelR
previously approved these changes
Sep 22, 2026
terrerox
force-pushed
the
fix/name-collision-matching-and-batching
branch
2 times, most recently
from
September 30, 2026 02:25
6ce2539 to
510b4d1
Compare
terrerox
force-pushed
the
fix/name-collision-matching-and-batching
branch
2 times, most recently
from
October 7, 2026 01:03
08a7437 to
16b1766
Compare
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.
…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.
…stency in handling existing items
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.
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.
Skipping a colliding folder upload now keeps the existing folder and uploads only the files and subfolders that do not exist yet, merging colliding subfolders recursively so the folder structure is preserved.
Covers keep both for files and folders, replacing a folder, merging a skipped folder into the existing one, replace, keep both and skip for moved files, and versioned replace. The Drive mock now tracks folders, folder creation, moves and a versioning flag; moves are driven by dispatching the HTML5 drag events on the row's drop zone.
terrerox
force-pushed
the
fix/name-collision-matching-and-batching
branch
from
October 7, 2026 13:25
16b1766 to
5a3ca29
Compare
terrerox
removed this pull request from stack #2151
October 7, 2026 13:28
…restore-e2e [PB-6531]: e2e coverage for same-name restore collisions
…atch [PB-6531]: resolve same-name items within a move collision batch
…ge-on-skip [PB-6531]: merge folder uploads into the existing folder on skip
[PB-6531]: e2e coverage for the skip option in name collisions
…collision [PB-6531]feat/skip item option in name collision
terrerox
merged commit Oct 7, 2026
bef2cfe
into
refactor/extract-name-collision-upload-actions
1 of 3 checks passed
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Stacked on
refactor/extract-name-collision-upload-actions. Two fixes to how name collisions are resolved, split out of #2039 so that PR only adds the Skip option.findExistingItemForinnameCollision.utils.ts, and items without a match are left out of the replace.moveItemsToTrash,moveItemsThunk, the upload managers) in one call instead of one call per item, so batching, concurrency limits and per-item error handling are inherited rather than reimplemented. Versioned replacements still run one at a time because that upload bypasses the queue. The destination folder is refreshed once per group instead of once per item.Unit tests cover the matching rules and every resolution path.
Related Issues
Related Pull Requests
Checklist
Testing Process
yarn vitest run src/app/drive/components/NameCollisionDialog, plus the manual upload/move collision flows (keep both, replace, with and without versioning).