Skip to content

[PB-6531]: extract name collision logic out of the container - #2145

Closed
terrerox wants to merge 3 commits into
feat/skip-item-option-in-name-collisionfrom
refactor/extract-name-collision-logic
Closed

terrerox wants to merge 3 commits into
feat/skip-item-option-in-name-collisionfrom
refactor/extract-name-collision-logic

Conversation

@terrerox

Copy link
Copy Markdown
Contributor

Description

Stacked on #2039. Pure refactor, no behaviour change.

The name-collision container had the same operationType + operation switch twice (once for "apply to all", once for the one-at-a-time flow) and held all the item matching and per-operation side effects inline. This moves that logic out of the component:

  • nameCollision.utils.ts: pure helpers for matching an incoming item to its existing counterpart, pairing items, finding the next pending group and computing the remaining groups after a decision.
  • nameCollision.actions.ts: the side effects (keep/replace for move and upload, versioned replace) behind a single resolveCollision that takes the items to handle plus a small context (dispatch, workspace, upload limit, versioning flag). Both dialog paths call it, so the duplicated switch is gone.
  • NameCollisionContainer.tsx now only reads store state, builds the context and decides whether to close or reopen the dialog.

Both modules have unit tests (100% line coverage on the new files). #2129 is rebased on top of this so the merge-on-skip logic lands as one extra branch in resolveCollision.

Related Issues

Related Pull Requests

Checklist

  • Changes have been tested locally.
  • Unit tests have been written or updated as necessary.
  • The code adheres to the repository's coding standards.
  • Relevant documentation has been added or updated.
  • No new warnings or errors have been introduced.
  • SonarCloud issues have been reviewed and addressed.
  • QA Passed

Testing Process

yarn vitest run src/app/drive/components/NameCollisionDialog plus the same manual flows as #2039 (upload/move collisions with keep, replace, skip, per item and apply to all).

jaaaaavier and others added 3 commits August 25, 2026 20:38
- offer the skip option for move collisions and reword the dialog to be operation-neutral
- toggle the apply-to-all checkbox from its label
- match each duplicated item to its existing counterpart by name and type instead of by index
- apply actions to all duplicates through the array-based trash/move/upload primitives and close the dialog immediately
…ainer

Move the item matching, pairing and remaining-group logic into
nameCollision.utils and the per-operation side effects into
nameCollision.actions, so both the apply-to-all and one-at-a-time paths
share a single resolveCollision instead of two duplicated switches.
Both modules are covered by unit tests.
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying drive-web with  Cloudflare Pages  Cloudflare Pages

Latest commit: 52e8618
Status: ✅  Deploy successful!
Preview URL: https://dae7e2a6.drive-web.pages.dev
Branch Preview URL: https://refactor-extract-name-collis.drive-web.pages.dev

View logs

@sonarqubecloud

Copy link
Copy Markdown

@terrerox
terrerox force-pushed the feat/skip-item-option-in-name-collision branch from 9ce4b8b to 41817e2 Compare September 11, 2026 03:40
@terrerox

Copy link
Copy Markdown
Contributor Author

Superseded by the reordered stack: the refactor now lands before the feature as #2146 and #2147, with the matching/batching fixes in #2148, followed by #2039 and #2129.

@terrerox terrerox closed this Sep 11, 2026
@terrerox
terrerox deleted the refactor/extract-name-collision-logic branch September 11, 2026 03:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants