diff --git a/frontend/src/components/ScenarioManagementModal.test.tsx b/frontend/src/components/ScenarioManagementModal.test.tsx new file mode 100644 index 000000000..8824e5da3 --- /dev/null +++ b/frontend/src/components/ScenarioManagementModal.test.tsx @@ -0,0 +1,96 @@ +/** + * Regression test for the "list vanishes during delete" bug reported in + * staff testing (April 2026). See ScenarioContext.test.tsx for full context. + * + * The modal must keep rendering the scenario cards while a delete mutation + * is pending — only the initial query-fetch state should swap the list for + * a "Loading scenarios..." placeholder. + */ +import { describe, it, expect, vi } from 'vitest' +import { render, screen } from '@testing-library/react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import type { ReactNode } from 'react' +import { Toaster } from 'react-hot-toast' + +// Mock useSyncStatusAPI — modal uses it for the CampMinder "synced" line. +vi.mock('../hooks/useSyncStatusAPI', () => ({ + useSyncStatusAPI: () => ({ data: undefined }), +})) + +// Mock useYear — modal reads currentYear for clear-scenario calls. +vi.mock('../hooks/useCurrentYear', async () => { + const actual = await vi.importActual('../hooks/useCurrentYear') + return { ...actual, useYear: () => 2026 } +}) + +// Render the modal without its child modals doing anything. +vi.mock('./ScenarioEditModal', () => ({ default: () => null })) +vi.mock('./NewScenarioModal', () => ({ default: () => null })) + +import ScenarioManagementModal from './ScenarioManagementModal' +import { ScenarioContext } from '../hooks/useScenario' +import type { ScenarioContextType, Scenario } from '../hooks/useScenario' + +function makeContext(overrides: Partial): ScenarioContextType { + const scenarios: Scenario[] = [ + { + id: 'scenario-1', + name: 'Dorm Cabin Plan A', + session_cm_id: 1000001, + is_active: true, + description: '', + created: '2026-04-01T00:00:00Z', + updated: '2026-04-01T00:00:00Z', + }, + ] + return { + currentScenario: null, + isProductionMode: true, + scenarios, + loading: false, + isLoading: false, + isMutating: false, + error: null, + loadScenarios: vi.fn().mockResolvedValue(undefined), + createScenario: vi.fn(), + selectScenario: vi.fn(), + updateScenario: vi.fn().mockResolvedValue(undefined), + deleteScenario: vi.fn().mockResolvedValue(undefined), + clearScenario: vi.fn().mockResolvedValue(undefined), + ...overrides, + } +} + +function renderModal(ctx: ScenarioContextType) { + const qc = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }) + const wrapper = ({ children }: { children: ReactNode }) => ( + + {children} + + + ) + return render(, { wrapper }) +} + +describe('ScenarioManagementModal loading state', () => { + it('shows "Loading scenarios..." during initial query fetch', () => { + // Matches what ScenarioContext produces when the query is first loading: + // loading=true AND isLoading=true (scenarios list not yet fetched). + renderModal(makeContext({ loading: true, isLoading: true, scenarios: [] })) + expect(screen.getByText('Loading scenarios...')).toBeInTheDocument() + }) + + it('keeps scenario cards visible while a delete mutation is pending', () => { + // Simulates real context state mid-delete: the combined loading flag + // is true (because isMutating is true), but the query has already + // resolved (isLoading=false). Prior to the fix, the modal consumed the + // combined `loading` and swapped the list for the placeholder — this + // test asserts the fix by requiring the cards to remain visible. + renderModal(makeContext({ loading: true, isLoading: false, isMutating: true })) + + expect(screen.getByText('Dorm Cabin Plan A')).toBeInTheDocument() + expect(screen.queryByText('Loading scenarios...')).not.toBeInTheDocument() + }) +}) diff --git a/frontend/src/components/ScenarioManagementModal.tsx b/frontend/src/components/ScenarioManagementModal.tsx index 90fd84b2c..e6fd0f345 100644 --- a/frontend/src/components/ScenarioManagementModal.tsx +++ b/frontend/src/components/ScenarioManagementModal.tsx @@ -30,6 +30,10 @@ export default function ScenarioManagementModal({ onClose, }: ScenarioManagementModalProps) { const currentYear = useYear() + // Read `isLoading` (initial query fetch) rather than the combined + // `loading` flag — otherwise the scenario list is replaced with a + // "Loading scenarios..." placeholder while a delete/clear is in flight, + // making the list appear to vanish behind the confirmation dialog. const { scenarios, currentScenario, @@ -37,7 +41,7 @@ export default function ScenarioManagementModal({ updateScenario, deleteScenario, clearScenario, - loading, + isLoading, } = useScenario() const { data: syncStatus } = useSyncStatusAPI() @@ -157,7 +161,7 @@ export default function ScenarioManagementModal({ {/* Scenarios List */}
- {loading ? ( + {isLoading ? (
Loading scenarios...
) : scenarios.length === 0 ? (
diff --git a/frontend/src/contexts/ScenarioContext.test.tsx b/frontend/src/contexts/ScenarioContext.test.tsx new file mode 100644 index 000000000..1c4b00ed8 --- /dev/null +++ b/frontend/src/contexts/ScenarioContext.test.tsx @@ -0,0 +1,152 @@ +/** + * Tests for ScenarioContext loading/mutation state split. + * + * Regression: staff testing (April 2026) reported that clicking "Delete" on a + * scenario caused the entire list in ScenarioManagementModal to vanish — only + * the hardcoded "CampMinder" card remained visible behind the confirmation + * prompt. Root cause: the provider's single `loading` flag went true for any + * pending mutation (create / update / delete / clear), and the modal rendered + * a "Loading scenarios..." placeholder in that branch, replacing the scenario + * cards. The fix exposes `isLoading` (initial query fetch) separately from + * `isMutating` (any mutation pending) so consumers can distinguish the two. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { renderHook, act, waitFor } from '@testing-library/react' +import type { ReactNode } from 'react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' + +// Mock pocketbase before importing the context +vi.mock('../lib/pocketbase', () => { + const collections: Record = {} + return { + pb: { + collection: vi.fn((name: string) => { + collections[name] ??= { + getFullList: vi.fn().mockResolvedValue([]), + getOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + } + return collections[name] + }), + }, + getCurrentUser: vi.fn(() => ({ id: 'user-1', email: 'user@example.com' })), + } +}) + +// Mock auth so useSavedScenarios doesn't gate on isLoading +vi.mock('./AuthContext', () => ({ + useAuth: () => ({ isLoading: false }), +})) + +// Mock useYear so provider gets a stable year +vi.mock('../hooks/useCurrentYear', async () => { + const actual = await vi.importActual('../hooks/useCurrentYear') + return { + ...actual, + useYear: () => 2026, + } +}) + +import { pb } from '../lib/pocketbase' +import type { Mock } from 'vitest' +import { ScenarioProvider } from './ScenarioContext' +import { useScenario } from '../hooks/useScenario' + +interface CollectionMock { + getFullList: Mock + getOne: Mock + create: Mock + update: Mock + delete: Mock +} + +function getCollection(name: string): CollectionMock { + return (pb.collection as Mock)(name) as CollectionMock +} + +function createWrapper() { + const qc = new QueryClient({ + defaultOptions: { mutations: { retry: false }, queries: { retry: false } }, + }) + return ({ children }: { children: ReactNode }) => ( + + {children} + + ) +} + +beforeEach(() => { + vi.clearAllMocks() +}) + +describe('ScenarioContext loading vs mutating', () => { + it('exposes isLoading (query fetch) separately from isMutating (mutation pending)', () => { + const { result } = renderHook(() => useScenario(), { wrapper: createWrapper() }) + // Both flags must exist on the public context shape. + expect(result.current).toHaveProperty('isLoading') + expect(result.current).toHaveProperty('isMutating') + }) + + it('does not flip isLoading to true during a delete mutation', async () => { + const savedScenarios = getCollection('saved_scenarios') + const drafts = getCollection('bunk_assignments_draft') + + savedScenarios.getFullList.mockResolvedValue([ + { + id: 'scenario-1', + name: 'Test Scenario', + session: 'pb-session-1', + year: 2026, + is_active: true, + description: '', + created: '2026-04-01T00:00:00Z', + updated: '2026-04-01T00:00:00Z', + collectionId: 'c1', + collectionName: 'saved_scenarios', + expand: { session: { cm_id: 1000001 } }, + }, + ]) + + // Delete path: getFullList for drafts, then delete scenario. + // Keep the delete promise pending so we can observe state mid-flight. + drafts.getFullList.mockResolvedValue([]) + let resolveScenarioDelete: (value?: unknown) => void = () => {} + savedScenarios.delete.mockImplementation( + () => + new Promise((resolve) => { + resolveScenarioDelete = resolve + }) + ) + + const { result } = renderHook(() => useScenario(), { wrapper: createWrapper() }) + + // Let the initial query settle. + await act(async () => { + await result.current.loadScenarios(1000001) + }) + await waitFor(() => expect(result.current.scenarios).toHaveLength(1)) + expect(result.current.isLoading).toBe(false) + + // Start the delete but do NOT await it yet — we want to observe the + // in-flight state. + let deletePromise: Promise = Promise.resolve() + act(() => { + deletePromise = result.current.deleteScenario('scenario-1') + }) + + // While the delete is pending: isMutating true, isLoading stays false. + await waitFor(() => expect(result.current.isMutating).toBe(true)) + expect(result.current.isLoading).toBe(false) + + // Resolve and drain. + await act(async () => { + resolveScenarioDelete() + await deletePromise + }) + + await waitFor(() => expect(result.current.isMutating).toBe(false)) + expect(result.current.isLoading).toBe(false) + }) +}) diff --git a/frontend/src/contexts/ScenarioContext.tsx b/frontend/src/contexts/ScenarioContext.tsx index 4bc40472e..3065278e6 100644 --- a/frontend/src/contexts/ScenarioContext.tsx +++ b/frontend/src/contexts/ScenarioContext.tsx @@ -41,14 +41,21 @@ export const ScenarioProvider: FC = ({ children }) => { clearScenarioMutation.error?.message ?? null - // Combined loading state - const loading = - isLoading || + // Any mutation in flight. Tracked separately from `isLoading` so UI that + // renders list content (e.g. ScenarioManagementModal) doesn't replace the + // list with a placeholder while a delete/clear is processing. + const isMutating = createScenarioMutation.isPending || updateScenarioMutation.isPending || deleteScenarioMutation.isPending || clearScenarioMutation.isPending + // Combined loading state (initial fetch OR mutation pending). Kept for + // backward compatibility with any consumer that just needs a unified + // "busy" signal. Consumers that drive empty-state/placeholder UI should + // read `isLoading` instead. + const loading = isLoading || isMutating + // Load scenarios for a session const loadScenarios = useCallback(async (sessionId: number) => { setCurrentSessionId(sessionId) @@ -220,6 +227,8 @@ export const ScenarioProvider: FC = ({ children }) => { isProductionMode, scenarios, loading, + isLoading, + isMutating, error, loadScenarios, createScenario, diff --git a/frontend/src/hooks/useScenario.ts b/frontend/src/hooks/useScenario.ts index 344704904..35f62a532 100644 --- a/frontend/src/hooks/useScenario.ts +++ b/frontend/src/hooks/useScenario.ts @@ -18,7 +18,15 @@ export interface ScenarioContextType { scenarios: Scenario[] // Loading states + // `isLoading` reflects the initial React Query fetch — use this for + // empty/placeholder UI. `isMutating` reflects any in-flight mutation + // (create/update/delete/clear) — don't use it to hide list content, since + // doing so causes the list to "vanish" mid-mutation. `loading` is the + // combined flag, retained for existing consumers that only care that + // *something* is happening. loading: boolean + isLoading: boolean + isMutating: boolean error: string | null // Actions