diff --git a/BATSHIT_REPORT.md b/BATSHIT_REPORT.md deleted file mode 100644 index 777239d6..00000000 --- a/BATSHIT_REPORT.md +++ /dev/null @@ -1,167 +0,0 @@ -# Batch Room Meeting Status Queries — Handoff Report - -## Business Goal - -The rooms list page (`/rooms`) fires **2N+2 HTTP requests** for N rooms. Each room card renders a `MeetingStatus` component that independently calls two hooks: - -- `useRoomActiveMeetings(roomName)` → `GET /v1/rooms/{room_name}/meetings/active` -- `useRoomUpcomingMeetings(roomName)` → `GET /v1/rooms/{room_name}/meetings/upcoming` - -Each of those endpoints internally does a room lookup by name (`LIMIT 1`) plus a data query. For 10 rooms, that's 22 HTTP requests and 20 DB queries on page load. This is a classic N+1 problem at the API layer. - -**Goal: collapse all per-room meeting status queries into a single bulk POST request.** - -## Approach: DataLoader-style Batching - -We use [`@yornaath/batshit`](https://github.com/yornaath/batshit) — a lightweight DataLoader pattern for JS. It collects individual `.fetch(roomName)` calls within a 10ms window, deduplicates them, and dispatches one bulk request. Each caller gets back only their slice of the response. - -This is **isomorphic**: removing the batcher and reverting the hooks to direct API calls would still work. The backend bulk endpoint is additive, the individual endpoints remain untouched. - -## What Was Built - -### Branch - -`fix-room-query-batching` (worktree at `.worktrees/fix-room-query-batching/`) - -### Backend Changes - -**New bulk DB methods** (3 files): - -| File | Method | Purpose | -|------|--------|---------| -| `server/reflector/db/rooms.py` | `RoomController.get_by_names(names)` | Fetch rooms by name list using `IN` clause | -| `server/reflector/db/meetings.py` | `MeetingController.get_all_active_for_rooms(room_ids, current_time)` | Active meetings for multiple rooms, one query | -| `server/reflector/db/calendar_events.py` | `CalendarEventController.get_upcoming_for_rooms(room_ids, minutes_ahead)` | Upcoming events for multiple rooms, one query | - -**New endpoint** in `server/reflector/views/rooms.py`: - -``` -POST /v1/rooms/meetings/bulk-status -Body: { "room_names": ["room-a", "room-b", ...] } -Response: { "room-a": { "active_meetings": [...], "upcoming_events": [...] }, ... } -``` - -- 3 total DB queries regardless of room count (rooms lookup, active meetings, upcoming events) -- Auth masking applied: non-owners get `host_room_url=""` for Whereby, `description=null`/`attendees=null` for calendar events -- Registered before `/{room_id}` route to avoid path conflict -- Request/response models: `BulkStatusRequest`, `RoomMeetingStatus` - -### Frontend Changes - -**New dependency**: `@yornaath/batshit` (added to `www/package.json`) - -**New file**: `www/app/lib/meetingStatusBatcher.ts` - -```typescript -export const meetingStatusBatcher = create({ - fetcher: async (roomNames: string[]) => { - const unique = [...new Set(roomNames)]; - const { data } = await client.POST("/v1/rooms/meetings/bulk-status", { - body: { room_names: unique }, - }); - return roomNames.map((name) => ({ - roomName: name, - active_meetings: data?.[name]?.active_meetings ?? [], - upcoming_events: data?.[name]?.upcoming_events ?? [], - })); - }, - resolver: keyResolver("roomName"), - scheduler: windowScheduler(10), // 10ms batching window -}); -``` - -**Modified hooks** in `www/app/lib/apiHooks.ts`: - -- `useRoomActiveMeetings` — changed from `$api.useQuery("get", "/v1/rooms/{room_name}/meetings/active", ...)` to `useQuery` + `meetingStatusBatcher.fetch(roomName)` -- `useRoomUpcomingMeetings` — same pattern -- `useRoomsCreateMeeting` — cache invalidation updated from `$api.queryOptions(...)` to `meetingStatusKeys.active(roomName)` -- Added `meetingStatusKeys` query key factory: `{ active: (name) => ["rooms", name, "meetings/active"], upcoming: (name) => ["rooms", name, "meetings/upcoming"] }` - -**Cache invalidation compatibility**: the new query keys contain `"meetings/active"` and `"meetings/upcoming"` as string elements, which the existing `useMeetingDeactivate` predicate matches via `.includes()`. No changes needed there. - -**Regenerated**: `www/app/reflector-api.d.ts` from OpenAPI spec. - -### Files Modified - -| File | Change | -|------|--------| -| `server/reflector/db/rooms.py` | +`get_by_names()` | -| `server/reflector/db/meetings.py` | +`get_all_active_for_rooms()` | -| `server/reflector/db/calendar_events.py` | +`get_upcoming_for_rooms()` | -| `server/reflector/views/rooms.py` | +endpoint, +`BulkStatusRequest`, +`RoomMeetingStatus`, +imports (`asyncio`, `defaultdict`) | -| `www/package.json` | +`@yornaath/batshit` | -| `www/pnpm-lock.yaml` | Updated | -| `www/app/lib/meetingStatusBatcher.ts` | **New file** | -| `www/app/lib/apiHooks.ts` | Rewrote 2 hooks, added key factory, updated 1 invalidation | -| `www/app/reflector-api.d.ts` | Regenerated | - -## Verification Status - -**Tested:** -- Backend lint (ruff): clean -- Backend tests: 351 passed, 8 skipped (2 pre-existing flaky tests unrelated to this work: `test_transcript_rtc_and_websocket`, `test_transcript_upload_file`) -- TypeScript type check (`tsc --noEmit`): clean -- OpenAPI spec: bulk-status endpoint present and correctly typed -- Pre-commit hooks: all passed - -**NOT tested (requires manual browser verification):** -- Open rooms list page → Network tab shows single `POST /v1/rooms/meetings/bulk-status` instead of 2N GETs -- Active meeting badges render correctly per room -- Upcoming meeting indicators render correctly per room -- Single room page (`MeetingSelection.tsx`) still works (batcher handles batch-of-1) -- Meeting deactivation → cache invalidates and meeting status refreshes -- Creating a meeting → active meetings badge updates - -## Frontend Testing (Next Steps) - -See `FRONTEND_TEST_RESEARCH.md` for a full research document on how to write unit tests for these hooks. Summary: - -- **Approach**: `jest.mock()` on module-level `apiClient` and `meetingStatusBatcher`, `renderHook`/`waitFor` from `@testing-library/react` -- **Batcher testing**: unit test batcher directly with mock `client.POST`; test hooks with mock batcher module -- **New deps needed**: `@testing-library/react`, `@testing-library/jest-dom`, `@testing-library/dom`, `jest-environment-jsdom` -- **Key gotcha**: `openapi-react-query` reconstructed from mock client to test actual integration, or mock `$api` methods directly -- **Potential issues**: `ts-jest` v29 / Jest 30 compatibility, ESM handling for `openapi-react-query` - -## How to Run - -### Backend (Docker, from worktree) - -```bash -cd .worktrees/fix-room-query-batching - -# Symlink env files (not in git) -ln -sf /path/to/main/server/.env server/.env -ln -sf /path/to/main/www/.env.local www/.env.local -ln -sf /path/to/main/www/.env www/.env - -# Start services -docker compose up -d redis postgres server -``` - -### Frontend (manual, per project convention) - -```bash -cd .worktrees/fix-room-query-batching/www -pnpm install # if not done -pnpm dev -``` - -### Backend Tests - -```bash -cd .worktrees/fix-room-query-batching/server -REDIS_HOST=localhost CELERY_BROKER_URL=redis://localhost:6379/1 CELERY_RESULT_BACKEND=redis://localhost:6379/1 uv run pytest tests/ -q -``` - -### Regenerate OpenAPI Types - -Requires the backend server running on port 1250: - -```bash -cd .worktrees/fix-room-query-batching/server -REDIS_HOST=localhost CELERY_BROKER_URL=redis://localhost:6379/1 CELERY_RESULT_BACKEND=redis://localhost:6379/1 \ - uv run python -c "import json; from reflector.app import app; json.dump(app.openapi(), open('/tmp/openapi.json','w'))" 2>/dev/null - -cd ../www -npx openapi-typescript /tmp/openapi.json -o ./app/reflector-api.d.ts -``` diff --git a/FRONTEND_TEST_RESEARCH.md b/FRONTEND_TEST_RESEARCH.md deleted file mode 100644 index 6330f9d6..00000000 --- a/FRONTEND_TEST_RESEARCH.md +++ /dev/null @@ -1,533 +0,0 @@ -# Frontend Hook Testing Research - -## Context - -Hooks in `www/app/lib/apiHooks.ts` use two patterns: -1. `$api.useQuery("get", "/v1/rooms", ...)` -- openapi-react-query wrapping openapi-fetch -2. `useQuery({ queryFn: () => meetingStatusBatcher.fetch(roomName!) })` -- plain react-query with batshit batcher - -Key dependencies: module-level `client` and `$api` in `apiClient.tsx`, module-level `meetingStatusBatcher` in `meetingStatusBatcher.ts`, `useAuth()` context, `useError()` context. - ---- - -## 1. Recommended Mocking Strategy - -**Use `jest.mock()` on the module-level clients + `renderHook` from `@testing-library/react`.** - -Reasons against MSW: -- `openapi-fetch` creates its `client` at module import time. MSW's `server.listen()` must run before the client is created (see [openapi-ts/openapi-typescript#1878](https://github.com/openapi-ts/openapi-typescript/issues/1878)). This requires dynamic imports or Proxy workarounds -- fragile. -- MSW adds infrastructure overhead (handlers, server lifecycle) for unit tests that only need to verify hook behavior. -- The batcher calls `client.POST(...)` directly, not `fetch()`. Mocking the client is more direct. - -Reasons against `openapi-fetch-mock` middleware: -- Requires `client.use()` / `client.eject()` per test -- less isolated than module mock. -- Our `client` already has auth middleware; test middleware ordering gets messy. - -**Winner: Direct module mocking with `jest.mock()`.** - -### Pattern for `$api` hooks - -Mock the entire `apiClient` module. The `$api.useQuery` / `$api.useMutation` from openapi-react-query are just wrappers around react-query that call `client.GET`, `client.POST`, etc. Mock at the `client` method level: - -```ts -// __mocks__ or inline -jest.mock("../apiClient", () => { - const mockClient = { - GET: jest.fn(), - POST: jest.fn(), - PUT: jest.fn(), - PATCH: jest.fn(), - DELETE: jest.fn(), - use: jest.fn(), - }; - const createFetchClient = require("openapi-react-query").default; - return { - client: mockClient, - $api: createFetchClient(mockClient), - API_URL: "http://test", - WEBSOCKET_URL: "ws://test", - configureApiAuth: jest.fn(), - }; -}); -``` - -**Problem**: `openapi-react-query` calls `client.GET(path, init)` and expects `{ data, error, response }`. So the mock must return that shape: - -```ts -import { client } from "../apiClient"; -const mockClient = client as jest.Mocked; - -mockClient.GET.mockResolvedValue({ - data: { items: [], total: 0 }, - error: undefined, - response: new Response(), -}); -``` - -### Pattern for batcher hooks - -Mock the batcher module: - -```ts -jest.mock("../meetingStatusBatcher", () => ({ - meetingStatusBatcher: { - fetch: jest.fn(), - }, -})); - -import { meetingStatusBatcher } from "../meetingStatusBatcher"; -const mockBatcher = meetingStatusBatcher as jest.Mocked; - -mockBatcher.fetch.mockResolvedValue({ - roomName: "test-room", - active_meetings: [{ id: "m1" }], - upcoming_events: [], -}); -``` - ---- - -## 2. renderHook: Use `@testing-library/react` (NOT `@testing-library/react-hooks`) - -`@testing-library/react-hooks` is **deprecated** since React 18. The `renderHook` function is now built into `@testing-library/react` v13+. - -```ts -import { renderHook, waitFor } from "@testing-library/react"; -``` - -Key differences from the old package: -- No `waitForNextUpdate` -- use `waitFor(() => expect(...))` instead -- No separate `result.current.error` for error boundaries -- Works with React 18 concurrent features - ---- - -## 3. Mocking/Intercepting openapi-fetch Client - -Three viable approaches ranked by simplicity: - -### A. Mock the module (recommended) - -As shown above. `jest.mock("../apiClient")` replaces the entire module. The `$api` wrapper can be reconstructed from the mock client using the real `openapi-react-query` factory, or you can mock `$api` methods directly. - -**Caveat**: If you reconstruct `$api` from a mock client, `$api.useQuery` will actually call `mockClient.GET(path, init)`. This is good -- it tests the integration between openapi-react-query and the client. - -### B. Inject mock fetch into client - -```ts -const mockFetch = jest.fn().mockResolvedValue( - new Response(JSON.stringify({ data: "test" }), { status: 200 }) -); -const testClient = createClient({ baseUrl: "http://test", fetch: mockFetch }); -``` - -Problem: requires the hooks to accept the client as a parameter or use dependency injection. Our hooks use module-level `$api` -- doesn't work without refactoring. - -### C. MSW (not recommended for unit tests) - -As discussed, the module-import-time client creation conflicts with MSW's server lifecycle. Workable but fragile. - ---- - -## 4. Testing Batshit Batcher Behavior - -### Unit test the batcher directly (no hooks) - -The batcher is a pure module -- test it without React: - -```ts -import { create, keyResolver, windowScheduler } from "@yornaath/batshit"; - -test("batches multiple room queries into one POST", async () => { - const mockPost = jest.fn().mockResolvedValue({ - data: { - "room-a": { active_meetings: [], upcoming_events: [] }, - "room-b": { active_meetings: [], upcoming_events: [] }, - }, - }); - - const batcher = create({ - fetcher: async (roomNames: string[]) => { - const unique = [...new Set(roomNames)]; - const { data } = await mockPost("/v1/rooms/meetings/bulk-status", { - body: { room_names: unique }, - }); - return roomNames.map((name) => ({ - roomName: name, - active_meetings: data?.[name]?.active_meetings ?? [], - upcoming_events: data?.[name]?.upcoming_events ?? [], - })); - }, - resolver: keyResolver("roomName"), - scheduler: windowScheduler(10), - }); - - // Fire multiple fetches within the 10ms window - const [resultA, resultB] = await Promise.all([ - batcher.fetch("room-a"), - batcher.fetch("room-b"), - ]); - - // Only one POST call - expect(mockPost).toHaveBeenCalledTimes(1); - expect(mockPost).toHaveBeenCalledWith("/v1/rooms/meetings/bulk-status", { - body: { room_names: ["room-a", "room-b"] }, - }); - - expect(resultA.active_meetings).toEqual([]); - expect(resultB.active_meetings).toEqual([]); -}); - -test("deduplicates same room name", async () => { - const mockPost = jest.fn().mockResolvedValue({ - data: { "room-a": { active_meetings: [{ id: "m1" }], upcoming_events: [] } }, - }); - - const batcher = create({ - fetcher: async (roomNames: string[]) => { - const unique = [...new Set(roomNames)]; - const { data } = await mockPost("/v1/rooms/meetings/bulk-status", { - body: { room_names: unique }, - }); - return roomNames.map((name) => ({ - roomName: name, - active_meetings: data?.[name]?.active_meetings ?? [], - upcoming_events: data?.[name]?.upcoming_events ?? [], - })); - }, - resolver: keyResolver("roomName"), - scheduler: windowScheduler(10), - }); - - const [r1, r2] = await Promise.all([ - batcher.fetch("room-a"), - batcher.fetch("room-a"), - ]); - - expect(mockPost).toHaveBeenCalledTimes(1); - // Both resolve to same data - expect(r1.active_meetings).toEqual([{ id: "m1" }]); - expect(r2.active_meetings).toEqual([{ id: "m1" }]); -}); -``` - -### Test the actual batcher module with mocked client - -```ts -jest.mock("../apiClient", () => ({ - client: { POST: jest.fn() }, -})); - -import { client } from "../apiClient"; -import { meetingStatusBatcher } from "../meetingStatusBatcher"; - -const mockClient = client as jest.Mocked; - -test("meetingStatusBatcher calls bulk-status endpoint", async () => { - mockClient.POST.mockResolvedValue({ - data: { "room-x": { active_meetings: [], upcoming_events: [] } }, - error: undefined, - response: new Response(), - }); - - const result = await meetingStatusBatcher.fetch("room-x"); - expect(result.active_meetings).toEqual([]); - expect(mockClient.POST).toHaveBeenCalledWith( - "/v1/rooms/meetings/bulk-status", - expect.objectContaining({ body: { room_names: ["room-x"] } }), - ); -}); -``` - ---- - -## 5. Minimal Jest Setup - -### Install dependencies - -```bash -cd www && pnpm add -D @testing-library/react @testing-library/jest-dom jest-environment-jsdom -``` - -Note: `jest` v30 and `ts-jest` are already in devDependencies. `@types/jest` v30 is already present. `@testing-library/react` v16 supports React 18. - -### jest.config.ts - -```ts -import type { Config } from "jest"; - -const config: Config = { - testEnvironment: "jest-environment-jsdom", - transform: { - "^.+\\.tsx?$": ["ts-jest", { - tsconfig: "tsconfig.json", - }], - }, - moduleNameMapper: { - // Handle module aliases if you add them later - // "^@/(.*)$": "/$1", - }, - setupFilesAfterSetup: ["/jest.setup.ts"], - // Ignore Next.js build output - testPathIgnorePatterns: ["/.next/", "/node_modules/"], -}; - -export default config; -``` - -### jest.setup.ts - -```ts -import "@testing-library/jest-dom"; -``` - -### Mocking modules that fail in jsdom - -The `apiClient.tsx` module calls `getSession()` and `getClientEnv()` at import time. These will fail in jsdom. Mock them: - -```ts -// In test file or __mocks__ -jest.mock("next-auth/react", () => ({ - getSession: jest.fn().mockResolvedValue(null), - signIn: jest.fn(), - signOut: jest.fn(), - useSession: jest.fn().mockReturnValue({ data: null, status: "unauthenticated" }), -})); - -jest.mock("../next", () => ({ - isBuildPhase: false, -})); - -jest.mock("../clientEnv", () => ({ - getClientEnv: () => ({ - API_URL: "http://test-api", - WEBSOCKET_URL: "ws://test-api", - FEATURE_REQUIRE_LOGIN: false, - FEATURE_PRIVACY: null, - FEATURE_BROWSE: null, - FEATURE_SEND_TO_ZULIP: null, - FEATURE_ROOMS: null, - }), -})); -``` - ---- - -## 6. Testing Hooks Without Prop-Based DI - -The hooks use module-level `$api` and `meetingStatusBatcher`. No way to inject via props. Two approaches: - -### A. `jest.mock()` the modules (recommended) - -Already shown. Works cleanly. Each test can `mockResolvedValue` differently. - -### B. Manual mock files (`__mocks__/`) - -Create `www/app/lib/__mocks__/apiClient.tsx`: - -```ts -const mockClient = { - GET: jest.fn(), - POST: jest.fn(), - PUT: jest.fn(), - PATCH: jest.fn(), - DELETE: jest.fn(), - use: jest.fn(), -}; - -// Use real openapi-react-query wrapping the mock client -const createFetchClient = jest.requireActual("openapi-react-query").default; - -export const client = mockClient; -export const $api = createFetchClient(mockClient); -export const API_URL = "http://test"; -export const WEBSOCKET_URL = "ws://test"; -export const configureApiAuth = jest.fn(); -``` - -Then in tests: `jest.mock("../apiClient")` with no factory -- picks up the `__mocks__` file automatically. - ---- - -## 7. Complete Example Test - -```ts -// www/app/lib/__tests__/apiHooks.test.ts - -import { renderHook, waitFor } from "@testing-library/react"; -import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import React from "react"; - -// Mock modules before imports -jest.mock("next-auth/react", () => ({ - getSession: jest.fn().mockResolvedValue(null), - signIn: jest.fn(), - signOut: jest.fn(), - useSession: jest.fn().mockReturnValue({ - data: null, - status: "unauthenticated", - }), -})); - -jest.mock("../clientEnv", () => ({ - getClientEnv: () => ({ - API_URL: "http://test", - WEBSOCKET_URL: "ws://test", - FEATURE_REQUIRE_LOGIN: false, - FEATURE_PRIVACY: null, - FEATURE_BROWSE: null, - FEATURE_SEND_TO_ZULIP: null, - FEATURE_ROOMS: null, - }), -})); - -jest.mock("../next", () => ({ isBuildPhase: false })); - -jest.mock("../meetingStatusBatcher", () => ({ - meetingStatusBatcher: { fetch: jest.fn() }, -})); - -// Must mock apiClient BEFORE importing hooks -jest.mock("../apiClient", () => { - const mockClient = { - GET: jest.fn(), - POST: jest.fn(), - PATCH: jest.fn(), - DELETE: jest.fn(), - use: jest.fn(), - }; - const createFetchClient = - jest.requireActual("openapi-react-query").default; - return { - client: mockClient, - $api: createFetchClient(mockClient), - API_URL: "http://test", - WEBSOCKET_URL: "ws://test", - configureApiAuth: jest.fn(), - }; -}); - -// Now import the hooks under test -import { useRoomActiveMeetings, useTranscriptsSearch } from "../apiHooks"; -import { client } from "../apiClient"; -import { meetingStatusBatcher } from "../meetingStatusBatcher"; -import { ErrorProvider } from "../../(errors)/errorContext"; - -const mockClient = client as jest.Mocked; -const mockBatcher = meetingStatusBatcher as { fetch: jest.Mock }; - -// Wrapper with required providers -function createWrapper() { - const queryClient = new QueryClient({ - defaultOptions: { - queries: { retry: false }, - }, - }); - return function Wrapper({ children }: { children: React.ReactNode }) { - return React.createElement( - QueryClientProvider, - { client: queryClient }, - React.createElement(ErrorProvider, null, children), - ); - }; -} - -describe("useRoomActiveMeetings", () => { - afterEach(() => jest.clearAllMocks()); - - it("returns active meetings from batcher", async () => { - mockBatcher.fetch.mockResolvedValue({ - roomName: "test-room", - active_meetings: [{ id: "m1", room_name: "test-room" }], - upcoming_events: [], - }); - - const { result } = renderHook( - () => useRoomActiveMeetings("test-room"), - { wrapper: createWrapper() }, - ); - - await waitFor(() => expect(result.current.isSuccess).toBe(true)); - - expect(result.current.data).toEqual([ - { id: "m1", room_name: "test-room" }, - ]); - expect(mockBatcher.fetch).toHaveBeenCalledWith("test-room"); - }); - - it("is disabled when roomName is null", () => { - const { result } = renderHook( - () => useRoomActiveMeetings(null), - { wrapper: createWrapper() }, - ); - - expect(result.current.fetchStatus).toBe("idle"); - expect(mockBatcher.fetch).not.toHaveBeenCalled(); - }); -}); - -describe("useTranscriptsSearch", () => { - afterEach(() => jest.clearAllMocks()); - - it("fetches transcripts via $api", async () => { - mockClient.GET.mockResolvedValue({ - data: { items: [{ id: "t1", title: "Test" }], total: 1 }, - error: undefined, - response: new Response(), - }); - - const { result } = renderHook( - () => useTranscriptsSearch("hello"), - { wrapper: createWrapper() }, - ); - - await waitFor(() => expect(result.current.isSuccess).toBe(true)); - - expect(result.current.data).toEqual({ - items: [{ id: "t1", title: "Test" }], - total: 1, - }); - expect(mockClient.GET).toHaveBeenCalledWith( - "/v1/transcripts/search", - expect.objectContaining({ - params: expect.objectContaining({ - query: expect.objectContaining({ q: "hello" }), - }), - }), - ); - }); -}); -``` - ---- - -## 8. Summary of Decisions - -| Question | Answer | -|----------|--------| -| Mocking approach | `jest.mock()` on module-level clients | -| renderHook source | `@testing-library/react` (not deprecated hooks lib) | -| Intercept openapi-fetch | Mock `client.GET/POST/...` methods, reconstruct `$api` with real `openapi-react-query` | -| Test batcher | Unit test batcher directly with mock POST fn; test hooks with mock batcher module | -| Auth context | Mock `next-auth/react`, disable `requireLogin` feature flag | -| Error context | Wrap with real `ErrorProvider` (it's simple state) | -| QueryClient | New instance per test, `retry: false` | - -### New packages needed - -```bash -cd www && pnpm add -D @testing-library/react @testing-library/jest-dom @testing-library/dom jest-environment-jsdom -``` - -### Files to create - -1. `www/jest.config.ts` -- jest configuration -2. `www/jest.setup.ts` -- `import "@testing-library/jest-dom"` -3. `www/app/lib/__tests__/apiHooks.test.ts` -- hook tests -4. `www/app/lib/__tests__/meetingStatusBatcher.test.ts` -- batcher unit tests - -### Potential issues - -- `ts-jest` v29 may not fully support Jest 30. Watch for compatibility errors. May need `ts-jest@next` or switch to `@swc/jest`. -- `openapi-react-query` imports may need ESM handling in Jest. If `jest.requireActual("openapi-react-query")` fails, mock `$api` methods directly instead of reconstructing. -- `"use client"` directive at top of `apiHooks.ts` / `apiClient.tsx` -- Jest ignores this (it's a no-op outside Next.js bundler), but verify it doesn't cause parse errors. diff --git a/server/reflector/views/rooms.py b/server/reflector/views/rooms.py index 59369b44..f461ee56 100644 --- a/server/reflector/views/rooms.py +++ b/server/reflector/views/rooms.py @@ -8,13 +8,14 @@ from typing import Annotated, Any, Literal, Optional from fastapi import APIRouter, Depends, HTTPException from fastapi_pagination import Page from fastapi_pagination.ext.databases import apaginate -from pydantic import BaseModel +from pydantic import BaseModel, Field from redis.exceptions import LockError import reflector.auth as auth from reflector.db import get_database from reflector.db.calendar_events import calendar_events_controller from reflector.db.meetings import meetings_controller +from reflector.db.rooms import Room as DbRoom from reflector.db.rooms import rooms_controller from reflector.redis_cache import RedisAsyncLock from reflector.schemas.platform import Platform @@ -198,7 +199,7 @@ async def rooms_list( class BulkStatusRequest(BaseModel): - room_names: list[str] + room_names: list[str] = Field(max_length=100) class RoomMeetingStatus(BaseModel): @@ -213,8 +214,14 @@ async def rooms_bulk_meeting_status( ): user_id = user["sub"] if user else None - rooms = await rooms_controller.get_by_names(request.room_names) - room_by_id: dict[str, Any] = {r.id: r for r in rooms} + all_rooms = await rooms_controller.get_by_names(request.room_names) + # Filter to rooms the user can see (owned or shared), matching rooms_list behavior + rooms = [ + r + for r in all_rooms + if r.is_shared or (user_id is not None and r.user_id == user_id) + ] + room_by_id: dict[str, DbRoom] = {r.id: r for r in rooms} room_ids = list(room_by_id.keys()) current_time = datetime.now(timezone.utc) diff --git a/www/app/lib/__tests__/meetingStatusBatcher.test.tsx b/www/app/lib/__tests__/meetingStatusBatcher.test.tsx index 43b5a4be..dca47f4e 100644 --- a/www/app/lib/__tests__/meetingStatusBatcher.test.tsx +++ b/www/app/lib/__tests__/meetingStatusBatcher.test.tsx @@ -67,19 +67,13 @@ function mockBulkStatusEndpoint( mockClient.POST.mockImplementation( async (_path: string, options: { body: { room_names: string[] } }) => { const roomNames: string[] = options.body.room_names; - const data = roomData - ? Object.fromEntries( - roomNames.map((name) => [ - name, - roomData[name] ?? { active_meetings: [], upcoming_events: [] }, - ]), - ) - : Object.fromEntries( - roomNames.map((name) => [ - name, - { active_meetings: [], upcoming_events: [] }, - ]), - ); + const src = roomData ?? {}; + const data = Object.fromEntries( + roomNames.map((name) => [ + name, + src[name] ?? { active_meetings: [], upcoming_events: [] }, + ]), + ); return { data, error: undefined, response: {} }; }, ); @@ -152,7 +146,6 @@ describe("meeting status batcher integration", () => { ); // Without batching this would be 20 calls (2 hooks x 10 rooms). - // With the 200ms test window, all queries land in one batch → exactly 1 POST. expect(postCalls).toHaveLength(1); // The single call should contain all 10 rooms (deduplicated) diff --git a/www/app/lib/meetingStatusBatcher.ts b/www/app/lib/meetingStatusBatcher.ts index 3bb2e29e..22f78906 100644 --- a/www/app/lib/meetingStatusBatcher.ts +++ b/www/app/lib/meetingStatusBatcher.ts @@ -14,13 +14,19 @@ export function createMeetingStatusBatcher(windowMs: number = BATCH_WINDOW_MS) { return create({ fetcher: async (roomNames: string[]): Promise => { const unique = [...new Set(roomNames)]; - const { data } = await client.POST("/v1/rooms/meetings/bulk-status", { - body: { room_names: unique }, - }); + const { data, error } = await client.POST( + "/v1/rooms/meetings/bulk-status", + { body: { room_names: unique } }, + ); + if (error || !data) { + throw new Error( + `bulk-status fetch failed: ${JSON.stringify(error ?? "no data")}`, + ); + } return roomNames.map((name) => ({ roomName: name, - active_meetings: data?.[name]?.active_meetings ?? [], - upcoming_events: data?.[name]?.upcoming_events ?? [], + active_meetings: data[name]?.active_meetings ?? [], + upcoming_events: data[name]?.upcoming_events ?? [], })); }, resolver: keyResolver("roomName"), diff --git a/www/jest.config.js b/www/jest.config.js index e526a641..8348aa25 100644 --- a/www/jest.config.js +++ b/www/jest.config.js @@ -12,8 +12,7 @@ module.exports = { module: "esnext", moduleResolution: "bundler", esModuleInterop: true, - strict: false, - strictNullChecks: true, + strict: true, downlevelIteration: true, lib: ["dom", "dom.iterable", "esnext"], },