Repository navigation
[PB-6531]: extract name collision move actions out of the container - #2146
Merged
Merged
Conversation
This was referenced Sep 11, 2026
Deploying drive-web with
|
| Latest commit: |
1598ef4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0f552e67.drive-web.pages.dev |
| Branch Preview URL: | https://refactor-extract-name-collis-twf5.drive-web.pages.dev |
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
CandelR
reviewed
Sep 14, 2026
CandelR
previously approved these changes
Sep 16, 2026
terrerox
force-pushed
the
refactor/extract-name-collision-move-actions
branch
2 times, most recently
from
September 30, 2026 02:25
b30a5cd to
7d4f851
Compare
terrerox
dismissed
CandelR’s stale review
October 2, 2026 11:55
The merge-base changed after approval.
terrerox
force-pushed
the
refactor/extract-name-collision-move-actions
branch
2 times, most recently
from
October 7, 2026 01:03
7fff079 to
7311fe3
Compare
Move the keep/replace handling for moved items into nameCollision.actions behind resolveMoveCollision, with unit tests. No behaviour change.
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
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.
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
refactor/extract-name-collision-move-actions
branch
from
October 7, 2026 13:25
7311fe3 to
6f45d60
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
…d-batching [PB-6531]: match name collisions by name and type and batch their resolution
…n-upload-actions [PB-6531]: extract name collision upload actions out of the container
larryrider
previously approved these changes
Oct 8, 2026
larryrider
left a comment
larryrider
left a comment
Contributor
There was a problem hiding this comment.
Check and/or fix SonarCloud issues before merge
|
larryrider
approved these changes
Oct 8, 2026
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
3 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
First of a stack that ends in #2039 (skip option) and #2129 (merge folders on skip). Pure refactor, no behaviour change.
The name-collision container held every side effect inline. This moves the move-collision handling (keep both with a unique name, replace by trashing the existing item) into
nameCollision.actions.tsbehindresolveMoveCollision, which takes the items plus a small context (dispatch, workspace, upload limit, versioning flag) instead of closing over component state. Upload collisions stay in the container for now and move in the next PR.Unit tests cover both move resolutions.
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).