diff --git a/conductor/room-editor-refactor-core.md b/conductor/room-editor-refactor-core.md index e77470f..d448967 100644 --- a/conductor/room-editor-refactor-core.md +++ b/conductor/room-editor-refactor-core.md @@ -1,69 +1,90 @@ # Implementation Plan: Room Editor Refactor (Core & Logic) -**Objective:** Standardize the room layout data model, align backend/frontend types, and refactor the core editor logic for robustness and grid-based precision. +**Objective:** Fix the pixel vs. grid-unit mismatch in stored room data, and robustify the editor for professional room planning. -**Background:** -The current room implementation suffers from naming inconsistencies (`type` vs `kind`) and coordinate system mismatches (pixels vs grid units). The editor logic in `RoomCanvas.svelte` is basic and needs to be more robust to support professional room planning. +**Background:** +The editor (`RoomCanvas.svelte`) already stores and renders in grid units (1 unit = 40 px). However, the demo seed (`demo_seed.sql`) was written with raw pixel values (e.g. `width: 200`), causing demo Room A to render broken (200 grid units = 8000 px). Any room created via the editor since launch is correct; any room predating the editor's grid-unit switch (or the demo room) is broken. A one-time data migration is the first priority. + +**Note on the `type`/`kind` field:** `backend/src/models.rs:81` already bridges this with `#[serde(rename = "type")] pub kind: String`. The wire format is `type` and `frontend/src/lib/types.ts:33` already uses `type`. **No rename is needed.** If the Rust internal name is ever changed to `type`, a raw identifier (`r#type`) is required since `type` is reserved. + +**Note on backend validation:** `backend/src/routes/rooms.rs:18–69` already implements `validate_layout` with empty-layout check, unique IDs, allowed types (`seat`, `table`, `gap`, `door`), unique seat labels, and non-negative geometry. Tests at lines 184–322 cover all of it. **Task 2 below replaces the previously planned duplicate work.** + +**Note on `SeatMap.svelte`:** This plan does **not** touch `SeatMap.svelte`. Its retirement and replacement with a dynamic renderer is handled by the sibling visualization plan. Any `LayoutElement` contract change made here must be cross-checked against `backend/src/routes/checkin.rs:53,194` (which deserialises it) and `frontend/src/lib/types.ts:86` (`CheckinInfo.layout`). --- -## 1. Data Model & Type Alignment +## 1. Data Migration & Seed Fix -### Task 1: Standardize LayoutElement Naming +### Task 1: Pixel → Grid-Unit Migration **Files to Modify:** -- `backend/src/models.rs` -- `frontend/src/lib/types.ts` +- `backend/migrations/003_normalize_room_layout_units.sql` *(create)* - `backend/demo/demo_seed.sql` +- `backend/src/routes/rooms.rs` (update tests that assert large numeric coordinates) **Changes:** -- Unified field name `type` (using `#[serde(rename = "type")]` if necessary in Rust, or changing it consistently). -- Standardize coordinate units: All `x`, `y`, `width`, `height` values in the database will represent **grid units** (e.g., 1 unit = 40px) rather than raw pixels. -- Update `demo_seed.sql` to use these normalized grid units. +- Write `003_normalize_room_layout_units.sql`. For each row in `rooms`, parse `layout_json`; if any element has `x`, `y`, `width`, or `height` > 50, divide all four by 40 and update the row. This heuristic is safe because grid-unit values are small integers/half-steps (max ~30), while pixel values are large (typically 80–800). +- Update `demo_seed.sql:16–41` to use grid units (e.g. `width: 200` → `width: 5`). The 24 elements in demo Room A need to be re-measured in grid units. +- Update any integration tests in `rooms.rs` that rely on large pixel-scale layout values. -### Task 2: Backend Validation +### Task 2: Backend Validation (Scope Reduction) **Files to Modify:** - `backend/src/routes/rooms.rs` -**Changes:** -- Add validation logic to `POST /api/admin/rooms` and `PUT /api/admin/rooms/:id/layout`. -- Ensure all elements have unique IDs. -- Validate that `type` is one of the allowed strings (`seat`, `table`, `door`, `gap`). +**Changes (additive only — do not duplicate existing logic):** +- Add upper-bound validation: `x` and `y` must be < a `MAX_CANVAS` constant (e.g. 100 grid units). Reject elements that fall off the canvas. +- Add grid-step validation: `x`, `y`, `width`, `height` must be multiples of 0.5 (i.e. `(value * 2) % 1 == 0`). Apply post-migration so existing data has already been normalised. +- Add a test for each new validator. --- ## 2. Editor Core Refactor -### Task 3: RoomCanvas Logic Overhaul +### Task 3: RoomCanvas State & Behaviour **Files to Modify:** - `frontend/src/lib/RoomCanvas.svelte` +**Current state (188 lines):** +- Drag: `draggingId / startX / startY` only (lines 26–28). No resize state or handles exist. +- Snap: lines 47–48 snap to 0.25 grid units (`Math.round(.../10)*10/40`). This is partially correct but the increment should be configurable (0.5 default). +- Rendering: already multiplies by `GRID_SIZE = 40` (line 85). Unit separation is mostly correct. +- Bug: `onmousemove` / `onmouseup` are bound on the SVG only (lines 69–71). Releasing the cursor outside the SVG strands the drag. Move these listeners to `window` for the duration of a drag. + **Changes:** -- **Grid Snap:** Implement mandatory snap-to-grid (0.5 or 1.0 unit increments) during dragging and resizing. -- **State Management:** Refactor internal dragging state to be cleaner and more predictable. -- **Selection:** Improve the selection highlight and event propagation. -- **Unit Separation:** Ensure the component strictly thinks in grid units, with the rendering layer handling the pixel scaling. +- **Build resize from scratch.** Add resize handles (e.g. bottom-right corner hit area) per element. Track `resizingId`, `resizeStartX`, `resizeStartY`, `resizeStartW`, `resizeStartH` as drag state. Snap resize delta to 0.5 increments. +- **Fix drag escape.** Bind `mousemove`/`mouseup` to `window` when dragging begins; remove them on drop. +- **Snap increment.** Change snap to 0.5 grid units (from 0.25). Accept an optional `snapStep` prop (default `0.5`) for the snap-toggle feature below. ### Task 4: Editor UI Improvements **Files to Modify:** - `frontend/src/routes/admin/rooms/[roomId]/+page.svelte` -**Changes:** -- Add a "Snap to Grid" toggle. -- Add numeric inputs for precise coordinate editing (X, Y, W, H). -- Implement "duplicate element" functionality. -- Better error handling and visual feedback during saving. +**What already exists (do not re-add):** +- Width/height inputs with `step="0.5"` (lines 90–97) +- Label input (line 87) +- Add seat/table/door buttons (lines 64–66) +- Delete button (line 101) + +**What to add:** +- **X/Y numeric inputs** (with `step="0.5"`) for precise coordinate editing of the selected element, bound to its `x` and `y` fields. +- **"+ Gap" button** alongside the existing add buttons. `gap` is accepted by `validate_layout` but is currently unreachable from the UI. +- **"Snap to Grid" toggle.** Bind to a boolean state; pass as `snapStep={snapEnabled ? 0.5 : 0}` to `RoomCanvas`. +- **"Duplicate element" button.** Copies the selected element with a new UUID and offsets it by 1 grid unit. +- **Surface save errors.** `saveLayout` (lines 27–29) currently only `console.error`. Display an inline error message in the UI. --- ## 3. Verification ### Automated Tests: -- `backend/src/routes/rooms.rs`: Add unit tests for layout validation. -- `frontend/tests/rooms.spec.ts`: Create a new Playwright test for room editing (creating elements, dragging, snapping, and saving). +- `backend/src/routes/rooms.rs`: Tests for the new upper-bound and grid-step validators. +- `backend/migrations/`: Verify migration 003 runs cleanly on the test DB (use `sqlx migrate run`). +- `frontend/tests/rooms.spec.ts` *(new)*: Playwright test — create a room, add table and two seats via the UI, drag a seat (verify snap), save and reload, assert coordinates are preserved. ### Manual Verification: -1. Create a new room. -2. Add a table and two seats. -3. Verify that dragging snaps to the grid. -4. Save and reload to ensure coordinates are preserved exactly. -5. Inspect the SQLite database to confirm coordinates are stored as small grid units (e.g., `2.5`) instead of large pixel values. +1. `make seed-demo` — reseed with the fixed `demo_seed.sql`. +2. Open `Admin → Rooms → Room A` in the editor. All elements must appear at sensible grid positions (not far off-screen). +3. Drag an element: verify it snaps to 0.5-unit increments. +4. Resize an element: verify handles appear and snap correctly. +5. Add a Gap element and verify it can be placed and saved. +6. Inspect the SQLite DB directly: `SELECT layout_json FROM rooms LIMIT 1`. All element coordinates must be small numbers (≤ 30), not pixel values (≥ 80). +7. Save and reload: verify coordinates are exactly preserved (no rounding drift). diff --git a/conductor/room-editor-refactor-visualization.md b/conductor/room-editor-refactor-visualization.md index c6c38dd..b44cafd 100644 --- a/conductor/room-editor-refactor-visualization.md +++ b/conductor/room-editor-refactor-visualization.md @@ -1,27 +1,46 @@ # Implementation Plan: Room Editor Refactor (Unified Visualization) -**Objective:** Replace hardcoded seat maps with a unified, dynamic, and high-fidelity room visualization system that works across Admin and Student views. - -**Background:** -Currently, the application uses a hardcoded `SeatMap.svelte` for Live Views and Student Check-ins, while using a dynamic `RoomCanvas.svelte` for editing. This leads to data mismatches and prevents users from using custom room layouts. This plan unifies the visualization layer. +**Objective:** Replace the broken hardcoded `SeatMap.svelte` with a unified, dynamic room renderer that works across Admin Live View and Student Check-in. --- -## 1. Unified Visualization Component +## Pre-flight: Existing Bugs This Work Fixes -### Task 1: Create `DynamicRoomView.svelte` -**Files to Create/Modify:** -- `frontend/src/lib/components/DynamicRoomView.svelte` -- (Optionally) Merge into `frontend/src/lib/RoomCanvas.svelte` +Acknowledge these before starting — do not assume current behaviour is correct. + +1. **Student check-in seat selection is silently broken in production.** `frontend/src/lib/components/SeatMap.svelte:33–58` uses hardcoded seat IDs (`T1-1`…`T4-5`). These IDs do not exist in any room's `layout_json`, so `POST /api/checkin` is rejected by `backend/src/routes/checkin.rs:200–207` (`"invalid seat"`) for every seat click. Migrating to the dynamic renderer is the fix. +2. **Admin live view shows no occupancy data.** `frontend/src/routes/admin/live/[slotId]/+page.svelte:161` calls `` with no `assignments` / `students` props — so even though `attendances` is loaded, nothing renders in the seat map. +3. **Check-in response is mistyped on the frontend.** `s/[code]/+page.svelte:82` treats the response of `POST /api/checkin` as an `Attendance` object, but the backend returns `{"ok": true}` (`checkin.rs:159–281`). Currently masked because `loadInfo()` is re-called immediately after. Fix this type error while migrating. + +**Note on other consumers of `SeatMap`:** A grep of `frontend/src/` found `SeatMap` is used only in the two routes described below. There are no dashboard, print-view, or mobile-only consumers. + +--- + +## 1. Extend `RoomCanvas` (Decision: Don't Fork) + +### Task 1: Add Read-Only / Interactive Modes to `RoomCanvas.svelte` +**Files to Modify:** +- `frontend/src/lib/RoomCanvas.svelte` + +**Decision rationale:** `RoomCanvas.svelte:14–24` already accepts `occupiedSeatIds`, `mySeatId`, `studentNames`, `selectedId`, `onElementClick`, and `editable`. Creating a separate `DynamicRoomView.svelte` would duplicate the SVG/element rendering logic and risk divergence. **Extend `RoomCanvas` instead.** **Changes:** -- Create a read-only/interactive component that renders SVG layouts based on the `LayoutElement[]` data. -- **High Fidelity:** Implement the aesthetic details from the design handoff (rounded tables, specific seat styling, label positioning). -- **Responsive Scaling:** Implement an `autoScale` or `viewBox` based system so the room fills the available width on mobile and desktop without breaking coordinates. -- **Interaction Modes:** - - `mode="checkin"`: Seats are clickable for students. - - `mode="notes"`: Seats are clickable for tutors to open note editors. - - `mode="display"`: Read-only view for dashboard/monitoring. +- Add a `clickable: boolean` prop (default `false`) — enables `onElementClick` in read-only mode without enabling edit handles. This maps to the `checkin` and `notes` use cases. +- Remove the hardcoded `width="800" height="600"` (line 65). Replace with a `viewBox` computed from element extents (or a configured canvas size), `preserveAspectRatio="xMidYMid meet"`, and a CSS `width: 100%; height: auto` so the SVG scales responsively to its container. Verify this does not break the editor (pass a fixed `style="width:800px"` wrapper in the editor route). +- Add high-fidelity styling per the design handoff: rounded tables (`rx`/`ry` on rect), specific seat styling (circle or rounded-rect), label positioning (centred on table, below seat icon), seat-state colours (vacant / occupied / mine). + +**Prop summary after changes:** + +| Prop | Type | Purpose | +|---|---|---| +| `elements` | `LayoutElement[]` | Layout data | +| `editable` | `boolean` | Enables drag, resize, add, delete | +| `clickable` | `boolean` | Enables `onElementClick` in read-only mode | +| `occupiedSeatIds` | `string[]` | Seats to style as occupied | +| `mySeatId` | `string \| null` | Seat to style as "mine" | +| `studentNames` | `Record` | Labels overlaid on occupied seats | +| `selectedId` | `string \| null` | Currently selected element (editor) | +| `onElementClick` | `(id: string) => void` | Click callback | --- @@ -30,37 +49,67 @@ Currently, the application uses a hardcoded `SeatMap.svelte` for Live Views and ### Task 2: Replace `SeatMap` in Admin Live View **Files to Modify:** - `frontend/src/routes/admin/live/[slotId]/+page.svelte` +- `backend/src/routes/attendance.rs` *(extend API response)* -**Changes:** -- Replace `SeatMap` with `DynamicRoomView`. -- Connect the `onSeatClick` event to the note-taking and manual attendance logic. -- Ensure attendance data (who sits where) is correctly overlaid on the dynamic layout. +**Pre-step — extend the backend API response (required):** +`attendance.rs:62–105` (`get_session_attendance`) returns `SessionAttendance` but does not include room layouts. The page has no way to know the layout. Options: +- **(Recommended)** Extend `SessionAttendance` in `backend/src/models.rs` and `attendance.rs` to include each slot's `layout: Option>` keyed per slot. This avoids an N+1 fetch. +- Alternative: have the page call `api.admin.rooms.get(slot.room_id)` after loading the slot. Simpler but adds a round-trip. + +**Frontend changes:** +- Replace `` at line 161 with ``. +- `occupiedSeatIds`: derive from `attendances.map(a => a.seat_id).filter(Boolean)`. +- `studentNames`: derive from `attendances` as `{ [seat_id]: student.name }`. +- **Seat → student mapping (new logic required):** The existing note-editor is driven by `selectedStudentId` and `toggleAttendance` takes `studentId`. The new `onElementClick(seatId)` must look up `attendances.find(a => a.seat_id === seatId)?.student_id` to populate `selectedStudentId`. Add this mapping in `handleSeatClick`. ### Task 3: Replace `SeatMap` in Student Check-in **Files to Modify:** - `frontend/src/routes/s/[code]/+page.svelte` -**Changes:** -- Replace `SeatMap` with `DynamicRoomView`. -- Connect seat selection to the `POST /api/checkin` API. -- Ensure the "current seat" (mySeatId) is visually highlighted in the dynamic view. +**There are 4 call sites, not 1.** `SeatMap` is called at lines 210, 248, 316, and 368 (phone + desktop × seat-pick step + confirmed step). All four need to be replaced. + +**Layout data is already on the wire.** `GET /api/checkin/:code` returns `CheckinInfo.layout: LayoutElement[] | null` (populated by `checkin.rs:53–68` and typed in `types.ts:84–88`), but `s/[code]/+page.svelte:43–58` discards `res.layout`. Read and store it: `let layout = $state([])` and assign `layout = res.layout ?? []`. + +**Per call site:** +- Lines 210, 316 (seat-pick step, phone and desktop): Replace with ``. +- Lines 248, 368 (confirmed step, phone and desktop): Replace with ``. + +**Derived state to add:** +```ts +const occupiedSeatIds = $derived(attendances.map(a => a.seat_id).filter(Boolean) as string[]); +``` + +**Fix the response-typing bug** (Pre-flight item 3): `api.ts` types `checkin.post` as `Promise` but the backend returns `{ok: true}`. Change the return type to `Promise<{ok: boolean}>` and update `s/[code]/+page.svelte:82` accordingly. ### Task 4: Deprecate `SeatMap.svelte` -**Files to Modify:** -- Delete `frontend/src/lib/components/SeatMap.svelte` once integration is verified. +**Files to Delete:** +- `frontend/src/lib/components/SeatMap.svelte` + +**Hard ordering — do not delete until all of the following are true:** +1. All 6 call sites are migrated (1 in admin live view + 4 in `s/[code]`). +2. `grep -rn "SeatMap" frontend/src/` returns zero results. +3. All Playwright tests pass (see Task 5). --- ## 3. Verification -### Automated Tests: -- `frontend/tests/checkin-dynamic.spec.ts`: E2E test to verify student check-in on a **custom-created** room layout. -- `frontend/tests/admin-live-dynamic.spec.ts`: E2E test to verify that tutors can see students on a **custom-created** room layout and click them to leave notes. +### Recommended Ordering +1. Extend the backend API (Task 2 pre-step) — unblocks frontend. +2. Extend `RoomCanvas` with `clickable`, responsive `viewBox`, and high-fidelity styling (Task 1). +3. Migrate `s/[code]` (Task 3) — backend already returns layout; this is the quickest win and immediately unblocks the broken check-in. +4. Migrate admin live view (Task 2) — needs new backend data and the seat→student mapping. +5. Run Playwright tests. +6. Delete `SeatMap.svelte` (Task 4). -### Manual Verification: -1. Create a non-standard room layout in the Admin Editor (e.g., a "U" shape). +### Automated Tests +- `frontend/tests/checkin-dynamic.spec.ts` *(new)*: E2E test — create a custom (non-square) room layout via the API, create a session + slot using it, open the student check-in link, verify the layout renders (not a blank grid), click a seat, verify `POST /api/checkin` succeeds and the seat turns green. Mirror the seat IDs already used in `backend/src/routes/checkin.rs:290–653` (`s1`, `s2`). +- `frontend/tests/admin-live-dynamic.spec.ts` *(new)*: E2E test — using the same custom room, manually add attendance for a student on seat `s1`, open the tutor live view, verify the student's name appears on the correct seat. + +### Manual Verification +1. Create a U-shaped room layout in `Admin → Rooms`. 2. Create a session and slot using this room. -3. Open the Student Check-in link on a mobile device (browser simulation). -4. Verify the "U" shape is rendered correctly and scaled to fit the screen. -5. Check in as a student and verify the seat turns green. -6. Open the Tutor Live View on a desktop and verify the same student is visible on the same seat in the "U" shape. +3. Open the student check-in link on a mobile viewport. Verify the U-shape is rendered and scaled to fit. +4. Check in as a student by clicking a seat. Verify the seat turns green. +5. Open the tutor Live View on desktop. Verify the same student appears on the correct seat in the U-shape. +6. Click the occupied seat. Verify the note-editor opens for that student. diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index a5708db..f43979f 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -7,6 +7,8 @@ import { auth } from './auth.svelte'; const BASE = '/api'; +let isRefreshing = false; + export async function request(path: string, init?: RequestInit): Promise { const res = await fetch(BASE + path, { ...init, @@ -18,6 +20,20 @@ export async function request(path: string, init?: RequestInit): Promise { }); if (res.status === 401 && browser) { + if (!isRefreshing && path !== '/auth/refresh') { + isRefreshing = true; + try { + const refreshed = await fetch(BASE + '/auth/refresh', { method: 'POST', credentials: 'include' }); + if (refreshed.ok) { + isRefreshing = false; + return request(path, init); + } + } catch (_e) { + // refresh failed, fall through to logout + } finally { + isRefreshing = false; + } + } auth.logout(); throw new Error('Unauthorized'); } diff --git a/frontend/src/lib/auth.svelte.ts b/frontend/src/lib/auth.svelte.ts index 085eeb1..9481fc7 100644 --- a/frontend/src/lib/auth.svelte.ts +++ b/frontend/src/lib/auth.svelte.ts @@ -14,7 +14,13 @@ export const auth = { async init() { if (!browser || _initialized) return; try { - const res = await fetch(`/api/auth/me?t=${Date.now()}`, { credentials: 'include' }); + let res = await fetch(`/api/auth/me?t=${Date.now()}`, { credentials: 'include' }); + if (!res.ok) { + const refreshed = await fetch('/api/auth/refresh', { method: 'POST', credentials: 'include' }); + if (refreshed.ok) { + res = await fetch(`/api/auth/me?t=${Date.now()}`, { credentials: 'include' }); + } + } if (res.ok) { const me = await res.json(); _isSuperadmin = me.is_superadmin; diff --git a/frontend/src/routes/admin/+layout.svelte b/frontend/src/routes/admin/+layout.svelte index 3822edd..4b37294 100644 --- a/frontend/src/routes/admin/+layout.svelte +++ b/frontend/src/routes/admin/+layout.svelte @@ -12,8 +12,11 @@ let course = $state(null); + const isLoginRoute = $derived($page.url.pathname === '/admin/login'); + onMount(async () => { await auth.init(); + if (isLoginRoute) return; if (!auth.authenticated) { goto('/admin/login'); return; @@ -27,7 +30,7 @@ }); $effect(() => { - if (auth.initialized && !auth.authenticated) goto('/admin/login'); + if (auth.initialized && !auth.authenticated && !isLoginRoute) goto('/admin/login'); }); const activePath = $derived($page.url.pathname); @@ -42,6 +45,8 @@ > {@render children()} + {:else if isLoginRoute} + {@render children()} {:else}
Redirecting to login...
{/if} diff --git a/frontend/tests/auth.spec.ts b/frontend/tests/auth.spec.ts new file mode 100644 index 0000000..f2b5fde --- /dev/null +++ b/frontend/tests/auth.spec.ts @@ -0,0 +1,20 @@ +import { test, expect } from '@playwright/test'; + +test.describe('Login page accessibility', () => { + test.use({ storageState: { cookies: [], origins: [] } }); + + test('renders login form without auth cookies (regression: redirect trap)', async ({ page }) => { + await page.goto('/admin/login'); + await expect(page.locator('#email')).toBeVisible(); + await expect(page.locator('#password')).toBeVisible(); + await expect(page.locator('button[type="submit"]')).toBeVisible(); + await expect(page.locator('text=Willkommen zurück')).toBeVisible(); + await expect(page.locator('text=Redirecting to login')).not.toBeVisible(); + }); + + test('unauthenticated /admin redirects to login form', async ({ page }) => { + await page.goto('/admin'); + await page.waitForURL(/\/admin\/login/); + await expect(page.locator('#email')).toBeVisible(); + }); +});