selfhostyourtech/reflector
2
0
Fork 0
mirror of https://github.com/Monadical-SAS/reflector.git synced 2026-02-06 10:46:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: address code review findings

Browse Source 
- Add max_length=100 on BulkStatusRequest.room_names to prevent abuse
- Filter bulk endpoint results to rooms user can see (owned or shared)
- Throw on bulk-status fetch error instead of silently returning empty data
- Fix room_by_id type annotation: dict[str, DbRoom] instead of Any
- Remove stale "200ms" comment in test
- Enable strict: true in jest tsconfig
- Remove working docs from tracked files
- Simplify redundant ternary in test helper
This commit is contained in:
Igor Loskutov
2026-02-05 19:36:17 -05:00
parent 931c344ddf
commit 9dc6c20ef8
6 changed files with 30 additions and 725 deletions
Download Patch File Download Diff File Expand all files Collapse all files

167
BATSHIT_REPORT.md
View File

@@ -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
```

533
FRONTEND_TEST_RESEARCH.md
View File

@@ -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<typeof client>;
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<typeof meetingStatusBatcher>;
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<paths>({ 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<typeof client>;
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
// "^@/(.*)$": "<rootDir>/$1",
},
setupFilesAfterSetup: ["<rootDir>/jest.setup.ts"],
// Ignore Next.js build output
testPathIgnorePatterns: ["<rootDir>/.next/", "<rootDir>/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<typeof client>;
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.

15
server/reflector/views/rooms.py
View File

@@ -8,13 +8,14 @@ from typing import Annotated, Any, Literal, Optional
from fastapi import APIRouter, Depends, HTTPException from fastapi import APIRouter, Depends, HTTPException
from fastapi_pagination import Page from fastapi_pagination import Page
from fastapi_pagination.ext.databases import apaginate from fastapi_pagination.ext.databases import apaginate
from pydantic import BaseModel from pydantic import BaseModel, Field
from redis.exceptions import LockError from redis.exceptions import LockError
import reflector.auth as auth import reflector.auth as auth
from reflector.db import get_database from reflector.db import get_database
from reflector.db.calendar_events import calendar_events_controller from reflector.db.calendar_events import calendar_events_controller
from reflector.db.meetings import meetings_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.db.rooms import rooms_controller
from reflector.redis_cache import RedisAsyncLock from reflector.redis_cache import RedisAsyncLock
from reflector.schemas.platform import Platform from reflector.schemas.platform import Platform
@@ -198,7 +199,7 @@ async def rooms_list(
class BulkStatusRequest(BaseModel): class BulkStatusRequest(BaseModel):
room_names: list[str] room_names: list[str] = Field(max_length=100)
class RoomMeetingStatus(BaseModel): class RoomMeetingStatus(BaseModel):
@@ -213,8 +214,14 @@ async def rooms_bulk_meeting_status(
): ):
user_id = user["sub"] if user else None user_id = user["sub"] if user else None
rooms = await rooms_controller.get_by_names(request.room_names) all_rooms = await rooms_controller.get_by_names(request.room_names)
room_by_id: dict[str, Any] = {r.id: r for r in rooms} # 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()) room_ids = list(room_by_id.keys())
current_time = datetime.now(timezone.utc) current_time = datetime.now(timezone.utc)

21
www/app/lib/__tests__/meetingStatusBatcher.test.tsx
View File

@@ -67,19 +67,13 @@ function mockBulkStatusEndpoint(
mockClient.POST.mockImplementation( mockClient.POST.mockImplementation(
async (_path: string, options: { body: { room_names: string[] } }) => { async (_path: string, options: { body: { room_names: string[] } }) => {
const roomNames: string[] = options.body.room_names; const roomNames: string[] = options.body.room_names;
const data = roomData const src = roomData ?? {};
? Object.fromEntries( const data = Object.fromEntries(
roomNames.map((name) => [ roomNames.map((name) => [
name, name,
roomData[name] ?? { active_meetings: [], upcoming_events: [] }, src[name] ?? { active_meetings: [], upcoming_events: [] },
]), ]),
) );
: Object.fromEntries(
roomNames.map((name) => [
name,
{ active_meetings: [], upcoming_events: [] },
]),
);
return { data, error: undefined, response: {} }; 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). // 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); expect(postCalls).toHaveLength(1);
// The single call should contain all 10 rooms (deduplicated) // The single call should contain all 10 rooms (deduplicated)

16
www/app/lib/meetingStatusBatcher.ts
View File

@@ -14,13 +14,19 @@ export function createMeetingStatusBatcher(windowMs: number = BATCH_WINDOW_MS) {
return create({ return create({
fetcher: async (roomNames: string[]): Promise<MeetingStatusResult[]> => { fetcher: async (roomNames: string[]): Promise<MeetingStatusResult[]> => {
const unique = [...new Set(roomNames)]; const unique = [...new Set(roomNames)];
const { data } = await client.POST("/v1/rooms/meetings/bulk-status", { const { data, error } = await client.POST(
body: { room_names: unique }, "/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) => ({ return roomNames.map((name) => ({
roomName: name, roomName: name,
active_meetings: data?.[name]?.active_meetings ?? [], active_meetings: data[name]?.active_meetings ?? [],
upcoming_events: data?.[name]?.upcoming_events ?? [], upcoming_events: data[name]?.upcoming_events ?? [],
})); }));
}, },
resolver: keyResolver("roomName"), resolver: keyResolver("roomName"),

3
www/jest.config.js
View File

@@ -12,8 +12,7 @@ module.exports = {
module: "esnext", module: "esnext",
moduleResolution: "bundler", moduleResolution: "bundler",
esModuleInterop: true, esModuleInterop: true,
strict: false, strict: true,
strictNullChecks: true,
downlevelIteration: true, downlevelIteration: true,
lib: ["dom", "dom.iterable", "esnext"], lib: ["dom", "dom.iterable", "esnext"],
}, },