[6.x] New Asset Exists Modal Resolution Feature#14433
[6.x] New Asset Exists Modal Resolution Feature#14433jackmcdade wants to merge 5 commits into6.xfrom
Conversation
Works with uploading and moving assets. Resolves #14381.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| } | ||
|
|
||
| $ids[] = $asset->move($folder)->id(); | ||
| } |
There was a problem hiding this comment.
Partially moved assets cause stale IDs on retry
High Severity
When moving multiple assets, the run() method moves them sequentially and throws AssetConflictException on the first conflict. Assets moved before the conflict are not rolled back — their IDs change because their paths change. However, pendingSelections on the client still holds the original (now stale) IDs for those already-moved assets. When continueMoveConflictResolution re-sends those stale IDs in the next request, the server fails to resolve them, causing errors or silently losing track of those assets.
Additional Locations (1)
This updates the asset conflict UX by making modal-dismiss behavior resolve as cancel, only showing “apply to all” when multiple upload conflicts exist, and replacing recursive move-conflict continuation with an iterative loop. It also adds MoveAsset tests for non-conflicting moves, same-folder no-op moves, and explicit cancel strategy handling.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ad6f0b2. Configure here.
| : __('statamic::messages.asset_conflict_a_newer'); | ||
| $existingAge = $sourceLastModified >= $destinationLastModified | ||
| ? __('statamic::messages.asset_conflict_older') | ||
| : __('statamic::messages.asset_conflict_newer'); |
There was a problem hiding this comment.
Unused $existingAge variable computed but never displayed
Low Severity
The $existingAge variable is computed and passed as the :existing_age replacement to the translation message, but the asset_conflict_message template string doesn't contain an :existing_age placeholder. This is dead code that adds confusion about whether the message was intended to include this information.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ad6f0b2. Configure here.
1 participant
Works with uploading and moving assets. Resolves #14381.
asset-conflicts.mp4
Note
Medium Risk
Touches both CP frontend and backend action/upload flows to introduce new 409 conflict responses and resolution strategies; mistakes could cause unintended overwrites or stuck uploads/moves, though changes are scoped and covered by tests for move conflicts.
Overview
Adds first-class asset conflict resolution for both uploads and drag/drop moves in the CP.
Conflicting uploads now return structured
409JSON with conflict details, the browser queues these conflicts and presents a modal to Cancel, Keep Both (timestamp), or Overwrite, with optional apply to all, and cache-busts affected previews/thumbnails after overwrite/timestamp resolutions.Conflicting moves now detect existing destination files in
MoveAsset, throw anAssetConflictExceptionwith conflict metadata, and the asset browser intercepts this to show the same modal flow (including multi-selection handling, optional apply-to-all policy, and glide cache clearing on overwrite).Reviewed by Cursor Bugbot for commit ad6f0b2. Bugbot is set up for automated code reviews on this repo. Configure here.