From dda4c71790d3b39382a3b3cc98ad3fcec38b9f88 Mon Sep 17 00:00:00 2001 From: Benjamin Welsh Date: Thu, 23 Apr 2026 20:31:48 -0400 Subject: [PATCH] add zustand store for websocket data --- README.md | 3 +- frontend/package-lock.json | 40 ++- frontend/package.json | 3 +- .../src/components/ScheduleSectionRowView.tsx | 21 +- .../src/components/SectionCalendarGrid.tsx | 2 +- ...Socket.test.ts => useScheduleData.test.ts} | 218 +++++++----- frontend/src/hooks/useScheduleData.ts | 72 ++++ frontend/src/hooks/useScheduleWebSocket.ts | 212 ------------ frontend/src/pages/Faculty.tsx | 29 +- frontend/src/pages/Schedules.test.tsx | 18 +- frontend/src/pages/Schedules.tsx | 4 +- frontend/src/stores/scheduleDataStore.ts | 321 ++++++++++++++++++ 12 files changed, 597 insertions(+), 346 deletions(-) rename frontend/src/hooks/{useScheduleWebSocket.test.ts => useScheduleData.test.ts} (55%) create mode 100644 frontend/src/hooks/useScheduleData.ts delete mode 100644 frontend/src/hooks/useScheduleWebSocket.ts create mode 100644 frontend/src/stores/scheduleDataStore.ts diff --git a/README.md b/README.md index a05a5b7..f789e5a 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,7 @@ automated-course-scheduler/ │ │ ├── components/ # Reusable UI components │ │ ├── hooks/ # Custom React hooks │ │ ├── pages/ # Route-level page components +│ │ ├── stores/ # Zustand stores (shared client state) │ │ ├── types/ # TypeScript type definitions │ │ └── utils/ # Helper utilities │ ├── package.json @@ -219,7 +220,7 @@ FastAPI automatically exposes the full OpenAPI spec at `/docs` (Swagger UI) and ### Real-Time Updates -The backend exposes a WebSocket endpoint (`/ws`) that broadcasts schedule change events to all connected clients. The frontend subscribes via the `useScheduleWebSocket` hook so edits made by one user are reflected immediately in other open sessions — no polling required. +The backend exposes a WebSocket endpoint (`/ws`) that broadcasts schedule change events to all connected clients. The frontend subscribes via the `useScheduleData` hook, which reads from a shared zustand store so the Schedules and Faculty pages reuse one cached connection per schedule — no polling and no duplicate fetches on navigation. ## License diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 33c36b4..424e067 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -12,7 +12,8 @@ "axios": "^1.7.2", "react": "^18.3.1", "react-dom": "^18.3.1", - "react-router-dom": "^6.24.1" + "react-router-dom": "^6.24.1", + "zustand": "^5.0.12" }, "devDependencies": { "@eslint/js": "^9.39.4", @@ -3286,14 +3287,14 @@ "version": "15.7.15", "resolved": "https://registry.npmjs.org/@types/prop-types/-/prop-types-15.7.15.tgz", "integrity": "sha512-F6bEyamV9jKGAFBEmlQnesRPGOQqS2+Uwi0Em15xenOxHaf2hv6L8YCVn3rPdPJOiJfPiCnLIRyvwVaqMY3MIw==", - "dev": true, + "devOptional": true, "license": "MIT" }, "node_modules/@types/react": { "version": "18.3.28", "resolved": "https://registry.npmjs.org/@types/react/-/react-18.3.28.tgz", "integrity": "sha512-z9VXpC7MWrhfWipitjNdgCauoMLRdIILQsAEV+ZesIzBq/oUlxk0m3ApZuMFCXdnS4U7KrI+l3WRUEGQ8K1QKw==", - "dev": true, + "devOptional": true, "license": "MIT", "dependencies": { "@types/prop-types": "*", @@ -4360,7 +4361,7 @@ "version": "3.2.3", "resolved": "https://registry.npmjs.org/csstype/-/csstype-3.2.3.tgz", "integrity": "sha512-z1HGKcYy2xA8AGQfwrn0PAy+PB7X/GSj3UVJW9qKyn43xWa+gl5nXmU4qqLMRzWVLFC8KusUX8T/0kCiOYpAIQ==", - "dev": true, + "devOptional": true, "license": "MIT" }, "node_modules/data-urls": { @@ -5829,7 +5830,7 @@ "version": "9.0.21", "resolved": "https://registry.npmjs.org/immer/-/immer-9.0.21.tgz", "integrity": "sha512-bc4NBHqOqSfRW7POMkHd51LvClaeMXpm8dx0e8oE2GORbq5aRK7Bxl4FyzVLdGtLmvLKL7BTDBG5ACQm4HWjTA==", - "dev": true, + "devOptional": true, "license": "MIT", "funding": { "type": "opencollective", @@ -9718,6 +9719,35 @@ "peerDependencies": { "zod": "^3.25.0 || ^4.0.0" } + }, + "node_modules/zustand": { + "version": "5.0.12", + "resolved": "https://registry.npmjs.org/zustand/-/zustand-5.0.12.tgz", + "integrity": "sha512-i77ae3aZq4dhMlRhJVCYgMLKuSiZAaUPAct2AksxQ+gOtimhGMdXljRT21P5BNpeT4kXlLIckvkPM029OljD7g==", + "license": "MIT", + "engines": { + "node": ">=12.20.0" + }, + "peerDependencies": { + "@types/react": ">=18.0.0", + "immer": ">=9.0.6", + "react": ">=18.0.0", + "use-sync-external-store": ">=1.2.0" + }, + "peerDependenciesMeta": { + "@types/react": { + "optional": true + }, + "immer": { + "optional": true + }, + "react": { + "optional": true + }, + "use-sync-external-store": { + "optional": true + } + } } } } diff --git a/frontend/package.json b/frontend/package.json index dc1243e..eb68297 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -18,7 +18,8 @@ "axios": "^1.7.2", "react": "^18.3.1", "react-dom": "^18.3.1", - "react-router-dom": "^6.24.1" + "react-router-dom": "^6.24.1", + "zustand": "^5.0.12" }, "devDependencies": { "@eslint/js": "^9.39.4", diff --git a/frontend/src/components/ScheduleSectionRowView.tsx b/frontend/src/components/ScheduleSectionRowView.tsx index 1cc271d..85f7230 100644 --- a/frontend/src/components/ScheduleSectionRowView.tsx +++ b/frontend/src/components/ScheduleSectionRowView.tsx @@ -2,7 +2,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import type { CourseResponse, InstructorInfo, SectionRichResponse, TimeBlockInfo, WarningResponse } from '../api/generated'; import { getAutomatedCourseSchedulerAPI, Severity } from '../api/generated'; import { axiosInstance } from '../api/axiosInstance'; -import type { LockInfo } from '../hooks/useScheduleWebSocket'; +import type { LockInfo } from '../stores/scheduleDataStore'; import CrosslistSectionHint from './CrosslistSectionHint'; import FacultyTooltip from './FacultyTooltip'; import MultiSearchableSelect from './MultiSearchableSelect'; @@ -97,8 +97,27 @@ export default function ScheduleSectionRowView({ const [timeBlockFilterIds, setTimeBlockFilterIds] = useState([]); const [selectedSection, setSelectedSection] = useState(null); const [editingSection, setEditingSection] = useState(null); + const editingSectionRef = useRef(null); const [creating, setCreating] = useState(false); const [lockError, setLockError] = useState<{ sectionId: number; msg: string } | null>(null); + + useEffect(() => { + editingSectionRef.current = editingSection; + }, [editingSection]); + + // Release any held lock on unmount so navigating away with the editor open + // does not leave a dangling lock (the shared WebSocket no longer closes on + // page navigation, so the server's disconnect-release failsafe won't fire). + useEffect(() => { + return () => { + const active = editingSectionRef.current; + if (active) { + void getAutomatedCourseSchedulerAPI() + .releaseLockSectionsSectionIdUnlockPost(active.section_id) + .catch(() => {}); + } + }; + }, []); const [hoveredInstructor, setHoveredInstructor] = useState<{ instructor: InstructorInfo; rect: DOMRect; diff --git a/frontend/src/components/SectionCalendarGrid.tsx b/frontend/src/components/SectionCalendarGrid.tsx index 2856ec1..3aadec3 100644 --- a/frontend/src/components/SectionCalendarGrid.tsx +++ b/frontend/src/components/SectionCalendarGrid.tsx @@ -1,6 +1,6 @@ import { useMemo } from 'react'; import type { InstructorInfo, SectionRichResponse } from '../api/generated'; -import type { LockInfo } from '../hooks/useScheduleWebSocket'; +import type { LockInfo } from '../stores/scheduleDataStore'; import { expandDays, parseTimeToMinutes } from '../utils/scheduleCalendar'; function dayLabel(letter: string): string { diff --git a/frontend/src/hooks/useScheduleWebSocket.test.ts b/frontend/src/hooks/useScheduleData.test.ts similarity index 55% rename from frontend/src/hooks/useScheduleWebSocket.test.ts rename to frontend/src/hooks/useScheduleData.test.ts index c2f1e38..dcc6d42 100644 --- a/frontend/src/hooks/useScheduleWebSocket.test.ts +++ b/frontend/src/hooks/useScheduleData.test.ts @@ -1,6 +1,7 @@ import { renderHook, act, waitFor } from '@testing-library/react'; -import { vi, describe, it, expect, beforeAll, afterAll, beforeEach } from 'vitest'; -import { useScheduleWebSocket } from './useScheduleWebSocket'; +import { vi, describe, it, expect, beforeAll, afterAll, beforeEach, afterEach } from 'vitest'; +import { useScheduleData } from './useScheduleData'; +import { __testing } from '../stores/scheduleDataStore'; import * as generated from '../api/generated'; import type { SectionRichResponse } from '../api/generated'; @@ -11,7 +12,7 @@ vi.mock('@auth0/auth0-react', () => ({ }), })); -// ── API mock (initial locks fetch) ──────────────────────────────────────────── +// ── API mock ────────────────────────────────────────────────────────────────── vi.mock('../api/generated', async (importOriginal) => { const actual = await importOriginal(); return { @@ -25,6 +26,7 @@ vi.mock('../api/generated', async (importOriginal) => { // ── WebSocket mock ──────────────────────────────────────────────────────────── interface MockWs { + url: string; send: ReturnType; close: ReturnType; onopen: ((e: Event) => void) | null; @@ -34,11 +36,13 @@ interface MockWs { } let mockWs: MockWs; - -const MockWebSocket = vi.fn().mockImplementation(() => { +const MockWebSocket = vi.fn().mockImplementation((url: string) => { mockWs = { + url, send: vi.fn(), - close: vi.fn(), + close: vi.fn().mockImplementation(function (this: MockWs) { + this.onclose?.({ code: 1000 }); + }), onopen: null, onmessage: null, onclose: null, @@ -47,11 +51,18 @@ const MockWebSocket = vi.fn().mockImplementation(() => { return mockWs; }); -// Stub WebSocket once for the whole suite so stale async coroutines -// never hit Node 24's built-in WebSocket after unstub. -beforeAll(() => { vi.stubGlobal('WebSocket', MockWebSocket); }); -afterAll(() => { vi.unstubAllGlobals(); }); -beforeEach(() => { MockWebSocket.mockClear(); }); +beforeAll(() => { + vi.stubGlobal('WebSocket', MockWebSocket); +}); +afterAll(() => { + vi.unstubAllGlobals(); +}); +beforeEach(() => { + MockWebSocket.mockClear(); +}); +afterEach(() => { + __testing.reset(); +}); // ── Fixtures ────────────────────────────────────────────────────────────────── const makeSection = (overrides: Partial = {}): SectionRichResponse => ({ @@ -67,15 +78,14 @@ const makeSection = (overrides: Partial = {}): SectionRichR ...overrides, }); -// ── Helpers ─────────────────────────────────────────────────────────────────── - -/** Render hook and advance to the connected state. */ async function renderConnected(scheduleId = 10) { - const hookResult = renderHook(() => useScheduleWebSocket(scheduleId)); - // Wait for connect()'s async chain (getToken → new WebSocket) to complete + const hookResult = renderHook(({ id }: { id: number }) => useScheduleData(id), { + initialProps: { id: scheduleId }, + }); await waitFor(() => expect(MockWebSocket).toHaveBeenCalled()); - // Trigger onopen and flush state - await act(async () => { mockWs.onopen?.(new Event('open')); }); + await act(async () => { + mockWs.onopen?.(new Event('open')); + }); return hookResult; } @@ -86,70 +96,69 @@ function sendMessage(type: string, payload: unknown) { } // ── Tests ───────────────────────────────────────────────────────────────────── -describe('useScheduleWebSocket', () => { +describe('useScheduleData', () => { it('starts loading/connecting and reaches connected on open', async () => { - const { result } = renderHook(() => useScheduleWebSocket(10)); - // Before WebSocket connects: loading state + const { result } = renderHook(() => useScheduleData(10)); expect(result.current.loading).toBe(true); expect(result.current.status).toBe('connecting'); expect(result.current.sections).toEqual([]); - // Wait for WS creation and open await waitFor(() => expect(MockWebSocket).toHaveBeenCalled()); - await act(async () => { mockWs.onopen?.(new Event('open')); }); + await act(async () => { + mockWs.onopen?.(new Event('open')); + }); expect(result.current.status).toBe('connected'); expect(mockWs.send).toHaveBeenCalledWith(JSON.stringify({ action: 'init' })); }); - it('passes scheduleId to the WebSocket hook', async () => { - renderHook(() => useScheduleWebSocket(7)); + it('passes scheduleId to the WebSocket URL', async () => { + renderHook(() => useScheduleData(7)); await waitFor(() => expect(MockWebSocket).toHaveBeenCalled()); expect(MockWebSocket.mock.calls[0][0]).toContain('/ws/7'); }); + it('returns empty state when scheduleId is null and does not open a socket', () => { + const { result } = renderHook(() => useScheduleData(null)); + expect(result.current.loading).toBe(false); + expect(result.current.status).toBe('disconnected'); + expect(MockWebSocket).not.toHaveBeenCalled(); + }); + it('sets sections and clears loading on "schedule" event', async () => { const { result } = await renderConnected(); - const section = makeSection(); sendMessage('schedule', [section]); - expect(result.current.sections).toEqual([section]); expect(result.current.loading).toBe(false); }); - it('appends section on "section_created" event', async () => { + it('appends section on "section_created"', async () => { const { result } = await renderConnected(); - sendMessage('schedule', [makeSection({ section_id: 1 })]); sendMessage('section_created', makeSection({ section_id: 2, section_number: 2 })); - expect(result.current.sections).toHaveLength(2); expect(result.current.sections[1].section_id).toBe(2); }); - it('patches the correct row on "section_updated" event', async () => { + it('patches the correct row on "section_updated"', async () => { const { result } = await renderConnected(); - sendMessage('schedule', [ makeSection({ section_id: 1 }), makeSection({ section_id: 2, section_number: 2 }), ]); sendMessage('section_updated', { section_id: 1, data: makeSection({ section_id: 1, capacity: 99 }) }); - expect(result.current.sections[0].capacity).toBe(99); expect(result.current.sections[1].section_id).toBe(2); }); - it('removes the correct row on "section_deleted" event', async () => { + it('removes the correct row on "section_deleted"', async () => { const { result } = await renderConnected(); - sendMessage('schedule', [ makeSection({ section_id: 1 }), makeSection({ section_id: 2, section_number: 2 }), ]); sendMessage('section_deleted', { section_id: 1 }); - expect(result.current.sections).toHaveLength(1); expect(result.current.sections[0].section_id).toBe(2); }); @@ -157,32 +166,19 @@ describe('useScheduleWebSocket', () => { it('increments comment_count on "comment_added"', async () => { const { result } = await renderConnected(); sendMessage('schedule', [makeSection({ section_id: 1, comment_count: 2 })]); - sendMessage('comment_added', { section_id: 1, comment_id: 9, user_id: 1, content: 'x' }); + sendMessage('comment_added', { section_id: 1 }); expect(result.current.sections[0].comment_count).toBe(3); }); - it('decrements comment_count on "comment_deleted"', async () => { - const { result } = await renderConnected(); - sendMessage('schedule', [makeSection({ section_id: 1, comment_count: 2 })]); - sendMessage('comment_deleted', { section_id: 1, comment_id: 9 }); - expect(result.current.sections[0].comment_count).toBe(1); - }); - - it('decrements comment_count by deleted_count when parent delete removes replies', async () => { + it('decrements comment_count by deleted_count on "comment_deleted"', async () => { const { result } = await renderConnected(); sendMessage('schedule', [makeSection({ section_id: 1, comment_count: 2 })]); - sendMessage('comment_deleted', { - section_id: 1, - comment_id: 9, - deleted_comment_ids: [9, 10], - deleted_count: 2, - }); + sendMessage('comment_deleted', { section_id: 1, deleted_count: 2 }); expect(result.current.sections[0].comment_count).toBe(0); }); it('adds a lock on "lock_acquired" and removes it on "lock_released"', async () => { const { result } = await renderConnected(); - const lockInfo = { section_id: 1, locked_by: 42, display_name: 'Jane Doe', expires_at: '2099-01-01T00:00:00Z' }; sendMessage('lock_acquired', lockInfo); expect(result.current.locks.get(1)).toEqual(lockInfo); @@ -190,41 +186,89 @@ describe('useScheduleWebSocket', () => { sendMessage('lock_released', { section_id: 1 }); expect(result.current.locks.has(1)).toBe(false); }); +}); - it('sets status=disconnected and schedules reconnect on close', async () => { - // Connect with real timers, then switch to fake timers for reconnect assertions - const { result } = await renderConnected(); - MockWebSocket.mockClear(); - - vi.useFakeTimers(); - try { - await act(async () => { mockWs.onclose?.({ code: 1001 }); }); - expect(result.current.status).toBe('disconnected'); - - await act(async () => { vi.advanceTimersByTime(1500); }); - expect(MockWebSocket).toHaveBeenCalledTimes(1); - } finally { - vi.useRealTimers(); - } - }); - - it('closes WebSocket and cancels reconnect on unmount', async () => { - // Connect with real timers, then switch to fake timers for unmount assertions - const { result, unmount } = await renderConnected(); - const wsRef = mockWs; - - vi.useFakeTimers(); - try { - await act(async () => { unmount(); }); - - MockWebSocket.mockClear(); - await act(async () => { vi.advanceTimersByTime(5000); }); - - expect(wsRef.close).toHaveBeenCalled(); - expect(MockWebSocket).not.toHaveBeenCalled(); - void result; - } finally { - vi.useRealTimers(); - } +describe('useScheduleData — subscriber lifecycle', () => { + it('two subscribers to the same scheduleId share one WebSocket', async () => { + const hook1 = renderHook(() => useScheduleData(10)); + await waitFor(() => expect(MockWebSocket).toHaveBeenCalledTimes(1)); + await act(async () => { + mockWs.onopen?.(new Event('open')); + }); + + const hook2 = renderHook(() => useScheduleData(10)); + await act(async () => { + // no new socket should open + }); + expect(MockWebSocket).toHaveBeenCalledTimes(1); + + const section = makeSection(); + sendMessage('schedule', [section]); + expect(hook1.result.current.sections).toEqual([section]); + expect(hook2.result.current.sections).toEqual([section]); + }); + + it('keeps the socket open when one of two subscribers unmounts', async () => { + const hook1 = await renderConnected(10); + const socketRef = mockWs; + + const hook2 = renderHook(() => useScheduleData(10)); + expect(MockWebSocket).toHaveBeenCalledTimes(1); + + await act(async () => { + hook1.unmount(); + await new Promise((r) => setTimeout(r, 5)); + }); + + expect(socketRef.close).not.toHaveBeenCalled(); + expect(__testing.getSubscriberCount()).toBe(1); + void hook2; + }); + + it('defers close on last unsubscribe and cancels it if a new subscriber mounts for the same scheduleId', async () => { + const hook1 = await renderConnected(10); + const socketRef = mockWs; + + await act(async () => { + hook1.unmount(); + }); + // Subscriber count is 0 but the close is deferred via setTimeout(0). + expect(socketRef.close).not.toHaveBeenCalled(); + + // Re-subscribe in the same tick (simulates route change Schedules → Faculty). + const hook2 = renderHook(() => useScheduleData(10)); + await act(async () => { + await new Promise((r) => setTimeout(r, 5)); + }); + + expect(socketRef.close).not.toHaveBeenCalled(); + expect(MockWebSocket).toHaveBeenCalledTimes(1); + void hook2; + }); + + it('closes the socket after the deferred window if no re-subscribe arrives', async () => { + const hook1 = await renderConnected(10); + const socketRef = mockWs; + + await act(async () => { + hook1.unmount(); + await new Promise((r) => setTimeout(r, 5)); + }); + + expect(socketRef.close).toHaveBeenCalled(); + expect(__testing.getSubscriberCount()).toBe(0); + }); + + it('switches connection when a subscriber requests a different scheduleId', async () => { + const { rerender } = await renderConnected(10); + const firstSocket = mockWs; + + await act(async () => { + rerender({ id: 20 }); + }); + await waitFor(() => expect(MockWebSocket).toHaveBeenCalledTimes(2)); + + expect(firstSocket.close).toHaveBeenCalled(); + expect(MockWebSocket.mock.calls[1][0]).toContain('/ws/20'); }); }); diff --git a/frontend/src/hooks/useScheduleData.ts b/frontend/src/hooks/useScheduleData.ts new file mode 100644 index 0000000..8460ee9 --- /dev/null +++ b/frontend/src/hooks/useScheduleData.ts @@ -0,0 +1,72 @@ +import { useEffect, useRef } from 'react'; +import { useAuth0 } from '@auth0/auth0-react'; +import { + subscribeToSchedule, + unsubscribeFromSchedule, + useScheduleDataStore, + type LockInfo, + type WsStatus, +} from '../stores/scheduleDataStore'; +import type { SectionRichResponse, WarningResponse } from '../api/generated'; + +export type { LockInfo, WsStatus }; + +export interface UseScheduleDataResult { + sections: SectionRichResponse[]; + locks: Map; + warnings: WarningResponse[]; + loading: boolean; + status: WsStatus; +} + +const EMPTY_LOCKS: Map = new Map(); +const EMPTY_SECTIONS: SectionRichResponse[] = []; +const EMPTY_WARNINGS: WarningResponse[] = []; + +export function useScheduleData(scheduleId: number | null): UseScheduleDataResult { + const { getAccessTokenSilently } = useAuth0(); + const getTokenRef = useRef(getAccessTokenSilently); + + useEffect(() => { + getTokenRef.current = getAccessTokenSilently; + }, [getAccessTokenSilently]); + + useEffect(() => { + if (scheduleId == null) return; + const getToken = () => getTokenRef.current(); + subscribeToSchedule(scheduleId, getToken); + return () => { + unsubscribeFromSchedule(scheduleId); + }; + }, [scheduleId]); + + const activeScheduleId = useScheduleDataStore((s) => s.scheduleId); + const sections = useScheduleDataStore((s) => s.sections); + const locks = useScheduleDataStore((s) => s.locks); + const warnings = useScheduleDataStore((s) => s.warnings); + const loading = useScheduleDataStore((s) => s.loading); + const status = useScheduleDataStore((s) => s.status); + + if (scheduleId == null) { + return { + sections: EMPTY_SECTIONS, + locks: EMPTY_LOCKS, + warnings: EMPTY_WARNINGS, + loading: false, + status: 'disconnected', + }; + } + + // Guard against returning data from a different schedule during a switch. + if (activeScheduleId !== scheduleId) { + return { + sections: EMPTY_SECTIONS, + locks: EMPTY_LOCKS, + warnings: EMPTY_WARNINGS, + loading: true, + status: 'connecting', + }; + } + + return { sections, locks, warnings, loading, status }; +} diff --git a/frontend/src/hooks/useScheduleWebSocket.ts b/frontend/src/hooks/useScheduleWebSocket.ts deleted file mode 100644 index a9c07ae..0000000 --- a/frontend/src/hooks/useScheduleWebSocket.ts +++ /dev/null @@ -1,212 +0,0 @@ -import { useEffect, useRef, useState } from 'react'; -import { useAuth0 } from '@auth0/auth0-react'; -import { getAutomatedCourseSchedulerAPI, type SectionRichResponse, type WarningResponse } from '../api/generated'; - -const WS_BASE = import.meta.env.VITE_API_BASE_URL - ? import.meta.env.VITE_API_BASE_URL.replace(/^http/, 'ws') - : 'ws://localhost:8000'; - -export type WsStatus = 'connecting' | 'connected' | 'disconnected'; - -export interface LockInfo { - section_id: number; - locked_by: number; - display_name: string; - expires_at: string; -} - -export interface UseScheduleWebSocketResult { - sections: SectionRichResponse[]; - locks: Map; - warnings: WarningResponse[]; - loading: boolean; - status: WsStatus; -} - -export function useScheduleWebSocket(scheduleId: number): UseScheduleWebSocketResult { - const { getAccessTokenSilently } = useAuth0(); - const getAccessTokenSilentlyRef = useRef(getAccessTokenSilently); - getAccessTokenSilentlyRef.current = getAccessTokenSilently; - - const [sections, setSections] = useState([]); - const [locks, setLocks] = useState>(new Map()); - const [warnings, setWarnings] = useState([]); - const [loading, setLoading] = useState(true); - const [status, setStatus] = useState('connecting'); - const sectionsRef = useRef([]); - const locksRef = useRef>(new Map()); - const warningsRef = useRef([]); - - // Fetch initial lock + warning state (neither is broadcast on connect, only on change) - useEffect(() => { - const api = getAutomatedCourseSchedulerAPI(); - api.getScheduleLocksSchedulesScheduleIdLocksGet(scheduleId).then((activeLocks) => { - const map = new Map(); - for (const l of activeLocks) { - map.set(l.section_id, l); - } - locksRef.current = map; - setLocks(new Map(map)); - }).catch(() => {}); - api.getScheduleWarningsSchedulesScheduleIdWarningsGet(scheduleId).then((list) => { - warningsRef.current = list; - setWarnings(list); - }).catch(() => {}); - }, [scheduleId]); - - useEffect(() => { - let ws: WebSocket | null = null; - let reconnectTimer: ReturnType | null = null; - let attempts = 0; - let cancelled = false; - - function fetchWarnings() { - getAutomatedCourseSchedulerAPI() - .getScheduleWarningsSchedulesScheduleIdWarningsGet(scheduleId) - .then((list) => { warningsRef.current = list; setWarnings(list); }) - .catch(() => {}); - } - - async function connect() { - if (cancelled) return; - - let token: string; - try { - token = await getAccessTokenSilentlyRef.current(); - } catch { - if (!cancelled) setStatus('disconnected'); - return; - } - - if (cancelled) return; - setStatus('connecting'); - - ws = new WebSocket(`${WS_BASE}/ws/${scheduleId}?token=${encodeURIComponent(token)}`); - - ws.onopen = () => { - if (cancelled) { ws?.close(); return; } - attempts = 0; - setStatus('connected'); - ws?.send(JSON.stringify({ action: 'init' })); - }; - - ws.onmessage = (event: MessageEvent) => { - if (cancelled) return; - let msg: { type: string; payload: unknown }; - try { - msg = JSON.parse(event.data as string); - } catch { - return; - } - - switch (msg.type) { - case 'schedule': { - const list = msg.payload as SectionRichResponse[]; - sectionsRef.current = list; - setSections(list); - setLoading(false); - break; - } - case 'section_created': { - const created = msg.payload as SectionRichResponse; - sectionsRef.current = [...sectionsRef.current, created]; - setSections(sectionsRef.current); - break; - } - case 'section_updated': { - const { section_id, data } = msg.payload as { - section_id: number; - data: SectionRichResponse; - }; - sectionsRef.current = sectionsRef.current.map((s) => - s.section_id === section_id ? data : s, - ); - setSections(sectionsRef.current); - break; - } - case 'section_deleted': { - const { section_id } = msg.payload as { section_id: number }; - sectionsRef.current = sectionsRef.current.filter( - (s) => s.section_id !== section_id, - ); - setSections(sectionsRef.current); - // Clear any lock for the deleted section - locksRef.current = new Map(locksRef.current); - locksRef.current.delete(section_id); - setLocks(new Map(locksRef.current)); - break; - } - case 'lock_acquired': { - const lockInfo = msg.payload as LockInfo; - locksRef.current = new Map(locksRef.current); - locksRef.current.set(lockInfo.section_id, lockInfo); - setLocks(new Map(locksRef.current)); - break; - } - case 'lock_released': { - const { section_id } = msg.payload as { section_id: number }; - locksRef.current = new Map(locksRef.current); - locksRef.current.delete(section_id); - setLocks(new Map(locksRef.current)); - break; - } - case 'comment_added': { - const payload = msg.payload as { section_id: number }; - const { section_id: sid } = payload; - sectionsRef.current = sectionsRef.current.map((s) => - s.section_id === sid - ? { ...s, comment_count: (s.comment_count ?? 0) + 1 } - : s, - ); - setSections([...sectionsRef.current]); - break; - } - case 'comment_deleted': { - const payload = msg.payload as { - section_id: number; - deleted_count?: number; - }; - const { section_id: sid, deleted_count } = payload; - const n = - typeof deleted_count === 'number' && Number.isFinite(deleted_count) - ? Math.max(1, Math.floor(deleted_count)) - : 1; - sectionsRef.current = sectionsRef.current.map((s) => - s.section_id === sid - ? { ...s, comment_count: Math.max(0, (s.comment_count ?? 0) - n) } - : s, - ); - setSections([...sectionsRef.current]); - break; - } - case 'section_warnings': { - fetchWarnings(); - break; - } - } - }; - - ws.onclose = () => { - if (cancelled) return; - setStatus('disconnected'); - const delay = Math.min(1000 * 2 ** attempts, 10_000); - attempts++; - reconnectTimer = setTimeout(connect, delay); - }; - - ws.onerror = () => { - ws?.close(); - }; - } - - connect(); - - return () => { - cancelled = true; - if (reconnectTimer) clearTimeout(reconnectTimer); - ws?.close(); - }; - }, [scheduleId]); - - return { sections, locks, warnings, loading, status }; -} diff --git a/frontend/src/pages/Faculty.tsx b/frontend/src/pages/Faculty.tsx index f3d5afc..6a14c75 100644 --- a/frontend/src/pages/Faculty.tsx +++ b/frontend/src/pages/Faculty.tsx @@ -5,10 +5,10 @@ import { type CampusResponse, type InviteResponse, type ScheduleResponse, - type SectionRichResponse, } from '../api/generated'; import FacultyDrawer, { type FacultyRecord } from '../components/FacultyDrawer'; import SearchableSelect, { type SelectOption } from '../components/SearchableSelect'; +import { useScheduleData } from '../hooks/useScheduleData'; // ── Histogram helpers ────────────────────────────────────────────────────── @@ -68,8 +68,7 @@ export default function Faculty() { // Schedule / sections (shared with histogram) const [schedules, setSchedules] = useState([]); const [selectedScheduleId, setSelectedScheduleId] = useState(null); - const [sections, setSections] = useState([]); - const [sectionsLoading, setSectionsLoading] = useState(false); + const { sections, loading: sectionsLoading } = useScheduleData(selectedScheduleId); // Campuses const [campuses, setCampuses] = useState([]); @@ -134,30 +133,6 @@ export default function Faculty() { .finally(() => setFacultyLoading(false)); }, []); - // Fetch sections when schedule changes - useEffect(() => { - if (!selectedScheduleId) { - setSections([]); - return; - } - let cancelled = false; - setSectionsLoading(true); - getAutomatedCourseSchedulerAPI() - .getScheduleSectionsRichSchedulesScheduleIdSectionsRichGet(selectedScheduleId) - .then((secs) => { - if (!cancelled) setSections(secs); - }) - .catch(() => { - if (!cancelled) setSections([]); - }) - .finally(() => { - if (!cancelled) setSectionsLoading(false); - }); - return () => { - cancelled = true; - }; - }, [selectedScheduleId]); - // Current load per faculty in selected schedule const loadMap = useMemo(() => { const map = new Map(); diff --git a/frontend/src/pages/Schedules.test.tsx b/frontend/src/pages/Schedules.test.tsx index e78ad25..7df51ee 100644 --- a/frontend/src/pages/Schedules.test.tsx +++ b/frontend/src/pages/Schedules.test.tsx @@ -2,7 +2,7 @@ import { render, screen } from '@testing-library/react'; import { MemoryRouter, Route, Routes } from 'react-router-dom'; import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'; import Schedules from './Schedules'; -import * as wsModule from '../hooks/useScheduleWebSocket'; +import * as wsModule from '../hooks/useScheduleData'; import * as generated from '../api/generated'; import type { SectionRichResponse, UserResponse } from '../api/generated'; @@ -61,7 +61,7 @@ const mockSection: SectionRichResponse = { instructors: [], }; -const defaultWsReturn: wsModule.UseScheduleWebSocketResult = { +const defaultWsReturn: wsModule.UseScheduleDataResult = { sections: [], locks: new Map(), warnings: [], @@ -101,7 +101,7 @@ describe('Schedules page', () => { }); it('shows spinner while WebSocket is loading', () => { - vi.spyOn(wsModule, 'useScheduleWebSocket').mockReturnValue({ + vi.spyOn(wsModule, 'useScheduleData').mockReturnValue({ ...defaultWsReturn, loading: true, status: 'connecting', }); renderAtRoute('/schedules/42'); @@ -110,7 +110,7 @@ describe('Schedules page', () => { }); it('renders ScheduleSectionRowView with sections once loaded', () => { - vi.spyOn(wsModule, 'useScheduleWebSocket').mockReturnValue({ + vi.spyOn(wsModule, 'useScheduleData').mockReturnValue({ ...defaultWsReturn, sections: [mockSection], }); renderAtRoute('/schedules/42'); @@ -121,7 +121,7 @@ describe('Schedules page', () => { }); it('passes parsed scheduleId from URL to the WebSocket hook', () => { - const hookSpy = vi.spyOn(wsModule, 'useScheduleWebSocket').mockReturnValue(defaultWsReturn); + const hookSpy = vi.spyOn(wsModule, 'useScheduleData').mockReturnValue(defaultWsReturn); renderAtRoute('/schedules/7'); expect(hookSpy).toHaveBeenCalledWith(7); }); @@ -138,26 +138,26 @@ describe('Schedules page', () => { }); it('renders breadcrumb link back to /schedules', () => { - vi.spyOn(wsModule, 'useScheduleWebSocket').mockReturnValue(defaultWsReturn); + vi.spyOn(wsModule, 'useScheduleData').mockReturnValue(defaultWsReturn); renderAtRoute('/schedules/42'); expect(screen.getByRole('link', { name: 'Schedules' })).toHaveAttribute('href', '/schedules'); }); it('displays schedule name fetched from API', async () => { - vi.spyOn(wsModule, 'useScheduleWebSocket').mockReturnValue(defaultWsReturn); + vi.spyOn(wsModule, 'useScheduleData').mockReturnValue(defaultWsReturn); renderAtRoute('/schedules/42'); expect(await screen.findByRole('heading', { name: 'Fall 2025' })).toBeInTheDocument(); }); it('does not show Faculty/Admin toggle for non-admin users', async () => { - vi.spyOn(wsModule, 'useScheduleWebSocket').mockReturnValue(defaultWsReturn); + vi.spyOn(wsModule, 'useScheduleData').mockReturnValue(defaultWsReturn); renderAtRoute('/schedules/42'); await screen.findByRole('heading', { name: 'Fall 2025' }); expect(screen.queryByRole('group', { name: 'Schedule mode' })).not.toBeInTheDocument(); }); it('shows Faculty/Admin mode toggle for ADMIN users', async () => { - vi.spyOn(wsModule, 'useScheduleWebSocket').mockReturnValue(defaultWsReturn); + vi.spyOn(wsModule, 'useScheduleData').mockReturnValue(defaultWsReturn); mockUserValue = { me: adminUser, meError: null, meLoading: false }; renderAtRoute('/schedules/42'); diff --git a/frontend/src/pages/Schedules.tsx b/frontend/src/pages/Schedules.tsx index 57cdc69..49929e1 100644 --- a/frontend/src/pages/Schedules.tsx +++ b/frontend/src/pages/Schedules.tsx @@ -2,7 +2,7 @@ import { useEffect, useState } from 'react'; import { Link, useParams } from 'react-router-dom'; import ScheduleSectionRowView from '../components/ScheduleSectionRowView'; import AlgorithmWarningsBanner from '../components/AlgorithmWarningsBanner'; -import { useScheduleWebSocket, type WsStatus } from '../hooks/useScheduleWebSocket'; +import { useScheduleData, type WsStatus } from '../hooks/useScheduleData'; import { getAutomatedCourseSchedulerAPI, type ScheduleResponse } from '../api/generated'; import { useUser } from '../context/UserContext'; import { downloadScheduleCsv } from '../utils/exportCsv'; @@ -52,7 +52,7 @@ function CalendarIcon({ active }: { active: boolean }) { } function ScheduleView({ scheduleId, readOnly }: { scheduleId: number; readOnly?: boolean }) { - const { sections, locks, warnings, loading, status } = useScheduleWebSocket(scheduleId); + const { sections, locks, warnings, loading, status } = useScheduleData(scheduleId); const [viewMode, setViewMode] = useState('table'); const [schedulePersona, setSchedulePersona] = useState('admin'); const [selectedCourseCount, setSelectedCourseCount] = useState(0); diff --git a/frontend/src/stores/scheduleDataStore.ts b/frontend/src/stores/scheduleDataStore.ts new file mode 100644 index 0000000..665564c --- /dev/null +++ b/frontend/src/stores/scheduleDataStore.ts @@ -0,0 +1,321 @@ +import { create } from 'zustand'; +import { + getAutomatedCourseSchedulerAPI, + type SectionRichResponse, + type WarningResponse, +} from '../api/generated'; + +export type WsStatus = 'connecting' | 'connected' | 'disconnected'; + +export interface LockInfo { + section_id: number; + locked_by: number; + display_name: string; + expires_at: string; +} + +type GetToken = () => Promise; + +interface ScheduleDataState { + scheduleId: number | null; + sections: SectionRichResponse[]; + locks: Map; + warnings: WarningResponse[]; + status: WsStatus; + loading: boolean; +} + +const WS_BASE = import.meta.env.VITE_API_BASE_URL + ? import.meta.env.VITE_API_BASE_URL.replace(/^http/, 'ws') + : 'ws://localhost:8000'; + +const initialState: ScheduleDataState = { + scheduleId: null, + sections: [], + locks: new Map(), + warnings: [], + status: 'disconnected', + loading: true, +}; + +export const useScheduleDataStore = create(() => ({ ...initialState })); + +// ── Connection lifecycle (module-scoped, not reactive state) ───────────────── + +let subscriberCount = 0; +let currentScheduleId: number | null = null; +let ws: WebSocket | null = null; +let reconnectTimer: ReturnType | null = null; +let closeTimer: ReturnType | null = null; +let attempts = 0; +let intentionalClose = false; + +function resetState(scheduleId: number) { + useScheduleDataStore.setState({ + scheduleId, + sections: [], + locks: new Map(), + warnings: [], + status: 'connecting', + loading: true, + }); +} + +function fetchInitialSnapshot(scheduleId: number) { + const api = getAutomatedCourseSchedulerAPI(); + api + .getScheduleLocksSchedulesScheduleIdLocksGet(scheduleId) + .then((active) => { + if (currentScheduleId !== scheduleId) return; + const map = new Map(); + for (const l of active) map.set(l.section_id, l); + useScheduleDataStore.setState({ locks: map }); + }) + .catch(() => {}); + api + .getScheduleWarningsSchedulesScheduleIdWarningsGet(scheduleId) + .then((list) => { + if (currentScheduleId !== scheduleId) return; + useScheduleDataStore.setState({ warnings: list }); + }) + .catch(() => {}); +} + +function handleMessage(msg: { type: string; payload: unknown }, scheduleId: number) { + if (currentScheduleId !== scheduleId) return; + const state = useScheduleDataStore.getState(); + switch (msg.type) { + case 'schedule': { + const list = msg.payload as SectionRichResponse[]; + useScheduleDataStore.setState({ sections: list, loading: false }); + break; + } + case 'section_created': { + const created = msg.payload as SectionRichResponse; + useScheduleDataStore.setState({ sections: [...state.sections, created] }); + break; + } + case 'section_updated': { + const { section_id, data } = msg.payload as { + section_id: number; + data: SectionRichResponse; + }; + useScheduleDataStore.setState({ + sections: state.sections.map((s) => (s.section_id === section_id ? data : s)), + }); + break; + } + case 'section_deleted': { + const { section_id } = msg.payload as { section_id: number }; + const newLocks = new Map(state.locks); + newLocks.delete(section_id); + useScheduleDataStore.setState({ + sections: state.sections.filter((s) => s.section_id !== section_id), + locks: newLocks, + }); + break; + } + case 'lock_acquired': { + const info = msg.payload as LockInfo; + const newLocks = new Map(state.locks); + newLocks.set(info.section_id, info); + useScheduleDataStore.setState({ locks: newLocks }); + break; + } + case 'lock_released': { + const { section_id } = msg.payload as { section_id: number }; + const newLocks = new Map(state.locks); + newLocks.delete(section_id); + useScheduleDataStore.setState({ locks: newLocks }); + break; + } + case 'comment_added': { + const { section_id } = msg.payload as { section_id: number }; + useScheduleDataStore.setState({ + sections: state.sections.map((s) => + s.section_id === section_id + ? { ...s, comment_count: (s.comment_count ?? 0) + 1 } + : s, + ), + }); + break; + } + case 'comment_deleted': { + const payload = msg.payload as { section_id: number; deleted_count?: number }; + const n = + typeof payload.deleted_count === 'number' && Number.isFinite(payload.deleted_count) + ? Math.max(1, Math.floor(payload.deleted_count)) + : 1; + useScheduleDataStore.setState({ + sections: state.sections.map((s) => + s.section_id === payload.section_id + ? { ...s, comment_count: Math.max(0, (s.comment_count ?? 0) - n) } + : s, + ), + }); + break; + } + case 'section_warnings': { + getAutomatedCourseSchedulerAPI() + .getScheduleWarningsSchedulesScheduleIdWarningsGet(scheduleId) + .then((list) => { + if (currentScheduleId !== scheduleId) return; + useScheduleDataStore.setState({ warnings: list }); + }) + .catch(() => {}); + break; + } + } +} + +async function connect(scheduleId: number, getToken: GetToken) { + if (currentScheduleId !== scheduleId) return; + + let token: string; + try { + token = await getToken(); + } catch { + if (currentScheduleId === scheduleId) { + useScheduleDataStore.setState({ status: 'disconnected' }); + } + return; + } + if (currentScheduleId !== scheduleId) return; + + useScheduleDataStore.setState({ status: 'connecting' }); + + const socket = new WebSocket( + `${WS_BASE}/ws/${scheduleId}?token=${encodeURIComponent(token)}`, + ); + ws = socket; + + socket.onopen = () => { + if (ws !== socket) { + socket.close(); + return; + } + attempts = 0; + useScheduleDataStore.setState({ status: 'connected' }); + socket.send(JSON.stringify({ action: 'init' })); + }; + + socket.onmessage = (event: MessageEvent) => { + if (ws !== socket) return; + let msg: { type: string; payload: unknown }; + try { + msg = JSON.parse(event.data as string); + } catch { + return; + } + handleMessage(msg, scheduleId); + }; + + socket.onclose = () => { + if (ws === socket) ws = null; + if (intentionalClose) return; + if (currentScheduleId !== scheduleId || subscriberCount === 0) return; + useScheduleDataStore.setState({ status: 'disconnected' }); + const delay = Math.min(1000 * 2 ** attempts, 10_000); + attempts++; + reconnectTimer = setTimeout(() => { + reconnectTimer = null; + if (currentScheduleId === scheduleId && subscriberCount > 0) { + void connect(scheduleId, getToken); + } + }, delay); + }; + + socket.onerror = () => { + socket.close(); + }; +} + +function openConnection(scheduleId: number, getToken: GetToken) { + intentionalClose = false; + attempts = 0; + fetchInitialSnapshot(scheduleId); + void connect(scheduleId, getToken); +} + +function closeSocket() { + if (reconnectTimer) { + clearTimeout(reconnectTimer); + reconnectTimer = null; + } + if (ws) { + intentionalClose = true; + try { + ws.close(); + } catch { + // ignore + } + ws = null; + } + attempts = 0; +} + +export function subscribeToSchedule(scheduleId: number, getToken: GetToken): void { + if (closeTimer) { + clearTimeout(closeTimer); + closeTimer = null; + } + + if (currentScheduleId !== scheduleId) { + closeSocket(); + currentScheduleId = scheduleId; + subscriberCount = 1; + resetState(scheduleId); + openConnection(scheduleId, getToken); + return; + } + + subscriberCount++; + if (!ws) { + // Socket was deferred-closed; state may still be populated. Reopen without clearing sections. + useScheduleDataStore.setState({ status: 'connecting' }); + openConnection(scheduleId, getToken); + } +} + +export function unsubscribeFromSchedule(scheduleId: number): void { + if (currentScheduleId !== scheduleId) return; + subscriberCount = Math.max(0, subscriberCount - 1); + if (subscriberCount === 0) { + if (closeTimer) clearTimeout(closeTimer); + closeTimer = setTimeout(() => { + closeTimer = null; + if (subscriberCount === 0) { + closeSocket(); + useScheduleDataStore.setState({ status: 'disconnected' }); + } + }, 0); + } +} + +// ── Test-only helpers ─────────────────────────────────────────────────────── + +export const __testing = { + reset() { + if (reconnectTimer) clearTimeout(reconnectTimer); + if (closeTimer) clearTimeout(closeTimer); + reconnectTimer = null; + closeTimer = null; + if (ws) { + intentionalClose = true; + try { + ws.close(); + } catch { + // ignore + } + } + ws = null; + subscriberCount = 0; + currentScheduleId = null; + attempts = 0; + intentionalClose = false; + useScheduleDataStore.setState({ ...initialState }); + }, + getSubscriberCount: () => subscriberCount, + getCurrentScheduleId: () => currentScheduleId, + getWs: () => ws, +};