fix: stop a half-completed media collection move reporting success (#6017) - #6123
Merged
Conversation
…6017) Moving items between media collections is two writes: add to the target, then remove from the source. bulkMoveOrCopy swallowed every removal error into null and only counted add failures, so a move whose removals all failed still toasted "Moved N to ..." while the items sat duplicated in both collections — and clearing the selection left the user with nothing to retry. Removal failures are now tracked separately. When any removal fails the toast says "Copied N ..., but M could not be removed from ..." (never a success toast), the collection still reconciles to the authoritative server state from the last successful removal, and select mode stays open with only the half-moved items selected so the move can be re-run on exactly what failed. Add failures keep their own count in the same message. Claude-Session: https://claude.ai/code/session_01VjkWVTfzKyRuAv3HEsspwN
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
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.
Summary
Moving items between media collections is two writes: add to the target, then remove from the source.
bulkMoveOrCopyinclient/src/pages/MediaCollectionDetail.jsxswallowed every removal error intonull(.catch(() => null)) and computed its failure count asselectedItems.length - added - dupes— which only accounts for add failures. A move whose removals all failed still toastedMoved N to "…"while the items sat duplicated in both collections, andexitSelectMode()cleared the selection so the user had nothing left to retry.This change:
removeFailedKeysset, separate from the add-failure count.Copied N to "<target>", but M could not be removed from "<source>"— deliberately "Copied", because calling a half-completed move a "Move" is exactly the false success this branch exists to prevent. An add failure appends; K failed to addto the same message.collectionto the authoritative server state returned by the last successful removal, so a partial failure doesn't leave the view showing items that did move out.The synthetic Unsorted source is unaffected — it never issues removals, it just re-derives via
refresh().Rejected alternative (per the issue): rolling back the adds in the target collection with automated DELETEs. Accurate partial-progress reporting leaves the user in control instead of cascading more writes onto an already-unstable destination.
Test plan
client/src/pages/MediaCollectionDetail.test.jsxgains three cases coveringbulkMoveOrCopy: every removal succeeds (plain success toast, select mode exits), every removal fails (error toast, no success toast, all items stay selected), and a mixed run where one add and one removal fail (both counts reported, collection reconciled to the last successful removal, selection narrowed to the one failed key).BulkTargetPickeris now mocked with a clickable destination row so the flow can be driven end to end.cd client && npx vitest run— 832 files / 10220 tests pass. One unrelated pre-existing date-boundary flake insrc/components/calendar/ReviewTab.test.jsxfailed on the first run and passed on re-run; it is untouched by this diff.Closes #6017
https://claude.ai/code/session_01VjkWVTfzKyRuAv3HEsspwN