diff --git a/frontend/taskdeck-web/src/store/board/boardCrudStore.ts b/frontend/taskdeck-web/src/store/board/boardCrudStore.ts index 45f5e391f3..120616c30a 100644 --- a/frontend/taskdeck-web/src/store/board/boardCrudStore.ts +++ b/frontend/taskdeck-web/src/store/board/boardCrudStore.ts @@ -28,14 +28,24 @@ export interface BoardListFetchOptions { * Skip the throttle window, and NOTHING else — an explicit user request for * fresh data, which the 5 s gap between mounts was never meant to answer. * - * The stamp is written only after a success, so a retry that follows a FAILED - * list read was never blocked by it. What this exists for is the stamp an - * EARLIER success left behind: `state.error` is shared by every board action, - * so a create/rename/archive failure two seconds after a good list read puts - * BoardsListView on its error branch with a Retry control, and without this - * the click returned here before touching `loading` or issuing a request — - * no skeleton, no request, a dead button until the window passed (#2689 - * round-2 finding 1). + * The stamp is written only after a success, but that is not the same as + * "a failure reopens the window": a stamp an EARLIER success left behind + * outlives every later failure, so the window is open for the whole 5 s + * regardless of what happened in between. Two ways in: `state.error` is + * shared by every board action, so a create/rename/archive failure two + * seconds after a good list read puts BoardsListView on its error branch + * with a Retry control; and a FILTERED list read (the activity selector's + * `includeArchived` one) writes the same shared `error` when it fails, and + * a filtered FAILURE leaves the stamp untouched — though a filtered success + * writes it like any other success, since `lastFetchBoardsAt` below is not + * gated on `isFilteredRequest`. + * + * So only `force` gets past the THROTTLE; the in-flight share and demo mode + * still apply, which is why this is a skipped window rather than a + * guaranteed request. That is enough for the retry path, which always passes + * it. Without it the click returned here before touching `loading` or + * issuing a request: no skeleton, no request, a dead button until the window + * passed (#2689 round-2 finding 1, docblock corrected in #2689 item 7). * * Deliberately NOT a bypass of the in-flight share: joining a read that is * already on the wire is the correct answer to a second caller, and forcing @@ -174,6 +184,32 @@ export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers) // error still identical to the one this path observed. The marker is // dropped on any current-generation success, matched or not, so a stale // message can never authorise a later clear. + // + // The documented limitation, at the same granularity as that + // precedent: the guard compares MESSAGES, and the copy collapses. Every + // client-side timeout maps to the one `boards.error.timeout` string and + // every offline failure to axios's "Network Error" (no response, so + // `getErrorMessage` falls through to `err.message`), for reads and + // mutations alike, so two different surfaces routinely produce + // byte-identical text and this comparison cannot tell them apart. + // + // Concretely, offline: this list read fails with "Network Error", the + // user submits the create form, `createBoard` fails with the same two + // words during the forced retry, and the retry's success then clears an + // alert the list read did not raise. The timeout string collides the + // same way between the two BOUNDED reads — this one and the detail read + // in `startBoardFetch`, both carrying `BOARD_REQUEST_TIMEOUT_MS` — since + // both write `boards.error.timeout` into the one shared `error`. Note + // which actor is NOT available for that half: `boardsApi.createBoard` + // is a bare `http.post` and the axios instance sets no default timeout + // (see the bound's own comment above), so a mutation can never produce + // the timeout string — only the offline string. + // + // It is not exotic; offline is the ordinary case. Distinguishing them + // needs an owner tag on the error surface rather than a string compare, + // which is a wider change than this seam (#2689 item 8); the losing + // surface's toast survives either way, so the failure is still + // reported. const listReadErrorToClear = lastListReadError lastListReadError = null if (listReadErrorToClear !== null && state.error.value === listReadErrorToClear) { diff --git a/frontend/taskdeck-web/src/store/board/index.ts b/frontend/taskdeck-web/src/store/board/index.ts index 0009e4975b..e7b03a0137 100644 --- a/frontend/taskdeck-web/src/store/board/index.ts +++ b/frontend/taskdeck-web/src/store/board/index.ts @@ -3,7 +3,11 @@ export type { CardFilters, BoardState } from './boardState' export { createBoardHelpers } from './boardStoreHelpers' export type { BoardHelpers } from './boardStoreHelpers' export { createBoardCrudActions } from './boardCrudStore' -export type { BoardFetchIntent, BoardFetchOptions } from './boardCrudStore' +export type { + BoardFetchIntent, + BoardFetchOptions, + BoardListFetchOptions, +} from './boardCrudStore' export { createColumnActions } from './columnStore' export { createCardActions } from './cardStore' export { createCardCommentActions } from './cardCommentStore' 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 0abfc48e48..272ce2c0bd 100644 --- a/frontend/taskdeck-web/src/tests/store/board/boardCrudStore.spec.ts +++ b/frontend/taskdeck-web/src/tests/store/board/boardCrudStore.spec.ts @@ -187,12 +187,16 @@ describe('boardCrudStore', () => { }) // #2689 round-2 finding 1. The throttle stamp is written only after a - // SUCCESS, so a retry following a failed read was never blocked by it — but - // the stamp an EARLIER success left behind is a different matter. `error` - // is shared by every board action, so the boards list can be sitting on its - // error branch with a live Retry control (a create/rename/archive failure) - // while this window is still open. An explicit retry has to get through; - // an ordinary mount still must not. + // SUCCESS, but a later failure does not reopen the window: the stamp an + // EARLIER success left behind survives it, including a filtered read's + // failure, which writes the shared `error` while leaving the stamp + // untouched (a filtered SUCCESS writes it like any other success). Only + // `force` gets past the throttle — the in-flight share and demo mode still + // apply. `error` is shared by every board action, so the boards list can be + // sitting on its error branch with a live Retry control (a + // create/rename/archive failure) while this window is still open. An + // explicit retry has to get through; an ordinary mount still must not. + // (Comment corrected in #2689 item 7.) it('lets a forced read through the throttle window while an unforced one is still skipped', async () => { vi.useFakeTimers() mockBoardsApi.getBoards.mockResolvedValue([{ id: 'board-1', name: 'My Board' }]) @@ -206,7 +210,12 @@ describe('boardCrudStore', () => { await fetchBoards() expect(mockBoardsApi.getBoards).toHaveBeenCalledTimes(1) - // Same window, forced: a real request, and the skeleton the view needs. + // Same window, forced: a real request goes out. The request count is the + // load-bearing assertion here; `loading` back at false only says the read + // settled, which every settled read satisfies (#2689 item 10). That the + // view actually SHOWS a skeleton while a forced retry is in flight is + // proven in BoardsListView.spec.ts, "forces past the throttle window when + // the alert came from another action after a good read". await fetchBoards(undefined, false, { force: true }) expect(mockBoardsApi.getBoards).toHaveBeenCalledTimes(2) expect(state.loading.value).toBe(false) diff --git a/frontend/taskdeck-web/src/tests/views/BoardsListView.spec.ts b/frontend/taskdeck-web/src/tests/views/BoardsListView.spec.ts index 0de062d91e..224c221b4a 100644 --- a/frontend/taskdeck-web/src/tests/views/BoardsListView.spec.ts +++ b/frontend/taskdeck-web/src/tests/views/BoardsListView.spec.ts @@ -262,6 +262,62 @@ describe('BoardsListView', () => { wrapper.unmount() }) + // #2689 item 6, the other half of that restore. The retry read is bounded + // at 10 s and the create panel sits ABOVE the loading chain, so it stays + // interactive while the retry is in flight: the user can open "+ New Board" + // and start typing during those ten seconds. Restoring unconditionally then + // pulled the caret out of the name input the moment the read failed, and + // the next Space or Enter re-fired Retry instead of typing. The restore + // exists for focus that was LOST, so it is guarded on + // `document.activeElement` being null or
. + it('leaves the caret alone when the user moved into the create form during the retry', async () => { + mockBoardStore.error = 'Failed to load boards' + + const wrapper = mount(BoardsListView, { attachTo: document.body }) + await waitForUi() + + const firstButton = wrapper.find('[data-action="retry-board-load"]') + .element as HTMLButtonElement + firstButton.focus() + expect(document.activeElement).toBe(firstButton) + + let failRead!: () => void + mockBoardStore.fetchBoards.mockImplementation(() => { + mockBoardStore.loading = true + return new Promise