From 4fa39113f0f284cc3d7f552b1edb547a255f8b3c Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Tue, 22 Sep 2026 20:27:31 +0100 Subject: [PATCH] Preserve board loading until each operation settles --- OUTSTANDING_TASKS.md | 5 + docs/STATUS.md | 19 +- .../src/store/board/boardCrudStore.ts | 66 ++--- .../src/store/board/boardState.ts | 3 + .../src/store/board/boardStoreHelpers.ts | 12 + .../src/store/board/cardCommentStore.ts | 14 +- .../taskdeck-web/src/store/board/cardStore.ts | 18 +- .../src/store/board/columnStore.ts | 63 +++-- .../src/store/board/labelStore.ts | 14 +- .../tests/store/board/boardCrudStore.spec.ts | 3 + .../store/board/boardLoadingOwnership.spec.ts | 227 ++++++++++++++++++ .../board/columnStoreRouteOwnership.spec.ts | 21 +- .../taskdeck-web/stryker.smoke.contract.mjs | 4 +- .../smart-ci/mutation-smoke-contract.test.mjs | 2 +- 14 files changed, 365 insertions(+), 106 deletions(-) create mode 100644 frontend/taskdeck-web/src/tests/store/board/boardLoadingOwnership.spec.ts diff --git a/OUTSTANDING_TASKS.md b/OUTSTANDING_TASKS.md index 97889711d..b8e207020 100644 --- a/OUTSTANDING_TASKS.md +++ b/OUTSTANDING_TASKS.md @@ -189,6 +189,11 @@ to move from line 667 to 673. The expression, column bounds and negative checks This small test-data change still touches a declared control path; independent review and passing CI do not supply the maintainer decision under ADR-0066. +The successor for [#3305](https://github.com/Chris0Jeky/Taskdeck/issues/3305) needs the same +maintainer review: replacing shared loading moves that unchanged mutation target from line +673 to 656, including the literal companion assertion under `scripts/ci/`. Its review and +verification remain separate from approval to merge. + - [ ] **Review PR #2787 post hoc and review new CI-control candidates before merge.** The coordinator merged prompt-v3 PR #2787 at `0cebd938d79f045a20ce99bff495b986c25cf267` with hosted checks and independent Terra review, but without the maintainer review required by the ADR-0066 amendment. Its changed surface includes the Windows archive acceptance script and matching tests. The earlier SC-10 delegation covered twelve named PRs and did not include #2787. Please review that merged change; the coordinator has not inferred acknowledgement or reverted it. New nightly observation PR #2791 and the #2335 control-trust test PR must finish independent review and exact-head hosted qualification before the maintainer reviews their final heads. This checkpoint grants no release, repository-settings, or selective-execution approval. ### J.3. Twelve control-plane PRs merged outside the ADR-0066 per-PR review (2026-09-09 to 2026-09-10) diff --git a/docs/STATUS.md b/docs/STATUS.md index f4fc9cc26..54624cd0e 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -2,6 +2,21 @@ Last Updated: 2026-09-22 +## Board loading belongs to pending operations (#3305) + +Board list/detail reads and the mutations that show shared loading now retain individual +owners. One request finishing cannot dismiss the skeleton or enable retries while another +operation remains pending. Filtered list reads and queued column/label writes participate +in the same ownership set; joining an existing list request does not add another owner. +Logout clears the set synchronously through the existing session generation, and late +settlements cannot clear a new account's work. Replacing an explicit detail read releases +its old owner even when the transport ignores cancellation. Background refreshes, direct +child reads and archive/restore retain their existing behavior without shared loading. + +Deferred tests cover cross-module overlap, rejection, logout, independent list requests, +detail replacement and queued background work. This reduces misleading empty/loading +states during board maintenance while preserving review-first proposal behavior. + ## Logout retires board mutation settlements (#3306) Board, card (including archive/restore), label and comment operations capture the shared @@ -19,7 +34,7 @@ notifications. Store results and errors still settle for the original caller. This prevents old-account data from reappearing and reduces cleanup after account switching. It preserves review-first proposal behavior and does not undo server writes. Same-session -loading arbitration remains #3305; card/comment ordering PRs #3312/#3304 require separate +loading arbitration is covered by #3305 above; card/comment ordering PRs #3312/#3304 require separate reconciliation with these guards before integration. ## Column writes follow their board visit (#3314) @@ -37,7 +52,7 @@ for that recovery and keeps its later result. This reduces navigation-induced board maintenance while preserving existing review-first proposal behavior. It does not cancel a write already accepted by the server. Shared ownership for card, comment, label and board mutations is covered by #3306 above; shared loading arbitration -remains tracked in #3305. Deferred-response store tests and BoardView lifecycle tests cover the +is covered by #3305 above. Deferred-response store tests and BoardView lifecycle tests cover the route/session boundary, and the existing three ordering assertions now compare actual reactive array identities as well as full contents. diff --git a/frontend/taskdeck-web/src/store/board/boardCrudStore.ts b/frontend/taskdeck-web/src/store/board/boardCrudStore.ts index c1705f093..1566e35a9 100644 --- a/frontend/taskdeck-web/src/store/board/boardCrudStore.ts +++ b/frontend/taskdeck-web/src/store/board/boardCrudStore.ts @@ -10,7 +10,7 @@ import { buildDemoBoardList } from '../../utils/demoData' import { applyBoardCardCounts } from '../../utils/boardCardCounts' import type { CreateBoardDto, UpdateBoardDto } from '../../types/board' import { initialCardFilters, type BoardState } from './boardState' -import { captureBoardSession, type BoardHelpers } from './boardStoreHelpers' +import { beginBoardLoading, captureBoardSession, type BoardHelpers } from './boardStoreHelpers' // Minimum gap between board-list fetches. Multiple views (BoardsListView, // ActivityView, ReviewView, etc.) can call fetchBoards on mount in quick @@ -76,6 +76,7 @@ interface ActiveBoardFetch { backgroundFailureMessage?: string preserveCardComments: boolean controller: AbortController + finishLoading: () => void promise: Promise } @@ -129,10 +130,13 @@ export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers) return } if (helpers.isDemoMode) { - state.loading.value = true - state.error.value = null - state.boards.value = buildDemoBoardList() - state.loading.value = false + const finishLoading = beginBoardLoading(state) + try { + state.error.value = null + state.boards.value = buildDemoBoardList() + } finally { + finishLoading() + } return } @@ -140,10 +144,10 @@ export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers) const isCurrentListGeneration = () => requestGeneration === boardListGeneration const controller = new AbortController() inFlightBoardListReads.add(controller) + const finishLoading = beginBoardLoading(state) const request = (async () => { try { - state.loading.value = true state.error.value = null // Bounded exactly like the detail read below (`startBoardFetch`), and // for a reason the share made sharper: once every unfiltered caller in @@ -262,25 +266,7 @@ export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers) throw e } finally { inFlightBoardListReads.delete(controller) - // Gated for the same reason the detail path gates its own loading - // write: by the time a superseded read settles, the flag belongs to - // the read that replaced it. Clearing it here would drop the next - // session's skeleton and show that user an empty account until their - // own read resolves. - // - // That makes the gate correct only while every bumper of - // boardListGeneration also clears state.loading in the same synchronous - // turn, so no read is left owning a flag nobody will clear. - // resetForLogout is the only bumper today and does exactly that. A - // list-side cancel helper modelled on cancelBackgroundBoardFetch — - // which bumps boardFetchGeneration and deliberately leaves the flag - // alone — would strand loading true and leave BoardsListView on its - // skeleton for good. Clear the flag alongside any new bumper, or - // replace this gate with a per-request ownership token that does not - // depend on the coupling. - if (isCurrentListGeneration()) { - state.loading.value = false - } + finishLoading() } })() @@ -425,6 +411,10 @@ export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers) // recovery uses it to reject a server response that was already in flight // when the write was refused. state.currentBoardRequestGeneration.value = requestGeneration + const finishLoading = intent === 'explicit' ? beginBoardLoading(state) : () => {} + // Retire the old visible owner even if its transport ignores cancellation. + // Acquire the replacement first so an explicit-to-explicit handoff stays busy. + activeBoardFetch?.finishLoading() activeBoardFetch?.controller.abort() const controller = new AbortController() const mutationEpoch = helpers.getBoardDetailMutationEpoch(id) @@ -435,6 +425,7 @@ export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers) backgroundFailureMessage, preserveCardComments, controller, + finishLoading, promise: Promise.resolve(false), } satisfies ActiveBoardFetch @@ -472,7 +463,6 @@ export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers) const performFetch = async (): Promise => { if (helpers.isDemoMode) { if (intent === 'explicit') { - state.loading.value = true state.error.value = null } const [board, cards] = await Promise.all([ @@ -493,15 +483,11 @@ export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers) state.currentBoardCards.value = cards state.currentBoardLabels.value = [] if (!preserveCurrentComments) state.cardCommentsByCardId.value = {} - if (intent === 'explicit') { - state.loading.value = false - } return true } try { if (intent === 'explicit') { - state.loading.value = true state.error.value = null } const readOptions: BoardReadOptions = { @@ -572,14 +558,11 @@ export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers) helpers.handleApiError(e, 'Failed to fetch board') throw e - } finally { - if (intent === 'explicit' && isCurrentGeneration()) { - state.loading.value = false - } } } const promise = performFetch().finally(() => { + finishLoading() if (activeBoardFetch !== request) { return } @@ -595,8 +578,8 @@ export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers) async function createBoard(board: CreateBoardDto) { helpers.guardDemoMutation() const isCurrentSession = captureBoardSession(state) + const finishLoading = beginBoardLoading(state) try { - state.loading.value = true state.error.value = null const newBoard = await boardsApi.createBoard(board) if (!isCurrentSession()) return newBoard @@ -607,15 +590,15 @@ export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers) if (isCurrentSession()) helpers.handleApiError(e, 'Failed to create board') throw e } finally { - if (isCurrentSession()) state.loading.value = false + finishLoading() } } async function updateBoard(boardId: string, board: UpdateBoardDto) { helpers.guardDemoMutation() const isCurrentSession = captureBoardSession(state) + const finishLoading = beginBoardLoading(state) try { - state.loading.value = true state.error.value = null const updatedBoard = await boardsApi.updateBoard(boardId, board) if (!isCurrentSession()) return updatedBoard @@ -640,15 +623,15 @@ export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers) if (isCurrentSession()) helpers.handleApiError(e, 'Failed to update board') throw e } finally { - if (isCurrentSession()) state.loading.value = false + finishLoading() } } async function deleteBoard(boardId: string) { helpers.guardDemoMutation() const isCurrentSession = captureBoardSession(state) + const finishLoading = beginBoardLoading(state) try { - state.loading.value = true state.error.value = null await boardsApi.deleteBoard(boardId) if (!isCurrentSession()) return @@ -682,7 +665,7 @@ export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers) if (isCurrentSession()) helpers.handleApiError(e, 'Failed to archive board') throw e } finally { - if (isCurrentSession()) state.loading.value = false + finishLoading() } } @@ -715,6 +698,8 @@ export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers) */ function resetForLogout() { state.boardMutationSessionGeneration.value++ + state.loadingOperations.clear() + state.loading.value = false state.boardViewVisit.value = { boardId: null } boardListGeneration++ // The bump comes first so the rejection each abort produces lands on a @@ -753,7 +738,6 @@ export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers) state.cardCommentsByCardId.value = {} state.boardPresenceMembers.value = [] state.editingCardId.value = null - state.loading.value = false state.error.value = null state.filters.value = initialCardFilters() } diff --git a/frontend/taskdeck-web/src/store/board/boardState.ts b/frontend/taskdeck-web/src/store/board/boardState.ts index 9c07274f9..2bd0b6f03 100644 --- a/frontend/taskdeck-web/src/store/board/boardState.ts +++ b/frontend/taskdeck-web/src/store/board/boardState.ts @@ -63,6 +63,8 @@ export function createBoardState() { const boardPresenceMembers = ref([]) const editingCardId = ref(null) const loading = ref(false) + // Not reactive: owners publish the visible flag when they enter or leave. + const loadingOperations = new Set() const error = ref(null) const filters = ref(initialCardFilters()) @@ -81,6 +83,7 @@ export function createBoardState() { boardPresenceMembers, editingCardId, loading, + loadingOperations, error, filters, } diff --git a/frontend/taskdeck-web/src/store/board/boardStoreHelpers.ts b/frontend/taskdeck-web/src/store/board/boardStoreHelpers.ts index 7a0bf651e..996ea0988 100644 --- a/frontend/taskdeck-web/src/store/board/boardStoreHelpers.ts +++ b/frontend/taskdeck-web/src/store/board/boardStoreHelpers.ts @@ -19,6 +19,18 @@ export function captureBoardSession(state: BoardState): () => boolean { return () => state.boardMutationSessionGeneration.value === generation } +/** Release only this operation; logout retires every owner through the same session epoch. */ +export function beginBoardLoading(state: BoardState): () => void { + const isCurrentSession = captureBoardSession(state) + const token = Symbol() + state.loadingOperations.add(token) + state.loading.value = true + return () => { + if (!isCurrentSession() || !state.loadingOperations.delete(token)) return + state.loading.value = state.loadingOperations.size > 0 + } +} + /** * Whether this failure is a client-side timeout — a routine outcome on every * board read since #2685 bounded them (`timeout: BOARD_REQUEST_TIMEOUT_MS`, diff --git a/frontend/taskdeck-web/src/store/board/cardCommentStore.ts b/frontend/taskdeck-web/src/store/board/cardCommentStore.ts index 65618d8cc..17df0032e 100644 --- a/frontend/taskdeck-web/src/store/board/cardCommentStore.ts +++ b/frontend/taskdeck-web/src/store/board/cardCommentStore.ts @@ -4,7 +4,7 @@ import { cardCommentsApi } from '../../api/cardCommentsApi' import type { CardComment, CreateCardCommentDto, UpdateCardCommentDto } from '../../types/comments' import type { BoardState } from './boardState' -import { captureBoardSession, type BoardHelpers } from './boardStoreHelpers' +import { beginBoardLoading, captureBoardSession, type BoardHelpers } from './boardStoreHelpers' export function createCardCommentActions(state: BoardState, helpers: BoardHelpers) { function getCardComments(cardId: string): CardComment[] { @@ -31,8 +31,8 @@ export function createCardCommentActions(state: BoardState, helpers: BoardHelper async function createCardComment(boardId: string, cardId: string, dto: CreateCardCommentDto) { helpers.guardDemoMutation() const isCurrentSession = captureBoardSession(state) + const finishLoading = beginBoardLoading(state) try { - state.loading.value = true state.error.value = null const createdComment = await cardCommentsApi.createComment(boardId, cardId, dto) if (!isCurrentSession()) return createdComment @@ -51,7 +51,7 @@ export function createCardCommentActions(state: BoardState, helpers: BoardHelper if (isCurrentSession()) helpers.handleApiError(e, 'Failed to create card comment') throw e } finally { - if (isCurrentSession()) state.loading.value = false + finishLoading() } } @@ -63,8 +63,8 @@ export function createCardCommentActions(state: BoardState, helpers: BoardHelper ) { helpers.guardDemoMutation() const isCurrentSession = captureBoardSession(state) + const finishLoading = beginBoardLoading(state) try { - state.loading.value = true state.error.value = null const updatedComment = await cardCommentsApi.updateComment(boardId, cardId, commentId, dto) if (!isCurrentSession()) return updatedComment @@ -82,15 +82,15 @@ export function createCardCommentActions(state: BoardState, helpers: BoardHelper if (isCurrentSession()) helpers.handleApiError(e, 'Failed to update card comment') throw e } finally { - if (isCurrentSession()) state.loading.value = false + finishLoading() } } async function deleteCardComment(boardId: string, cardId: string, commentId: string) { helpers.guardDemoMutation() const isCurrentSession = captureBoardSession(state) + const finishLoading = beginBoardLoading(state) try { - state.loading.value = true state.error.value = null await cardCommentsApi.deleteComment(boardId, cardId, commentId) if (!isCurrentSession()) return @@ -104,7 +104,7 @@ export function createCardCommentActions(state: BoardState, helpers: BoardHelper if (isCurrentSession()) helpers.handleApiError(e, 'Failed to delete card comment') throw e } finally { - if (isCurrentSession()) state.loading.value = false + finishLoading() } } diff --git a/frontend/taskdeck-web/src/store/board/cardStore.ts b/frontend/taskdeck-web/src/store/board/cardStore.ts index c0da2f426..315773bee 100644 --- a/frontend/taskdeck-web/src/store/board/cardStore.ts +++ b/frontend/taskdeck-web/src/store/board/cardStore.ts @@ -5,7 +5,7 @@ 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 { captureBoardSession, type BoardHelpers } from './boardStoreHelpers' +import { beginBoardLoading, captureBoardSession, type BoardHelpers } from './boardStoreHelpers' import type { BoardFetchOptions } from './boardCrudStore' export function createCardActions( @@ -74,8 +74,8 @@ export function createCardActions( async function createCard(boardId: string, card: CreateCardDto) { helpers.guardDemoMutation() const isCurrentSession = captureBoardSession(state) + const finishLoading = beginBoardLoading(state) try { - state.loading.value = true state.error.value = null const newCard = await cardsApi.createCard(boardId, card) if (!isCurrentSession()) return newCard @@ -98,15 +98,15 @@ export function createCardActions( if (isCurrentSession()) helpers.handleApiError(e, 'Failed to create card') throw e } finally { - if (isCurrentSession()) state.loading.value = false + finishLoading() } } async function updateCard(boardId: string, cardId: string, card: UpdateCardDto) { helpers.guardDemoMutation() const isCurrentSession = captureBoardSession(state) + const finishLoading = beginBoardLoading(state) try { - state.loading.value = true state.error.value = null const existingCard = state.currentBoardCards.value.find((c) => c.id === cardId) const request = { @@ -134,7 +134,7 @@ export function createCardActions( } throw e } finally { - if (isCurrentSession()) state.loading.value = false + finishLoading() } } @@ -142,8 +142,8 @@ export function createCardActions( helpers.guardDemoMutation() const isCurrentSession = captureBoardSession(state) let refreshChildren = false + const finishLoading = beginBoardLoading(state) try { - state.loading.value = true state.error.value = null await cardsApi.deleteCard(boardId, cardId, confirmation) if (!isCurrentSession()) return @@ -176,7 +176,7 @@ export function createCardActions( if (isCurrentSession()) helpers.handleApiError(e, 'Failed to delete card') throw e } finally { - if (isCurrentSession()) state.loading.value = false + finishLoading() } // Finish mutation-owned loading/error writes before a refresh can outlive navigation. if (isCurrentSession() && refreshChildren) await refreshDetachedChildren(boardId) @@ -190,8 +190,8 @@ export function createCardActions( ) { helpers.guardDemoMutation() const isCurrentSession = captureBoardSession(state) + const finishLoading = beginBoardLoading(state) try { - state.loading.value = true state.error.value = null const existingCard = @@ -236,7 +236,7 @@ export function createCardActions( if (isCurrentSession()) helpers.handleApiError(e, 'Failed to move card') throw e } finally { - if (isCurrentSession()) state.loading.value = false + finishLoading() } } diff --git a/frontend/taskdeck-web/src/store/board/columnStore.ts b/frontend/taskdeck-web/src/store/board/columnStore.ts index 772fe11eb..43c499753 100644 --- a/frontend/taskdeck-web/src/store/board/columnStore.ts +++ b/frontend/taskdeck-web/src/store/board/columnStore.ts @@ -5,7 +5,7 @@ import { watch } from 'vue' import { columnsApi } from '../../api/columnsApi' import type { CreateColumnDto, UpdateColumnDto } from '../../types/board' import type { BoardState, BoardViewVisit } from './boardState' -import type { BoardHelpers } from './boardStoreHelpers' +import { beginBoardLoading, type BoardHelpers } from './boardStoreHelpers' interface ColumnMutationVisit { boardId: string @@ -78,33 +78,38 @@ export function createColumnActions(state: BoardState, helpers: BoardHelpers) { visit: ColumnMutationVisit, mutation: () => Promise, ): Promise { - const previous = mutationTailByBoardId.get(visit.boardId) - let operation: Promise + const finishLoading = beginBoardLoading(state) + try { + const previous = mutationTailByBoardId.get(visit.boardId) + let operation: Promise - if (previous) { - operation = previous.catch(() => undefined).then(() => { + if (previous) { + operation = previous.catch(() => undefined).then(() => { + if (!isCurrentVisit(visit)) throw new StaleBoardVisitError() + return mutation() + }) + } else { + // The first intent starts transport in the initiating call stack. Only a + // later intent is queued and therefore needs a pre-transport session gate. if (!isCurrentVisit(visit)) throw new StaleBoardVisitError() - return mutation() - }) - } else { - // The first intent starts transport in the initiating call stack. Only a - // later intent is queued and therefore needs a pre-transport session gate. - if (!isCurrentVisit(visit)) throw new StaleBoardVisitError() - operation = mutation() - } + operation = mutation() + } - const tail = operation.then( - () => undefined, - () => undefined, - ) - mutationTailByBoardId.set(visit.boardId, tail) + const tail = operation.then( + () => undefined, + () => undefined, + ) + mutationTailByBoardId.set(visit.boardId, tail) - try { - return await operation - } finally { - if (mutationTailByBoardId.get(visit.boardId) === tail) { - mutationTailByBoardId.delete(visit.boardId) + try { + return await operation + } finally { + if (mutationTailByBoardId.get(visit.boardId) === tail) { + mutationTailByBoardId.delete(visit.boardId) + } } + } finally { + finishLoading() } } @@ -147,7 +152,6 @@ export function createColumnActions(state: BoardState, helpers: BoardHelpers) { const visit = captureColumnVisit(boardId) return runColumnMutation(visit, async () => { try { - state.loading.value = true state.error.value = null const newColumn = await columnsApi.createColumn(boardId, column) markColumnMutation(visit) @@ -172,8 +176,6 @@ export function createColumnActions(state: BoardState, helpers: BoardHelpers) { helpers.handleApiError(e, 'Failed to create column') } throw e - } finally { - if (isCurrentVisit(visit)) state.loading.value = false } }) } @@ -183,7 +185,6 @@ export function createColumnActions(state: BoardState, helpers: BoardHelpers) { const visit = captureColumnVisit(boardId) return runColumnMutation(visit, async () => { try { - state.loading.value = true state.error.value = null const updatedColumn = await columnsApi.updateColumn(boardId, columnId, column) markColumnMutation(visit) @@ -203,8 +204,6 @@ export function createColumnActions(state: BoardState, helpers: BoardHelpers) { helpers.handleApiError(e, 'Failed to update column') } throw e - } finally { - if (isCurrentVisit(visit)) state.loading.value = false } }) } @@ -214,7 +213,6 @@ export function createColumnActions(state: BoardState, helpers: BoardHelpers) { const visit = captureColumnVisit(boardId) return runColumnMutation(visit, async () => { try { - state.loading.value = true state.error.value = null await columnsApi.deleteColumn(boardId, columnId) markColumnMutation(visit) @@ -236,8 +234,6 @@ export function createColumnActions(state: BoardState, helpers: BoardHelpers) { helpers.handleApiError(e, 'Failed to delete column') } throw e - } finally { - if (isCurrentVisit(visit)) state.loading.value = false } }) } @@ -247,7 +243,6 @@ export function createColumnActions(state: BoardState, helpers: BoardHelpers) { const visit = captureColumnVisit(boardId) return runColumnMutation(visit, async () => { try { - state.loading.value = true state.error.value = null const reorderedColumns = await columnsApi.reorderColumns(boardId, columnIds) markColumnMutation(visit) @@ -265,8 +260,6 @@ export function createColumnActions(state: BoardState, helpers: BoardHelpers) { helpers.handleApiError(e, 'Failed to reorder columns') } throw e - } finally { - if (isCurrentVisit(visit)) state.loading.value = false } }) } diff --git a/frontend/taskdeck-web/src/store/board/labelStore.ts b/frontend/taskdeck-web/src/store/board/labelStore.ts index 4c7050271..95157b8b1 100644 --- a/frontend/taskdeck-web/src/store/board/labelStore.ts +++ b/frontend/taskdeck-web/src/store/board/labelStore.ts @@ -10,7 +10,7 @@ import { watch } from 'vue' import { labelsApi } from '../../api/labelsApi' import type { CreateLabelDto, Label, UpdateLabelDto } from '../../types/board' import type { BoardState } from './boardState' -import { captureBoardSession, type BoardHelpers } from './boardStoreHelpers' +import { beginBoardLoading, captureBoardSession, type BoardHelpers } from './boardStoreHelpers' interface LabelCacheVisit { boardId: string @@ -182,8 +182,8 @@ export function createLabelActions(state: BoardState, helpers: BoardHelpers) { async function createLabel(boardId: string, label: CreateLabelDto) { helpers.guardDemoMutation() const visit = captureLabelVisit(boardId) + const finishLoading = beginBoardLoading(state) try { - state.loading.value = true state.error.value = null const newLabel = await labelsApi.createLabel(boardId, label) if (!visit.isCurrentSession()) return newLabel @@ -208,15 +208,15 @@ export function createLabelActions(state: BoardState, helpers: BoardHelpers) { } throw e } finally { - if (isCurrentBoardVisit(visit)) state.loading.value = false + finishLoading() } } async function updateLabel(boardId: string, labelId: string, label: UpdateLabelDto) { helpers.guardDemoMutation() const visit = captureLabelVisit(boardId) + const finishLoading = beginBoardLoading(state) try { - state.loading.value = true state.error.value = null const updatedLabel = await runLabelMutation( boardId, @@ -244,15 +244,15 @@ export function createLabelActions(state: BoardState, helpers: BoardHelpers) { } throw e } finally { - if (isCurrentBoardVisit(visit)) state.loading.value = false + finishLoading() } } async function deleteLabel(boardId: string, labelId: string) { helpers.guardDemoMutation() const visit = captureLabelVisit(boardId) + const finishLoading = beginBoardLoading(state) try { - state.loading.value = true state.error.value = null await runLabelMutation( boardId, @@ -278,7 +278,7 @@ export function createLabelActions(state: BoardState, helpers: BoardHelpers) { } throw e } finally { - if (isCurrentBoardVisit(visit)) state.loading.value = false + finishLoading() } } diff --git a/frontend/taskdeck-web/src/tests/store/board/boardCrudStore.spec.ts b/frontend/taskdeck-web/src/tests/store/board/boardCrudStore.spec.ts index d83e4d571..7062cf04e 100644 --- a/frontend/taskdeck-web/src/tests/store/board/boardCrudStore.spec.ts +++ b/frontend/taskdeck-web/src/tests/store/board/boardCrudStore.spec.ts @@ -766,6 +766,7 @@ describe('boardCrudStore', () => { const second = fetchBoard('board-2') await expect(second).resolves.toBe(true) + expect(state.loading.value).toBe(false) await expect(first).resolves.toBe(false) expect(oldCardsAborted).toBe(true) expect(oldLabelsAborted).toBe(true) @@ -796,6 +797,7 @@ describe('boardCrudStore', () => { cardsB.resolve([{ id: 'card-b', columnId: 'column-b' }]) labelsB.resolve([{ id: 'label-b', name: 'Bug' }]) await expect(second).resolves.toBe(true) + expect(state.loading.value).toBe(false) boardA.resolve({ id: 'board-a', name: 'Board A', columns: [{ id: 'column-a', cardCount: 0 }] }) cardsA.resolve([{ id: 'card-a', columnId: 'column-a' }]) @@ -862,6 +864,7 @@ describe('boardCrudStore', () => { explicitCards.resolve([{ id: 'card-explicit', columnId: 'column-1' }]) explicitLabels.resolve([{ id: 'label-explicit', name: 'Explicit' }]) await expect(explicit).resolves.toBe(true) + expect(state.loading.value).toBe(false) expect(mockBoardsApi.getBoard).toHaveBeenCalledTimes(2) expect(mockCardsApi.getCards).toHaveBeenCalledTimes(2) diff --git a/frontend/taskdeck-web/src/tests/store/board/boardLoadingOwnership.spec.ts b/frontend/taskdeck-web/src/tests/store/board/boardLoadingOwnership.spec.ts new file mode 100644 index 000000000..69d922e42 --- /dev/null +++ b/frontend/taskdeck-web/src/tests/store/board/boardLoadingOwnership.spec.ts @@ -0,0 +1,227 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { createPinia, setActivePinia } from 'pinia' +import { boardsApi } from '../../../api/boardsApi' +import { cardsApi } from '../../../api/cardsApi' +import { cardCommentsApi } from '../../../api/cardCommentsApi' +import { columnsApi } from '../../../api/columnsApi' +import { labelsApi } from '../../../api/labelsApi' +import { useBoardStore } from '../../../store/boardStore' +import type { Board, BoardDetail, Card, Column, Label } from '../../../types/board' +import type { CardComment } from '../../../types/comments' + +vi.mock('../../../api/boardsApi') +vi.mock('../../../api/cardsApi') +vi.mock('../../../api/cardCommentsApi') +vi.mock('../../../api/columnsApi') +vi.mock('../../../api/labelsApi') + +const time = '2026-09-22T12:00:00Z' + +const columnA: Column = { + id: 'column-a', boardId: 'board-a', name: 'Todo', position: 0, + wipLimit: null, cardCount: 1, createdAt: time, updatedAt: time, +} +const columnB: Column = { ...columnA, id: 'column-b', boardId: 'board-b' } +const boardA: BoardDetail = { + id: 'board-a', name: 'Account A board', description: null, isArchived: false, + createdAt: time, updatedAt: time, columns: [columnA], +} +const boardB: BoardDetail = { + ...boardA, id: 'board-b', name: 'Account B board', columns: [columnB], +} +const cardA: Card = { + id: 'card-a', boardId: 'board-a', columnId: 'column-a', title: 'A card', + description: '', dueDate: null, isBlocked: false, blockReason: null, + position: 0, labels: [], createdAt: time, updatedAt: time, +} +const cardB: Card = { ...cardA, id: 'card-b', boardId: 'board-b', columnId: 'column-b', title: 'B card' } +const labelA: Label = { + id: 'label-a', boardId: 'board-a', name: 'A label', colorHex: '#123456', + createdAt: time, updatedAt: time, +} +const labelB: Label = { ...labelA, id: 'label-b', boardId: 'board-b', name: 'B label' } +const commentA: CardComment = { + id: 'comment-a', boardId: 'board-a', cardId: 'card-a', parentCommentId: null, + authorUserId: 'account-a', authorUsername: 'account-a', content: 'A comment', + isDeleted: false, editedAt: null, mentions: [], createdAt: time, updatedAt: time, +} +const commentB: CardComment = { ...commentA, id: 'comment-b', boardId: 'board-b', cardId: 'card-b', content: 'B comment' } + +function deferred() { + let resolve!: (value: T | PromiseLike) => void + let reject!: (reason?: unknown) => void + const promise = new Promise((yes, no) => { resolve = yes; reject = no }) + return { promise, resolve, reject } +} + +type Store = ReturnType + +function installBoardA(store: Store) { + store.currentBoard = structuredClone(boardA) + store.currentBoardCards = [structuredClone(cardA)] + store.currentBoardLabels = [structuredClone(labelA)] + store.cardCommentsByCardId = { 'card-a': [structuredClone(commentA)] } +} + +function startBoardBDetail(store: Store) { + const board = deferred() + const cards = deferred() + const labels = deferred() + vi.mocked(boardsApi.getBoard).mockReturnValueOnce(board.promise) + vi.mocked(cardsApi.getCards).mockReturnValueOnce(cards.promise) + vi.mocked(labelsApi.getLabels).mockReturnValueOnce(labels.promise) + const read = store.fetchBoard('board-b') + return { read, board, cards, labels } +} + +const detailMutationCases = [ + { + name: 'board', + api: boardsApi.updateBoard, + result: boardA, + start: (store: Store) => store.updateBoard('board-a', { name: 'A board' }), + }, + { + name: 'card', + api: cardsApi.updateCard, + result: cardA, + start: (store: Store) => store.updateCard('board-a', 'card-a', { title: 'A card' }), + }, + { + name: 'label', + api: labelsApi.updateLabel, + result: labelA, + start: (store: Store) => store.updateLabel('board-a', 'label-a', { name: 'A label' }), + }, + { + name: 'comment', + api: cardCommentsApi.updateComment, + result: commentA, + start: (store: Store) => store.updateCardComment('board-a', 'card-a', 'comment-a', { content: 'A comment' }), + }, + { + name: 'column', + api: columnsApi.updateColumn, + result: columnA, + start: (store: Store) => store.updateColumn('board-a', 'column-a', { name: 'Todo' }), + }, +] + +describe('board loading ownership', () => { + beforeEach(() => { + vi.resetAllMocks() + setActivePinia(createPinia()) + }) + + for (const mutation of detailMutationCases) { + it(`${mutation.name} mutation cannot clear loading owned by a newer board detail read`, async () => { + const store = useBoardStore() + installBoardA(store) + const pendingMutation = deferred() + vi.mocked(mutation.api).mockReturnValueOnce(pendingMutation.promise as never) + + const call = mutation.start(store) + expect(store.loading).toBe(true) + const detail = startBoardBDetail(store) + + pendingMutation.resolve(mutation.result) + await expect(call).resolves.toEqual(mutation.result) + expect(store.loading).toBe(true) + + detail.board.resolve(structuredClone(boardB)) + detail.cards.resolve([structuredClone(cardB)]) + detail.labels.resolve([structuredClone(labelB)]) + await expect(detail.read).resolves.toBe(true) + expect(store.loading).toBe(false) + }) + } + + for (const outcome of ['success', 'rejection'] as const) { + it(`a ${outcome} from one mutation module cannot clear another module's loading owner`, async () => { + const store = useBoardStore() + installBoardA(store) + const first = deferred() + const second = deferred() + vi.mocked(cardsApi.updateCard).mockReturnValueOnce(first.promise) + vi.mocked(cardCommentsApi.createComment).mockReturnValueOnce(second.promise) + + const firstCall = store.updateCard('board-a', 'card-a', { title: 'A card' }) + const secondCall = store.createCardComment('board-a', 'card-a', { content: 'B comment' }) + expect(store.loading).toBe(true) + + if (outcome === 'success') { + first.resolve(structuredClone(cardA)) + await expect(firstCall).resolves.toEqual(cardA) + } else { + const failure = new Error('first mutation failed') + const rejected = expect(firstCall).rejects.toBe(failure) + first.reject(failure) + await rejected + } + expect(store.loading).toBe(true) + + second.resolve(structuredClone(commentB)) + await expect(secondCall).resolves.toEqual(commentB) + expect(store.loading).toBe(false) + }) + } + + for (const outcome of ['success', 'rejection'] as const) { + it(`logout retires an old ${outcome} without clearing a new deferred operation`, async () => { + const store = useBoardStore() + const old = deferred() + vi.mocked(boardsApi.updateBoard).mockReturnValueOnce(old.promise as never) + const oldCall = store.updateBoard('board-a', { name: 'Old account' }) + + store.resetForLogout() + expect(store.loading).toBe(false) + + const current = deferred() + vi.mocked(boardsApi.updateBoard).mockReturnValueOnce(current.promise) + const currentCall = store.updateBoard('board-b', { name: 'Current account' }) + expect(store.loading).toBe(true) + + if (outcome === 'success') { + old.resolve(structuredClone(boardA)) + await expect(oldCall).resolves.toEqual(boardA) + } else { + const failure = new Error('old account failed') + const rejected = expect(oldCall).rejects.toBe(failure) + old.reject(failure) + await rejected + } + expect(store.loading).toBe(true) + + current.resolve(structuredClone(boardB)) + await expect(currentCall).resolves.toEqual(boardB) + expect(store.loading).toBe(false) + }) + } + + for (const firstKind of ['unfiltered', 'filtered'] as const) { + it(`keeps loading while the ${firstKind} and the other board list read overlap`, async () => { + const store = useBoardStore() + const first = deferred() + const second = deferred() + vi.mocked(boardsApi.getBoards) + .mockReturnValueOnce(first.promise) + .mockReturnValueOnce(second.promise) + + const firstRead = firstKind === 'unfiltered' + ? store.fetchBoards(undefined, false, { force: true }) + : store.fetchBoards('account', false, { force: true }) + const secondRead = firstKind === 'unfiltered' + ? store.fetchBoards('account', false, { force: true }) + : store.fetchBoards(undefined, false, { force: true }) + expect(store.loading).toBe(true) + + first.resolve([structuredClone(boardA)]) + await expect(firstRead).resolves.toBeUndefined() + expect(store.loading).toBe(true) + + second.resolve([structuredClone(boardB)]) + await expect(secondRead).resolves.toBeUndefined() + expect(store.loading).toBe(false) + }) + } +}) diff --git a/frontend/taskdeck-web/src/tests/store/board/columnStoreRouteOwnership.spec.ts b/frontend/taskdeck-web/src/tests/store/board/columnStoreRouteOwnership.spec.ts index 82079f95d..02b1bd1d6 100644 --- a/frontend/taskdeck-web/src/tests/store/board/columnStoreRouteOwnership.spec.ts +++ b/frontend/taskdeck-web/src/tests/store/board/columnStoreRouteOwnership.spec.ts @@ -8,6 +8,8 @@ const { api } = vi.hoisted(() => ({ api: { reorderColumns: vi.fn(), getColumns: vi.fn(), } })) vi.mock('../../../api/columnsApi', () => ({ columnsApi: api })) +const { boardApi } = vi.hoisted(() => ({ boardApi: { createBoard: vi.fn() } })) +vi.mock('../../../api/boardsApi', () => ({ boardsApi: boardApi })) import { createColumnActions } from '../../../store/board/columnStore' import { createBoardCrudActions } from '../../../store/board/boardCrudStore' @@ -44,6 +46,16 @@ function setup() { return { state, helpers, ui, visit, actions } } +function startNewScreenOperation(state: ReturnType, helpers: ReturnType['helpers']) { + const response = deferred() + boardApi.createBoard.mockReturnValueOnce(response.promise) + const pending = createBoardCrudActions(state, helpers as never).createBoard({ name: 'New board' }) + return async () => { + response.resolve({ id: 'new-board', name: 'New board', columns: [] } as unknown as BoardDetail) + await pending + } +} + describe('column mutations follow the board screen lifetime', () => { beforeEach(() => vi.resetAllMocks()) @@ -63,8 +75,8 @@ describe('column mutations follow the board screen lifetime', () => { : actions.reorderColumns('board-1', ['b', 'a']) ui.endBoardViewVisit(visit) + const finishNewScreenOperation = startNewScreenOperation(state, helpers) state.error.value = 'New screen error' - state.loading.value = true const result = kind === 'delete' ? undefined : kind === 'reorder' ? [...columns].reverse() : { ...columns[0], id: kind === 'create' ? 'new' : 'a', name: 'Changed' } response.resolve(result as never) @@ -79,6 +91,8 @@ describe('column mutations follow the board screen lifetime', () => { expect(helpers.handleApiError).not.toHaveBeenCalled() expect(state.error.value).toBe('New screen error') expect(state.loading.value).toBe(true) + await finishNewScreenOperation() + expect(state.loading.value).toBe(false) }, ) @@ -101,6 +115,7 @@ describe('column mutations follow the board screen lifetime', () => { expect(state.currentBoard.value!.columns).toBe(cached) expect(state.currentBoard.value!.columns).toEqual(columns) expect(helpers.toast.success).not.toHaveBeenCalled() + expect(state.loading.value).toBe(false) }) it.each(['departure', 'logout'])('does not publish an old failure after %s', async (boundary) => { @@ -110,14 +125,16 @@ describe('column mutations follow the board screen lifetime', () => { const pending = actions.updateColumn('board-1', 'a', { name: 'Changed' }) if (boundary === 'logout') createBoardCrudActions(state, helpers as never).resetForLogout() else ui.endBoardViewVisit(visit) + const finishNewScreenOperation = startNewScreenOperation(state, helpers) state.error.value = 'New screen error' - state.loading.value = true const failure = new Error('Late failure') response.reject(failure) await expect(pending).rejects.toBe(failure) expect(helpers.handleApiError).not.toHaveBeenCalled() expect(state.error.value).toBe('New screen error') expect(state.loading.value).toBe(true) + await finishNewScreenOperation() + expect(state.loading.value).toBe(false) }) it('drops pre-logout queued work even when a new login opens the same board', async () => { diff --git a/frontend/taskdeck-web/stryker.smoke.contract.mjs b/frontend/taskdeck-web/stryker.smoke.contract.mjs index 89da6d5b3..19f61cb14 100644 --- a/frontend/taskdeck-web/stryker.smoke.contract.mjs +++ b/frontend/taskdeck-web/stryker.smoke.contract.mjs @@ -19,8 +19,8 @@ export const mutationSmokeContract = Object.freeze({ schemaVersion: '1.0', file: 'src/store/board/boardCrudStore.ts', - start: Object.freeze({ line: 673, column: 28 }), - end: Object.freeze({ line: 673, column: 78 }), + start: Object.freeze({ line: 656, column: 28 }), + end: Object.freeze({ line: 656, column: 78 }), source: 'state.boards.value.filter((b) => b.id !== boardId)', }) diff --git a/scripts/ci/smart-ci/mutation-smoke-contract.test.mjs b/scripts/ci/smart-ci/mutation-smoke-contract.test.mjs index daadbf04f..136c7c746 100644 --- a/scripts/ci/smart-ci/mutation-smoke-contract.test.mjs +++ b/scripts/ci/smart-ci/mutation-smoke-contract.test.mjs @@ -70,7 +70,7 @@ test('the shared range addresses the seam in the real tracked source', async () // are 0-based while the contract's are 1-based, so a derived assertion would // pass whichever base `mutationSmokeRange` happened to use. Passing column 28 // starts the range inside the expression and drops its outermost mutant. - assert.equal(mutationSmokeRange, 'src/store/board/boardCrudStore.ts:673:27-673:77') + assert.equal(mutationSmokeRange, 'src/store/board/boardCrudStore.ts:656:27-656:77') assert.equal(mutationSmokeContract.source.length, 50) // Real payload, not a fabricated line: the contract must address the seam in