diff --git a/.planning/phases/12-initial-setup-wizard/deferred-items.md b/.planning/phases/12-initial-setup-wizard/deferred-items.md new file mode 100644 index 0000000..0100354 --- /dev/null +++ b/.planning/phases/12-initial-setup-wizard/deferred-items.md @@ -0,0 +1,12 @@ +# Deferred Items — Phase 12 + +Out-of-scope discoveries logged during execution. Not fixed by the originating plan. + +## 12-07 — Pre-existing PWA lint errors (out of scope) + +Discovered during 12-07 verification (`pnpm lint` in apps/pwa). 22 errors, NOT introduced by 12-07 (the four files 12-07 touched lint clean): + +- `apps/pwa/src/api/setupClient.contract.test.ts` — `@typescript-eslint/no-unsafe-*` (any-typed `res.body` access in contract assertions) +- `apps/pwa/src/routes/SetupPage.test.tsx:152` — `no-unused-vars` (`container` assigned but unused) + +Both files were last modified in earlier Phase-12 commits (e.g. 066b69f), confirming pre-existing. Left untouched per the executor SCOPE BOUNDARY rule (only auto-fix issues directly caused by the current task). Recommend a follow-up lint-cleanup quick task. diff --git a/apps/pwa/src/components/SetupBanner.test.tsx b/apps/pwa/src/components/SetupBanner.test.tsx new file mode 100644 index 0000000..ccd7a69 --- /dev/null +++ b/apps/pwa/src/components/SetupBanner.test.tsx @@ -0,0 +1,130 @@ +/** + * SetupBanner.test.tsx — regression tests for the gap-6 ['me'] freshness fix + * (Phase 12 Plan 07 Task 2) plus the success-only dismissal contract. + * + * gap 6 mechanism (ii) — ['me'] staleness, NOT a backend linking gap: + * The first-login claim in upsertUser (apps/api/src/auth/user.ts) preserves the + * same users.id, so the wizard-stored CalDAV credential stays linked and the DB + * correctly reports needsProviderSetup=false after the operator authenticates. + * The bug was purely client-cache: a ['me'] entry populated BEFORE the claim + * (e.g. a pre-auth visit) served a stale needsProviderSetup=true for up to the + * 5-minute staleTime, so the "Set up your calendar" banner kept showing. + * Fix: the banner's ['me'] query uses staleTime 0, so a stale cache entry is + * refetched on mount and the banner reflects the just-claimed credential. + * + * Tests cover: + * - needsProviderSetup=false → banner absent + * - needsProviderSetup=true → banner present (no dismiss/X button) + * - staleTime 0: a pre-seeded stale ['me'] (needsProviderSetup=true) is refetched + * on mount; once fetchMe resolves needsProviderSetup=false, the banner hides + * (mirrors the post-wizard claim where the DB now reports false) + */ + +import React from 'react'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; + +// CredentialSheet is rendered (closed) by SetupBanner — mock it to avoid the +// real sheet's DOM/focus-trap complexity. We're testing banner visibility only. +vi.mock('./CredentialSheet.js', () => ({ + CredentialSheet: () => null, +})); + +vi.mock('../api/client.js', () => ({ + fetchMe: vi.fn(), +})); + +import { fetchMe } from '../api/client.js'; +import type { Mock } from 'vitest'; +import { SetupBanner } from './SetupBanner.js'; + +const mockFetchMe = fetchMe as Mock; + +function makeQueryClient() { + return new QueryClient({ + defaultOptions: { + queries: { retry: false }, + mutations: { retry: false }, + }, + }); +} + +function renderBanner(queryClient: QueryClient) { + return render( + + + , + ); +} + +function meUser(needsProviderSetup: boolean) { + return { + user: { + id: 1, + displayName: 'Test User', + color: '#4a90d9', + isAdmin: false, + needsProviderSetup, + }, + }; +} + +describe('SetupBanner', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('does not render when needsProviderSetup is false', async () => { + mockFetchMe.mockResolvedValue(meUser(false)); + + const queryClient = makeQueryClient(); + renderBanner(queryClient); + + // Give the query a tick to resolve, then confirm the banner stays absent + await new Promise((r) => setTimeout(r, 50)); + expect(screen.queryByText('Set up your calendar')).toBeNull(); + }); + + it('renders the banner (no dismiss button) when needsProviderSetup is true', async () => { + mockFetchMe.mockResolvedValue(meUser(true)); + + const queryClient = makeQueryClient(); + renderBanner(queryClient); + + await waitFor(() => { + expect(screen.getByText('Set up your calendar')).toBeInTheDocument(); + }); + + // The ONLY action is "Set up now" — there is NO dismiss/X button (T-05-24). + expect(screen.getByRole('button', { name: 'Set up now' })).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /dismiss/i })).toBeNull(); + expect(screen.queryByRole('button', { name: /close/i })).toBeNull(); + }); + + // gap 6 regression: a stale ['me'] cache entry (needsProviderSetup=true, as it + // would be from a pre-claim visit) must be refetched on mount because the query + // uses staleTime 0. After the fresh fetch resolves needsProviderSetup=false (the + // post-wizard-claim DB truth), the banner must hide. + it('refetches a stale ["me"] on mount (staleTime 0) and hides the banner once needsProviderSetup is false', async () => { + const queryClient = makeQueryClient(); + + // Seed a STALE cache entry as if populated before the wizard claim. + queryClient.setQueryData(['me'], meUser(true)); + + // The server now reports the claimed credential → needsProviderSetup=false. + mockFetchMe.mockResolvedValue(meUser(false)); + + renderBanner(queryClient); + + // staleTime 0 → the seeded entry is stale → refetch fires on mount. + await waitFor(() => { + expect(mockFetchMe).toHaveBeenCalled(); + }); + + // After the refetch resolves false, the banner must disappear. + await waitFor(() => { + expect(screen.queryByText('Set up your calendar')).toBeNull(); + }); + }); +}); diff --git a/apps/pwa/src/components/SetupBanner.tsx b/apps/pwa/src/components/SetupBanner.tsx index a5e5d42..24d0f7b 100644 --- a/apps/pwa/src/components/SetupBanner.tsx +++ b/apps/pwa/src/components/SetupBanner.tsx @@ -33,11 +33,18 @@ export function SetupBanner() { // Use HTMLButtonElement for the ref (assignable to the CredentialSheet's HTMLElement trigger) const ctaRef = useRef(null); + // staleTime 0 (gap 6): this banner gates on needsProviderSetup, which is only + // correct if ['me'] is fresh on entry to the authenticated shell after the + // wizard claim (see App.tsx for the full mechanism note). With a 5-minute + // staleTime a pre-claim cache entry kept needsProviderSetup=true and the banner + // showed even though the operator already configured the calendar in the wizard. + // The success-only dismissal contract is unchanged: this banner still clears + // ONLY when needsProviderSetup becomes false (no dismiss/X button). const meQuery = useQuery({ queryKey: ['me'], queryFn: fetchMe, retry: false, - staleTime: 5 * 60 * 1000, + staleTime: 0, }); // Only show when needsProviderSetup is explicitly true