fix(board): serialize per-card move and delete settlements #3312
+423
−85
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,18 +1,102 @@ | ||
| /** | ||
| * Card operations: fetch, create, update, delete, move cards, and provenance. | ||
| */ | ||
| import { watch } from 'vue' | ||
| import { cardsApi } from '../../api/cardsApi' | ||
| import { getErrorMessage } from '../../utils/errorMessage' | ||
| import type { CardDetachPreview, CreateCardDto, UpdateCardDto, CardCaptureProvenance } from '../../types/board' | ||
| import type { BoardState } from './boardState' | ||
| import type { BoardHelpers } from './boardStoreHelpers' | ||
| import type { BoardFetchOptions } from './boardCrudStore' | ||
|
|
||
| interface CardMutationVisit { | ||
| boardId: string | ||
| generation: number | ||
| } | ||
|
|
||
| class StaleBoardVisitError extends Error { | ||
| constructor() { | ||
| super('The board visit that queued this card change has ended.') | ||
| this.name = 'StaleBoardVisitError' | ||
| } | ||
| } | ||
|
|
||
| export function createCardActions( | ||
| state: BoardState, | ||
| helpers: BoardHelpers, | ||
| refreshBoard: (boardId: string, options?: BoardFetchOptions) => Promise<boolean>, | ||
| ) { | ||
| // Move and delete target the same durable card and neither API exposes a | ||
| // shared client mutation token. Serialize only that per-card lane so server | ||
| // commit order follows user intent, while unrelated cards remain concurrent. | ||
| // The visit generation prevents a queued pre-logout intent from starting | ||
| // under a later session's credentials. | ||
| const mutationTailByCardId = new Map<string, Promise<void>>() | ||
| let boardVisitGeneration = 0 | ||
|
|
||
| watch( | ||
| () => state.currentBoard.value?.id ?? null, | ||
| (nextBoardId, previousBoardId) => { | ||
| if (nextBoardId !== previousBoardId) boardVisitGeneration++ | ||
| }, | ||
| { flush: 'sync' }, | ||
| ) | ||
|
|
||
| function captureCardMutationVisit(boardId: string): CardMutationVisit { | ||
| return { boardId, generation: boardVisitGeneration } | ||
| } | ||
|
|
||
| function isCurrentCardMutationVisit(visit: CardMutationVisit) { | ||
| const currentBoard = state.currentBoard.value | ||
| return ( | ||
| (currentBoard === null || currentBoard.id === visit.boardId) && | ||
| boardVisitGeneration === visit.generation | ||
| ) | ||
| } | ||
|
|
||
| async function runCardMutation<T>( | ||
| cardId: string, | ||
| visit: CardMutationVisit, | ||
| mutation: () => Promise<T>, | ||
| ): Promise<T> { | ||
| const previous = mutationTailByCardId.get(cardId) | ||
| let operation: Promise<T> | ||
|
|
||
| if (previous) { | ||
| operation = previous.catch(() => undefined).then(() => { | ||
| if (!isCurrentCardMutationVisit(visit)) throw new StaleBoardVisitError() | ||
| return mutation() | ||
| }) | ||
| } else { | ||
| // The first intent is already submitted by the caller; do not defer its | ||
| // transport to a microtask where immediate navigation could cancel it. | ||
| if (!isCurrentCardMutationVisit(visit)) throw new StaleBoardVisitError() | ||
| operation = mutation() | ||
| } | ||
|
|
||
| const tail = operation.then( | ||
| () => undefined, | ||
| () => undefined, | ||
| ) | ||
| mutationTailByCardId.set(cardId, tail) | ||
|
|
||
| try { | ||
| return await operation | ||
| } finally { | ||
| if (mutationTailByCardId.get(cardId) === tail) { | ||
| mutationTailByCardId.delete(cardId) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function isOlderCardSnapshot(candidateUpdatedAt: string, currentUpdatedAt: string) { | ||
| const candidateTime = Date.parse(candidateUpdatedAt) | ||
| const currentTime = Date.parse(currentUpdatedAt) | ||
| return Number.isFinite(candidateTime) && | ||
| Number.isFinite(currentTime) && | ||
| candidateTime < currentTime | ||
|
Comment on lines
+93
to
+97
There was a problem hiding this comment.
When a refresh installs a newer card less than one millisecond after the pending move, Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Confirmed. The current helper reduces both 2026-09-22T00:00:00.1234567Z and 2026-09-22T00:00:00.1234999Z to 1790035200123; the older-than predicate therefore returns false. Reproduced that exact precision loss locally in Node, without treating it as full project execution. The 8f89bfc commit only reconciles the two older fixtures; it does not address this new finding. Leaving this thread unresolved and the PR draft. The correction needs a precision-preserving instant comparison, not lexical comparison of unnormalised offset strings, plus canonical pending-move/authoritative-refresh tests at sub-millisecond differences, equal instants with different offsets, and normal newer/equal controls. Card payload and column counts must both remain authoritative when the older response settles. |
||
| } | ||
|
|
||
| async function refreshDetachedChildren(boardId: string) { | ||
| // The mutation already committed. It only changes hierarchy ownership, not | ||
| // surviving comment threads, so keep the open editor's same-board cache | ||
|
|
@@ -130,44 +214,44 @@ export function createCardActions( | |
|
|
||
| async function deleteCard(boardId: string, cardId: string, confirmation?: CardDetachPreview) { | ||
| helpers.guardDemoMutation() | ||
| const visit = captureCardMutationVisit(boardId) | ||
| return runCardMutation(cardId, visit, async () => { | ||
| let refreshChildren = false | ||
| try { | ||
| state.loading.value = true | ||
| state.error.value = null | ||
| await cardsApi.deleteCard(boardId, cardId, confirmation) | ||
| helpers.markBoardDetailMutation(boardId) | ||
|
|
||
| // A move, realtime refresh, or navigation can replace this state while | ||
| // the DELETE is in flight. Commit only into the exact initiating board | ||
| // visit and derive the count delta from the card that exists NOW. If an | ||
| // authoritative refresh already removed it, its count is already settled. | ||
| if (isCurrentCardMutationVisit(visit)) { | ||
| const committedCard = state.currentBoardCards.value.find((card) => card.id === cardId) | ||
| state.currentBoardCards.value = state.currentBoardCards.value.filter((card) => card.id !== cardId) | ||
| if (state.cardCommentsByCardId.value[cardId]) { | ||
| const { [cardId]: _, ...remainingComments } = state.cardCommentsByCardId.value | ||
| state.cardCommentsByCardId.value = remainingComments | ||
| } | ||
|
|
||
| if (committedCard) { | ||
| helpers.updateColumnCardCount(committedCard.columnId, -1) | ||
| } | ||
|
|
||
| refreshChildren = state.currentBoard.value?.id === boardId && | ||
| state.currentBoardCards.value.some(card => card.parentCardId === cardId) | ||
| helpers.toast.success('Card deleted successfully') | ||
| } | ||
| } catch (e: unknown) { | ||
| helpers.handleApiError(e, 'Failed to delete card') | ||
| throw e | ||
| } finally { | ||
| state.loading.value = false | ||
| } | ||
| // Finish mutation-owned loading/error writes before a refresh can outlive navigation. | ||
| if (refreshChildren) await refreshDetachedChildren(boardId) | ||
| }) | ||
| } | ||
|
|
||
| async function moveCard( | ||
|
|
@@ -177,53 +261,56 @@ export function createCardActions( | |
| targetPosition: number, | ||
| ) { | ||
| helpers.guardDemoMutation() | ||
| const visit = captureCardMutationVisit(boardId) | ||
| return runCardMutation(cardId, visit, async () => { | ||
| try { | ||
| state.loading.value = true | ||
| state.error.value = null | ||
|
|
||
| const updatedCard = await cardsApi.moveCard(boardId, cardId, { | ||
| targetColumnId, | ||
| targetPosition, | ||
| }) | ||
| helpers.markBoardDetailMutation(boardId) | ||
|
|
||
| if (!isCurrentCardMutationVisit(visit)) { | ||
| return updatedCard | ||
| } | ||
|
|
||
| // Resolve the committed card after the await. Its current column owns | ||
| // any count delta; a pre-request snapshot has no settlement authority. | ||
| const commitIndex = state.currentBoardCards.value.findIndex((card) => card.id === cardId) | ||
| if (commitIndex === -1) { | ||
| // A later delete or authoritative refresh removed the card. Never | ||
| // resurrect it from an older move response. Preserve the historical | ||
| // null-board preload behavior only when no board session exists yet. | ||
| if (state.currentBoard.value === null && state.currentBoardCards.value.length === 0) { | ||
| state.currentBoardCards.value.push(updatedCard) | ||
| helpers.toast.success('Card moved successfully') | ||
| } | ||
| return updatedCard | ||
| } | ||
|
|
||
| const committedCard = state.currentBoardCards.value[commitIndex] | ||
| if (isOlderCardSnapshot(updatedCard.updatedAt, committedCard.updatedAt)) { | ||
| return updatedCard | ||
| } | ||
|
|
||
| state.currentBoardCards.value[commitIndex] = updatedCard | ||
| if (committedCard.columnId !== updatedCard.columnId) { | ||
| helpers.updateColumnCardCount(committedCard.columnId, -1) | ||
| helpers.updateColumnCardCount(updatedCard.columnId, 1) | ||
| } | ||
|
|
||
| helpers.toast.success('Card moved successfully') | ||
| return updatedCard | ||
| } catch (e: unknown) { | ||
| helpers.handleApiError(e, 'Failed to move card') | ||
| throw e | ||
| } finally { | ||
| state.loading.value = false | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| async function fetchCardProvenance( | ||
|
|
||
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a second same-card mutation is queued and the user navigates from board A while the first request is pending, this watcher may not advance the generation because
currentBoardremains the last committed board until the next board fetch succeeds, andBoardViewalso leaves it intact on unmount. If the first request settles during a slow board-B load—or after leaving the board route—the queued mutation still passes the visit check and starts against A, despite belonging to the abandoned visit; bind invalidation to the route/fetch visit lifecycle rather than the committed board payload.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed against exact head 8f89bfc: BoardView's route watcher changes its local boardId before awaiting fetchBoard, while the store's mutation watcher observes only currentBoard.id. On unmount the view cancels a background fetch and clears presence/editing state, but does not retire this mutation-visit generation. startBoardFetch advances its own request generation before transport without changing the committed payload. Thus the queued A intent can still start while B is pending or the board route has closed.
No fix for this new finding is claimed by the fixture-only commit. Keep the thread open and PR draft. A separate explicit route/mutation-visit boundary must retire queued intent before a new foreground route load or view teardown, while preserving ordinary same-board background refreshes and already-dispatched transport. Add integration-shaped deferred tests for slow/failed B loads, route unmount and A-to-B-to-A, including no extra API call under the abandoned owner. Simply observing every board-fetch generation would incorrectly cancel legitimate same-board refresh work.