fix(kanban): a rejected drag rolls the card back on the external-data path too (#4138) - #4204
Merged
Merged
Conversation
… path too (#4138) `handleCardMove`'s failure revert was gated on `!hasExternalData`, so the ListView-hosted board (which receives records via the `data` prop) left the card in the target column after a server rejection until a manual reload. The comment's premise — "parent handles refresh" — holds for an accepted move but not a rejected one: nothing changes server-side, so no refetch is ever triggered. The revert is now unconditional, which is what makes it one code path for both ownerships. The card's on-screen position lives in `KanbanImpl`'s `boardColumns`, re-synced from the `columns` prop on every identity change, so a re-render of `ObjectKanban` is what un-says the move: on internal data the map corrects the record, on external data the fresh array identity re-renders and the board re-buckets from the parent's unchanged records. The optimistic write on the way in stays gated deliberately — doing it on the external path would snap an accepted move back before the server answered — and both directions are now pinned. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
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.
Fixes #4138
An illegal Kanban drag that the server rejects (
PATCH400invalid_transition) left the card stuck in the target column until a manual reload — but only when the board is hosted by a parent supplying records through thedataprop, which is the ListView/console path real users meet. The toast fired and the server value was unchanged, so the board kept showing a move that had not happened.Root cause, confirmed at
origin/main@b1e42d09bpackages/plugin-kanban/src/ObjectKanban.tsx—handleCardMoveran its failure revert only insideif (!hasExternalData). The premise recorded beside it ("parent handles refresh", line 147) holds for an accepted move: the parent's mutation subscription refetches and the new value propagates. A rejected move changes nothing server-side, so that refetch is never triggered and nothing un-said the optimistic move. Premise of the issue verified, unchanged, and reproduced as a failing test before any edit.Where the optimistic state actually lives (measured first)
Not in
ObjectKanban. This was measured before writing the fix, because the revert has to clear exactly the layer that holds the move:KanbanImpl'shandleDragEndmoves the card inside its ownboardColumnsstate and only then callsonCardMove. That is why the card visibly moves on both data paths, with no parent re-render involved.boardColumnsis re-synced from thecolumnsprop by an effect keyed on that prop's identity.columnscomes fromKanbanRenderer, which re-buckets records into a fresh array whenever itsschemaprop changes identity — andObjectKanbanhands it a new object literal on every render.So a re-render of
ObjectKanbanis what un-says the move, by re-bucketing the source-of-truth records.fetchedDatais read in exactly one place (therawDataline, 219), and on the external pathexternalDatawins there — which is why the old gated revert was a no-op that nothing could observe.The fix
One unconditional revert serving both ownerships — deliberately not a second revert path:
fetchedDatais the source of truth, so the map corrects the record and re-renders.fetchedDatais unread and normally empty, butArray#mapalways allocates, so the fresh identity re-renders the component and the board re-buckets fromexternalData— which the server never changed. That is the revert: the card returns tofromColumnId.The optimistic write on the way in stays gated on internal data deliberately, and the asymmetry is now pinned by a control: writing
fetchedDataon the external path would re-render against the unchanged parent records and snap an accepted move back before the server had answered.No host-notification API was added (per the direction ruling on the card); none turned out to be needed, since the revert is the board's own display state.
Tests
New pin
packages/plugin-kanban/src/ObjectKanban.rejectedMoveRollback.test.tsxdrives the board throughDndContext's realonDragEnd, soKanbanImpl's local move, the persist, the failure branch and the re-bucket are all the production code path.Red-first, pre-fix, exactly the QA signature —
Backlogempty, card left in the target column:Reverse verification (after committing,
git checkout origin/main -- packages/plugin-kanban/src/ObjectKanban.tsx, tests kept). Direction predicted before running: RED on the external+rejected case only, with the stuck-card signature; the four controls stay green. Confirmed exactly —Tests 1 failed | 4 passed (5), same assertion as above. Restored withgit checkout HEAD -- ....Full package, on the restored tree:
Dependency closure built first (
pnpm --filter '@object-ui/plugin-kanban^...' build) so the type-check read fresh declarations rather than staledist.Changeset:
.changeset/kanban-rejected-move-rollback-4138.md(@object-ui/plugin-kanbanpatch) — user-visible behavior change.Generated by Claude Code