s0wlz (Matthias Puchstein)
08cb668bab
All checks were successful
Release / release (push) Successful in 7m12s
The admin layout guard rendered only a "Redirecting to login..." placeholder for the /admin/login child route, trapping every unauthenticated visitor. Exempt the login route from the auth gate so the form renders correctly. Also wire the new POST /api/auth/refresh endpoint (from the dual-token migration) into both auth.init() and the api request() 401 handler, so sessions survive the 15-minute access-token lifetime without a hard logout. Adds a Playwright regression test asserting the login form is visible in a clean (no-cookie) browser context.
116 lines
8.2 KiB
Markdown
116 lines
8.2 KiB
Markdown
# Implementation Plan: Room Editor Refactor (Unified Visualization)
|
||
|
||
**Objective:** Replace the broken hardcoded `SeatMap.svelte` with a unified, dynamic room renderer that works across Admin Live View and Student Check-in.
|
||
|
||
---
|
||
|
||
## Pre-flight: Existing Bugs This Work Fixes
|
||
|
||
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 `<SeatMap variant="tutor" scale={0.78} />` 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:**
|
||
- 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<string,string>` | Labels overlaid on occupied seats |
|
||
| `selectedId` | `string \| null` | Currently selected element (editor) |
|
||
| `onElementClick` | `(id: string) => void` | Click callback |
|
||
|
||
---
|
||
|
||
## 2. Integration & Cleanup
|
||
|
||
### 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)*
|
||
|
||
**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<Vec<LayoutElement>>` 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 `<SeatMap variant="tutor" scale={0.78} />` at line 161 with `<RoomCanvas elements={slot.layout ?? []} clickable={true} occupiedSeatIds={...} studentNames={...} onElementClick={handleSeatClick} />`.
|
||
- `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`
|
||
|
||
**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<LayoutElement[]>([])` and assign `layout = res.layout ?? []`.
|
||
|
||
**Per call site:**
|
||
- Lines 210, 316 (seat-pick step, phone and desktop): Replace with `<RoomCanvas elements={layout} clickable={true} occupiedSeatIds={occupiedSeatIds} mySeatId={null} onElementClick={selectSeat} />`.
|
||
- Lines 248, 368 (confirmed step, phone and desktop): Replace with `<RoomCanvas elements={layout} clickable={false} occupiedSeatIds={occupiedSeatIds} mySeatId={myAttendance?.seat_id ?? null} />`.
|
||
|
||
**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<Attendance>` 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 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
|
||
|
||
### 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).
|
||
|
||
### 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 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.
|