Skip to content

feat(teacher): THI-235 Sprint 2.A étape 2 — Teacher Dashboard CRUD#268

Merged
thierryvm merged 3 commits into
mainfrom
feature/thi-235-2A-teacher-dashboard-crud
May 19, 2026
Merged

feat(teacher): THI-235 Sprint 2.A étape 2 — Teacher Dashboard CRUD#268
thierryvm merged 3 commits into
mainfrom
feature/thi-235-2A-teacher-dashboard-crud

Conversation

@thierryvm

@thierryvm thierryvm commented May 19, 2026

Copy link
Copy Markdown
Owner

Summary

Route /app/teacher gated by RequireRole allowed={['teacher', 'super_admin']} :

  • liste "Mes classes" avec card par classe (nom, code 12-hex, count élèves, copy URL d'invitation)
  • création via inline form expandable (Shadcn-free — voir Design decisions)
  • Sidebar entry conditionnelle role-gated

Étape 2 du Sprint 2.A (cf. THI-235). L'étape 3 (page /app/join consommant la RPC join_class_by_code) suit dans 1-2 jours.

Self-review

Files

  • New: TeacherDashboard.tsx, teacher/ClassCard.tsx, teacher/CreateClassForm.tsx, useTeacherClasses.ts, tests Vitest (25 nouveaux : 14 TeacherDashboard + 11 ClassCard), migrations 020_classes_hardening.sql + 021_invitation_code_qualify_extensions.sql
  • Modified: Sidebar.tsx (entry conditionnelle), routes.ts (lazy import + path), types/database.ts (invitation_code + RPC signature, removed Insert override)
  • Auto: sitemap.xml (lastmod auto-regen build)

Cascade pré-merge

Check Verdict
npm run type-check ✅ Clean
npm run lint ✅ Clean
npm run test ✅ 1545 passed / 20 skipped (+25 nouveaux)
npm run build ✅ TeacherDashboard chunk 8.88 kB / 3.24 kB gzip
ui-auditor agent ✅ SHIP-READY (0 CRITICAL/HIGH/MEDIUM, 1 LOW deferred Dialog)
security-auditor agent ⚠️→✅ 8.7/10 initial → 2 HIGH fixés (migration 020 + 021), 3 MEDIUM follow-ups
rbac-flow-tester agent ⚠️→✅ Initial flag pgcrypto schema (raison) → fix migration 021 + empirical validation PASS
Empirical INSERT test prod (via Supabase MCP) ✅ H1 CHECK length=81 raises 23514, H2 forced code overwriten, cleanup OK
Sourcery review ⏭️ SKIPPED rate-limit hebdomadaire (acceptable per CLAUDE.md projet)

Migrations appliquées en prod via Supabase MCP

  • 020 classes_hardening : CHECK constraint length(name) BETWEEN 1 AND 80 (security-auditor H1) + CHECK invitation_code ~ '^[0-9a-f]{12}$' (H2) + trigger force regenerate
  • 021 invitation_code_qualify_extensions (FIX CRITICAL) : generate_invitation_code() raised 42883 function gen_random_bytes(integer) does not exist because pgcrypto is in extensions schema while migration 017 set search_path=public. Fix : qualify call as extensions.gen_random_bytes(6). Discovered empirically via INSERT test, mirrors the bug 42702 process lesson (happy path empirical test missing on migration ship).

Tickets follow-up Backlog créés

  • THI-240 UI : Sidebar refactor "Mes outils" collapsible (si 3+ entries role-gated Sprint 2.B+)
  • THI-241 Security M1 : map PostgREST/RLS error messages to generic UX (info disclosure)
  • THI-242 Security M3 : decide institution_admin SELECT scope on invitation_code (ADR class)

Design decisions

Inline form vs Shadcn Dialog

Intentionally not a modal. @radix-ui/react-dialog not installed, AdminPanel.tsx has no modals either. For a 1-input form (class name only, institution_id auto-inherited from profile), inline expandable form keeps deps + complexity low. If Sprint 2.B adds 2+ more modals, shadcn Dialog will be installed then — pattern documented in CreateClassForm.tsx header.

Anonymous-friendly UX preserved

Sidebar entry is conditional (role === 'teacher' || role === 'super_admin'), invisible for guests/student. Route /app/teacher is gated server-side by RequireRole — defense-in-depth (client hide + server guard).

URL copy & étape 3 dependency

${window.location.origin}/app/join?code=<code> — the /app/join route lands in Sprint 2.A étape 3 (20-22 mai). The URL is functional shape-wise but currently 404s. Acceptable risk for next 24-72h (no teacher will share codes before étape 3 ships) — noted in security-auditor M2 (no exploit, just UX).

invitation_code Insert type removal

Removed invitation_code?: string from the TypeScript Insert type. Migration 020 trigger now ALWAYS regenerates — letting tests think they can override would be misleading. For test fixtures needing a fixed code, INSERT then UPDATE.

Mea culpa migration 021 discovery

The bug 42883 (function gen_random_bytes does not exist) was first flagged by the rbac-flow-tester agent. I initially dismissed it as a false positive because SELECT gen_random_bytes(6) worked in my service_role test. Only after running an empirical INSERT did I confirm the agent was right (the trigger's search_path=public from migration 017 broke the bare reference). Migration 021 qualifies the call as extensions.gen_random_bytes. Same process lesson as bug 42702 PR #266 : test the happy path empirically before declaring migrations green.

Test plan (live)

  • CI green (type-check + lint + test + build)
  • Sourcery SKIPPED (rate-limit, acceptable)
  • Vercel preview deployed
  • Empirical INSERT test prod (migrations 020 + 021 verified PASS)
  • Voie A Chrome MCP : /app/teacher desktop + iPhone 14
    • Anonymous → fallback "Vous devez être connecté"
    • Student → fallback "Accès réservé"
    • Teacher → dashboard accessible, créer classe, copier URL
    • Super_admin → dashboard accessible
    • Sidebar entry visible uniquement pour teacher/super_admin
  • Validation visuelle @Thierry sur Brave (Voie B confirmation)
  • CHANGELOG.md + plan.md updated dans commit fixup ou PR docs séparée
  • Obsidian handoff écrit en fin de session

🤖 Generated with Claude Code

Route `/app/teacher` gated by RequireRole `teacher`/`super_admin` :
liste "Mes classes", form inline création (Shadcn-free, see below),
ClassCard avec copy URL d'invitation `/app/join?code=<12-hex>`.

## Files

* `src/app/components/TeacherDashboard.tsx` — wrapper + listing +
  empty state (RequireRole guard).
* `src/app/components/teacher/CreateClassForm.tsx` — inline expandable
  form (intentionally NOT a shadcn Dialog — see "Design decisions").
* `src/app/components/teacher/ClassCard.tsx` — display class + copy
  URL with inline "Copié"/"Copie impossible" feedback (no toast dep).
* `src/lib/hooks/useTeacherClasses.ts` — fetch + create class via
  Supabase REST, PostgREST embedded aggregate for enrollment_count.
* `src/app/components/Sidebar.tsx` — entry "Mes classes"
  conditionnelle role-gated (teacher OR super_admin).
* `src/app/routes.ts` — `/app/teacher` lazy route.
* `src/app/types/database.ts` — added `invitation_code: string` to
  classes Row, `join_class_by_code` RPC signature for étape 3 ready.
  Removed `invitation_code?` from Insert type (security-auditor H2 fix —
  trigger now always regenerates, client cannot force a value).
* `supabase/migrations/020_classes_hardening.sql` — applied to prod :
  CHECK constraint `length(name) BETWEEN 1 AND 80` (security-auditor H1)
  + CHECK constraint `invitation_code ~ '^[0-9a-f]{12}$'` (H2)
  + trigger `set_invitation_code_before_insert()` now ALWAYS regens
  (defense-in-depth even if CHECK is dropped). Pre-flight verified
  existing data conforms (1 row, OK).
* `src/test/teacherDashboard.test.tsx` (14 tests) + `classCard.test.tsx`
  (11 tests) — RBAC, listing states, create flow, copy URL feedback,
  clipboard API failures, plural/singular labels, SSR-safe URL build.
* `public/sitemap.xml` — auto-regen lastmod 16/05 → 19/05 from build.

## Design decisions

**Inline form vs Shadcn Dialog** : intentionally NOT using a modal.
@radix-ui/react-dialog is not installed, AdminPanel.tsx has no modals
either. For a 1-input form (class name only, institution_id auto-
inherited), an inline expandable form keeps deps + complexity low.
If Sprint 2.B adds 2+ more modals (Edit class, Delete class confirm),
shadcn Dialog will be installed then. Documented in component header.

**Anonymous-friendly UX preserved** : Sidebar entry is conditional
(`role === 'teacher' || 'super_admin'`), invisible for guests/student.
Route `/app/teacher` is gated server-side by RequireRole — defense-in-depth.

**URL copy** : `${window.location.origin}/app/join?code=<code>`. The
`/app/join` route lands in étape 3 (20-22 mai). The URL is functional
shape-wise but currently 404s — acceptable risk for next 24-72h, noted
in security-auditor M2 (no exploit, just UX).

## Sources of truth aligned

* Linear : THI-235 (umbrella) marche, THI-240 (Sidebar refactor "Mes outils"
  if 3+ role-gated entries) + THI-241 (security-auditor M1 error message
  sanitization) + THI-242 (security-auditor M3 institution_admin code
  visibility ADR decision) tickets created as Backlog follow-ups.
* Pre-merge cascade : ✅ type-check + lint + 1545 tests (1520 → +25)
  + build (TeacherDashboard chunk 8.88 kB / 3.24 kB gzip)
  + ui-auditor SHIP-READY
  + security-auditor 8.7/10 (2 HIGH fixed in this commit, 3 MEDIUM in follow-ups)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@vercel

vercel Bot commented May 19, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
terminal-learning Ready Ready Preview, Comment May 19, 2026 0:31am

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @thierryvm, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

…ytes

CRITICAL bug discovered empirically via INSERT test 19/05/2026 after
migration 020 shipped : the `generate_invitation_code()` trigger function
raised `42883 function gen_random_bytes(integer) does not exist` on every
teacher INSERT attempt.

## Root cause

Supabase places `pgcrypto` in the `extensions` schema by default
(verified empirically : `pg_extension` shows `pgcrypto` v1.3 in
`extensions`, not `public`). Migration 017 added `SET search_path = public`
to `generate_invitation_code()` to satisfy the
`function_search_path_mutable` advisor warning. After that change, the
bare reference `gen_random_bytes(6)` in the function body could no longer
be resolved because the search_path no longer includes `extensions`.

The existing `Terminal 101` class row was inserted BEFORE migration 017,
hiding the regression until Sprint 2.A étape 2 introduced the Teacher
Dashboard which exercises the create-class path empirically.

## Fix

Qualify the call as `extensions.gen_random_bytes(6)` so the function
resolves correctly under the strict `search_path = public`. Preserves
advisor compliance (immutable search_path) AND restores trigger.

## Validation post-fix (empirical via Supabase MCP)

1. INSERT class with `invitation_code='000000000000'` (forced) →
   trigger overwrites to `aff8a032a126` (12 hex), H2 fix preserved
2. INSERT class with `name=repeat('x',81)` → raises `check_violation`,
   H1 fix preserved
3. SELECT count(*) FROM classes → 1 (just the original Terminal 101),
   test rows cleaned up

## Process lesson

The empirical INSERT test should have been part of migration 016/020
validation, not discovered Sprint 2.A étape 2 by accident. This mirrors
the bug 42702 process lesson : RPCs and triggers need *happy path*
empirical tests with realistic auth context BEFORE merge, not just
edge case rejections.

Mea culpa : I initially dismissed the rbac-flow-tester agent finding as
a false positive without running the empirical INSERT test myself.
Correctly flagged by the agent, validated by direct execute_sql once
I tested empirically.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…entry

CHANGELOG.md updated with full entry for PR #268 :
- Route /app/teacher RequireRole gating
- Sidebar entry conditionnelle role-gated
- Inline form (Shadcn-free intentional)
- ClassCard copy URL invitation
- Migration 020 hardening + 021 fix CRITICAL (extensions.gen_random_bytes)
- Cascade pré-merge results (ui-auditor SHIP-READY, security-auditor 8.7/10)
- Follow-up tickets THI-240/241/242

Linked from PR body.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@thierryvm
thierryvm merged commit 320706c into main May 19, 2026
4 checks passed
thierryvm added a commit that referenced this pull request May 19, 2026
… redirect (#269)

* feat(ux): THI-235 Sprint 2.A étape 2.bis — role-aware navigation hub + login redirect

Follow-up to PR #268 — fixes UX gap empirically confirmed by @Thierry
("je n'avais même pas vu le lien dans la sidebar") : a teacher arriving
on /app has no discoverable entry point to /app/teacher beyond the
Sidebar entry which is visually subtle.

## Changes

### 1. StaffQuickActions section on /app Dashboard
* `src/app/components/dashboard/StaffQuickActions.tsx` — role-gated
  cards section "MES OUTILS" inserted between "Progression globale"
  and "Stats row" on Dashboard. Renders nothing for anonymous + student
  (preserves anonymous-friendly UX).
* `teacher` OR `super_admin` → "Mes classes" card → /app/teacher
* `super_admin` → "Administration" card → /app/admin
* Pattern : industry-standard 2026 B2B SaaS "role-specific KPIs"
  (DAR Design, Orbix research) — non-intrusive, discoverable, no
  onboarding wizard required (anti-pattern 2026).

### 2. Sidebar entry "Administration" for super_admin
* `src/app/components/Sidebar.tsx` — entry conditionnelle after
  "Mes classes", visible only when `role === 'super_admin'`. Will
  move into a "Mes outils" collapsible section (THI-240) once
  Sprint 2.B adds 3+ role-gated entries.

### 3. Login redirect flow with open-redirect protection
* `src/lib/auth/validateReturnTo.ts` — security helper, strict
  allowlist regex `/^\/app(\/[a-zA-Z0-9_-]+)*\/?$/` + length cap
  200 chars + pre-checks for backslash, null byte, path traversal,
  URL encoding. Returns safe fallback `/app` on rejection. NEVER
  throws, NEVER logs the input (could be attacker payload).
* `src/lib/auth/returnToStorage.ts` — sessionStorage wrapper.
  setReturnTo(path) writes; consumeReturnTo() reads + clears +
  validates atomically (one-shot semantics, can never be replayed).
* `RequireRole` + `RequireAuth` unauthenticated fallback now shows
  "Se connecter" (primary, emerald-soft) + "Retour à l'accueil"
  (ghost). Click "Se connecter" stores location.pathname and
  navigates to /?login=open.
* `Landing.tsx` detects `?login=open` on mount, auto-opens
  LoginModal, immediately strips the query param (clean URL +
  prevents reopen on history.back).
* `AuthCallback.tsx` reads consumeReturnTo() after successful
  session resolution, validates the path, navigates there.
  Failed login → /  (unchanged).

## Security analysis

**Threat model — open redirect after login** (OWASP A01:2021) :
An attacker stores a malicious returnTo via XSS or social engineering,
the user logs in, the app redirects them to attacker.com or executes
`javascript:` URI.

**Defense layers** :
1. `validateReturnTo` strict allowlist regex `^/app/...$`
2. Length cap 200 chars (defensive, no legit path exceeds)
3. Pre-checks reject `\`, null byte, whitespace, `%`, `..`, `~`, `//`
4. sessionStorage (tab-scoped, not URL query param) reduces vector
   from "shareable malicious link" to "XSS-only" (smaller surface)
5. consumeReturnTo() one-shot read+clear — replay impossible
6. Validation at READ time (not write) — even an XSS-injected value
   gets rejected at consume

**Tests** : `src/test/validateReturnTo.test.ts` (51 tests across
accepted paths, rejected URLs/protocols/traversal/encoding/whitespace,
non-string inputs, length boundaries, adversarial input that must
not throw) + `returnToStorage.test.ts` (11 tests : happy path,
one-shot semantics, validation at consume, defensive when storage
unavailable, storage key isolation).

## Files

**New :**
* `src/lib/auth/validateReturnTo.ts` (helper)
* `src/lib/auth/returnToStorage.ts` (sessionStorage wrapper)
* `src/app/components/dashboard/StaffQuickActions.tsx` (role-gated cards)
* `src/test/validateReturnTo.test.ts` (51 tests)
* `src/test/returnToStorage.test.ts` (11 tests)
* `src/test/staffQuickActions.test.tsx` (10 tests)

**Modified :**
* `src/app/components/Dashboard.tsx` (insert StaffQuickActions)
* `src/app/components/Sidebar.tsx` (entry Administration super_admin)
* `src/app/components/Landing.tsx` (?login=open auto-open modal)
* `src/app/components/auth/AuthCallback.tsx` (consume returnTo)
* `src/app/components/auth/RequireAuth.tsx` (Se connecter button)
* `src/app/components/auth/RequireRole.tsx` (Se connecter button)
* `src/test/requireRole.test.tsx` (+4 tests Se connecter)

## Cascade pré-merge

* `npm run type-check` ✅ Clean
* `npm run lint` ✅ Clean (1 eslint-disable justifié dans Landing
  useEffect query param handler — pattern aligné useTeacherClasses)
* `npm run test` ✅ 1645 passed / 20 skipped (+50 nouveaux)
* `npm run build` ✅ Dashboard chunk inchangé (StaffQuickActions
  inline in main bundle, no separate chunk needed)

## Follow-ups Sprint 2.B (créés)

* Pattern adaptive routing post-login per role (e.g. teacher → /app/teacher
  default landing) deferred to Sprint 2.B as "opinionated v2" — needs
  product validation @Thierry first

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(ux): Landing query param cleanup runs for authenticated users too

security-auditor finding M1 (cosmetic, PR #269 audit) : the `?login=open`
query param was only being stripped from the URL when the visitor was
NOT authenticated. An already-logged-in user visiting `/?login=open`
would see the param stick in the address bar indefinitely (visible in
analytics / logs, mildly confusing).

Fix : extract the cleanup outside the `!user` guard so it runs whenever
the param is present, regardless of auth state. The modal only opens
for guests (preserves the intended behavior).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* docs(changelog): THI-235 Sprint 2.A étape 2.bis entry — role-aware nav hub

Entry détaillée PR #269 :
- Context empirical (Thierry n'avait pas vu sidebar entry)
- Industry research 2026 (DAR Design, Orbix, Lollypop)
- Cards quick actions Dashboard + Sidebar Administration + login redirect
- 7 défense layers open-redirect protection
- Tests +72 (51 + 11 + 10 helpers/composants + 4 updates RequireRole)
- Cascade pré-merge results (ui-auditor SHIP-READY, security-auditor 9.5/10)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
@thierryvm
thierryvm deleted the feature/thi-235-2A-teacher-dashboard-crud branch May 19, 2026 17:26
thierryvm added a commit that referenced this pull request May 19, 2026
…hness sync (#272)

Drift identifié par `session-orchestrator` agent (shutdown 19/05 ~21h) :
4 fichiers .md vitaux stale post Sprint 2.A teacher workflow (PRs
#267-271). Fixes mécaniques (concaténation entrée Update, count agents)
appliqués maintenant ; refactors lourds (ARCHITECTURE, CONVENTIONS,
GUIDELINES, teacher-guide, STORY narrative) reportés en tickets Linear
THI-245/246/247/248 (effort 30-60 min chacun, besoin tête fraîche).

## Files updated

- `docs/plan.md` : entrée Update 19 mai ~21h CEST avec récap 5 PRs
  (#267-#271), migrations 020+021, tests 1545→1604, cascade
  ALL GREEN (security-auditor 9.4-9.5, ui-auditor SHIP-READY,
  rbac-flow-tester 11/11, happy path RPC empirique, Voie A Chrome
  MCP), 6 follow-ups Linear créés, industry research codifiée
  (DAR Design / Orbix / Lollypop 2026), mea culpa session reconnu,
  mutuelle entretien en attente accord boutons dons
- `docs/ROADMAP.md` : Last updated bumped 17 mai → 19 mai, summary
  Sprint 2.A 75% shipped + scores audits 19 mai
- `docs/README.md` : Last updated bumped 16 mai → 19 mai avec lien
  CHANGELOG pour détails
- `.claude/agents/README.md` : count 15 → 16 (drift count corrigé),
  mention validation `session-orchestrator` usage shutdown réussi
  (rapport 8 sections : git, Linear, docs vitaux fresh check,
  agent coverage gaps, orphan cleanup, recommandations prioritaires,
  verdict), nouvelle section "Patterns récurrents" avec startup +
  shutdown agent invocation pour éviter répétition process

## Voie C — docs-only

- ✅ Zero file `src/`, `api/`, `supabase/`, `package.json`,
  `vercel.json`
- ✅ Zero impact runtime / SEO / performance
- ✅ Type-check + lint + test inchangés (4 fichiers .md)
- ✅ Acceptable pour merge rapide sans Voie A

## Tickets Linear refactor follow-up (créés)

- THI-245 (Medium) : Refactor ARCHITECTURE.md post Sprint 2.A
  + Phase 9 (drift > 5 semaines, 13 avril → manque Phase 7c LTI,
  Phase 9 Admin Panel, Sprint 2.A Teacher, migrations 016-021)
- THI-246 (Low) : Update CONVENTIONS.md + GUIDELINES.md ou
  pointeurs vers CLAUDE.md (sas 48h RETIRÉE, Voies A/B/C,
  graduation STOP sécurité, happy path testing, framework updates)
- THI-247 (Low) : Update teacher-guide.md post Teacher Dashboard
  CRUD (#268)
- THI-248 (Low) : STORY.md narrative Sprint 2.A 1ère personne
  (bug 42702, agent rbac-flow-tester avait raison, "je n'avais
  pas vu le lien", "super_admin pas teacher de base")

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant