From d33f57935be2814ebdbbb24b92dbc8a043f3a5b9 Mon Sep 17 00:00:00 2001 From: Alimedhat000 Date: Wed, 26 Aug 2026 03:26:18 +0300 Subject: [PATCH 1/2] fix(client): clear stale collaborator error on new add/remove attempts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit useCollaborators only reset its error inside the initial fetch effect, so a failed add or remove left 'Failed to add/remove collaborator' showing indefinitely — even after a successful retry or closing/reopening the dropdown. Reset the error at the start of each action, mirroring the fetch effect. Adds a browser-mode vitest project for hook-level tests (stories can't exercise API-dependent logic) with coverage for error clearing on successful retries. --- client/.gitignore | 3 + .../__tests__/use-collaborators.test.tsx | 136 ++++++++++++++++++ client/src/hooks/use-collaborators.ts | 2 + client/vite.config.ts | 40 ++++-- 4 files changed, 169 insertions(+), 12 deletions(-) create mode 100644 client/src/hooks/__tests__/use-collaborators.test.tsx diff --git a/client/.gitignore b/client/.gitignore index 51af31d..5a569ad 100644 --- a/client/.gitignore +++ b/client/.gitignore @@ -35,3 +35,6 @@ storybook-static /blob-report/ /playwright/.auth/ /playwright/.cache/ + +# Vitest browser failure screenshots +__screenshots__/ diff --git a/client/src/hooks/__tests__/use-collaborators.test.tsx b/client/src/hooks/__tests__/use-collaborators.test.tsx new file mode 100644 index 0000000..f8b17c5 --- /dev/null +++ b/client/src/hooks/__tests__/use-collaborators.test.tsx @@ -0,0 +1,136 @@ +import { act } from 'react'; +import { createElement } from 'react'; +import type { ReactNode } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { api } from '@/lib/api'; + +vi.mock('@/lib/api', () => ({ + api: { + get: vi.fn(), + post: vi.fn(), + delete: vi.fn(), + }, +})); + +import { useCollaborators } from '../use-collaborators'; + +const mockApi = vi.mocked(api, true); + +/** + * Renders a hook inside a probe component mounted on document.body and + * exposes its latest return value, since no DOM-testing library is wired + * into the client suite. + * + * @param useHook - Hook factory invoked on every render of the probe. + * @returns The latest hook result plus unmount for cleanup. + */ +function renderHook(useHook: () => T) { + let result!: T; + let root!: Root; + + const Probe = () => { + result = useHook(); + return null; + }; + + const host = document.createElement('div'); + document.body.appendChild(host); + + act(() => { + root = createRoot(host); + root.render(createElement(Probe) as ReactNode); + }); + + return { + get current() { + return result; + }, + unmount: () => { + act(() => root.unmount()); + host.remove(); + }, + }; +} + +describe('useCollaborators', () => { + beforeEach(() => { + ( + globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean } + ).IS_REACT_ACT_ENVIRONMENT = true; + mockApi.get.mockResolvedValue({ data: [] }); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('clears a stale add error when a retry succeeds', async () => { + const hook = renderHook(() => useCollaborators('doc1')); + await act(async () => {}); // flush initial fetch + + mockApi.post.mockRejectedValueOnce(new Error('boom')); + await act(async () => { + await hook.current.addCollaborator('a@b.c'); + }); + expect(hook.current.error).toBe('Failed to add collaborator'); + + mockApi.post.mockResolvedValueOnce({}); + let added = false; + await act(async () => { + added = await hook.current.addCollaborator('a@b.c'); + }); + + expect(added).toBe(true); + expect(hook.current.error).toBeNull(); + hook.unmount(); + }); + + it('clears a stale remove error when a retry succeeds', async () => { + const hook = renderHook(() => useCollaborators('doc1')); + await act(async () => {}); + + mockApi.delete.mockRejectedValueOnce(new Error('boom')); + await act(async () => { + await hook.current.removeCollaborator('u1'); + }); + expect(hook.current.error).toBe('Failed to remove collaborator'); + + mockApi.delete.mockResolvedValueOnce({}); + await act(async () => { + await hook.current.removeCollaborator('u1'); + }); + + expect(hook.current.error).toBeNull(); + hook.unmount(); + }); + + it('surfaces an error when adding fails', async () => { + const hook = renderHook(() => useCollaborators('doc1')); + await act(async () => {}); + + mockApi.post.mockRejectedValueOnce(new Error('boom')); + let added = true; + await act(async () => { + added = await hook.current.addCollaborator('a@b.c'); + }); + + expect(added).toBe(false); + expect(hook.current.error).toBe('Failed to add collaborator'); + hook.unmount(); + }); + + it('surfaces an error when removing fails', async () => { + const hook = renderHook(() => useCollaborators('doc1')); + await act(async () => {}); + + mockApi.delete.mockRejectedValueOnce(new Error('boom')); + await act(async () => { + await hook.current.removeCollaborator('u1'); + }); + + expect(hook.current.error).toBe('Failed to remove collaborator'); + hook.unmount(); + }); +}); diff --git a/client/src/hooks/use-collaborators.ts b/client/src/hooks/use-collaborators.ts index 92e70e8..5b7b133 100644 --- a/client/src/hooks/use-collaborators.ts +++ b/client/src/hooks/use-collaborators.ts @@ -41,6 +41,7 @@ export const useCollaborators = (docId?: string) => { const removeCollaborator = async (userId: string) => { if (!docId) return; + setError(null); try { await api.delete(`/document/${docId}/collaborators/${userId}`); setCollaborators((prev) => prev.filter((c) => c.id !== userId)); @@ -52,6 +53,7 @@ export const useCollaborators = (docId?: string) => { const addCollaborator = async (email: string) => { if (!docId || !email) return false; + setError(null); try { await api.post(`/document/${docId}/collaborators`, { email }); const res = await api.get(`/document/${docId}/collaborators`); diff --git a/client/vite.config.ts b/client/vite.config.ts index 58a6bc2..2cac025 100644 --- a/client/vite.config.ts +++ b/client/vite.config.ts @@ -13,7 +13,21 @@ const dirname = ? __dirname : path.dirname(fileURLToPath(import.meta.url)); -// More info at: https://storybook.js.org/docs/next/writing-tests/integrations/vitest-addon +// Fresh object per project: vitest mutates browser instance configs while +// registering nested projects, so sharing one literal collides. +const browserConfig = () => + ({ + enabled: true, + headless: true, + provider: 'playwright', + instances: [ + { + browser: 'chromium', + }, + ], + }) as const; + +// More info: https://storybook.js.org/docs/next/writing-tests/integrations/vitest-addon export default defineConfig({ plugins: [react(), tailwindcss()], resolve: { @@ -51,21 +65,23 @@ export default defineConfig({ test: { name: 'storybook', // One browser instance already stretches CI runners; parallel - // files alongside the server suite starves vitest's runner. + // files alongside the server suite starve vitest's runner. fileParallelism: false, - browser: { - enabled: true, - headless: true, - provider: 'playwright', - instances: [ - { - browser: 'chromium', - }, - ], - }, + browser: browserConfig(), setupFiles: ['.storybook/vitest.setup.ts'], }, }, + { + // Hook-level tests run in a real browser too — stories can't + // exercise logic that needs API interactions. + extends: true, + test: { + name: 'browser-unit', + include: ['src/**/__tests__/*.test.{ts,tsx}'], + fileParallelism: false, + browser: browserConfig(), + }, + }, ], }, }); From cab7dc4944717100da783e7b35a4b49f8a94136e Mon Sep 17 00:00:00 2001 From: Alimedhat000 Date: Wed, 26 Aug 2026 20:26:57 +0300 Subject: [PATCH 2/2] test(client): harden use-collaborators harness against CI flakes Replace bare await act(async () => {}) flushes with a polling waitFor that waits for loading to settle. Empty act flushes can miss the initial fetch's microtask in CI timing. --- .../__tests__/use-collaborators.test.tsx | 30 ++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/client/src/hooks/__tests__/use-collaborators.test.tsx b/client/src/hooks/__tests__/use-collaborators.test.tsx index f8b17c5..de669af 100644 --- a/client/src/hooks/__tests__/use-collaborators.test.tsx +++ b/client/src/hooks/__tests__/use-collaborators.test.tsx @@ -54,6 +54,28 @@ function renderHook(useHook: () => T) { }; } +/** + * Flushes pending React updates and microtasks. Polls until the + * predicate holds, so `await act(async () => {})` empty flushes don't + * flake when the initial fetch resolves on the next microtask. + * + * @param predicate - Condition to wait for. + * @param timeoutMs - Fail after this long. + */ +async function waitFor(predicate: () => boolean, timeoutMs = 1000) { + const start = Date.now(); + while (!predicate()) { + if (Date.now() - start > timeoutMs) { + throw new Error('waitFor timeout'); + } + await act(async () => { + await new Promise((r) => { + setTimeout(r, 0); + }); + }); + } +} + describe('useCollaborators', () => { beforeEach(() => { ( @@ -68,7 +90,7 @@ describe('useCollaborators', () => { it('clears a stale add error when a retry succeeds', async () => { const hook = renderHook(() => useCollaborators('doc1')); - await act(async () => {}); // flush initial fetch + await waitFor(() => !hook.current.loading); mockApi.post.mockRejectedValueOnce(new Error('boom')); await act(async () => { @@ -89,7 +111,7 @@ describe('useCollaborators', () => { it('clears a stale remove error when a retry succeeds', async () => { const hook = renderHook(() => useCollaborators('doc1')); - await act(async () => {}); + await waitFor(() => !hook.current.loading); mockApi.delete.mockRejectedValueOnce(new Error('boom')); await act(async () => { @@ -108,7 +130,7 @@ describe('useCollaborators', () => { it('surfaces an error when adding fails', async () => { const hook = renderHook(() => useCollaborators('doc1')); - await act(async () => {}); + await waitFor(() => !hook.current.loading); mockApi.post.mockRejectedValueOnce(new Error('boom')); let added = true; @@ -123,7 +145,7 @@ describe('useCollaborators', () => { it('surfaces an error when removing fails', async () => { const hook = renderHook(() => useCollaborators('doc1')); - await act(async () => {}); + await waitFor(() => !hook.current.loading); mockApi.delete.mockRejectedValueOnce(new Error('boom')); await act(async () => {