diff --git a/docs/plans/2026-05-04-unified-fixes-and-room-editor-refactor.md b/docs/plans/2026-05-04-unified-fixes-and-room-editor-refactor.md index 795848f..b1bd37e 100644 --- a/docs/plans/2026-05-04-unified-fixes-and-room-editor-refactor.md +++ b/docs/plans/2026-05-04-unified-fixes-and-room-editor-refactor.md @@ -19,24 +19,93 @@ The intended outcome is one continuous batch of work that ships: secure auth (lo --- +## Phase 0 — Git Workflow + +### 0.1 Create a worktree + +From your main clone (assume it lives at `~/tutortool`): + +```sh +git worktree add ../tutortool-unified main +cd ../tutortool-unified +git switch -c feature/unified-fixes-room-editor +``` + +All implementation work happens in `../tutortool-unified`. +The original clone stays clean on `main`. + +### 0.2 Commit discipline + +- One logical commit per Phase/sub-phase, e.g.: + - `feat(security): fix RUSTSEC-2023-0071 via aws_lc_rs` + - `fix(api): handle empty 200 body + fix check-in typing` + - `feat(tutors): add is_active flag, deactivate + safe delete` + - `feat(rooms): pixel→grid migration + layout validators` + - `feat(canvas): fix drag/resize, click-to-select in edit mode` + - `feat(viz): replace SeatMap with dynamic RoomCanvas` +- `make lint && make test` must pass **before** each commit. + Gate with a pre-commit hook if you like: + ```sh + # .git/hooks/pre-commit + #!/bin/sh + cd backend && cargo clippy --all-targets -- -D warnings && cargo test + ``` + +### 0.3 Open the PR + +Once all phases are done and CI is green: + +```sh +git push -u origin feature/unified-fixes-room-editor +# then open a PR against main via Gitea/GitHub UI +# title: "Unified fixes, tutor lifecycle, room editor refactor" +# body: link this plan doc, list phases, reference Playwright spec results +``` + +Merge strategy: **squash-merge** each phase-branch if you split work, or a single **merge commit** if working on one branch (keeps phase history readable). + +### 0.4 Post-merge cleanup + +```sh +cd ~/tutortool # back to main clone +git pull # fast-forward to merged state +git worktree remove ../tutortool-unified +git branch -d feature/unified-fixes-room-editor +``` + +### 0.5 Verification gate before merge + +PR must have: + +- [ ] `make test` passes (all backend + new Phase 2.4/3.2/3.7 tests) +- [ ] `make lint` passes (zero Clippy warnings, zero TS errors) +- [ ] `sqlx migrate run` clean on fresh DB (migrations 003 + 004) +- [ ] All new Playwright specs green (`rooms`, `checkin-dynamic`, `admin-live-dynamic`, `admin-tutors`, `admin-rooms-delete`) +- [ ] `cargo audit` clean (no RUSTSEC-2023-0071) + ## Phase 1 — Security & Quick Fixes ### 1.1 RUSTSEC-2023-0071 (Marvin Attack) + - `backend/Cargo.toml` — set `jsonwebtoken` to `features = ["aws_lc_rs"]`. - `backend/audit.toml` — remove `ignore = ["RUSTSEC-2023-0071"]`. - `.gitea/workflows/ci.yml` and `.gitea/workflows/release.yml` — drop `--ignore RUSTSEC-2023-0071`. ### 1.2 JSON parse error on empty 200 body + - `frontend/src/lib/api.ts:46` — currently only 204 is short-circuited. Extend the empty-body branch to also handle 200-with-empty-body (probe `content-length === '0'` or fall through `await res.text()` and return `{} as T` if empty). Triggers from `assignTutor`, `unassignTutor`, etc. ### 1.3 Check-in API typing + - `frontend/src/lib/api.ts` — `checkin.post` currently typed `Promise`; backend returns `{ok: true}`. Change to `Promise<{ok: boolean}>`. - `frontend/src/routes/s/[code]/+page.svelte:82` — drop the local-state assignment from the response; rely on the subsequent `loadInfo()` to populate `myAttendance`. ### 1.4 Attendance count not assigned (issues doc #2) + - `frontend/src/routes/s/[code]/+page.svelte` `loadInfo` — assign `attendances = checkinAttendances` so the "Anwesende N / M" counter updates. ### 1.5 Admin logout UI + - `frontend/src/lib/components/TutorShell.svelte` (where the actual sidebar nav lives — `frontend/src/routes/admin/+layout.svelte` only renders the shell) — add an "Abmelden" entry at the bottom of the nav. On click: `await api.auth.logout()` → `auth.logout()` → `goto('/admin/login')`. Re-export `auth` and `api` if needed. --- @@ -46,6 +115,7 @@ The intended outcome is one continuous batch of work that ships: secure auth (lo The user wants **both** soft-deactivate (preserve history, hide from pickers) **and** real hard-delete (with safety pre-check). ### 2.1 Schema migration + - New file `backend/migrations/004_tutor_is_active.sql`: ```sql ALTER TABLE tutors ADD COLUMN is_active BOOLEAN NOT NULL DEFAULT 1; @@ -53,6 +123,7 @@ The user wants **both** soft-deactivate (preserve history, hide from pickers) ** No backfill needed; existing rows default to active. ### 2.2 Backend changes + **File:** `backend/src/routes/tutors.rs` - **`Tutor` model** (`backend/src/models.rs:11-17`) — add `is_active: bool`. @@ -62,13 +133,15 @@ The user wants **both** soft-deactivate (preserve history, hide from pickers) ** - `tutor_courses WHERE tutor_id = ?` - `slots WHERE tutor_id = ?` - `notes WHERE tutor_id = ?` - If any non-zero, return `AppError::Conflict(format!("Tutor:in hat noch {c} Kurszuordnung(en), {s} Slot(s) und {n} Notiz(en). Bitte zuerst entfernen oder deaktivieren."))`. + If any non-zero, return `AppError::Conflict(format!("Tutor:in hat noch {c} Kurszuordnung(en), {s} Slot(s) und {n} Notiz(en). Bitte zuerst entfernen oder deaktivieren."))`. `AppError::Conflict` already maps to 409 with `{"error": msg}` (`backend/src/error.rs`). + - **Auth login** (`backend/src/routes/auth_routes.rs:22-96`) — reject inactive tutors with `AppError::Unauthorized` (same response shape as wrong-password to avoid info leakage). Add this check after password verification. - **Tutor pickers** — grep for `SELECT … FROM tutors` outside of `list_tutors` and `auth_routes.rs`. Likely sites: `backend/src/routes/courses.rs` (tutor assignment list), `backend/src/routes/sessions.rs` or `slots` creation flow. Each must add `WHERE is_active = 1`. ### 2.3 Frontend changes + **File:** `frontend/src/routes/admin/tutors/+page.svelte` - Show three status pills: `Superadmin` / `Tutor:in` / `Inaktiv` (combine `is_superadmin` + `is_active`). @@ -77,6 +150,7 @@ The user wants **both** soft-deactivate (preserve history, hide from pickers) ** - **API client** `frontend/src/lib/api.ts` — add `api.admin.tutors.setActive(id, is_active)` calling `PATCH /admin/tutors/{id}/active`. ### 2.4 Tests (`backend/src/routes/tutors.rs` `#[cfg(test)]` block) + Pattern after `backend/src/routes/rooms.rs:184-322` and use `backend/src/test_helpers.rs`. - `delete_tutor_with_no_refs_succeeds` → 204, row gone. @@ -126,19 +200,21 @@ Reference: `conductor/room-editor-refactor-visualization.md`. One-liner per task ## Verification ### Automated + - `make test` — backend suite passes, including: - Phase 2.4 tutor delete + activate tests - Phase 3.2 layout validator tests - Phase 3.7 room delete tests - `sqlx migrate run` against a clean DB — migrations 003 and 004 apply cleanly. - `make lint` — zero warnings (per the project's mandate). -- `frontend/tests/rooms.spec.ts` *(new)* — create room, drag/snap, save/reload, assert coords preserved. -- `frontend/tests/checkin-dynamic.spec.ts` *(new)* — custom layout, click seat, `POST /api/checkin` succeeds, seat turns green. -- `frontend/tests/admin-live-dynamic.spec.ts` *(new)* — student appears on correct seat in tutor live view. -- `frontend/tests/admin-tutors.spec.ts` *(new)* — (i) delete tutor with no refs → row disappears; (ii) delete tutor attached to a course → red error mentions `Kurszuordnung`; unassign → retry succeeds; (iii) deactivate → tutor cannot log in; (iv) reactivate → tutor can log in again. -- `frontend/tests/admin-rooms-delete.spec.ts` *(new)* — delete unused room succeeds; delete room attached to a slot → 409 message visible. +- `frontend/tests/rooms.spec.ts` _(new)_ — create room, drag/snap, save/reload, assert coords preserved. +- `frontend/tests/checkin-dynamic.spec.ts` _(new)_ — custom layout, click seat, `POST /api/checkin` succeeds, seat turns green. +- `frontend/tests/admin-live-dynamic.spec.ts` _(new)_ — student appears on correct seat in tutor live view. +- `frontend/tests/admin-tutors.spec.ts` _(new)_ — (i) delete tutor with no refs → row disappears; (ii) delete tutor attached to a course → red error mentions `Kurszuordnung`; unassign → retry succeeds; (iii) deactivate → tutor cannot log in; (iv) reactivate → tutor can log in again. +- `frontend/tests/admin-rooms-delete.spec.ts` _(new)_ — delete unused room succeeds; delete room attached to a slot → 409 message visible. ### Manual + 1. `make seed-demo`. Open `Admin → Rooms → Room A` — all elements at sensible grid positions (no values > 50). 2. Drag and resize an element; release outside the SVG; confirm no stranded drag. 3. Click an element in **edit mode** — sidebar populates (issue #4 regression). @@ -158,8 +234,8 @@ Reference: `conductor/room-editor-refactor-visualization.md`. One-liner per task - `backend/Cargo.toml` - `backend/audit.toml` - `.gitea/workflows/ci.yml`, `.gitea/workflows/release.yml` -- `backend/migrations/003_normalize_room_layout_units.sql` *(new)* -- `backend/migrations/004_tutor_is_active.sql` *(new)* +- `backend/migrations/003_normalize_room_layout_units.sql` _(new)_ +- `backend/migrations/004_tutor_is_active.sql` _(new)_ - `backend/demo/demo_seed.sql` - `backend/src/models.rs` - `backend/src/routes/tutors.rs` @@ -171,7 +247,7 @@ Reference: `conductor/room-editor-refactor-visualization.md`. One-liner per task - `frontend/src/lib/api.ts` - `frontend/src/lib/types.ts` - `frontend/src/lib/RoomCanvas.svelte` -- `frontend/src/lib/components/SeatMap.svelte` *(delete in Phase 4.5)* +- `frontend/src/lib/components/SeatMap.svelte` _(delete in Phase 4.5)_ - `frontend/src/lib/components/TutorShell.svelte` (logout button) - `frontend/src/routes/admin/tutors/+page.svelte` - `frontend/src/routes/admin/rooms/+page.svelte`